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

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

[Stoiko Ivanov]

changes v2->v3:
* incoroporated Fabian's and Thomas' feedback - huge thanks:
** changed `p-b-t kernel next-boot <ver>` to `p-b-t kernel pin <ver> --next-boot`
** improved usage output
** style-fixes to proxmox-ve apt-hook
** 'untaint' the fd fetched from the environment in proxmox-ve apt-hook
* fixed a glitch in the non p-b-t booted case (next-boot pin followed by a
  permanent pin cause the next-boot to be ignored)
* output of `p-b-t kernel list` now only prints pinned versions if they are set

original cover-letter for v2:
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).

pve-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 --next-boot option kernel pin command
  proxmox-boot: add pin/unpin functionality for non-p-b-t systems

 bin/proxmox-boot-tool                     | 97 +++++++++++++++++++++++
 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, 171 insertions(+)
 create mode 100644 proxmox-boot/proxmox-boot-cleanup.service

proxmox-ve:
Stoiko Ivanov (3):
  apt-hook: fix perlcritic warnings
  apt-hook: verify that fd is numeric
  apt-hook: add check preventing the removal of pinned kernels

 debian/apthook/pve-apt-hook | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta v3 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 v3 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        | 48 ++++++++++++++++++++++++++++++++++++
 proxmox-boot/functions       | 37 +++++++++++++++++++++++++++
 proxmox-boot/zz-proxmox-boot |  5 ++++
 3 files changed, 90 insertions(+)

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 93760fb..329df42 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -286,6 +286,13 @@ list_kernels() {
 	echo ""
 	echo "Automatically selected kernels:"
 	echo "$boot_kernels"
+
+	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
+	if [ -n "$pinned_kernel" ]; then
+		echo ""
+		echo "Pinned kernel:"
+		echo "${pinned_kernel}"
+	fi
 }
 
 usage() {
@@ -296,6 +303,8 @@ usage() {
 	warn "       $0 clean [--dry-run]"
 	warn "       $0 refresh [--hook <name>]"
 	warn "       $0 kernel <add|remove> <kernel-version>"
+	warn "       $0 kernel pin <kernel-version>"
+	warn "       $0 kernel unpin"
 	warn "       $0 kernel list"
 	warn "       $0 status [--quiet]"
 	warn "       $0 help"
@@ -323,6 +332,15 @@ help() {
 	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 ""
+	echo "USAGE: $0 kernel pin <kernel-version>"
+	echo ""
+	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 pinning kernels"
+	echo ""
+	echo "USAGE: $0 kernel unpin"
+	echo ""
+	echo "    unpin sets the latest kernel as the default entry (undoes a previous pin)"
+	echo ""
 	echo "USAGE: $0 kernel list"
 	echo ""
 	echo "    list kernel versions currently selected for inclusion on ESPs."
@@ -392,6 +410,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 +500,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..5a56b74 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 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
+
+	# replaces the current default entry, if one exists else append it at the end of the file
+	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 v3 3/4] proxmox-boot: add --next-boot option kernel pin 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                     | 53 ++++++++++++++++++-----
 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, 69 insertions(+), 11 deletions(-)
 create mode 100644 proxmox-boot/proxmox-boot-cleanup.service

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 329df42..90c4f5d 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -288,11 +288,17 @@ list_kernels() {
 	echo "$boot_kernels"
 
 	pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
+	nextboot_kernel="$(get_first_line "$NEXT_BOOT_PIN")"
 	if [ -n "$pinned_kernel" ]; then
 		echo ""
 		echo "Pinned kernel:"
 		echo "${pinned_kernel}"
 	fi
+	if [ -n "$nextboot_kernel" ]; then
+		echo ""
+		echo "Kernel pinned on next-boot:"
+		echo "${nextboot_kernel}"
+	fi
 }
 
 usage() {
@@ -303,8 +309,8 @@ usage() {
 	warn "       $0 clean [--dry-run]"
 	warn "       $0 refresh [--hook <name>]"
 	warn "       $0 kernel <add|remove> <kernel-version>"
-	warn "       $0 kernel pin <kernel-version>"
-	warn "       $0 kernel unpin"
+	warn "       $0 kernel pin <kernel-version> [--next-boot]"
+	warn "       $0 kernel unpin [--next-boot]"
 	warn "       $0 kernel list"
 	warn "       $0 status [--quiet]"
 	warn "       $0 help"
@@ -332,14 +338,16 @@ help() {
 	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 ""
-	echo "USAGE: $0 kernel pin <kernel-version>"
+	echo "USAGE: $0 kernel pin <kernel-version> [--next-boot]"
 	echo ""
 	echo "    pin pve-kernel with ABI <kernel-version> as the default entry to be booted."
+	echo "    with --next-boot sets <kernel-version> only 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"
+	echo "USAGE: $0 kernel unpin [--next-boot]"
 	echo ""
-	echo "    unpin sets the latest kernel as the default entry (undoes a previous pin)"
+	echo "    unpin removes pinned and next-boot kernel settings."
+	echo "    with --next-boot only removes the pin for the next boot."
 	echo ""
 	echo "USAGE: $0 kernel list"
 	echo ""
@@ -412,6 +420,7 @@ status() {
 
 pin_kernel() {
 	ver="$1"
+	pin_file="$2"
 
 	if [ -z "$ver" ]; then
 		warn "E: <kernel-version> is mandatory"
@@ -419,17 +428,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 "Set kernel '$ver' $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+	echo "$ver" > "$pin_file"
+	echo "Set kernel '$ver' in $pin_file. 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
@@ -501,11 +518,25 @@ case "$1" in
 				exit 0
 			;;
 			'pin')
-				pin_kernel "$2"
+				if [ "$#" -eq 3 ] && [ "$3" = '--next-boot' ]; then
+					pin_kernel "$2" "${NEXT_BOOT_PIN}"
+				elif [ "$#" -eq 2 ]; then
+					pin_kernel "$2"
+				else
+					usage
+					exit 1
+				fi
 				exit 0
 			;;
 			'unpin')
-				unpin_kernel "$2"
+				if [ "$#" -eq 2 ] && [ "$2" = '--next-boot' ]; then
+					unpin_kernel "$2"
+				elif [ "$#" -eq 1 ]; then
+					unpin_kernel
+				else
+					usage
+					exit 1
+				fi
 				exit 0
 			;;
 			*)
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 5a56b74..b46d198 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 v3 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.

fetching the next-boot version explicitly again before setting the
provided version is to cover the sequence:
p-b-t kernel pin <ver1> --next-boot ; p-b-t kernel pin <ver2>

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

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 90c4f5d..178b6fb 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -437,15 +437,33 @@ pin_kernel() {
 		exit 1
 	fi
 	echo "$ver" > "$pin_file"
-	echo "Set kernel '$ver' in $pin_file. 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
+		next_boot_ver=$(get_first_line "${NEXT_BOOT_PIN}")
+		pin_ver="${next_boot_ver:-$ver}"
+		echo "Setting '$pin_ver' as grub default entry and running update-grub."
+		set_grub_default "$pin_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 v3 1/3] apt-hook: fix perlcritic warnings

[Stoiko Ivanov]

Co-authored-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
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..2053fef 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, "<&=", $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 v3 2/3] apt-hook: verify that fd is numeric

[Stoiko Ivanov]

since we read it from the environment

Co-authored-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 debian/apthook/pve-apt-hook | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index 2053fef..8fa58c5 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -15,7 +15,7 @@ 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;
 }
-- 
2.30.2

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

[Stoiko Ivanov]

the $file_read_firstline helper code is copied from PVE::Tools, since
we do not want to depend on pve-common being in good shape when the
apt hook is executed

Co-authored-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
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 8fa58c5..47629bc 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 pve-kernel-meta/proxmox-ve v3] proxmox-boot: add kernel pinning functionality (#3761)

[Fabian Grünbichler]

On February 11, 2022 4:15 pm, Stoiko Ivanov wrote:
> changes v2->v3:
> * incoroporated Fabian's and Thomas' feedback - huge thanks:
> ** changed `p-b-t kernel next-boot <ver>` to `p-b-t kernel pin <ver> --next-boot`
> ** improved usage output
> ** style-fixes to proxmox-ve apt-hook
> ** 'untaint' the fd fetched from the environment in proxmox-ve apt-hook
> * fixed a glitch in the non p-b-t booted case (next-boot pin followed by a
>   permanent pin cause the next-boot to be ignored)
> * output of `p-b-t kernel list` now only prints pinned versions if they are set
> 
> original cover-letter for v2:
> 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.

for the whole series:

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

not applying for now in case we want to roll this out together with the 
final version of the other series.

> 
> 
> 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).
> 
> pve-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 --next-boot option kernel pin command
>   proxmox-boot: add pin/unpin functionality for non-p-b-t systems
> 
>  bin/proxmox-boot-tool                     | 97 +++++++++++++++++++++++
>  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, 171 insertions(+)
>  create mode 100644 proxmox-boot/proxmox-boot-cleanup.service
> 
> proxmox-ve:
> Stoiko Ivanov (3):
>   apt-hook: fix perlcritic warnings
>   apt-hook: verify that fd is numeric
>   apt-hook: add check preventing the removal of pinned kernels
> 
>  debian/apthook/pve-apt-hook | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 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 v3 2/4] proxmox-boot: fix #3671 add pin/unpin for kernel-version

[Oguz Bektas]

hi,

just noticed it should be bug #3761 and not #3671, careful when
applying this :)

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

