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

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

[Stoiko Ivanov]

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).

Stoiko Ivanov (5):
  proxmox-boot: drop unused potential_esps function
  proxmox-boot: add get_first_line_from_file helper and use it
  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                     | 65 +++++++++++++++++++++--
 debian/pve-kernel-helper.install          |  1 +
 debian/rules                              |  3 ++
 proxmox-boot/Makefile                     |  4 ++
 proxmox-boot/functions                    | 46 ++++++++++++++++
 proxmox-boot/proxmox-boot-cleanup.service | 15 ++++++
 proxmox-boot/zz-proxmox-boot              | 15 +++---
 7 files changed, 137 insertions(+), 12 deletions(-)
 create mode 100644 proxmox-boot/proxmox-boot-cleanup.service

-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta 1/5] proxmox-boot: drop unused potential_esps function

[Stoiko Ivanov]

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

diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot
index e516b61..90d4aa9 100755
--- a/proxmox-boot/zz-proxmox-boot
+++ b/proxmox-boot/zz-proxmox-boot
@@ -39,11 +39,6 @@ elif [ -d /usr/share/doc/proxmox-backup/ ]; then
 	LOADER_TITLE="Proxmox Backup Server"
 fi
 
-potential_esps(){
-	lsblk --list -o PATH,UUID,FSTYPE,PARTTYPE,MOUNTPOINT |
-	awk '$3 == "vfat" && $4 == "c12a7328-f81f-11d2-ba4b-00a0c93ec93b" && $5 == "" {print $1,$2}'
-}
-
 update_esps() {
 	if [ ! -f "${ESP_LIST}" ]; then
 	    warn "No ${ESP_LIST} found, skipping ESP sync."
-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta 2/5] proxmox-boot: add get_first_line_from_file helper and use it

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 proxmox-boot/functions       | 8 ++++++++
 proxmox-boot/zz-proxmox-boot | 4 +---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/proxmox-boot/functions b/proxmox-boot/functions
index 6e19c20..4515a2d 100755
--- a/proxmox-boot/functions
+++ b/proxmox-boot/functions
@@ -101,3 +101,11 @@ loop_esp_list() {
 		"$@"
 	done
 }
+
+get_first_line() {
+	file="$1"
+	while IFS= read -r line || [ -n "$line" ]; do
+		break
+	done < "${file}"
+	echo "$line"
+}
diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot
index 90d4aa9..db73166 100755
--- a/proxmox-boot/zz-proxmox-boot
+++ b/proxmox-boot/zz-proxmox-boot
@@ -46,9 +46,7 @@ update_esps() {
 	fi
 	if [ -f /etc/kernel/cmdline ]; then
 		# we can have cmdline files with multiple or no new line at all, handle both!
-		while IFS= read -r CMDLINE || [ -n "$CMDLINE" ]; do
-			break
-		done < /etc/kernel/cmdline
+		CMDLINE=$(get_first_line /etc/kernel/cmdline)
 		echo ${CMDLINE} | grep -q 'root=' || \
 			{ warn "No root= parameter in /etc/kernel/cmdline found!"; exit 1; }
 	else
-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta 3/5] 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 /etc/default/grub 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        | 47 +++++++++++++++++++++++++++++++++---
 proxmox-boot/functions       | 37 ++++++++++++++++++++++++++++
 proxmox-boot/zz-proxmox-boot |  6 +++++
 3 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 93760fb..75eea0e 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"
