From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 0B13461B05 for ; Thu, 10 Feb 2022 11:59:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 005D41BE81 for ; Thu, 10 Feb 2022 11:58:44 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 7383E1BE78 for ; Thu, 10 Feb 2022 11:58:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4BD2F419B7 for ; Thu, 10 Feb 2022 11:58:42 +0100 (CET) Date: Thu, 10 Feb 2022 11:58:34 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20220204184538.3139247-1-s.ivanov@proxmox.com> <20220204184538.3139247-3-s.ivanov@proxmox.com> In-Reply-To: <<20220204184538.3139247-3-s.ivanov@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1644489740.krkovtx8jg.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.195 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [systemd.io, proxmox.com] Subject: Re: [pve-devel] [PATCH pve-kernel-meta v2 2/4] proxmox-boot: fix #3671 add pin/unpin for kernel-version X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Feb 2022 10:59:14 -0000 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. >=20 > 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) >=20 > 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. >=20 > 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 >=20 > Tested with a BIOS and an UEFI VM with / on ZFS. >=20 > [0] https://systemd.io/BOOT_LOADER_SPECIFICATION/ >=20 > Signed-off-by: Stoiko Ivanov > --- > bin/proxmox-boot-tool | 46 ++++++++++++++++++++++++++++++++---- > proxmox-boot/functions | 37 +++++++++++++++++++++++++++++ > proxmox-boot/zz-proxmox-boot | 5 ++++ > 3 files changed, 83 insertions(+), 5 deletions(-) >=20 > 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=3D"None." > fi > + pinned_kernel=3D"$(get_first_line "$PINNED_KERNEL_CONF")" > =20 > echo "Manually selected kernels:" > echo "$manual_kernels" > echo "" > echo "Automatically selected kernels:" > echo "$boot_kernels" > + echo "" > + echo "Pinned kernel:" > + echo "${pinned_kernel:-None}" > } > =20 > usage() { > @@ -295,8 +299,8 @@ usage() { > warn " $0 init " > warn " $0 clean [--dry-run]" > warn " $0 refresh [--hook ]" > - warn " $0 kernel " > - warn " $0 kernel list" > + warn " $0 kernel " > + warn " $0 kernel " > warn " $0 status [--quiet]" > warn " $0 help" > } > @@ -318,14 +322,16 @@ help() { > echo "" > echo " refresh all configured EFI system partitions. Use --hook to o= nly run the specified hook, omit to run all." > echo "" > - echo "USAGE: $0 kernel " > + echo "USAGE: $0 kernel " > echo "" > echo " add/remove pve-kernel with ABI to list of sy= nced 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 sets it as the defau= lt 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 " echo "" echo " add/remove pve-kernel with ABI to list of sync= ed kernels, in addition to automatically selected ones." echo " NOTE: you need to manually run 'refresh' once you're finished w= ith adding/removing kernels from the list" + echi "" + echo "USAGE: $0 kernel pin " + echo " pin pve-kernel with ABI as the default entry t= o be booted." + echo " NOTE: you need to manually run 'refresh' once you're finished w= ith adding/removing/pinning kernels from the list" and then adding next-boot and unpin to this section? while pin/next-boot=20 share the argument with add/remove, they are kind of a different=20 operation? e.g., the full thing could then be: echo "" echo "USAGE: $0 kernel " echo "" echo " add/remove pve-kernel with ABI to list of synce= d kernels, in addition to automatically selected ones." echo "" echo "USAGE: $0 kernel " echo " pin pve-kernel with ABI sets the default entry = to be booted until unpinned." echo " next-boot pve-kernel with ABI sets the kernel v= ersion for the next boot." echo " NOTE: you need to manually run 'refresh' once you're finished wi= th pinning kernels" echo "" echo "USAGE: $0 kernel unpin [--next-boot]" echo "" echo " unpin removes pinned and next-boot kernel settings. Use --next-b= oot to only remove a next-boot setting." echo " NOTE: you need to manually run 'refresh' once you're finished wi= th 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=20 --next-boot' like with unpin? > echo "" > - echo "USAGE: $0 kernel list" > + echo "USAGE: $0 kernel " > echo "" > echo " list kernel versions currently selected for inclusion on ESPs= ." > + echo " unpin sets the latest kernel as the default entry (undoes a p= revious pin)" > echo "" > echo "USAGE: $0 status [--quiet]" > echo "" > @@ -392,6 +398,28 @@ status() { > fi > } > =20 > +pin_kernel() { > + ver=3D"$1" > + > + if [ -z "$ver" ]; then > + warn "E: is mandatory" > + warn "" > + exit 1 > + fi > + > + if [ ! -e "/boot/vmlinuz-$ver" ]; then > + warn "E: no kernel image found in /boot for '$ver', not setting defaul= t." > + 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=3D"/etc/kernel/proxmox-boot-uuids" > ESPTYPE=3D'c12a7328-f81f-11d2-ba4b-00a0c93ec93b' > =20 > MANUAL_KERNEL_LIST=3D"/etc/kernel/pve-efiboot-manual-kernels" > +PINNED_KERNEL_CONF=3D"/etc/kernel/proxmox-boot-pin" > =20 > MOUNTROOT=3D"${TMPDIR:-/var/tmp}/espmounts" > # relative to the ESP mountpoint > PMX_ESP_DIR=3D"EFI/proxmox" > PMX_LOADER_CONF=3D"loader/loader.conf" > +GRUB_PIN_SNIPPET=3D"/etc/default/grub.d/proxmox-kernel-pin.cfg" > =20 > # adapted from /etc/kernel/postinst.d/apt-auto-removal as present in > # debian's apt package: > @@ -21,6 +23,7 @@ PMX_LOADER_CONF=3D"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 > =20 > kernel_keep_versions() { > eval "$(apt-config shell DPKG Dir::bin::dpkg/f)" > @@ -56,6 +59,8 @@ kernel_keep_versions() { > manual_kernels=3D"$(cat "$MANUAL_KERNEL_LIST")" > fi > =20 > + pinned_kernel=3D"$(get_first_line "$PINNED_KERNEL_CONF")" > + > kernels=3D"$(cat <<-EOF > $running_version > $install_version > @@ -63,6 +68,7 @@ kernel_keep_versions() { > $latest_2_versions > $series_metapackages > $oldseries_latest_kernel > + $pinned_kernel > EOF > )" > =20 > @@ -114,3 +120,34 @@ get_first_line() { > done < "${file}" > echo "$line" > } > + > +set_grub_default() { > + kver=3D"$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=3D$(sed -rn "s/.*gnulinux-advanced-(.+)['] \{$/\1/p" \ > + /boot/grub/grub.cfg) > + entry=3D"gnulinux-advanced-${root_devid}>gnulinux-${kver}-advanced-${r= oot_devid}" > + echo "GRUB_DEFAULT=3D\"${entry}\"" > "${GRUB_PIN_SNIPPET}" > + fi > +} > + > +set_systemd_boot_default() { > + mountpoint=3D"$1" > + kver=3D"$2" > + if [ -z "${kver}" ]; then > + entry=3D"proxmox-*" > + else > + entry=3D"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=3D$(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 > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20