[Thomas Lamprecht]

On 11/02/2022 16:15, Stoiko Ivanov wrote:
> pve-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 --next-boot option kernel pin command
>   proxmox-boot: add pin/unpin functionality for non-p-b-t systems
> 
>  bin/proxmox-boot-tool                     | 97 +++++++++++++++++++++++
>  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, 171 insertions(+)
>  create mode 100644 proxmox-boot/proxmox-boot-cleanup.service
> 
> proxmox-ve:
> Stoiko Ivanov (3):
>   apt-hook: fix perlcritic warnings
>   apt-hook: verify that fd is numeric
>   apt-hook: add check preventing the removal of pinned kernels
> 
>  debian/apthook/pve-apt-hook | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 

applied, thanks!

amended the bug number in patch 2/4 (thx Oguz) and made some small followups
(i had those lying around since a while and only committed now, I think
I had some other small things in mind when revieiwing this originally, but
nothing critical at all), mostly outputting possible pve kversions options
to choose from in some error cases and outputting when a previous pin gets
overridden with new one.

We could add a proxmox-ve
Breaks: pve-kernel-helper (<< 7.1-13)

to ensure this always works cleanly (not that a pinnend kernel gets removed)
but the case when this can happen is very odd and people should really not
hold back upgrading proxmox-ve, makes not much sense to do.