[pve-devel] [PATCH pve-kernel-meta v2 2/4] proxmox-boot: fix #3671 add pin/unpin for kernel-version
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Feb 10 11:58:34 CET 2022
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 at 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 at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
More information about the pve-devel
mailing list