[pve-devel] [PATCH pve-kernel-meta/pve-installer] boot ZFS on legacy BIOS systems from vfat

[pve-devel] [PATCH pve-kernel-meta/pve-installer] boot ZFS on legacy BIOS systems from vfat

[Stoiko Ivanov]

the pve-kernel-meta patches were prepared on top of the pve-kernel-5.4 branch
rfc->v1:
* Thanks to Fabian's great feedback this version handles a few cases I did not
  think of for the RFC (e.g. an update to grub which runs grub-install and
  removes the core.img/stage0 pointing to the ESP (instead of the zfs pool)
* pve-kernel-helper now adds a diversion (dpkg-divert(1)) to grub-install,
  which should prevent users/grub-upgrades accidentally overwriting grub
  with a config which tries to boot from the zpool
* both `update-grub` and the config it generates adds a banner/warning if
  proxmox-boot is used (notifying the user where the correct place for editing
  is)
* the renaming from pve-efiboot-tool to proxmox-boot-tool was carried further
  to also include all hooks and snippets
* a first version of a patch for pve-installer was added (and very roughly
  tested)

original cover-letter for the RFC:
This patchset has been long overdue, and complements the solution to booting
ZFS on UEFI systems using systemd-boot.

With the upgrade of ZFS 2.0.0 (and it's support for ZSTD compression), quite
a few users found out that their systems were still booted with legacy bios
boot and were consequently rendered unbootable with enabling zstd
compression on (a dataset on rpool).

The solution is inspired by our community-forum, especially @avw, and seems
rather lightweight (patch 2/2 is best viewed with '-w').
My first approach was to generate a working grub-config ourselves, but
given that grub has a few years of handling special cases - bind-mounting
the ESP on /boot and running 'update-grub' seems like a less painful way.

* patch 1/2 renames pve-efiboot-tool to proxmox-boot-tool (which seems more
appropriate by now)
* patch 2/2 adds support for installing grub appropriately on the ESPs
  and running the kernel sync-logic in a way that update-grub feels fine
  with

Sending as RFC, because this is a proof-of-concept and missing quite a few
things.

What works:
* installing this version on a root ZFS RAID-Z@ PVE (based on an old pre 6.2
  install)
* reformatting all 4 ESPs `proxmox-boot-tool format /dev/sda2 --force)
* initializing them
* rebooting into 5.4.106 and zfs 2.0.0
* upgrading the pool, setting compression=zstd, writing a file, rebooting
  (successfully)
* rebooting into an old 5.3 kernel - and getting greeted by busy-box instead
  of grub-rescue

What's missing (at least):
* support in the installer
* the renaming is not quite through (the kernel-hooks are still containing
  pve/efi in their name)
* testing the removal part of the$kernel-sync

pve-kernel-meta:
Stoiko Ivanov (7):
  proxmox-boot-tool: rename from pve-efiboot-tool
  proxmox-boot-tool: add status command
  proxmox-boot-tool: sort and remove duplicates on clean
  proxmox-boot: rename uuid list file
  proxmox-boot-tool: handle legacy boot zfs installs
  proxmox-boot: add grub.cfg header snippet
  proxmox-boot: add grub-install wrapper

 Makefile                                      |  2 +-
 bin/Makefile                                  |  3 +-
 bin/grub-install-wrapper                      | 12 +++
 bin/{pve-efiboot-tool => proxmox-boot-tool}   | 52 +++++++++---
 debian/pve-kernel-helper.install              |  5 +-
 debian/pve-kernel-helper.links                |  1 +
 debian/pve-kernel-helper.postinst             | 10 +++
 debian/pve-kernel-helper.postrm               | 14 ++++
 debian/pve-kernel-helper.preinst              |  9 +++
 proxmox-boot/000_proxmox_boot_header          | 22 +++++
 {efiboot => proxmox-boot}/Makefile            |  8 +-
 {efiboot => proxmox-boot}/functions           |  2 +-
 .../proxmox-auto-removal                      |  0
 .../proxmox-boot-sync                         |  2 +-
 .../zz-proxmox-boot                           | 81 +++++++++++++------
 15 files changed, 182 insertions(+), 41 deletions(-)
 create mode 100755 bin/grub-install-wrapper
 rename bin/{pve-efiboot-tool => proxmox-boot-tool} (88%)
 create mode 100644 debian/pve-kernel-helper.links
 create mode 100644 debian/pve-kernel-helper.postinst
 create mode 100644 debian/pve-kernel-helper.postrm
 create mode 100644 debian/pve-kernel-helper.preinst
 create mode 100755 proxmox-boot/000_proxmox_boot_header
 rename {efiboot => proxmox-boot}/Makefile (72%)
 rename {efiboot => proxmox-boot}/functions (98%)
 rename efiboot/pve-auto-removal => proxmox-boot/proxmox-auto-removal (100%)
 rename efiboot/pve-efiboot-sync => proxmox-boot/proxmox-boot-sync (84%)
 rename efiboot/zz-pve-efiboot => proxmox-boot/zz-proxmox-boot (69%)

pve-installer:
Stoiko Ivanov (1):
  always boot zfs with proxmox-boot-tool

 proxinstall | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH pve-kernel-meta 1/7] proxmox-boot-tool: rename from pve-efiboot-tool

[Stoiko Ivanov]

We will be using the mechanics also for ZFS systems booting with BIOS
legacy boot, and the tool is used also in PMG and PBS.

A symlink is kept in place for compatibility reasons

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 Makefile                                                      | 2 +-
 bin/Makefile                                                  | 2 +-
 bin/{pve-efiboot-tool => proxmox-boot-tool}                   | 2 +-
 debian/pve-kernel-helper.install                              | 4 ++--
 debian/pve-kernel-helper.links                                | 1 +
 {efiboot => proxmox-boot}/Makefile                            | 4 ++--
 {efiboot => proxmox-boot}/functions                           | 0
 efiboot/pve-auto-removal => proxmox-boot/proxmox-auto-removal | 0
 efiboot/pve-efiboot-sync => proxmox-boot/proxmox-boot-sync    | 2 +-
 efiboot/zz-pve-efiboot => proxmox-boot/zz-proxmox-boot        | 0
 10 files changed, 9 insertions(+), 8 deletions(-)
 rename bin/{pve-efiboot-tool => proxmox-boot-tool} (99%)
 create mode 100644 debian/pve-kernel-helper.links
 rename {efiboot => proxmox-boot}/Makefile (87%)
 rename {efiboot => proxmox-boot}/functions (100%)
 rename efiboot/pve-auto-removal => proxmox-boot/proxmox-auto-removal (100%)
 rename efiboot/pve-efiboot-sync => proxmox-boot/proxmox-boot-sync (84%)
 rename efiboot/zz-pve-efiboot => proxmox-boot/zz-proxmox-boot (100%)

diff --git a/Makefile b/Makefile
index 0b62b3e..90d5989 100644
--- a/Makefile
+++ b/Makefile
@@ -13,7 +13,7 @@ BUILD_DIR=build
 
 DEBS=${KERNEL_DEB} ${HEADERS_DEB} ${HELPER_DEB}
 
-SUBDIRS = efiboot bin
+SUBDIRS = proxmox-boot bin
 
 .PHONY: all
 all: ${SUBDIRS}
diff --git a/bin/Makefile b/bin/Makefile
index 058c86f..b78fa42 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -5,7 +5,7 @@ all:
 
 install:
 	install -d ${SBINDIR}
-	install -m 0755 pve-efiboot-tool ${SBINDIR}/
+	install -m 0755 proxmox-boot-tool ${SBINDIR}/
 
 .PHONY: clean distclean
 distclean:
diff --git a/bin/pve-efiboot-tool b/bin/proxmox-boot-tool
similarity index 99%
rename from bin/pve-efiboot-tool
rename to bin/proxmox-boot-tool
index f57a752..2d625a6 100755
--- a/bin/pve-efiboot-tool
+++ b/bin/proxmox-boot-tool
@@ -199,7 +199,7 @@ clean() {
 
 refresh() {
 	hook=$1
-	hookscripts='pve-auto-removal zz-pve-efiboot'
+	hookscripts='proxmox-auto-removal zz-proxmox-boot'
 
 	if [ -n "$hook" ]; then
 	    if echo "$hookscripts" | grep -sqE "(^|[[:space:]]+)$hook([[:space:]]+|$)"; then
diff --git a/debian/pve-kernel-helper.install b/debian/pve-kernel-helper.install
index 6f7f713..aae9494 100644
--- a/debian/pve-kernel-helper.install
+++ b/debian/pve-kernel-helper.install
@@ -1,5 +1,5 @@
 etc/kernel/postinst.d/*
 etc/kernel/postrm.d/*
-etc/initramfs/post-update.d/pve-efiboot-sync
-usr/sbin/pve-efiboot-tool
+etc/initramfs/post-update.d/proxmox-boot-sync
+usr/sbin/proxmox-boot-tool
 usr/share/pve-kernel-helper/scripts/functions
diff --git a/debian/pve-kernel-helper.links b/debian/pve-kernel-helper.links
new file mode 100644
index 0000000..70bf372
--- /dev/null
+++ b/debian/pve-kernel-helper.links
@@ -0,0 +1 @@
+/usr/sbin/proxmox-boot-tool /usr/sbin/pve-efiboot-tool
diff --git a/efiboot/Makefile b/proxmox-boot/Makefile
similarity index 87%
rename from efiboot/Makefile
rename to proxmox-boot/Makefile
index fc9e333..3a36cb7 100644
--- a/efiboot/Makefile
+++ b/proxmox-boot/Makefile
@@ -1,5 +1,5 @@
-KERNEL_HOOKSCRIPTS = pve-auto-removal zz-pve-efiboot
-INITRAMFS_HOOKSCRIPTS = pve-efiboot-sync
+KERNEL_HOOKSCRIPTS = proxmox-auto-removal zz-proxmox-boot
+INITRAMFS_HOOKSCRIPTS = proxmox-boot-sync
 SHARE_FILES = functions
 
 POSTINSTHOOKDIR = ${DESTDIR}/etc/kernel/postinst.d
diff --git a/efiboot/functions b/proxmox-boot/functions
similarity index 100%
rename from efiboot/functions
rename to proxmox-boot/functions
diff --git a/efiboot/pve-auto-removal b/proxmox-boot/proxmox-auto-removal
similarity index 100%
rename from efiboot/pve-auto-removal
rename to proxmox-boot/proxmox-auto-removal
diff --git a/efiboot/pve-efiboot-sync b/proxmox-boot/proxmox-boot-sync
similarity index 84%
rename from efiboot/pve-efiboot-sync
rename to proxmox-boot/proxmox-boot-sync
index c3ccf8e..5bdd72e 100644
--- a/efiboot/pve-efiboot-sync
+++ b/proxmox-boot/proxmox-boot-sync
@@ -7,5 +7,5 @@ set -e
 # this variable will be set to 1 and we do nothing, since our pve-kernel
 # hooks will update the ESPs all at once anyway.
 if [ -z "$INITRAMFS_TOOLS_KERNEL_HOOK" ]; then
-	/usr/sbin/pve-efiboot-tool refresh --hook 'zz-pve-efiboot'
+	/usr/sbin/proxmox-boot-tool refresh --hook 'zz-proxmox-boot'
 fi
diff --git a/efiboot/zz-pve-efiboot b/proxmox-boot/zz-proxmox-boot
similarity index 100%
rename from efiboot/zz-pve-efiboot
rename to proxmox-boot/zz-proxmox-boot
-- 
2.20.1

[pve-devel] [PATCH pve-kernel-meta 2/7] proxmox-boot-tool: add status command

[Stoiko Ivanov]

currently simply checking if $ESP_LIST exists, and indicating via
the exit status if proxmox-boot-tool is used for booting the system.

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

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 2d625a6..7b06d57 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -283,6 +283,7 @@ usage() {
 	warn "       $0 refresh [--hook <name>]"
 	warn "       $0 kernel <add|remove> <kernel-version>"
 	warn "       $0 kernel list"
+	warn "       $0 status"
 	warn "       $0 help"
 }
 
@@ -312,6 +313,20 @@ help() {
 	echo ""
 	echo "    list kernel versions currently selected for inclusion on ESPs."
 	echo ""
+	echo "USAGE: $0 status [--verbose]"
+	echo ""
+	echo "    Exit with 0 if any ESP is configured, else with 2."
+	echo ""
+}
+
+status() {
+	verbose="$1"
+	if [ ! -e ${ESP_LIST} ]; then
+		if [ -n "$verbose" ]; then
+		    warn "E: $ESP_LIST does not exist."
+		fi
+		exit 2
+	fi
 }
 
 if [ -z "$1" ]; then
@@ -390,6 +405,11 @@ case "$1" in
 			;;
 		esac
 	;;
+	'status')
+		shift
+		status "$1"
+		exit 0
+	;;
 	'help')
 		shift
 		help
-- 
2.20.1

[pve-devel] [PATCH pve-kernel-meta 3/7] proxmox-boot-tool: sort and remove duplicates on clean

[Stoiko Ivanov]

This is mostly in preparation for renaming pve-efiboot-uuids into
proxmox-boot-uuids, but can help in general (since each duplicate uuid
causes excessive disk i/o upon kernel upgrades).

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

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 7b06d57..df80c73 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -195,6 +195,10 @@ clean() {
 	if [ -e "$ESP_LIST".tmp ]; then
 		mv "$ESP_LIST".tmp "$ESP_LIST"
 	fi
+
+	echo "Sorting and removing duplicate ESPs.."
+	sort -uo "$ESP_LIST".tmp "$ESP_LIST"
+	mv "$ESP_LIST".tmp "$ESP_LIST"
 }
 
 refresh() {
-- 
2.20.1

[pve-devel] [PATCH pve-kernel-meta 4/7] proxmox-boot: rename uuid list file

[Stoiko Ivanov]

in order to be consistent with the renaming of pve-efiboot-tool to
proxmox-boot-tool.

Sending as separate patch, since it changes and removes a file in
'/etc', which could be considered part of the external 'api' of
proxmox-boot-tool

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 debian/pve-kernel-helper.postinst | 10 ++++++++++
 proxmox-boot/functions            |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)
 create mode 100644 debian/pve-kernel-helper.postinst

diff --git a/debian/pve-kernel-helper.postinst b/debian/pve-kernel-helper.postinst
new file mode 100644
index 0000000..5299d26
--- /dev/null
+++ b/debian/pve-kernel-helper.postinst
@@ -0,0 +1,10 @@
+#! /bin/sh
+
+set -e
+
+if [ -e /etc/kernel/pve-efiboot-uuids ]; then
+	echo "$0: legacy ESP list /etc/kernel/pve-efiboot-uuids found merging and cleaning." 1>&2
+	cat /etc/kernel/pve-efiboot-uuids >> /etc/kernel/proxmox-boot-uuids
+	proxmox-boot-tool clean
+	rm -f /etc/kernel/pve-efiboot-uuids
+fi
diff --git a/proxmox-boot/functions b/proxmox-boot/functions
index 72fe15d..04bdc51 100755
--- a/proxmox-boot/functions
+++ b/proxmox-boot/functions
@@ -1,7 +1,7 @@
 #! /bin/sh
 set -e
 
-ESP_LIST="/etc/kernel/pve-efiboot-uuids"
+ESP_LIST="/etc/kernel/proxmox-boot-uuids"
 ESPTYPE='c12a7328-f81f-11d2-ba4b-00a0c93ec93b'
 
 MANUAL_KERNEL_LIST="/etc/kernel/pve-efiboot-manual-kernels"
-- 
2.20.1

[pve-devel] [PATCH pve-kernel-meta 5/7] proxmox-boot-tool: handle legacy boot zfs installs

[Stoiko Ivanov]

This patch adds support for booting non-uefi/legacy/bios-boot ZFS
installs, by using proxmox-boot-tool to copy the kernels to the ESP
and then generate a fitting grub config for booting from the vfat ESP:

* grub is installed onto the ESP and the MBR points to the ESP
* after copying/deleting the kernels proxmox-boot-tool bindmounts the
  ESP on /boot (inside the new mount namespace)
* grub-update then manages to generate a fitting config.

Some paths/sanity-checks needed adaptation (to differentiate between
EFI boot and not (based on the existence of /sys/firmware/efi)

The arguments for grub-install are taken from the pve-installer.

The approach is inspired by @avw in our community-forum [0].

[0] https://forum.proxmox.com/threads/zfs-error-no-such-device-error-unknown-filesystem-entering-rescue-mode.75122/post-374799

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
best viewed with `git show -w`
 bin/proxmox-boot-tool        | 26 ++++++++----
 proxmox-boot/zz-proxmox-boot | 81 +++++++++++++++++++++++++-----------
 2 files changed, 75 insertions(+), 32 deletions(-)

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index df80c73..8baa577 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -150,14 +150,24 @@ init() {
 	echo "Mounting '$part' on '$esp_mp'."
 	mount -t vfat "$part" "$esp_mp"
 
-	echo "Installing systemd-boot.."
-	mkdir -p "$esp_mp/$PMX_ESP_DIR"
-	bootctl --path "$esp_mp" install
-
-	echo "Configuring systemd-boot.."
-	echo "timeout 3" > "$esp_mp/$PMX_LOADER_CONF.tmp"
-	echo "default proxmox-*" >> "$esp_mp/$PMX_LOADER_CONF.tmp"
-	mv "$esp_mp/$PMX_LOADER_CONF.tmp" "$esp_mp/$PMX_LOADER_CONF"
+	if [ -d /sys/firmware/efi ]; then
+		echo "Installing systemd-boot.."
+		mkdir -p "$esp_mp/$PMX_ESP_DIR"
+		bootctl --path "$esp_mp" install
+
+		echo "Configuring systemd-boot.."
+		echo "timeout 3" > "$esp_mp/$PMX_LOADER_CONF.tmp"
+		echo "default proxmox-*" >> "$esp_mp/$PMX_LOADER_CONF.tmp"
+		mv "$esp_mp/$PMX_LOADER_CONF.tmp" "$esp_mp/$PMX_LOADER_CONF"
+	else
+		echo "Installing grub i386-pc target.."
+		grub-install \
+			--boot-directory $esp_mp \
+			--target i386-pc \
+			--no-floppy \
+			--bootloader-id='proxmox' \
+			"/dev/$PKNAME"
+	fi
 	echo "Unmounting '$part'."
 	umount "$part"
 
diff --git a/proxmox-boot/zz-proxmox-boot b/proxmox-boot/zz-proxmox-boot
index 1c4ad73..019f3ad 100755
--- a/proxmox-boot/zz-proxmox-boot
+++ b/proxmox-boot/zz-proxmox-boot
@@ -76,18 +76,30 @@ update_esp_func() {
 		{ warn "creation of mountpoint ${mountpoint} failed - skipping"; return; }
 	mount "${path}" "${mountpoint}" || \
 		{ warn "mount of ${path} failed - skipping"; return; }
-	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"
+	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"
 		return
 	fi
-
 	warn "Copying and configuring kernels on ${path}"
 	copy_and_config_kernels "${mountpoint}"
-	remove_old_kernels "${mountpoint}"
+	if [ -d /sys/firmware/efi ]; then
+		remove_old_kernels_efi "${mountpoint}"
+	else
+		remove_old_kernels_legacy "${mountpoint}"
+		mount --bind "${mountpoint}" "/boot"
+		update-grub
+		umount /boot
+
+	fi
 
 	umount "${mountpoint}" || \
 		{ warn "umount of ${path} failed - failure"; exit 0; }
@@ -113,26 +125,33 @@ copy_and_config_kernels() {
 			continue
 		fi
 
-		warn "	Copying kernel and creating boot-entry for ${kver}"
-		KERNEL_ESP_DIR="${PMX_ESP_DIR}/${kver}"
-		KERNEL_LIVE_DIR="${esp}/${KERNEL_ESP_DIR}"
-		mkdir -p "${KERNEL_LIVE_DIR}"
-		cp -u --preserve=timestamps "${linux_image}" "${KERNEL_LIVE_DIR}/"
-		cp -u --preserve=timestamps "${initrd}" "${KERNEL_LIVE_DIR}/"
-
-		# 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}
-		EOF
+		if [ -d /sys/firmware/efi ]; then
+
+			warn "	Copying kernel and creating boot-entry for ${kver}"
+			KERNEL_ESP_DIR="${PMX_ESP_DIR}/${kver}"
+			KERNEL_LIVE_DIR="${esp}/${KERNEL_ESP_DIR}"
+			mkdir -p "${KERNEL_LIVE_DIR}"
+			cp -u --preserve=timestamps "${linux_image}" "${KERNEL_LIVE_DIR}/"
+			cp -u --preserve=timestamps "${initrd}" "${KERNEL_LIVE_DIR}/"
+
+			# 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}
+			EOF
+		else
+			warn "	Copying kernel ${kver}"
+			cp -u --preserve=timestamps "${linux_image}" "${esp}/"
+			cp -u --preserve=timestamps "${initrd}" "${esp}/"
+		fi
 	done
 
 }
 
-remove_old_kernels() {
+remove_old_kernels_efi() {
 	esp="$1"
 
 	for kerneldir in "${esp}/${PMX_ESP_DIR}"/*; do
@@ -151,6 +170,20 @@ remove_old_kernels() {
 
 }
 
+remove_old_kernels_legacy() {
+	esp="$1"
+
+	for kernel in "${esp}/"vmlinuz-*; do
+		kver="$(echo "${kernel}" | sed -r "s#^${esp}/vmlinuz-(.+)\$#\\1#")"
+
+		echo "${BOOT_KVERS}" | grep -q "${kver}" && continue;
+		warn "	Removing old version ${kver}"
+		rm -rf "${esp}/vmlinuz-${kver}"
+		rm -rf "${esp}/initrd.img-${kver}"
+	done
+
+}
+
 set -- $DEB_MAINT_PARAMS
 mode="${1#\'}"
 mode="${mode%\'}"
-- 
2.20.1

[pve-devel] [PATCH pve-kernel-meta 6/7] proxmox-boot: add grub.cfg header snippet

[Stoiko Ivanov]

If the system seems to be booted using proxmox-boot, write a header at
the beginning of the grub.cfg generated when running `update-grub`

Additionally print a warning in case the script is run interactively.
This is determined by checking for DPKG_VERSION, which is set when
running as post-inst task (after a kernel install/removal)
and for PVE_EFIBOOT_UNSHARED, which is set by proxmox-boot-tool when
running `proxmox-boot-tool refresh.`

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 proxmox-boot/000_proxmox_boot_header | 22 ++++++++++++++++++++++
 proxmox-boot/Makefile                |  4 ++++
 2 files changed, 26 insertions(+)
 create mode 100755 proxmox-boot/000_proxmox_boot_header

diff --git a/proxmox-boot/000_proxmox_boot_header b/proxmox-boot/000_proxmox_boot_header
new file mode 100755
index 0000000..57dae2c
--- /dev/null
+++ b/proxmox-boot/000_proxmox_boot_header
@@ -0,0 +1,22 @@
+#! /bin/sh
+set -e
+
+. /usr/share/pve-kernel-helper/scripts/functions
+
+if proxmox-boot-tool status; then
+	cat <<- EOF
+	#
+	# This system is booted via proxmox-boot-tool! The grub-config used when
+	# booting from the disks configured with proxmox-boot-tool resides on the vfat
+	# partitions with UUIDs listed in ${ESP_LIST}.
+	# /boot/grub/grub.cfg is NOT read when booting from those disk!
+	EOF
+
+	if [ -z "$DPKG_VERSION" ] && [ -z "$PVE_EFIBOOT_UNSHARED" ]; then
+		warn "W: This system is booted via proxmox-boot-tool:"
+		warn "W: Running update-grub does not update the correct config!"
+		warn "W: Run 'proxmox-boot-tool refresh' instead."
+		warn ""
+	fi
+fi
+
diff --git a/proxmox-boot/Makefile b/proxmox-boot/Makefile
index 3a36cb7..effd726 100644
--- a/proxmox-boot/Makefile
+++ b/proxmox-boot/Makefile
@@ -1,11 +1,13 @@
 KERNEL_HOOKSCRIPTS = proxmox-auto-removal zz-proxmox-boot
 INITRAMFS_HOOKSCRIPTS = proxmox-boot-sync
 SHARE_FILES = functions
+GRUB_CFG_SNIPPET = 000_proxmox_boot_header
 
 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
 
 .PHONY: all
 all:
@@ -19,6 +21,8 @@ install:
 	install -m 0755 ${INITRAMFS_HOOKSCRIPTS} ${POSTINITRAMFSHOOKDIR}
 	install -d ${SHARE_SCRIPTDIR}
 	install -m 0755 ${SHARE_FILES} ${SHARE_SCRIPTDIR}
+	install -d ${GRUB_CFG_DIR}
+	install -m 0755 ${GRUB_CFG_SNIPPET} ${GRUB_CFG_DIR}
 
 .PHONY: clean distclean
 distclean:
-- 
2.20.1

[pve-devel] [PATCH pve-kernel-meta 7/7] proxmox-boot: add grub-install wrapper

[Stoiko Ivanov]

if a (legacy) system is booted with proxmox-boot-tool, running
`grub-install` without being aware of the fact can render the system
unbootable (e.g. when letting the early stage point to an incompatible
zpool instead of the ESP).

To prevent this we add a dpkg-diversion [0], which simply checks if
`proxmox-boot-tool status` indicates that proxmox-boot is used and
errors out in that case, and runs the actual grub-install else.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 bin/Makefile                     |  1 +
 bin/grub-install-wrapper         | 12 ++++++++++++
 bin/proxmox-boot-tool            |  2 +-
 debian/pve-kernel-helper.install |  1 +
 debian/pve-kernel-helper.postrm  | 14 ++++++++++++++
 debian/pve-kernel-helper.preinst |  9 +++++++++
 6 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100755 bin/grub-install-wrapper
 create mode 100644 debian/pve-kernel-helper.postrm
 create mode 100644 debian/pve-kernel-helper.preinst

diff --git a/bin/Makefile b/bin/Makefile
index b78fa42..2e18342 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -6,6 +6,7 @@ all:
 install:
 	install -d ${SBINDIR}
 	install -m 0755 proxmox-boot-tool ${SBINDIR}/
+	install -m 0755 grub-install-wrapper ${SBINDIR}/grub-install
 
 .PHONY: clean distclean
 distclean:
diff --git a/bin/grub-install-wrapper b/bin/grub-install-wrapper
new file mode 100755
index 0000000..d9ef9cc
--- /dev/null
+++ b/bin/grub-install-wrapper
@@ -0,0 +1,12 @@
+#! /bin/sh
+set -e
+
+. /usr/share/pve-kernel-helper/scripts/functions
+
+if proxmox-boot-tool status; then
+	warn "grub-install is disabled because this system is booted via proxmox-boot-tool, if you really need to run it, run /usr/sbin/grub-install.real"
+	exit 1
+else
+	grub-install.real "$@"
+fi
+
diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 8baa577..bf9af7c 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -161,7 +161,7 @@ init() {
 		mv "$esp_mp/$PMX_LOADER_CONF.tmp" "$esp_mp/$PMX_LOADER_CONF"
 	else
 		echo "Installing grub i386-pc target.."
-		grub-install \
+		grub-install.real \
 			--boot-directory $esp_mp \
 			--target i386-pc \
 			--no-floppy \
diff --git a/debian/pve-kernel-helper.install b/debian/pve-kernel-helper.install
index aae9494..e410ce7 100644
--- a/debian/pve-kernel-helper.install
+++ b/debian/pve-kernel-helper.install
@@ -2,4 +2,5 @@ etc/kernel/postinst.d/*
 etc/kernel/postrm.d/*
 etc/initramfs/post-update.d/proxmox-boot-sync
 usr/sbin/proxmox-boot-tool
+usr/sbin/grub-install
 usr/share/pve-kernel-helper/scripts/functions
diff --git a/debian/pve-kernel-helper.postrm b/debian/pve-kernel-helper.postrm
new file mode 100644
index 0000000..98e5550
--- /dev/null
+++ b/debian/pve-kernel-helper.postrm
@@ -0,0 +1,14 @@
+#! /bin/sh
+
+set -e
+
+if [ remove = "$1" ] || [ abort-install = "$1" ] || [ disappear = "$1" ]; then
+	dpkg-divert --package pve-kernel-helper --remove --rename \
+		--divert /usr/sbin/grub-install.real /usr/sbin/grub-install
+fi
+
+if [ abort-upgrade = "$1" ] && dpkg --compare-versions "$2" lt 6.3-9; then
+	dpkg-divert --package pve-kernel-helper --remove --rename \
+		--divert /usr/sbin/grub-install.real /usr/sbin/grub-install
+fi
+
diff --git a/debian/pve-kernel-helper.preinst b/debian/pve-kernel-helper.preinst
new file mode 100644
index 0000000..d0a8f67
--- /dev/null
+++ b/debian/pve-kernel-helper.preinst
@@ -0,0 +1,9 @@
+#! /bin/sh
+
+set -e
+
+if [ upgrade != "$1" ] || dpkg --compare-versions "$2" lt 6.3-9; then
+	dpkg-divert --package pve-kernel-helper --add --rename \
+		--divert /usr/sbin/grub-install.real /usr/sbin/grub-install
+fi
+
-- 
2.20.1

[pve-devel] [PATCH installer 1/1] always boot zfs with proxmox-boot-tool

[Stoiko Ivanov]

proxmox-boot-tool now supports configuring grub to boot from the ESPs
the installer creates on all (bootable) disks - use this approach
for zfs installs.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
best viewed with `git show -w`
 proxinstall | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/proxinstall b/proxinstall
index 03f6ec7..3395fa2 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1224,11 +1224,11 @@ my sub chroot_chmod {
 	die "chroot: unable to change permission mode for '$path'\n";
 }
 
-sub prepare_systemd_boot_esp {
+sub prepare_proxmox_boot_esp {
     my ($espdev, $targetdir) = @_;
 
-    syscmd("chroot $targetdir pve-efiboot-tool init $espdev") == 0 ||
-	die "unable to init ESP and install systemd-boot loader on '$espdev'\n";
+    syscmd("chroot $targetdir proxmox-boot-tool init $espdev") == 0 ||
+	die "unable to init ESP and install proxmox-boot loader on '$espdev'\n";
 }
 
 sub prepare_grub_efi_boot_esp {
@@ -1820,19 +1820,19 @@ _EOD
 
 		foreach my $di (@$bootdevinfo) {
 		    my $dev = $di->{devname};
-		    if (!$native_4k_disk_bootable) {
-			eval {
-			    syscmd("chroot $targetdir /usr/sbin/grub-install --target i386-pc --no-floppy --bootloader-id='proxmox' $dev") == 0 ||
-				    die "unable to install the i386-pc boot loader on '$dev'\n";
-			};
-			push @$bootloader_err_list, $@ if $@;
-		    }
+		    if ($use_zfs) {
+			prepare_proxmox_boot_esp($di->{esp}, $targetdir);
+		    } else {
+			if (!$native_4k_disk_bootable) {
+			    eval {
+				syscmd("chroot $targetdir /usr/sbin/grub-install --target i386-pc --no-floppy --bootloader-id='proxmox' $dev") == 0 ||
+					die "unable to install the i386-pc boot loader on '$dev'\n";
+			    };
+			    push @$bootloader_err_list, $@ if $@;
+			}
 
 		    eval {
 			if (my $esp = $di->{esp}) {
-			    if ($use_zfs) {
-				prepare_systemd_boot_esp($esp, $targetdir);
-			    } else {
 				prepare_grub_efi_boot_esp($dev, $esp, $targetdir);
 			    }
 			}
-- 
2.20.1

Re: [pve-devel] [PATCH pve-kernel-meta 1/7] proxmox-boot-tool: rename from pve-efiboot-tool

[Fabian Grünbichler]

On April 22, 2021 1:17 pm, Stoiko Ivanov wrote:
> We will be using the mechanics also for ZFS systems booting with BIOS
> legacy boot, and the tool is used also in PMG and PBS.
> 
> A symlink is kept in place for compatibility reasons
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  Makefile                                                      | 2 +-
>  bin/Makefile                                                  | 2 +-
>  bin/{pve-efiboot-tool => proxmox-boot-tool}                   | 2 +-
>  debian/pve-kernel-helper.install                              | 4 ++--
>  debian/pve-kernel-helper.links                                | 1 +
>  {efiboot => proxmox-boot}/Makefile                            | 4 ++--
>  {efiboot => proxmox-boot}/functions                           | 0
>  efiboot/pve-auto-removal => proxmox-boot/proxmox-auto-removal | 0
>  efiboot/pve-efiboot-sync => proxmox-boot/proxmox-boot-sync    | 2 +-
>  efiboot/zz-pve-efiboot => proxmox-boot/zz-proxmox-boot        | 0
>  10 files changed, 9 insertions(+), 8 deletions(-)
>  rename bin/{pve-efiboot-tool => proxmox-boot-tool} (99%)
>  create mode 100644 debian/pve-kernel-helper.links
>  rename {efiboot => proxmox-boot}/Makefile (87%)
>  rename {efiboot => proxmox-boot}/functions (100%)
>  rename efiboot/pve-auto-removal => proxmox-boot/proxmox-auto-removal (100%)
>  rename efiboot/pve-efiboot-sync => proxmox-boot/proxmox-boot-sync (84%)
>  rename efiboot/zz-pve-efiboot => proxmox-boot/zz-proxmox-boot (100%)

these three files are installed as five conffiles, so they need special 
care when being renamed/moved, see `man dpkg-maintscript-helper`. 
alternatively we could also just unconditionally remove them on 
upgrades I guess.

otherwise the old ones will stay around on upgrades, and also be 
called..

> 
> diff --git a/Makefile b/Makefile
> index 0b62b3e..90d5989 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -13,7 +13,7 @@ BUILD_DIR=build
>  
>  DEBS=${KERNEL_DEB} ${HEADERS_DEB} ${HELPER_DEB}
>  
> -SUBDIRS = efiboot bin
> +SUBDIRS = proxmox-boot bin
>  
>  .PHONY: all
>  all: ${SUBDIRS}
> diff --git a/bin/Makefile b/bin/Makefile
> index 058c86f..b78fa42 100644
> --- a/bin/Makefile
> +++ b/bin/Makefile
> @@ -5,7 +5,7 @@ all:
>  
>  install:
>  	install -d ${SBINDIR}
> -	install -m 0755 pve-efiboot-tool ${SBINDIR}/
> +	install -m 0755 proxmox-boot-tool ${SBINDIR}/
>  
>  .PHONY: clean distclean
>  distclean:
> diff --git a/bin/pve-efiboot-tool b/bin/proxmox-boot-tool
> similarity index 99%
> rename from bin/pve-efiboot-tool
> rename to bin/proxmox-boot-tool
> index f57a752..2d625a6 100755
> --- a/bin/pve-efiboot-tool
> +++ b/bin/proxmox-boot-tool
> @@ -199,7 +199,7 @@ clean() {
>  
>  refresh() {
>  	hook=$1
> -	hookscripts='pve-auto-removal zz-pve-efiboot'
> +	hookscripts='proxmox-auto-removal zz-proxmox-boot'
>  
>  	if [ -n "$hook" ]; then
>  	    if echo "$hookscripts" | grep -sqE "(^|[[:space:]]+)$hook([[:space:]]+|$)"; then
> diff --git a/debian/pve-kernel-helper.install b/debian/pve-kernel-helper.install
> index 6f7f713..aae9494 100644
> --- a/debian/pve-kernel-helper.install
> +++ b/debian/pve-kernel-helper.install
> @@ -1,5 +1,5 @@
>  etc/kernel/postinst.d/*
>  etc/kernel/postrm.d/*
> -etc/initramfs/post-update.d/pve-efiboot-sync
> -usr/sbin/pve-efiboot-tool
> +etc/initramfs/post-update.d/proxmox-boot-sync
> +usr/sbin/proxmox-boot-tool
>  usr/share/pve-kernel-helper/scripts/functions
> diff --git a/debian/pve-kernel-helper.links b/debian/pve-kernel-helper.links
> new file mode 100644
> index 0000000..70bf372
> --- /dev/null
> +++ b/debian/pve-kernel-helper.links
> @@ -0,0 +1 @@
> +/usr/sbin/proxmox-boot-tool /usr/sbin/pve-efiboot-tool
> diff --git a/efiboot/Makefile b/proxmox-boot/Makefile
> similarity index 87%
> rename from efiboot/Makefile
> rename to proxmox-boot/Makefile
> index fc9e333..3a36cb7 100644
> --- a/efiboot/Makefile
> +++ b/proxmox-boot/Makefile
> @@ -1,5 +1,5 @@
> -KERNEL_HOOKSCRIPTS = pve-auto-removal zz-pve-efiboot
> -INITRAMFS_HOOKSCRIPTS = pve-efiboot-sync
> +KERNEL_HOOKSCRIPTS = proxmox-auto-removal zz-proxmox-boot
> +INITRAMFS_HOOKSCRIPTS = proxmox-boot-sync
>  SHARE_FILES = functions
>  
>  POSTINSTHOOKDIR = ${DESTDIR}/etc/kernel/postinst.d
> diff --git a/efiboot/functions b/proxmox-boot/functions
> similarity index 100%
> rename from efiboot/functions
> rename to proxmox-boot/functions
> diff --git a/efiboot/pve-auto-removal b/proxmox-boot/proxmox-auto-removal
> similarity index 100%
> rename from efiboot/pve-auto-removal
> rename to proxmox-boot/proxmox-auto-removal
> diff --git a/efiboot/pve-efiboot-sync b/proxmox-boot/proxmox-boot-sync
> similarity index 84%
> rename from efiboot/pve-efiboot-sync
> rename to proxmox-boot/proxmox-boot-sync
> index c3ccf8e..5bdd72e 100644
> --- a/efiboot/pve-efiboot-sync
> +++ b/proxmox-boot/proxmox-boot-sync
> @@ -7,5 +7,5 @@ set -e
>  # this variable will be set to 1 and we do nothing, since our pve-kernel
>  # hooks will update the ESPs all at once anyway.
>  if [ -z "$INITRAMFS_TOOLS_KERNEL_HOOK" ]; then
> -	/usr/sbin/pve-efiboot-tool refresh --hook 'zz-pve-efiboot'
> +	/usr/sbin/proxmox-boot-tool refresh --hook 'zz-proxmox-boot'
>  fi
> diff --git a/efiboot/zz-pve-efiboot b/proxmox-boot/zz-proxmox-boot
> similarity index 100%
> rename from efiboot/zz-pve-efiboot
> rename to proxmox-boot/zz-proxmox-boot
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> 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 2/7] proxmox-boot-tool: add status command

[Fabian Grünbichler]

On April 22, 2021 1:17 pm, Stoiko Ivanov wrote:
> currently simply checking if $ESP_LIST exists, and indicating via
> the exit status if proxmox-boot-tool is used for booting the system.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  bin/proxmox-boot-tool | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
> index 2d625a6..7b06d57 100755
> --- a/bin/proxmox-boot-tool
> +++ b/bin/proxmox-boot-tool
> @@ -283,6 +283,7 @@ usage() {
>  	warn "       $0 refresh [--hook <name>]"
>  	warn "       $0 kernel <add|remove> <kernel-version>"
>  	warn "       $0 kernel list"
> +	warn "       $0 status"
>  	warn "       $0 help"
>  }
>  
> @@ -312,6 +313,20 @@ help() {
>  	echo ""
>  	echo "    list kernel versions currently selected for inclusion on ESPs."
>  	echo ""
> +	echo "USAGE: $0 status [--verbose]"
> +	echo ""
> +	echo "    Exit with 0 if any ESP is configured, else with 2."
> +	echo ""
> +}
> +
> +status() {
> +	verbose="$1"
> +	if [ ! -e ${ESP_LIST} ]; then
> +		if [ -n "$verbose" ]; then
> +		    warn "E: $ESP_LIST does not exist."

this is a bit strange (a command called status that has a verbose flag 
that does not output anything if everything is okay). maybe we could 
list the ESPs? or even, if --verbose is given, mount them and print 
THEIR status? ANY output would be great as a start ;)

> +		fi
> +		exit 2
> +	fi
>  }
>  
>  if [ -z "$1" ]; then
> @@ -390,6 +405,11 @@ case "$1" in
>  			;;
>  		esac
>  	;;
> +	'status')
> +		shift
> +		status "$1"
> +		exit 0
> +	;;
>  	'help')
>  		shift
>  		help
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>