+
+	if [ -e "$PINNED_KERNEL_CONF" ]; then
+		pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF" || true )"
+		echo ""
+		echo "Pinned kernel:"
+		echo "$pinned_kernel"
+	fi
 }
 
 usage() {
@@ -295,8 +302,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 +325,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 ""
-	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 +401,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 +491,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 4515a2d..9fa29ca 100755
--- a/proxmox-boot/functions
+++ b/proxmox-boot/functions
@@ -5,6 +5,7 @@ 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
@@ -56,6 +57,10 @@ kernel_keep_versions() {
 		manual_kernels="$(cat "$MANUAL_KERNEL_LIST")"
 	fi
 
+	if [ -e "$PINNED_KERNEL_CONF" ]; then
+		pinned_kernel="$(get_first_line "$PINNED_KERNEL_CONF")"
+	fi
+
 	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
 	)"
 
@@ -109,3 +115,34 @@ get_first_line() {
 	done < "${file}"
 	echo "$line"
 }
+
+set_grub_default() {
+	kver="$1"
+	if [ -z "${kver}" ]; then
+		entry="0"
+	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}"
+	fi
+	sed -ri "/^GRUB_DEFAULT/{h;s/=.*\$/=\"${entry}\"/};\${x;/^$/{s//GRUB_DEFAULT=\"${entry}\"/;H};x}" \
+		/etc/default/grub
+}
+
+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..27448b2 100755
--- a/proxmox-boot/zz-proxmox-boot
+++ b/proxmox-boot/zz-proxmox-boot
@@ -90,9 +90,15 @@ update_esp_func() {
 	fi
 	warn "Copying and configuring kernels on ${path}"
 	copy_and_config_kernels "${mountpoint}"
+
+	if [ -e "${PINNED_KERNEL_CONF}" ]; then
+	    pinned_kernel=$(get_first_line "${PINNED_KERNEL_CONF}")
+	fi
 	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 4/5] proxmox-boot: add kernel next-boot command

[Stoiko Ivanov]

by pinning the desired version and touching a flag file, which is used
by the systemd service as condition for unpinning and refreshing upon
reboot

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

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 75eea0e..005109a 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -302,7 +302,7 @@ 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 <add|remove|pin|next-boot> <kernel-version>"
 	warn "       $0 kernel <list|unpin>"
 	warn "       $0 status [--quiet]"
 	warn "       $0 help"
@@ -325,7 +325,7 @@ 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 "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing kernels from the list"
@@ -499,6 +499,11 @@ case "$1" in
 				unpin_kernel "$2"
 				exit 0
 			;;
+			'next-boot')
+				pin_kernel "$2"
+				touch "${PMX_NEXT_BOOT_FILE}"
+				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 9fa29ca..3bea421 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"
+PMX_NEXT_BOOT_FILE="/etc/kernel/next-boot-active"
 
 MOUNTROOT="${TMPDIR:-/var/tmp}/espmounts"
 # relative to the ESP mountpoint
diff --git a/proxmox-boot/proxmox-boot-cleanup.service b/proxmox-boot/proxmox-boot-cleanup.service
new file mode 100644
index 0000000..3c390de
--- /dev/null
+++ b/proxmox-boot/proxmox-boot-cleanup.service
@@ -0,0 +1,15 @@
+[Unit]
+Description=Clean up bootloader next-boot setting
+After=systemd-remount-fs.service
+ConditionPathExists=/etc/kernel/next-boot-active
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=/usr/sbin/proxmox-boot-tool kernel unpin
+ExecStart=/usr/sbin/proxmox-boot-tool refresh
+ExecStart=/bin/rm -f /etc/kernel/next-boot-active
+
+[Install]
+WantedBy=multi-user.target
+
-- 
2.30.2

[pve-devel] [PATCH pve-kernel-meta 5/5] 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 | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 005109a..90ebfb2 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -415,12 +415,25 @@ pin_kernel() {
 		exit 1
 	fi
 	echo "$ver" > "$PINNED_KERNEL_CONF"
-	echo "Set kernel '$ver' $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+
+	if [ -f "${ESP_LIST}" ]; then
+		echo "Set kernel '$ver' $PINNED_KERNEL_CONF. 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 "$PINNED_KERNEL_CONF"
-	echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+	if [ -f "${ESP_LIST}" ]; then
+		echo "Removed $PINNED_KERNEL_CONF. Use the 'refresh' command to update the ESPs."
+	else
+		echo "Reset default grub entry and running update-grub."
+		set_grub_default ""
+		update-grub
+	fi
 }
 
 if [ -z "$1" ]; then
-- 
2.30.2

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

[Aaron Lauterer]

One small nit at the end:

