[pve-devel] [PATCH pve-kernel-meta/proxmox-ve v2] proxmox-boot: add kernel pinning functionality (#3761)

[pve-devel] [PATCH pve-kernel-meta/proxmox-ve v2] proxmox-boot: add kernel pinning functionality (#3761)

[Stoiko Ivanov]

changes v1->v2:
* incorporated the feedback on the v1 (by Aaron and Fabian - huge thx!):
** a next-boot pin is now handled independently from a pin - i.e. if you
   both pin a kernel and set one for the next-boot - the system afterwards
   keeps the pinned version (instead of the latest)
** change from modifying /etc/default/grub to creating a snippet in
   /etc/default/grub.d/proxmox-boot-pin.cfg - I did not see a need for having
   two pinning files there (since they get written both at each relevant
   invocation anyways - thus also no need for prefixing with y_ and z_
** the semantics of unpin changed (it now takes an optional argument to
   remove the next-boot-pin only (made the cleanup-service cleaner)
** added a check to the apthook in proxmox-ve as Fabian suggested
* changed the semantics of get_first_line - to check for file existence
  itself, since it makes using it shorter at almost all call-sites
* fixed two perlcritic warnings in the pve apthook (which is quite
  independent of the series)

again tested on 3 VMs (ext4, zfs+uefi, zfs+legacy) - but would be grateful
if you find some use-case apart from - pin permanent, pin next-boot, reboot,
reboot.


original cover letter of v1:
The following series adds:
* proxmox-boot-tool kernel pin <kabi-version> (to permanently set the
  default entry of the respective bootloader)
* proxmox-boot-tool kernel unpin (to undo a previous pin)
* proxmox-boot-tool kernel next-boot (to do a pin+touch a file, which causes
  an unpin on next boot)

This is the first functionality which is available for 'regular grub-setups'
(i.e. systems setup with lvm-thin with our ISO or systems installed on top
of plain debian) as well.

The first two patches are cleanup+refactoring (and should not change any
functionality)

The choices (those I think might benefit from a bit of feedback) for this
implementation were:
* for grub - automaticially rewrite '/etc/default/grub' (as this is where
  I'd look to check whether some default is set)
* for systemd - set the entry in the loader.conf and not in the efivars
  (`bootctl set-default/set-once`) - mostly from my bias towards config
  files instead of UEFI vars (depending on implementation quality of the
  UEFI) - another reason was to keep the implementation close for both
  boot-loaders
* for p-b-t booted systems the need to run `p-b-t refresh` manually
  afterwards (following the behavior of `p-b-t kernel add/remove`) could
  be changed to invoking the refresh directly (as with non-p-b-t booted
  systems). Especially since it might make sense to 'add' multiple kernels
  and then do the mount+copy+configupdate only once, whereas you can only
  pin on version anyways

Tested on three VMs installed from the 7.1 ISO (UEFI+ZFS, legacy+ZFS,
UEFI+lvm-thin).


proxmox-kernel-meta:
Stoiko Ivanov (4):
  proxmox-boot: return empty if file does not exist in get_first_line
  proxmox-boot: fix #3671 add pin/unpin for kernel-version
  proxmox-boot: add kernel next-boot command
  proxmox-boot: add pin/unpin functionality for non-p-b-t systems

 bin/proxmox-boot-tool                     | 76 ++++++++++++++++++++++-
 debian/pve-kernel-helper.install          |  1 +
 debian/rules                              |  3 +
 proxmox-boot/Makefile                     |  4 ++
 proxmox-boot/functions                    | 45 ++++++++++++++
 proxmox-boot/proxmox-boot-cleanup.service | 13 ++++
 proxmox-boot/zz-proxmox-boot              |  8 +++
 7 files changed, 147 insertions(+), 3 deletions(-)
 create mode 100644 proxmox-boot/proxmox-boot-cleanup.service

proxmox-ve:
Stoiko Ivanov (2):
  apt-hook: fix perlcritic warnings
  apt-hook: add check preventing the removal of pinned kernels

 debian/apthook/pve-apt-hook | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta v2 1/4] proxmox-boot: return empty if file does not exist in get_first_line

[Stoiko Ivanov]

makes using this helper shorter in most cases

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 proxmox-boot/functions | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/proxmox-boot/functions b/proxmox-boot/functions
index 4515a2d..27da363 100755
--- a/proxmox-boot/functions
+++ b/proxmox-boot/functions
@@ -104,6 +104,11 @@ loop_esp_list() {
 
 get_first_line() {
 	file="$1"
+	if [ ! -e  "$file" ]; then
+	    echo ""
+	    return
+	fi
+
 	while IFS= read -r line || [ -n "$line" ]; do
 		break
 	done < "${file}"
-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta v2 2/4] proxmox-boot: fix #3671 add pin/unpin for kernel-version

[Stoiko Ivanov]

The 2 commands follow the mechanics of p-b-t kernel add/remove in
writing the desired abi-version to a config-file in /etc/kernel and
actually modifying the boot-loader configuration upon p-b-t refresh.

A dedicated new file is used instead of writing the version (with some
kind of annotation) to the manual kernel list to keep parsing the file
simple (and hopefully also cause fewer problems with manually edited
files)

For systemd-boot we write the entry into the loader.conf on the ESP(s)
instead of relying on the `bootctl set-default` mechanics (bootctl(1))
which write the entry in an EFI-var. This was preferred, because of a
few reports of unwriteable EFI-vars on some systems (e.g. DELL servers
have a setting preventing writing EFI-vars from the OS). The rationale
in `Why not simply rely on the EFI boot menu logic?` from [0] also
makes a few points in that direction.

For grub the following choices were made:
* write the pinned version (or actually the menu-path leading to it)
  to a snippet in /etc/default/grub.d instead of editing the grub.cfg
  files on the partition. Mostly to divert as little as possible from
  the grub-workflow I assume people are used to.
* the 'root-device-id' part of the menu-entries is parsed from
  /boot/grub/grug.cfg since it was stable (the same on all ESPs and in
  /boot/grub), saves us from copying the part of "find device behind
  /, mangle it if zfs/btrfs, call grub_probe a few times" part of
  grub-mkconfig - and seems a bit more robust

Tested with a BIOS and an UEFI VM with / on ZFS.

[0] https://systemd.io/BOOT_LOADER_SPECIFICATION/

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 bin/proxmox-boot-tool        | 46 ++++++++++++++++++++++++++++++++----
 proxmox-boot/functions       | 37 +++++++++++++++++++++++++++++
 proxmox-boot/zz-proxmox-boot |  5 ++++
 3 files changed, 83 insertions(+), 5 deletions(-)

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 93760fb..31342a6 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -280,12 +280,16 @@ list_kernels() {
 	if [ -z "$manual_kernels" ]; then
 		manual_kernels="None."
 	fi
+	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
 
 	echo "Manually selected kernels:"
 	echo "$manual_kernels"
 	echo ""
 	echo "Automatically selected kernels:"
 	echo "$boot_kernels"
+	echo ""
+	echo "Pinned kernel:"
+	echo "${pinned_kernel:-None}"
 }
 
 usage() {
@@ -295,8 +299,8 @@ usage() {
 	warn "       $0 init <partition>"
 	warn "       $0 clean [--dry-run]"
 	warn "       $0 refresh [--hook <name>]"
-	warn "       $0 kernel <add|remove> <kernel-version>"
-	warn "       $0 kernel list"
+	warn "       $0 kernel <add|remove|pin> <kernel-version>"
+	warn "       $0 kernel <list|unpin>"
 	warn "       $0 status [--quiet]"
 	warn "       $0 help"
 }
@@ -318,14 +322,16 @@ help() {
 	echo ""
 	echo "    refresh all configured EFI system partitions. Use --hook to only run the specified hook, omit to run all."
 	echo ""
-	echo "USAGE: $0 kernel <add|remove> <kernel-version>"
+	echo "USAGE: $0 kernel <add|remove|pin> <kernel-version>"
 	echo ""
 	echo "    add/remove pve-kernel with ABI <kernel-version> to list of synced kernels, in addition to automatically selected ones."
-	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing kernels from the list"
+	echo "    pin pve-kernel with ABI <kernel-version> sets it as the default entry to be booted."
+	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing/pinning kernels from the list"
 	echo ""
-	echo "USAGE: $0 kernel list"
+	echo "USAGE: $0 kernel <list|unpin>"
 	echo ""
 	echo "    list kernel versions currently selected for inclusion on ESPs."
+	echo "    unpin sets the latest kernel as the default entry (undoes a previous pin)"
 	echo ""
 	echo "USAGE: $0 status [--quiet]"
 	echo ""
@@ -392,6 +398,28 @@ status() {
 	fi
 }
 
+pin_kernel() {
+	ver="$1"
+
+	if [ -z "$ver" ]; then
+		warn "E: <kernel-version> is mandatory"
+		warn ""
+		exit 1
+	fi
+
+	if [ ! -e "/boot/vmlinuz-$ver" ]; then
+		warn "E: no kernel image found in /boot for '$ver', not setting default."
+		exit 1
+	fi
+	echo "$ver" > "$PINNED_KERNEL_CONF"
+	echo "Set kernel '$ver' $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+}
+
+unpin_kernel() {
+	rm -f "$PINNED_KERNEL_CONF"
+	echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+}
+
 if [ -z "$1" ]; then
     usage
     exit 0
@@ -460,6 +488,14 @@ case "$1" in
 				list_kernels
 				exit 0
 			;;
+			'pin')
+				pin_kernel "$2"
+				exit 0
+			;;
+			'unpin')
+				unpin_kernel "$2"
+				exit 0
+			;;
 			*)
 				warn "E: invalid 'kernel' subcommand '$cmd'."
 				warn ""
diff --git a/proxmox-boot/functions b/proxmox-boot/functions
index 27da363..d97a7a1 100755
--- a/proxmox-boot/functions
+++ b/proxmox-boot/functions
@@ -5,11 +5,13 @@ ESP_LIST="/etc/kernel/proxmox-boot-uuids"
 ESPTYPE='c12a7328-f81f-11d2-ba4b-00a0c93ec93b'
 
 MANUAL_KERNEL_LIST="/etc/kernel/pve-efiboot-manual-kernels"
+PINNED_KERNEL_CONF="/etc/kernel/proxmox-boot-pin"
 
 MOUNTROOT="${TMPDIR:-/var/tmp}/espmounts"
 # relative to the ESP mountpoint
 PMX_ESP_DIR="EFI/proxmox"
 PMX_LOADER_CONF="loader/loader.conf"
+GRUB_PIN_SNIPPET="/etc/default/grub.d/proxmox-kernel-pin.cfg"
 
 # adapted from /etc/kernel/postinst.d/apt-auto-removal as present in
 # debian's apt package:
@@ -21,6 +23,7 @@ PMX_LOADER_CONF="loader/loader.conf"
 #  - the second-latest kernel version
 #  - the latest kernel version of each series (e.g. 4.13, 4.15, 5.0) by
 #    marking the meta-packages
+#  - the currently pinned kernel if any
 
 kernel_keep_versions() {
 	eval "$(apt-config shell DPKG Dir::bin::dpkg/f)"
@@ -56,6 +59,8 @@ kernel_keep_versions() {
 		manual_kernels="$(cat "$MANUAL_KERNEL_LIST")"
 	fi
 
+	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
+
 	kernels="$(cat <<-EOF
 		$running_version
 		$install_version
@@ -63,6 +68,7 @@ kernel_keep_versions() {
 		$latest_2_versions
 		$series_metapackages
 		$oldseries_latest_kernel
+		$pinned_kernel
 		EOF
 	)"
 
@@ -114,3 +120,34 @@ get_first_line() {
 	done < "${file}"
 	echo "$line"
 }
+
+set_grub_default() {
+	kver="$1"
+
+	if [ -z "${kver}" ]; then
+		rm -f "${GRUB_PIN_SNIPPET}"
+	else
+		# grub menu entry ids contain the internal root-device id
+		# (e.g. for zfs the GUID of the pool printed in hex) as this
+		# as this is independent of the ESP (or grub location) take
+		# it from /boot/grub/grub.cfg
+		root_devid=$(sed -rn "s/.*gnulinux-advanced-(.+)['] \{$/\1/p" \
+			/boot/grub/grub.cfg)
+		entry="gnulinux-advanced-${root_devid}>gnulinux-${kver}-advanced-${root_devid}"
+		echo "GRUB_DEFAULT=\"${entry}\"" > "${GRUB_PIN_SNIPPET}"
+	fi
+}
+
+set_systemd_boot_default() {
+	mountpoint="$1"
+	kver="$2"
+	if [ -z "${kver}" ]; then
+		entry="proxmox-*"
+	else
+		entry="proxmox-${kver}.conf"
+	fi
+
+	sed -ri "/^default /{h;s/ .*\$/ ${entry}/};\${x;/^$/{s//default ${entry}/;H};x}" \
+		"${mountpoint}/$PMX_LOADER_CONF"
+
+}
diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot
index db73166..7958a5d 100755
--- a/proxmox-boot/zz-proxmox-boot
+++ b/proxmox-boot/zz-proxmox-boot
@@ -90,9 +90,14 @@ update_esp_func() {
 	fi
 	warn "Copying and configuring kernels on ${path}"
 	copy_and_config_kernels "${mountpoint}"
+
+	pinned_kernel=$(get_first_line "${PINNED_KERNEL_CONF}")
+
 	if [ -d /sys/firmware/efi ]; then
+		set_systemd_boot_default "${mountpoint}" "${pinned_kernel}"
 		remove_old_kernels_efi "${mountpoint}"
 	else
+		set_grub_default "${pinned_kernel}"
 		remove_old_kernels_legacy "${mountpoint}"
 		mount --bind "${mountpoint}" "/boot"
 		update-grub
-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta v2 3/4] proxmox-boot: add kernel next-boot command

[Stoiko Ivanov]

by setting the desired version in a dedicated file, which is used
by the systemd service as condition for removing it  and refreshing
upon reboot

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 bin/proxmox-boot-tool                     | 34 +++++++++++++++++------
 debian/pve-kernel-helper.install          |  1 +
 debian/rules                              |  3 ++
 proxmox-boot/Makefile                     |  4 +++
 proxmox-boot/functions                    |  3 ++
 proxmox-boot/proxmox-boot-cleanup.service | 13 +++++++++
 proxmox-boot/zz-proxmox-boot              |  3 ++
 7 files changed, 53 insertions(+), 8 deletions(-)
 create mode 100644 proxmox-boot/proxmox-boot-cleanup.service

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 31342a6..5d878f6 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -299,8 +299,9 @@ usage() {
 	warn "       $0 init <partition>"
 	warn "       $0 clean [--dry-run]"
 	warn "       $0 refresh [--hook <name>]"
-	warn "       $0 kernel <add|remove|pin> <kernel-version>"
-	warn "       $0 kernel <list|unpin>"
+	warn "       $0 kernel <add|remove|pin|next-boot> <kernel-version>"
+	warn "       $0 kernel list"
+	warn "       $0 kernel unpin [--next-boot]"
 	warn "       $0 status [--quiet]"
 	warn "       $0 help"
 }
@@ -322,16 +323,20 @@ help() {
 	echo ""
 	echo "    refresh all configured EFI system partitions. Use --hook to only run the specified hook, omit to run all."
 	echo ""
-	echo "USAGE: $0 kernel <add|remove|pin> <kernel-version>"
+	echo "USAGE: $0 kernel <add|remove|pin|next-boot> <kernel-version>"
 	echo ""
 	echo "    add/remove pve-kernel with ABI <kernel-version> to list of synced kernels, in addition to automatically selected ones."
 	echo "    pin pve-kernel with ABI <kernel-version> sets it as the default entry to be booted."
+	echo "    next-boot pve-kernel with ABI <kernel-version> sets the kernel version for the next boot."
 	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing/pinning kernels from the list"
 	echo ""
-	echo "USAGE: $0 kernel <list|unpin>"
+	echo "USAGE: $0 kernel list"
 	echo ""
 	echo "    list kernel versions currently selected for inclusion on ESPs."
-	echo "    unpin sets the latest kernel as the default entry (undoes a previous pin)"
+	echo ""
+	echo "USAGE: $0 kernel unpin [--next-boot]"
+	echo ""
+	echo "    unpin removes pinned and next-boot kernel settings. Use --next-boot to only remove a next-boot setting."
 	echo ""
 	echo "USAGE: $0 status [--quiet]"
 	echo ""
@@ -400,6 +405,7 @@ status() {
 
 pin_kernel() {
 	ver="$1"
+	pin_file="$2"
 
 	if [ -z "$ver" ]; then
 		warn "E: <kernel-version> is mandatory"
@@ -407,17 +413,25 @@ pin_kernel() {
 		exit 1
 	fi
 
+	if [ -z "$pin_file" ]; then
+	    pin_file="$PINNED_KERNEL_CONF"
+	fi
+
 	if [ ! -e "/boot/vmlinuz-$ver" ]; then
 		warn "E: no kernel image found in /boot for '$ver', not setting default."
 		exit 1
 	fi
-	echo "$ver" > "$PINNED_KERNEL_CONF"
+	echo "$ver" > "$pin_file"
 	echo "Set kernel '$ver' $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
 }
 
 unpin_kernel() {
-	rm -f "$PINNED_KERNEL_CONF"
-	echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+	rm -f "$NEXT_BOOT_PIN"
+	echo "Removed $NEXT_BOOT_PIN. Use the 'refresh' command to update the ESPs."
+	if [ -z "$1" ]; then
+		rm -f "$PINNED_KERNEL_CONF"
+		echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+	fi
 }
 
 if [ -z "$1" ]; then
@@ -496,6 +510,10 @@ case "$1" in
 				unpin_kernel "$2"
 				exit 0
 			;;
+			'next-boot')
+				pin_kernel "$2" "${NEXT_BOOT_PIN}"
+				exit 0
+			;;
 			*)
 				warn "E: invalid 'kernel' subcommand '$cmd'."
 				warn ""
diff --git a/debian/pve-kernel-helper.install b/debian/pve-kernel-helper.install
index 5f264aa..33170fb 100644
--- a/debian/pve-kernel-helper.install
+++ b/debian/pve-kernel-helper.install
@@ -2,6 +2,7 @@ etc/grub.d/000_proxmox_boot_header
 etc/kernel/postinst.d/*
 etc/kernel/postrm.d/*
 etc/initramfs/post-update.d/proxmox-boot-sync
+lib/systemd/system/proxmox-boot-cleanup.service
 usr/sbin/proxmox-boot-tool
 usr/sbin/grub-install
 usr/share/pve-kernel-helper/scripts/functions
diff --git a/debian/rules b/debian/rules
index 58f7f7d..3dd1bc8 100755
--- a/debian/rules
+++ b/debian/rules
@@ -12,5 +12,8 @@ debian/control: $(wildcard debian/*.in)
 %:
 	dh $@
 
+override_dh_installsystemd:
+	dh_installsystemd --no-start
+
 .PHONY: build clean
 build clean:
diff --git a/proxmox-boot/Makefile b/proxmox-boot/Makefile
index effd726..2b0685d 100644
--- a/proxmox-boot/Makefile
+++ b/proxmox-boot/Makefile
@@ -2,12 +2,14 @@ KERNEL_HOOKSCRIPTS = proxmox-auto-removal zz-proxmox-boot
 INITRAMFS_HOOKSCRIPTS = proxmox-boot-sync
 SHARE_FILES = functions
 GRUB_CFG_SNIPPET = 000_proxmox_boot_header
+SYSTEMD_SERVICES = proxmox-boot-cleanup.service
 
 POSTINSTHOOKDIR = ${DESTDIR}/etc/kernel/postinst.d
 POSTRMHOOKDIR = ${DESTDIR}/etc/kernel/postrm.d
 POSTINITRAMFSHOOKDIR = ${DESTDIR}/etc/initramfs/post-update.d
 SHARE_SCRIPTDIR = ${DESTDIR}/usr/share/pve-kernel-helper/scripts
 GRUB_CFG_DIR = ${DESTDIR}/etc/grub.d
+SERVICE_DIR = ${DESTDIR}/lib/systemd/system
 
 .PHONY: all
 all:
@@ -23,6 +25,8 @@ install:
 	install -m 0755 ${SHARE_FILES} ${SHARE_SCRIPTDIR}
 	install -d ${GRUB_CFG_DIR}
 	install -m 0755 ${GRUB_CFG_SNIPPET} ${GRUB_CFG_DIR}
+	install -d ${SERVICE_DIR}
+	install -m 0644 ${SYSTEMD_SERVICES} ${SERVICE_DIR}
 
 .PHONY: clean distclean
 distclean:
diff --git a/proxmox-boot/functions b/proxmox-boot/functions
index d97a7a1..b1a10a5 100755
--- a/proxmox-boot/functions
+++ b/proxmox-boot/functions
@@ -6,6 +6,7 @@ ESPTYPE='c12a7328-f81f-11d2-ba4b-00a0c93ec93b'
 
 MANUAL_KERNEL_LIST="/etc/kernel/pve-efiboot-manual-kernels"
 PINNED_KERNEL_CONF="/etc/kernel/proxmox-boot-pin"
+NEXT_BOOT_PIN="/etc/kernel/next-boot-pin"
 
 MOUNTROOT="${TMPDIR:-/var/tmp}/espmounts"
 # relative to the ESP mountpoint
@@ -60,6 +61,7 @@ kernel_keep_versions() {
 	fi
 
 	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
+	nextboot_kernel="$(get_first_line "$NEXT_BOOT_PIN")"
 
 	kernels="$(cat <<-EOF
 		$running_version
@@ -69,6 +71,7 @@ kernel_keep_versions() {
 		$series_metapackages
 		$oldseries_latest_kernel
 		$pinned_kernel
+		$nextboot_kernel
 		EOF
 	)"
 
diff --git a/proxmox-boot/proxmox-boot-cleanup.service b/proxmox-boot/proxmox-boot-cleanup.service
new file mode 100644
index 0000000..4f9da94
--- /dev/null
+++ b/proxmox-boot/proxmox-boot-cleanup.service
@@ -0,0 +1,13 @@
+[Unit]
+Description=Clean up bootloader next-boot setting
+After=systemd-remount-fs.service
+ConditionPathExists=/etc/kernel/next-boot-pin
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=/usr/sbin/proxmox-boot-tool kernel unpin --next-boot
+ExecStart=/usr/sbin/proxmox-boot-tool refresh
+
+[Install]
+WantedBy=multi-user.target
diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot
index 7958a5d..5fe16a6 100755
--- a/proxmox-boot/zz-proxmox-boot
+++ b/proxmox-boot/zz-proxmox-boot
@@ -93,6 +93,9 @@ update_esp_func() {
 
 	pinned_kernel=$(get_first_line "${PINNED_KERNEL_CONF}")
 
+	if [ -e "${NEXT_BOOT_PIN}" ]; then
+	    pinned_kernel=$(get_first_line "${NEXT_BOOT_PIN}")
+	fi
 	if [ -d /sys/firmware/efi ]; then
 		set_systemd_boot_default "${mountpoint}" "${pinned_kernel}"
 		remove_old_kernels_efi "${mountpoint}"
-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta v2 4/4] proxmox-boot: add pin/unpin functionality for non-p-b-t systems

[Stoiko Ivanov]

While running `update-grub` directly in this case is a divergence from
the semantics of the command when p-b-t handles booting it makes the
cleanup in the `next-boot` case a bit tidier.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 bin/proxmox-boot-tool | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 5d878f6..222d131 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -422,15 +422,31 @@ pin_kernel() {
 		exit 1
 	fi
 	echo "$ver" > "$pin_file"
-	echo "Set kernel '$ver' $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+
+	if [ -f "${ESP_LIST}" ]; then
+		echo "Set kernel '$ver' in $pin_file. Use the 'refresh' command to update the ESPs."
+	else
+		echo "Setting '$ver' as grub default entry and running update-grub."
+		set_grub_default "$ver"
+		update-grub
+	fi
 }
 
 unpin_kernel() {
 	rm -f "$NEXT_BOOT_PIN"
-	echo "Removed $NEXT_BOOT_PIN. Use the 'refresh' command to update the ESPs."
+	echo "Removed $NEXT_BOOT_PIN."
 	if [ -z "$1" ]; then
 		rm -f "$PINNED_KERNEL_CONF"
-		echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+		echo "Removed $PINNED_KERNEL_CONF."
+	fi
+
+	if [ -f "${ESP_LIST}" ]; then
+		echo "Use the 'refresh' command to update the ESPs."
+	else
+		echo "Reset default grub entry and running update-grub."
+		pinned_kernel=$(get_first_line "${PINNED_KERNEL_CONF}")
+		set_grub_default "$pinned_kernel"
+		update-grub
 	fi
 }
 
-- 
2.30.2

[pve-devel] [PATCH proxmox-ve v2 1/2] apt-hook: fix perlcritic warnings

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 debian/apthook/pve-apt-hook | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index 1f77a1a..50e50d1 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -20,14 +20,16 @@ if (!defined $fd || $fd == 0) {
   exit 0;
 }
 
-open(my $fh, "<&=${fd}") or die "E: could not open APT_HOOK_INFO_FD (${fd}) - $!\n";
+open(my $fh, q{<&=}, "${fd}") or die "E: could not open APT_HOOK_INFO_FD (${fd}) - $!\n";
 
 my $cleanup = sub {
   my ($rc, $confirm) = @_;
 
   close($fh);
 
-  my $line = <STDIN> if $confirm;
+  if ($confirm) {
+      my $line = <STDIN>;
+  }
 
   exit $rc;
 };
-- 
2.30.2

[pve-devel] [PATCH proxmox-ve v2 2/2] apt-hook: add check preventing the removal of pinned kernels

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 debian/apthook/pve-apt-hook | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index 50e50d1..6de56c4 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -34,6 +34,17 @@ my $cleanup = sub {
   exit $rc;
 };
 
+my $file_read_firstline = sub {
+    my ($filename) = @_;
+
+    my $fh = IO::File->new($filename, "r");
+    return undef if !$fh;
+    my $res = <$fh>;
+    chomp $res if $res;
+    $fh->close;
+    return $res;
+};
+
 chomp (my $ver = <$fh>);
 if ($ver ne "VERSION 2") {
   $log->("apt-pve-hook misconfigured, expecting hook protocol version 2\n");
@@ -84,6 +95,23 @@ while (my $line = <$fh>) {
       $cleanup->(0, 1);
     }
   }
+  if ($pkg =~ /^pve-kernel-/) {
+    if ($action eq '**REMOVE**') {
+      my $next_boot_ver = $file_read_firstline->("/etc/kernel/next-boot-pin");
+      my $pinned_ver = $file_read_firstline->("/etc/kernel/proxmox-boot-pin");
+      my $remove_pinned_ver = ($next_boot_ver && $pkg =~ /$next_boot_ver/);
+      $remove_pinned_ver ||= ($pinned_ver && $pkg =~ /$pinned_ver/);
+      if ($remove_pinned_ver) {
+        $log->("!! WARNING !!\n");
+        $log->("You are attempting to remove the currently pinned kernel '${pkg}'!\n");
+        $log->("\n");
+        $log->("If you really do not need the version anymore unpin it by running\n");
+        $log->("\tproxmox-boot-tool kernel unpin'\n");
+        $log->("and repeat your apt invocation.\n");
+        $cleanup->(1);
+      }
+    }
+  }
 }
 
 $cleanup->(0);
-- 
2.30.2

Re: [pve-devel] [PATCH proxmox-ve v2 2/2] apt-hook: add check preventing the removal of pinned kernels

[Stoiko Ivanov]

while talking off-list about this I realized that I forgot to mention that
file_read_firstline is copied from PVE::Tools.

The rationale was that we might end up in a situation where pve-common
might not be available and the hook might still be called.
Also we might eventually have this hook in some of our other products,
which do not depend on pve-common (PBS for now).

if the series is accepted as-is - feel free to update the commit message.
else - I'll include it in the v3

sorry for the noise

On Fri,  4 Feb 2022 19:45:38 +0100
Stoiko Ivanov <s.ivanov@proxmox.com> wrote:

> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  debian/apthook/pve-apt-hook | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
> index 50e50d1..6de56c4 100755
> --- a/debian/apthook/pve-apt-hook
> +++ b/debian/apthook/pve-apt-hook
> @@ -34,6 +34,17 @@ my $cleanup = sub {
>    exit $rc;
>  };
>  
> +my $file_read_firstline = sub {
> +    my ($filename) = @_;
> +
> +    my $fh = IO::File->new($filename, "r");
> +    return undef if !$fh;
> +    my $res = <$fh>;
> +    chomp $res if $res;
> +    $fh->close;
> +    return $res;
> +};
> +
>  chomp (my $ver = <$fh>);
>  if ($ver ne "VERSION 2") {
>    $log->("apt-pve-hook misconfigured, expecting hook protocol version 2\n");
> @@ -84,6 +95,23 @@ while (my $line = <$fh>) {
>        $cleanup->(0, 1);
>      }
>    }
> +  if ($pkg =~ /^pve-kernel-/) {
> +    if ($action eq '**REMOVE**') {
> +      my $next_boot_ver = $file_read_firstline->("/etc/kernel/next-boot-pin");
> +      my $pinned_ver = $file_read_firstline->("/etc/kernel/proxmox-boot-pin");
> +      my $remove_pinned_ver = ($next_boot_ver && $pkg =~ /$next_boot_ver/);
> +      $remove_pinned_ver ||= ($pinned_ver && $pkg =~ /$pinned_ver/);
> +      if ($remove_pinned_ver) {
> +        $log->("!! WARNING !!\n");
> +        $log->("You are attempting to remove the currently pinned kernel '${pkg}'!\n");
> +        $log->("\n");
> +        $log->("If you really do not need the version anymore unpin it by running\n");
> +        $log->("\tproxmox-boot-tool kernel unpin'\n");
> +        $log->("and repeat your apt invocation.\n");
> +        $cleanup->(1);
> +      }
> +    }
> +  }
>  }
>  
>  $cleanup->(0);

Re: [pve-devel] [PATCH pve-kernel-meta/proxmox-ve v2] proxmox-boot: add kernel pinning functionality (#3761)

[Fabian Grünbichler]

On February 4, 2022 7:45 pm, Stoiko Ivanov wrote:
> changes v1->v2:
> * incorporated the feedback on the v1 (by Aaron and Fabian - huge thx!):
> ** a next-boot pin is now handled independently from a pin - i.e. if you
>    both pin a kernel and set one for the next-boot - the system afterwards
>    keeps the pinned version (instead of the latest)
> ** change from modifying /etc/default/grub to creating a snippet in
>    /etc/default/grub.d/proxmox-boot-pin.cfg - I did not see a need for having
>    two pinning files there (since they get written both at each relevant
>    invocation anyways - thus also no need for prefixing with y_ and z_
> ** the semantics of unpin changed (it now takes an optional argument to
>    remove the next-boot-pin only (made the cleanup-service cleaner)
> ** added a check to the apthook in proxmox-ve as Fabian suggested
> * changed the semantics of get_first_line - to check for file existence
>   itself, since it makes using it shorter at almost all call-sites
> * fixed two perlcritic warnings in the pve apthook (which is quite
>   independent of the series)
> 
> again tested on 3 VMs (ext4, zfs+uefi, zfs+legacy) - but would be grateful
> if you find some use-case apart from - pin permanent, pin next-boot, reboot,
> reboot.

some small things noted as replies to patches (might warrant a v3 - so 
I'll wait for your replies ;)), and the following proxmox-ve followups 
as discussed off-list:

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index 6de56c4..ff9877e 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -15,12 +15,12 @@ my $log = sub {
   print "W: ($hook_name) $line";
 };
 
-if (!defined $fd || $fd == 0) {
+if (!defined $fd || $fd == 0 || $fd !~ /^\d+$/) {
   $log->("APT_HOOK_INFO_FD not correctly defined, skipping apt-pve-hook checks\n");
   exit 0;
 }
 
-open(my $fh, q{<&=}, "${fd}") or die "E: could not open APT_HOOK_INFO_FD (${fd}) - $!\n";
+open(my $fh, "<&=", $fd) or die "E: could not open APT_HOOK_INFO_FD (${fd}) - $!\n";
 
 my $cleanup = sub {
   my ($rc, $confirm) = @_;
@@ -99,8 +99,8 @@ while (my $line = <$fh>) {
     if ($action eq '**REMOVE**') {
       my $next_boot_ver = $file_read_firstline->("/etc/kernel/next-boot-pin");
       my $pinned_ver = $file_read_firstline->("/etc/kernel/proxmox-boot-pin");
-      my $remove_pinned_ver = ($next_boot_ver && $pkg =~ /$next_boot_ver/);
-      $remove_pinned_ver ||= ($pinned_ver && $pkg =~ /$pinned_ver/);
+      my $remove_pinned_ver = ($next_boot_ver && $pkg =~ /${next_boot_ver}$/);
+      $remove_pinned_ver ||= ($pinned_ver && $pkg =~ /${pinned_ver}$/);
       if ($remove_pinned_ver) {
         $log->("!! WARNING !!\n");
         $log->("You are attempting to remove the currently pinned kernel '${pkg}'!\n");

> 
> 
> original cover letter of v1:
> The following series adds:
> * proxmox-boot-tool kernel pin <kabi-version> (to permanently set the
>   default entry of the respective bootloader)
> * proxmox-boot-tool kernel unpin (to undo a previous pin)
> * proxmox-boot-tool kernel next-boot (to do a pin+touch a file, which causes
>   an unpin on next boot)
> 
> This is the first functionality which is available for 'regular grub-setups'
> (i.e. systems setup with lvm-thin with our ISO or systems installed on top
> of plain debian) as well.
> 
> The first two patches are cleanup+refactoring (and should not change any
> functionality)
> 
> The choices (those I think might benefit from a bit of feedback) for this
> implementation were:
> * for grub - automaticially rewrite '/etc/default/grub' (as this is where
>   I'd look to check whether some default is set)
> * for systemd - set the entry in the loader.conf and not in the efivars
>   (`bootctl set-default/set-once`) - mostly from my bias towards config
>   files instead of UEFI vars (depending on implementation quality of the
>   UEFI) - another reason was to keep the implementation close for both
>   boot-loaders
> * for p-b-t booted systems the need to run `p-b-t refresh` manually
>   afterwards (following the behavior of `p-b-t kernel add/remove`) could
>   be changed to invoking the refresh directly (as with non-p-b-t booted
>   systems). Especially since it might make sense to 'add' multiple kernels
>   and then do the mount+copy+configupdate only once, whereas you can only
>   pin on version anyways
> 
> Tested on three VMs installed from the 7.1 ISO (UEFI+ZFS, legacy+ZFS,
> UEFI+lvm-thin).
> 
> 
> proxmox-kernel-meta:
> Stoiko Ivanov (4):
>   proxmox-boot: return empty if file does not exist in get_first_line
>   proxmox-boot: fix #3671 add pin/unpin for kernel-version
>   proxmox-boot: add kernel next-boot command
>   proxmox-boot: add pin/unpin functionality for non-p-b-t systems
> 
>  bin/proxmox-boot-tool                     | 76 ++++++++++++++++++++++-
>  debian/pve-kernel-helper.install          |  1 +
>  debian/rules                              |  3 +
>  proxmox-boot/Makefile                     |  4 ++
>  proxmox-boot/functions                    | 45 ++++++++++++++
>  proxmox-boot/proxmox-boot-cleanup.service | 13 ++++
>  proxmox-boot/zz-proxmox-boot              |  8 +++
>  7 files changed, 147 insertions(+), 3 deletions(-)
>  create mode 100644 proxmox-boot/proxmox-boot-cleanup.service
> 
> proxmox-ve:
> Stoiko Ivanov (2):
>   apt-hook: fix perlcritic warnings
>   apt-hook: add check preventing the removal of pinned kernels
> 
>  debian/apthook/pve-apt-hook | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH pve-kernel-meta v2 3/4] proxmox-boot: add kernel next-boot command

[Fabian Grünbichler]

On February 4, 2022 7:45 pm, Stoiko Ivanov wrote:
> by setting the desired version in a dedicated file, which is used
> by the systemd service as condition for removing it  and refreshing
> upon reboot
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  bin/proxmox-boot-tool                     | 34 +++++++++++++++++------
>  debian/pve-kernel-helper.install          |  1 +
>  debian/rules                              |  3 ++
>  proxmox-boot/Makefile                     |  4 +++
>  proxmox-boot/functions                    |  3 ++
>  proxmox-boot/proxmox-boot-cleanup.service | 13 +++++++++
>  proxmox-boot/zz-proxmox-boot              |  3 ++
>  7 files changed, 53 insertions(+), 8 deletions(-)
>  create mode 100644 proxmox-boot/proxmox-boot-cleanup.service
> 
> diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
> index 31342a6..5d878f6 100755
> --- a/bin/proxmox-boot-tool
> +++ b/bin/proxmox-boot-tool
> @@ -299,8 +299,9 @@ usage() {
>  	warn "       $0 init <partition>"
>  	warn "       $0 clean [--dry-run]"
>  	warn "       $0 refresh [--hook <name>]"
> -	warn "       $0 kernel <add|remove|pin> <kernel-version>"
> -	warn "       $0 kernel <list|unpin>"
> +	warn "       $0 kernel <add|remove|pin|next-boot> <kernel-version>"
> +	warn "       $0 kernel list"
> +	warn "       $0 kernel unpin [--next-boot]"
>  	warn "       $0 status [--quiet]"
>  	warn "       $0 help"
>  }
> @@ -322,16 +323,20 @@ help() {
>  	echo ""
>  	echo "    refresh all configured EFI system partitions. Use --hook to only run the specified hook, omit to run all."
>  	echo ""
> -	echo "USAGE: $0 kernel <add|remove|pin> <kernel-version>"
> +	echo "USAGE: $0 kernel <add|remove|pin|next-boot> <kernel-version>"
>  	echo ""
>  	echo "    add/remove pve-kernel with ABI <kernel-version> to list of synced kernels, in addition to automatically selected ones."
>  	echo "    pin pve-kernel with ABI <kernel-version> sets it as the default entry to be booted."
> +	echo "    next-boot pve-kernel with ABI <kernel-version> sets the kernel version for the next boot."
>  	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing/pinning kernels from the list"
>  	echo ""
> -	echo "USAGE: $0 kernel <list|unpin>"
> +	echo "USAGE: $0 kernel list"
>  	echo ""
>  	echo "    list kernel versions currently selected for inclusion on ESPs."
> -	echo "    unpin sets the latest kernel as the default entry (undoes a previous pin)"
> +	echo ""
> +	echo "USAGE: $0 kernel unpin [--next-boot]"
> +	echo ""
> +	echo "    unpin removes pinned and next-boot kernel settings. Use --next-boot to only remove a next-boot setting."
>  	echo ""
>  	echo "USAGE: $0 status [--quiet]"
>  	echo ""
> @@ -400,6 +405,7 @@ status() {
>  
>  pin_kernel() {
>  	ver="$1"
> +	pin_file="$2"
>  
>  	if [ -z "$ver" ]; then
>  		warn "E: <kernel-version> is mandatory"
> @@ -407,17 +413,25 @@ pin_kernel() {
>  		exit 1
>  	fi
>  
> +	if [ -z "$pin_file" ]; then
> +	    pin_file="$PINNED_KERNEL_CONF"
> +	fi
> +
>  	if [ ! -e "/boot/vmlinuz-$ver" ]; then
>  		warn "E: no kernel image found in /boot for '$ver', not setting default."
>  		exit 1
>  	fi
> -	echo "$ver" > "$PINNED_KERNEL_CONF"
> +	echo "$ver" > "$pin_file"
>  	echo "Set kernel '$ver' $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."

this message is now wrong?

>  }
>  
>  unpin_kernel() {
> -	rm -f "$PINNED_KERNEL_CONF"
> -	echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
> +	rm -f "$NEXT_BOOT_PIN"
> +	echo "Removed $NEXT_BOOT_PIN. Use the 'refresh' command to update the ESPs."
> +	if [ -z "$1" ]; then

$1 has never been checked to be '--next-boot', so this can easily be 
misused (e.g., someone not reading the docs thinking 'kernel unpin $foo' 
is the reverse of 'kernel pin $foo'). might warrant a check below?

> +		rm -f "$PINNED_KERNEL_CONF"
> +		echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
> +	fi
>  }
>  
>  if [ -z "$1" ]; then
> @@ -496,6 +510,10 @@ case "$1" in
>  				unpin_kernel "$2"

here ^

>  				exit 0
>  			;;
> +			'next-boot')
> +				pin_kernel "$2" "${NEXT_BOOT_PIN}"
> +				exit 0
> +			;;
>  			*)
>  				warn "E: invalid 'kernel' subcommand '$cmd'."
>  				warn ""
> diff --git a/debian/pve-kernel-helper.install b/debian/pve-kernel-helper.install
> index 5f264aa..33170fb 100644
> --- a/debian/pve-kernel-helper.install
> +++ b/debian/pve-kernel-helper.install
> @@ -2,6 +2,7 @@ etc/grub.d/000_proxmox_boot_header
>  etc/kernel/postinst.d/*
>  etc/kernel/postrm.d/*
>  etc/initramfs/post-update.d/proxmox-boot-sync
> +lib/systemd/system/proxmox-boot-cleanup.service
>  usr/sbin/proxmox-boot-tool
>  usr/sbin/grub-install
>  usr/share/pve-kernel-helper/scripts/functions
> diff --git a/debian/rules b/debian/rules
> index 58f7f7d..3dd1bc8 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -12,5 +12,8 @@ debian/control: $(wildcard debian/*.in)
>  %:
>  	dh $@
>  
> +override_dh_installsystemd:
> +	dh_installsystemd --no-start
> +
>  .PHONY: build clean
>  build clean:
> diff --git a/proxmox-boot/Makefile b/proxmox-boot/Makefile
> index effd726..2b0685d 100644
> --- a/proxmox-boot/Makefile
> +++ b/proxmox-boot/Makefile
> @@ -2,12 +2,14 @@ KERNEL_HOOKSCRIPTS = proxmox-auto-removal zz-proxmox-boot
>  INITRAMFS_HOOKSCRIPTS = proxmox-boot-sync
>  SHARE_FILES = functions
>  GRUB_CFG_SNIPPET = 000_proxmox_boot_header
> +SYSTEMD_SERVICES = proxmox-boot-cleanup.service
>  
>  POSTINSTHOOKDIR = ${DESTDIR}/etc/kernel/postinst.d
>  POSTRMHOOKDIR = ${DESTDIR}/etc/kernel/postrm.d
>  POSTINITRAMFSHOOKDIR = ${DESTDIR}/etc/initramfs/post-update.d
>  SHARE_SCRIPTDIR = ${DESTDIR}/usr/share/pve-kernel-helper/scripts
>  GRUB_CFG_DIR = ${DESTDIR}/etc/grub.d
> +SERVICE_DIR = ${DESTDIR}/lib/systemd/system
>  
>  .PHONY: all
>  all:
> @@ -23,6 +25,8 @@ install:
>  	install -m 0755 ${SHARE_FILES} ${SHARE_SCRIPTDIR}
>  	install -d ${GRUB_CFG_DIR}
>  	install -m 0755 ${GRUB_CFG_SNIPPET} ${GRUB_CFG_DIR}
> +	install -d ${SERVICE_DIR}
> +	install -m 0644 ${SYSTEMD_SERVICES} ${SERVICE_DIR}
>  
>  .PHONY: clean distclean
>  distclean:
> diff --git a/proxmox-boot/functions b/proxmox-boot/functions
> index d97a7a1..b1a10a5 100755
> --- a/proxmox-boot/functions
> +++ b/proxmox-boot/functions
> @@ -6,6 +6,7 @@ ESPTYPE='c12a7328-f81f-11d2-ba4b-00a0c93ec93b'
>  
>  MANUAL_KERNEL_LIST="/etc/kernel/pve-efiboot-manual-kernels"
>  PINNED_KERNEL_CONF="/etc/kernel/proxmox-boot-pin"
> +NEXT_BOOT_PIN="/etc/kernel/next-boot-pin"
>  
>  MOUNTROOT="${TMPDIR:-/var/tmp}/espmounts"
>  # relative to the ESP mountpoint
> @@ -60,6 +61,7 @@ kernel_keep_versions() {
>  	fi
>  
>  	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
> +	nextboot_kernel="$(get_first_line "$NEXT_BOOT_PIN")"
>  
>  	kernels="$(cat <<-EOF
>  		$running_version
> @@ -69,6 +71,7 @@ kernel_keep_versions() {
>  		$series_metapackages
>  		$oldseries_latest_kernel
>  		$pinned_kernel
> +		$nextboot_kernel
>  		EOF
>  	)"
>  
> diff --git a/proxmox-boot/proxmox-boot-cleanup.service b/proxmox-boot/proxmox-boot-cleanup.service
> new file mode 100644
> index 0000000..4f9da94
> --- /dev/null
> +++ b/proxmox-boot/proxmox-boot-cleanup.service
> @@ -0,0 +1,13 @@
> +[Unit]
> +Description=Clean up bootloader next-boot setting
> +After=systemd-remount-fs.service
> +ConditionPathExists=/etc/kernel/next-boot-pin
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStart=/usr/sbin/proxmox-boot-tool kernel unpin --next-boot
> +ExecStart=/usr/sbin/proxmox-boot-tool refresh
> +
> +[Install]
> +WantedBy=multi-user.target
> diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot
> index 7958a5d..5fe16a6 100755
> --- a/proxmox-boot/zz-proxmox-boot
> +++ b/proxmox-boot/zz-proxmox-boot
> @@ -93,6 +93,9 @@ update_esp_func() {
>  
>  	pinned_kernel=$(get_first_line "${PINNED_KERNEL_CONF}")
>  
> +	if [ -e "${NEXT_BOOT_PIN}" ]; then
> +	    pinned_kernel=$(get_first_line "${NEXT_BOOT_PIN}")
> +	fi
>  	if [ -d /sys/firmware/efi ]; then
>  		set_systemd_boot_default "${mountpoint}" "${pinned_kernel}"
>  		remove_old_kernels_efi "${mountpoint}"
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH pve-kernel-meta v2 2/4] proxmox-boot: fix #3671 add pin/unpin for kernel-version

[Fabian Grünbichler]

On February 4, 2022 7:45 pm, Stoiko Ivanov wrote:
> The 2 commands follow the mechanics of p-b-t kernel add/remove in
> writing the desired abi-version to a config-file in /etc/kernel and
> actually modifying the boot-loader configuration upon p-b-t refresh.
> 
> A dedicated new file is used instead of writing the version (with some
> kind of annotation) to the manual kernel list to keep parsing the file
> simple (and hopefully also cause fewer problems with manually edited
> files)
> 
> For systemd-boot we write the entry into the loader.conf on the ESP(s)
> instead of relying on the `bootctl set-default` mechanics (bootctl(1))
> which write the entry in an EFI-var. This was preferred, because of a
> few reports of unwriteable EFI-vars on some systems (e.g. DELL servers
> have a setting preventing writing EFI-vars from the OS). The rationale
> in `Why not simply rely on the EFI boot menu logic?` from [0] also
> makes a few points in that direction.
> 
> For grub the following choices were made:
> * write the pinned version (or actually the menu-path leading to it)
>   to a snippet in /etc/default/grub.d instead of editing the grub.cfg
>   files on the partition. Mostly to divert as little as possible from
>   the grub-workflow I assume people are used to.
> * the 'root-device-id' part of the menu-entries is parsed from
>   /boot/grub/grug.cfg since it was stable (the same on all ESPs and in
>   /boot/grub), saves us from copying the part of "find device behind
>   /, mangle it if zfs/btrfs, call grub_probe a few times" part of
>   grub-mkconfig - and seems a bit more robust
> 
> Tested with a BIOS and an UEFI VM with / on ZFS.
> 
> [0] https://systemd.io/BOOT_LOADER_SPECIFICATION/
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  bin/proxmox-boot-tool        | 46 ++++++++++++++++++++++++++++++++----
>  proxmox-boot/functions       | 37 +++++++++++++++++++++++++++++
>  proxmox-boot/zz-proxmox-boot |  5 ++++
>  3 files changed, 83 insertions(+), 5 deletions(-)
> 
> diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
> index 93760fb..31342a6 100755
> --- a/bin/proxmox-boot-tool
> +++ b/bin/proxmox-boot-tool
> @@ -280,12 +280,16 @@ list_kernels() {
>  	if [ -z "$manual_kernels" ]; then
>  		manual_kernels="None."
>  	fi
> +	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
>  
>  	echo "Manually selected kernels:"
>  	echo "$manual_kernels"
>  	echo ""
>  	echo "Automatically selected kernels:"
>  	echo "$boot_kernels"
> +	echo ""
> +	echo "Pinned kernel:"
> +	echo "${pinned_kernel:-None}"
>  }
>  
>  usage() {
> @@ -295,8 +299,8 @@ usage() {
>  	warn "       $0 init <partition>"
>  	warn "       $0 clean [--dry-run]"
>  	warn "       $0 refresh [--hook <name>]"
> -	warn "       $0 kernel <add|remove> <kernel-version>"
> -	warn "       $0 kernel list"
> +	warn "       $0 kernel <add|remove|pin> <kernel-version>"
> +	warn "       $0 kernel <list|unpin>"
>  	warn "       $0 status [--quiet]"
>  	warn "       $0 help"
>  }
> @@ -318,14 +322,16 @@ help() {
>  	echo ""
>  	echo "    refresh all configured EFI system partitions. Use --hook to only run the specified hook, omit to run all."
>  	echo ""
> -	echo "USAGE: $0 kernel <add|remove> <kernel-version>"
> +	echo "USAGE: $0 kernel <add|remove|pin> <kernel-version>"
>  	echo ""
>  	echo "    add/remove pve-kernel with ABI <kernel-version> to list of synced kernels, in addition to automatically selected ones."
> -	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing kernels from the list"
> +	echo "    pin pve-kernel with ABI <kernel-version> sets it as the default entry to be booted."
> +	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing/pinning kernels from the list"

I wonder if this should be split? e.g.,

 	echo "USAGE: $0 kernel <add|remove> <kernel-version>"
 	echo ""
 	echo "    add/remove pve-kernel with ABI <kernel-version> to list of synced kernels, in addition to automatically selected ones."
 	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing kernels from the list"
+	echi ""
+	echo "USAGE: $0 kernel pin <kernel-version>"
+	echo "    pin pve-kernel with ABI <kernel-version> as the default entry to be booted."
+	echo "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing/pinning kernels from the list"

and then adding next-boot and unpin to this section? while pin/next-boot 
share the argument with add/remove, they are kind of a different 
operation?

e.g., the full thing could then be:

	echo ""
	echo "USAGE: $0 kernel <add|remove> <kernel-version>"
	echo ""
	echo "    add/remove pve-kernel with ABI <kernel-version> to list of synced kernels, in addition to automatically selected ones."
	echo ""
	echo "USAGE: $0 kernel <pin|next-boot> <kernel-version>"
	echo "    pin pve-kernel with ABI <kernel-version> sets the default entry to be booted until unpinned."
	echo "    next-boot pve-kernel with ABI <kernel-version> sets the kernel version for the next boot."
	echo "    NOTE: you need to manually run 'refresh' once you're finished with pinning kernels"
	echo ""
	echo "USAGE: $0 kernel unpin [--next-boot]"
	echo ""
	echo "    unpin removes pinned and next-boot kernel settings. Use --next-boot to only remove a next-boot setting."
	echo "    NOTE: you need to manually run 'refresh' once you're finished with unpinning kernels"
	echo ""
	echo "USAGE: $0 kernel list"
	echo ""
	echo "    list kernel versions currently selected for inclusion on ESPs."

and reading it like that - maybe it makes sense to have 'pin 
--next-boot' like with unpin?

>  	echo ""
> -	echo "USAGE: $0 kernel list"
> +	echo "USAGE: $0 kernel <list|unpin>"
>  	echo ""
>  	echo "    list kernel versions currently selected for inclusion on ESPs."
> +	echo "    unpin sets the latest kernel as the default entry (undoes a previous pin)"
>  	echo ""
>  	echo "USAGE: $0 status [--quiet]"
>  	echo ""
> @@ -392,6 +398,28 @@ status() {
>  	fi
>  }
>  
> +pin_kernel() {
> +	ver="$1"
> +
> +	if [ -z "$ver" ]; then
> +		warn "E: <kernel-version> is mandatory"
> +		warn ""
> +		exit 1
> +	fi
> +
> +	if [ ! -e "/boot/vmlinuz-$ver" ]; then
> +		warn "E: no kernel image found in /boot for '$ver', not setting default."
> +		exit 1
> +	fi
> +	echo "$ver" > "$PINNED_KERNEL_CONF"
> +	echo "Set kernel '$ver' $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
> +}
> +
> +unpin_kernel() {
> +	rm -f "$PINNED_KERNEL_CONF"
> +	echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
> +}
> +
>  if [ -z "$1" ]; then
>      usage
>      exit 0
> @@ -460,6 +488,14 @@ case "$1" in
>  				list_kernels
>  				exit 0
>  			;;
> +			'pin')
> +				pin_kernel "$2"
> +				exit 0
> +			;;
> +			'unpin')
> +				unpin_kernel "$2"
> +				exit 0
> +			;;
>  			*)
>  				warn "E: invalid 'kernel' subcommand '$cmd'."
>  				warn ""
> diff --git a/proxmox-boot/functions b/proxmox-boot/functions
> index 27da363..d97a7a1 100755
> --- a/proxmox-boot/functions
> +++ b/proxmox-boot/functions
> @@ -5,11 +5,13 @@ ESP_LIST="/etc/kernel/proxmox-boot-uuids"
>  ESPTYPE='c12a7328-f81f-11d2-ba4b-00a0c93ec93b'
>  
>  MANUAL_KERNEL_LIST="/etc/kernel/pve-efiboot-manual-kernels"
> +PINNED_KERNEL_CONF="/etc/kernel/proxmox-boot-pin"
>  
>  MOUNTROOT="${TMPDIR:-/var/tmp}/espmounts"
>  # relative to the ESP mountpoint
>  PMX_ESP_DIR="EFI/proxmox"
>  PMX_LOADER_CONF="loader/loader.conf"
> +GRUB_PIN_SNIPPET="/etc/default/grub.d/proxmox-kernel-pin.cfg"
>  
>  # adapted from /etc/kernel/postinst.d/apt-auto-removal as present in
>  # debian's apt package:
> @@ -21,6 +23,7 @@ PMX_LOADER_CONF="loader/loader.conf"
>  #  - the second-latest kernel version
>  #  - the latest kernel version of each series (e.g. 4.13, 4.15, 5.0) by
>  #    marking the meta-packages
> +#  - the currently pinned kernel if any
>  
>  kernel_keep_versions() {
>  	eval "$(apt-config shell DPKG Dir::bin::dpkg/f)"
> @@ -56,6 +59,8 @@ kernel_keep_versions() {
>  		manual_kernels="$(cat "$MANUAL_KERNEL_LIST")"
>  	fi
>  
> +	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
> +
>  	kernels="$(cat <<-EOF
>  		$running_version
>  		$install_version
> @@ -63,6 +68,7 @@ kernel_keep_versions() {
>  		$latest_2_versions
>  		$series_metapackages
>  		$oldseries_latest_kernel
> +		$pinned_kernel
>  		EOF
>  	)"
>  
> @@ -114,3 +120,34 @@ get_first_line() {
>  	done < "${file}"
>  	echo "$line"
>  }
> +
> +set_grub_default() {
> +	kver="$1"
> +
> +	if [ -z "${kver}" ]; then
> +		rm -f "${GRUB_PIN_SNIPPET}"
> +	else
> +		# grub menu entry ids contain the internal root-device id
> +		# (e.g. for zfs the GUID of the pool printed in hex) as this
> +		# as this is independent of the ESP (or grub location) take

nit: doubled 'as this'

> +		# it from /boot/grub/grub.cfg
> +		root_devid=$(sed -rn "s/.*gnulinux-advanced-(.+)['] \{$/\1/p" \
> +			/boot/grub/grub.cfg)
> +		entry="gnulinux-advanced-${root_devid}>gnulinux-${kver}-advanced-${root_devid}"
> +		echo "GRUB_DEFAULT=\"${entry}\"" > "${GRUB_PIN_SNIPPET}"
> +	fi
> +}
> +
> +set_systemd_boot_default() {
> +	mountpoint="$1"
> +	kver="$2"
> +	if [ -z "${kver}" ]; then
> +		entry="proxmox-*"
> +	else
> +		entry="proxmox-${kver}.conf"
> +	fi
> +
> +	sed -ri "/^default /{h;s/ .*\$/ ${entry}/};\${x;/^$/{s//default ${entry}/;H};x}" \
> +		"${mountpoint}/$PMX_LOADER_CONF"
> +
> +}
> diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot
> index db73166..7958a5d 100755
> --- a/proxmox-boot/zz-proxmox-boot
> +++ b/proxmox-boot/zz-proxmox-boot
> @@ -90,9 +90,14 @@ update_esp_func() {
>  	fi
>  	warn "Copying and configuring kernels on ${path}"
>  	copy_and_config_kernels "${mountpoint}"
> +
> +	pinned_kernel=$(get_first_line "${PINNED_KERNEL_CONF}")
> +
>  	if [ -d /sys/firmware/efi ]; then
> +		set_systemd_boot_default "${mountpoint}" "${pinned_kernel}"
>  		remove_old_kernels_efi "${mountpoint}"
>  	else
> +		set_grub_default "${pinned_kernel}"
>  		remove_old_kernels_legacy "${mountpoint}"
>  		mount --bind "${mountpoint}" "/boot"
>  		update-grub
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>