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 7104260693 for ; Wed, 16 Feb 2022 12:31:58 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 660563672 for ; Wed, 16 Feb 2022 12:31:58 +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 048C53669 for ; Wed, 16 Feb 2022 12:31:57 +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 D010541AA8 for ; Wed, 16 Feb 2022 12:31:50 +0100 (CET) Date: Wed, 16 Feb 2022 12:31:43 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20220214155737.1444136-1-s.ivanov@proxmox.com> <20220214155737.1444136-4-s.ivanov@proxmox.com> In-Reply-To: <<<20220214155737.1444136-4-s.ivanov@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1645010114.2oak8z910n.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.038 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 KAM_SHORT 0.001 Use of a URL Shortener for very short URL POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes 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 - Subject: Re: [pve-devel] [PATCH pve-kernel-meta 3/5] proxmox-boot: refresh based on bootloader config instead of bootmode 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: Wed, 16 Feb 2022 11:31:58 -0000 On February 14, 2022 4:57 pm, Stoiko Ivanov wrote: > ignore the current bootmode (uefi/legacy) when deciding which configs > to generate - make this decision based on the found boot loader > configs on the ESP. >=20 > Change systemd-boot to load the kernel+initrd from the ESPs root where > grub needs them. This prevents the double use of space for systems > having both boot-loaders present (since FAT does not support > symlinks). While this is against the recommendations of the > boot-loader-specification [0]) it works fine (and we deviate from [0] > by not having the machine-id in the kernel subdirs anyways). > The UEFI spec on the directory structure [1], also should be fine with > it. >=20 > Additionally adapt the output of `p-b-t status` to the new paths >=20 > [0] https://systemd.io/BOOT_LOADER_SPECIFICATION/ > [1] section 13.3.1.3 of > https://uefi.org/sites/default/files/resources/UEFI_Spec_2_9_2021_03_18.p= df >=20 > Signed-off-by: Stoiko Ivanov > --- > bin/proxmox-boot-tool | 14 +++---- > proxmox-boot/zz-proxmox-boot | 78 ++++++++++++------------------------ > 2 files changed, 32 insertions(+), 60 deletions(-) >=20 > diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool > index 5197f5b..320f49b 100755 > --- a/bin/proxmox-boot-tool > +++ b/bin/proxmox-boot-tool > @@ -396,20 +396,18 @@ _status_detail() { > =20 > result=3D"" > if [ -f "${mountpoint}/$PMX_LOADER_CONF" ]; then > - if [ ! -d "${mountpoint}/$PMX_ESP_DIR" ]; then > - warn "${path}/$PMX_ESP_DIR does not exist" > - fi > - versions_uefi=3D$(ls -1 ${mountpoint}/$PMX_ESP_DIR | awk '{printf (NR>= 1?", ":"") $0}') > - result=3D"uefi (versions: ${versions_uefi})" > + result=3D"uefi" > fi > if [ -d "${mountpoint}/grub" ]; then > - versions_grub=3D$(ls -1 ${mountpoint}/vmlinuz-* | awk '{ gsub(/.*\/vml= inuz-/, ""); printf (NR>1?", ":"") $0 }') > if [ -n "$result" ]; then > - result=3D"${result}, grub (versions: ${versions_grub})" > + result=3D"${result}, grub" > else > - result=3D"grub (versions: ${versions_grub})" > + result=3D"grub" this here > fi > fi > + versions=3D$(find "${mountpoint}" -maxdepth 1 -name 'vmlinuz-*' | awk '= { gsub(/.*\/vmlinuz-/, ""); printf (NR>1?", ":"") $0 }') > + result=3D"${result} (versions: ${versions})" > + > echo "$curr_uuid is configured with: $result" > umount "${mountpoint}" || \ > { warn "umount of ${path} failed - failure"; exit 0; } > diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot > index 5fe16a6..244373d 100755 > --- a/proxmox-boot/zz-proxmox-boot > +++ b/proxmox-boot/zz-proxmox-boot > @@ -75,33 +75,33 @@ update_esp_func() { > { warn "creation of mountpoint ${mountpoint} failed - skipping"; retur= n; } > mount "${path}" "${mountpoint}" || \ > { warn "mount of ${path} failed - skipping"; return; } > - if [ -d /sys/firmware/efi ]; then > - if [ ! -f "${mountpoint}/$PMX_LOADER_CONF" ]; then > - warn "${path} contains no loader.conf - skipping" > - return > - fi > - if [ ! -d "${mountpoint}/$PMX_ESP_DIR" ]; then > - warn "${path}/$PMX_ESP_DIR does not exist- skipping" > - return > - fi > - elif [ ! -d "${mountpoint}/grub" ]; then > - warn "${path} contains no grub directory - skipping" > + sd_boot=3D"" > + grub=3D"" > + if [ -f "${mountpoint}/$PMX_LOADER_CONF" ]; then > + sd_boot=3D1 > + fi > + if [ -d "${mountpoint}/grub" ]; then > + grub=3D1 > + fi and this here looks rather similar - maybe it makes sense to have=20 helpers for 'ESP contains grub' and 'ESP contains sd-boot' before we add=20 more copies? ;) > + if [ -z "$sd_boot" ] && [ -z "$grub" ]; then > + warn "${path} contains no bootloader config - skipping!" > return > fi > warn "Copying and configuring kernels on ${path}" > - copy_and_config_kernels "${mountpoint}" > + copy_and_config_kernels "${mountpoint}" "${sd_boot}" > =20 > pinned_kernel=3D$(get_first_line "${PINNED_KERNEL_CONF}") > =20 > if [ -e "${NEXT_BOOT_PIN}" ]; then > pinned_kernel=3D$(get_first_line "${NEXT_BOOT_PIN}") > fi > - if [ -d /sys/firmware/efi ]; then > + if [ -n "$sd_boot" ]; then > set_systemd_boot_default "${mountpoint}" "${pinned_kernel}" > - remove_old_kernels_efi "${mountpoint}" > - else > + remove_old_kernels "${mountpoint}" > + fi > + if [ -n "$grub" ]; then > set_grub_default "${pinned_kernel}" > - remove_old_kernels_legacy "${mountpoint}" > + remove_old_kernels "${mountpoint}" > mount --bind "${mountpoint}" "/boot" > update-grub > umount /boot > @@ -116,7 +116,7 @@ update_esp_func() { > =20 > copy_and_config_kernels() { > esp=3D"$1" > - > + sd_boot=3D"$2" > =20 > for kver in ${BOOT_KVERS}; do > =20 > @@ -132,52 +132,25 @@ copy_and_config_kernels() { > continue > fi > =20 > - if [ -d /sys/firmware/efi ]; then > - > - warn " Copying kernel and creating boot-entry for ${kver}" > - KERNEL_ESP_DIR=3D"${PMX_ESP_DIR}/${kver}" > - KERNEL_LIVE_DIR=3D"${esp}/${KERNEL_ESP_DIR}" > - mkdir -p "${KERNEL_LIVE_DIR}" > - cp --preserve=3Dtimestamps "${linux_image}" "${KERNEL_LIVE_DIR}/" > - cp --preserve=3Dtimestamps "${initrd}" "${KERNEL_LIVE_DIR}/" > + warn " Copying kernel and creating boot-entry for ${kver}" nit: technically only true for the uefi/sd_boot if below > + cp --preserve=3Dtimestamps "${linux_image}" "${esp}/" > + cp --preserve=3Dtimestamps "${initrd}" "${esp}/" > =20 > - # create loader entry > + if [ -n "$sd_boot" ]; then > + # create loader entry > cat > "${esp}/loader/entries/proxmox-${kver}.conf" <<- EOF > title ${LOADER_TITLE} > version ${kver} > options ${CMDLINE} > - linux /${KERNEL_ESP_DIR}/vmlinuz-${kver} > - initrd /${KERNEL_ESP_DIR}/initrd.img-${kver} > + linux /vmlinuz-${kver} > + initrd /initrd.img-${kver} > EOF > - else > - warn " Copying kernel ${kver}" > - cp --preserve=3Dtimestamps "${linux_image}" "${esp}/" > - cp --preserve=3Dtimestamps "${initrd}" "${esp}/" > - fi > - done > - > -} > - > -remove_old_kernels_efi() { > - esp=3D"$1" > - > - for kerneldir in "${esp}/${PMX_ESP_DIR}"/*; do > - if [ ! -d "${kerneldir}" ]; then > - warn " ${kerneldir} is not a directory - skipping" > - continue > fi > - > - kver=3D"$(echo "${kerneldir}" | sed -r "s#^${esp}/${PMX_ESP_DIR}/(.+)\= $#\\1#")" > - > - echo "${BOOT_KVERS}" | grep -q "${kver}" && continue; > - warn " Removing old version ${kver}" > - rm -rf "${kerneldir}" > - rm -f "${esp}/loader/entries/proxmox-${kver}.conf" > done this still needs to be done at least once per ESP post-upgrade, else all=20 the (now unused) kernels+initrds in the old efi location are kept around=20 forever, taking up precious space on our small ESPs ;) maybe replace it with a simple if dir_exists; then rm -rf ...; fi ? > =20 > } > =20 > -remove_old_kernels_legacy() { > +remove_old_kernels() { > esp=3D"$1" > =20 > for kernel in "${esp}/"vmlinuz-*; do > @@ -187,6 +160,7 @@ remove_old_kernels_legacy() { > warn " Removing old version ${kver}" > rm -rf "${esp}/vmlinuz-${kver}" > rm -rf "${esp}/initrd.img-${kver}" > + rm -rf "${esp}/loader/entries/proxmox-${kver}.conf" > done > =20 > } > --=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