On 1/31/22 18:59, Stoiko Ivanov wrote:
> by pinning the desired version and touching a flag file, which is used
> by the systemd service as condition for unpinning and refreshing upon
> reboot
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>   bin/proxmox-boot-tool                     |  9 +++++++--
>   debian/pve-kernel-helper.install          |  1 +
>   debian/rules                              |  3 +++
>   proxmox-boot/Makefile                     |  4 ++++
>   proxmox-boot/functions                    |  1 +
>   proxmox-boot/proxmox-boot-cleanup.service | 15 +++++++++++++++
>   6 files changed, 31 insertions(+), 2 deletions(-)
>   create mode 100644 proxmox-boot/proxmox-boot-cleanup.service
> 
> diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
> index 75eea0e..005109a 100755
> --- a/bin/proxmox-boot-tool
> +++ b/bin/proxmox-boot-tool
> @@ -302,7 +302,7 @@ 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 <add|remove|pin|next-boot> <kernel-version>"
>   	warn "       $0 kernel <list|unpin>"
>   	warn "       $0 status [--quiet]"
>   	warn "       $0 help"
> @@ -325,7 +325,7 @@ 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 "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing kernels from the list"
> @@ -499,6 +499,11 @@ case "$1" in
>   				unpin_kernel "$2"
>   				exit 0
>   			;;
> +			'next-boot')
> +				pin_kernel "$2"
> +				touch "${PMX_NEXT_BOOT_FILE}"
> +				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 9fa29ca..3bea421 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"
> +PMX_NEXT_BOOT_FILE="/etc/kernel/next-boot-active"
>   
>   MOUNTROOT="${TMPDIR:-/var/tmp}/espmounts"
>   # relative to the ESP mountpoint
> diff --git a/proxmox-boot/proxmox-boot-cleanup.service b/proxmox-boot/proxmox-boot-cleanup.service
> new file mode 100644
> index 0000000..3c390de
> --- /dev/null
> +++ b/proxmox-boot/proxmox-boot-cleanup.service
> @@ -0,0 +1,15 @@
> +[Unit]
> +Description=Clean up bootloader next-boot setting
> +After=systemd-remount-fs.service
> +ConditionPathExists=/etc/kernel/next-boot-active
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStart=/usr/sbin/proxmox-boot-tool kernel unpin
> +ExecStart=/usr/sbin/proxmox-boot-tool refresh
> +ExecStart=/bin/rm -f /etc/kernel/next-boot-active
> +
> +[Install]
> +WantedBy=multi-user.target
> +

Do we want a newline here at the end of the unit file?

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

[Aaron Lauterer]

Tested systemd-boot (uefi) on physical hardware with PVE and grub in a PMG VM.

(un)Pinning and setting next-boot worked fine.

Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>

On 1/31/22 18:59, Stoiko Ivanov wrote:
> 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).
> 
> Stoiko Ivanov (5):
>    proxmox-boot: drop unused potential_esps function
>    proxmox-boot: add get_first_line_from_file helper and use it
>    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                     | 65 +++++++++++++++++++++--
>   debian/pve-kernel-helper.install          |  1 +
>   debian/rules                              |  3 ++
>   proxmox-boot/Makefile                     |  4 ++
>   proxmox-boot/functions                    | 46 ++++++++++++++++
>   proxmox-boot/proxmox-boot-cleanup.service | 15 ++++++
>   proxmox-boot/zz-proxmox-boot              | 15 +++---
>   7 files changed, 137 insertions(+), 12 deletions(-)
>   create mode 100644 proxmox-boot/proxmox-boot-cleanup.service
> 

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

[Fabian Grünbichler]

On January 31, 2022 6:59 pm, Stoiko Ivanov wrote:
> by pinning the desired version and touching a flag file, which is used
> by the systemd service as condition for unpinning and refreshing upon
> reboot
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  bin/proxmox-boot-tool                     |  9 +++++++--
>  debian/pve-kernel-helper.install          |  1 +
>  debian/rules                              |  3 +++
>  proxmox-boot/Makefile                     |  4 ++++
>  proxmox-boot/functions                    |  1 +
>  proxmox-boot/proxmox-boot-cleanup.service | 15 +++++++++++++++
>  6 files changed, 31 insertions(+), 2 deletions(-)
>  create mode 100644 proxmox-boot/proxmox-boot-cleanup.service
> 
> diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
> index 75eea0e..005109a 100755
> --- a/bin/proxmox-boot-tool
> +++ b/bin/proxmox-boot-tool
> @@ -302,7 +302,7 @@ 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 <add|remove|pin|next-boot> <kernel-version>"
>  	warn "       $0 kernel <list|unpin>"
>  	warn "       $0 status [--quiet]"
>  	warn "       $0 help"
> @@ -325,7 +325,7 @@ 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 "    NOTE: you need to manually run 'refresh' once you're finished with adding/removing kernels from the list"

this doesn't really make it clear (to me) that 'next-boot' removes an 
existing pinning. I'd either add an explicit note about this (here and 
in *-docs ;)), or dis-entangle it:
- next-boot-active becomes next-boot and contains the temp. pinned 
  kernel version
- grub gets a second snippet file (see other mail) for temp 
  pinning/next-boot with higher priority than regular pinning
- systemd-boot checks for next-boot before pinned-kernel when refreshing
- systemd service removes 'next-boot' and extra grub snippet, then does 
  refresh

> @@ -499,6 +499,11 @@ case "$1" in
>  				unpin_kernel "$2"
>  				exit 0
>  			;;
> +			'next-boot')
> +				pin_kernel "$2"
> +				touch "${PMX_NEXT_BOOT_FILE}"
> +				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 9fa29ca..3bea421 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"
> +PMX_NEXT_BOOT_FILE="/etc/kernel/next-boot-active"
>  
>  MOUNTROOT="${TMPDIR:-/var/tmp}/espmounts"
>  # relative to the ESP mountpoint
> diff --git a/proxmox-boot/proxmox-boot-cleanup.service b/proxmox-boot/proxmox-boot-cleanup.service
> new file mode 100644
> index 0000000..3c390de
> --- /dev/null
> +++ b/proxmox-boot/proxmox-boot-cleanup.service
> @@ -0,0 +1,15 @@
> +[Unit]
> +Description=Clean up bootloader next-boot setting
> +After=systemd-remount-fs.service
> +ConditionPathExists=/etc/kernel/next-boot-active
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStart=/usr/sbin/proxmox-boot-tool kernel unpin
> +ExecStart=/usr/sbin/proxmox-boot-tool refresh
> +ExecStart=/bin/rm -f /etc/kernel/next-boot-active
> +
> +[Install]
> +WantedBy=multi-user.target
> +
> -- 
> 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 3/5] proxmox-boot: fix #3671 add pin/unpin for kernel-version

[Fabian Grünbichler]

On January 31, 2022 6:59 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)

one thing I noticed while playing around - the following sequence of 
actions is a bit surprising:

- pin (old) version FOO
- refresh
- ... (long time, different admin, ..)
- apt remove pve-kernel-$FOO

while this prints

 No linux-image /boot/vmlinuz-$FOO found - skipping

this is kind of hard to understand without knowing about p-b-t internals,
skipping here means we don't copy the kernel/initrd from /boot to the 
ESP (since there is no source). now the $FOO kernel (and initrd) are on 
the ESPs, but not in /boot. since the package is no longer installed, 
future ABI-compatible upgrades are not installed, and the initrd is 
never regenerated when triggered by other factors.

worse, if I pinned that kernel for important reasons (e.g., HW-compat), 
removing the pin (via unpin, pinning another version, or next-boot to 
try whether an updated kernel improves the situation!) will remove the 
only copy of it..

I am not sure what we can do here (except making the message more 
prominent?) - failing apt is ugly, removing the kernel on the ESP when 
removing it from /boot despite it being pinned only makes it worse..

OTOH since a pinned kernel is by definition never auto-removed, hooking 
into the APT hook might work since that would mean the removal is never 
started, and the resulting dpkg/apt state is clean? obviously only 
possible for our kernels where we know the naming scheme, anything 
custom could still run into the issue..

> 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 /etc/default/grub 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.

did you test whether adding a snippet overriding GRUB_DEFAULT also 
works? we already do that to set the distributor for the various 
products.. creating/deleting a 

/etc/default/grub.d/y_proxmox_pinned_kernel.cfg

and (if we want to make the latter be separate from pinning, see other 
mail)

/etc/default/grub.d/z_proxmox_next_boot.cfg

seems like the cleaner approach compared to modifying the admin-managed 
/etc/default/grub ..

> * 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>

[pve-devel] applied: [PATCH pve-kernel-meta 1/5] proxmox-boot: drop unused potential_esps function

[Thomas Lamprecht]

On 31.01.22 18:59, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  proxmox-boot/zz-proxmox-boot | 5 -----
>  1 file changed, 5 deletions(-)
> 
>

applied, thanks!

[pve-devel] applied: [PATCH pve-kernel-meta 2/5] proxmox-boot: add get_first_line_from_file helper and use it

[Thomas Lamprecht]

On 31.01.22 18:59, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  proxmox-boot/functions       | 8 ++++++++
>  proxmox-boot/zz-proxmox-boot | 4 +---
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
>

applied, thanks!