* [pve-devel] [PATCH kernel-helper/manager v2] check for fitting grub-meta package on uefi systems
@ 2023-10-09 12:52 Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 1/2] proxmox-boot-tool: do not exit early in kernel-hook Stoiko Ivanov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Stoiko Ivanov @ 2023-10-09 12:52 UTC (permalink / raw)
To: pve-devel
v1->v2:
* adapted Friedrich's feedback (huge thanks!)
** fixed the wrongly negated check for installed grub-efi-amd64 in the
boot-tool hook.
** Rephrased the error-message in pve7to8 to 2 sentences. I tried adding a
newline as well, however this results in the message not being printed
in the warning color anymore (most likely due to [0]) - and I felt this
to be more important than having it on a separate line.
[0] https://perldoc.perl.org/Term::ANSIColor#RESTRICTIONS
original cover-letter for v1:
The following patchset is a followup to the one for the installer:
https://lists.proxmox.com/pipermail/pve-devel/2023-September/059270.html
As suggested by Thomas - adding the check to proxmox-kernel-helper seems
like a good idea. While adding it to d/postinst I thought that this might
not be the best place - and that getting the warning upon every
kernel-upgrade would be better vs. upon every upgrade of
proxmox-kernel-helper (which are far less often).
(Can gladly send the version with d/postinst as well)
If the pve-manager patch gets applied - I'd push the equivalent change to
pmg and provide one for pbs.
Tested on legacy and uefi VMs installed with pve-8.0 iso and
grub-efi-amd64 (and systemd-boot) removed vs. installed.
proxmox-kernel-helper:
Stoiko Ivanov (2):
proxmox-boot-tool: do not exit early in kernel-hook
proxmox-boot-tool: check if correct grub metapackage is installed
src/proxmox-boot/zz-proxmox-boot | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
pve-manager:
Stoiko Ivanov (1):
pve7to8: check for proper grub meta-package for bootmode
PVE/CLI/pve7to8.pm | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH kernel-helper v2 1/2] proxmox-boot-tool: do not exit early in kernel-hook
2023-10-09 12:52 [pve-devel] [PATCH kernel-helper/manager v2] check for fitting grub-meta package on uefi systems Stoiko Ivanov
@ 2023-10-09 12:52 ` Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH manager v2 1/1] pve7to8: check for proper grub meta-package for bootmode Stoiko Ivanov
2 siblings, 0 replies; 7+ messages in thread
From: Stoiko Ivanov @ 2023-10-09 12:52 UTC (permalink / raw)
To: pve-devel
update_esps is called first in the actual execution below - exiting
early does not work for systems that don't use proxmox-boot-tool if a
check added later needs to work there too.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/proxmox-boot/zz-proxmox-boot | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/proxmox-boot/zz-proxmox-boot b/src/proxmox-boot/zz-proxmox-boot
index 793882b..1adc1b1 100755
--- a/src/proxmox-boot/zz-proxmox-boot
+++ b/src/proxmox-boot/zz-proxmox-boot
@@ -44,7 +44,7 @@ fi
update_esps() {
if [ ! -f "${ESP_LIST}" ]; then
warn "No ${ESP_LIST} found, skipping ESP sync."
- exit 0
+ return
fi
if [ -f /etc/kernel/cmdline ]; then
# we can have cmdline files with multiple or no new line at all, handle both!
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed
2023-10-09 12:52 [pve-devel] [PATCH kernel-helper/manager v2] check for fitting grub-meta package on uefi systems Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 1/2] proxmox-boot-tool: do not exit early in kernel-hook Stoiko Ivanov
@ 2023-10-09 12:52 ` Stoiko Ivanov
2023-10-11 9:39 ` Friedrich Weber
2023-10-09 12:52 ` [pve-devel] [PATCH manager v2 1/1] pve7to8: check for proper grub meta-package for bootmode Stoiko Ivanov
2 siblings, 1 reply; 7+ messages in thread
From: Stoiko Ivanov @ 2023-10-09 12:52 UTC (permalink / raw)
To: pve-devel
this part of the hook applies only to systems not using pbt for
bootmangement.
Currently our ISO installs grub-pc unconditionally - and never the
conflicting grub-efi-amd64. Both packages are responsible for
running grub-install (for the appropriate disks) upon an upgrade of
grub.
This results in grub currently not getting updated on uefi-booted
systems (which do not use proxmox-boot-tool).
The patch causes a warning to be printed to notify the user.
Also considered putting the check+warning in d/postinst - but this way
it will get triggered more often (upon every
kernel-upgrade/update-initramfs, instead of only on
proxmox-kernel-helper updates, which are less often), increasing the
chances of being noticed.
checking for the changelog-presence was chosen, over `dpkg-query` for
the status, for consistency with the similar patch for pve7to8 (and
potentially a small speed-gain).
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/proxmox-boot/zz-proxmox-boot | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/proxmox-boot/zz-proxmox-boot b/src/proxmox-boot/zz-proxmox-boot
index 1adc1b1..4dfa765 100755
--- a/src/proxmox-boot/zz-proxmox-boot
+++ b/src/proxmox-boot/zz-proxmox-boot
@@ -215,6 +215,23 @@ disable_systemd_boot_hook() {
}
+check_grub_efi_package() {
+
+ if [ -f "${ESP_LIST}" ]; then
+ return
+ fi
+
+ if [ ! -d /sys/firmware/efi ]; then
+ return
+ fi
+
+ if [ -f /usr/share/doc/grub-efi-amd64/changelog.Debian.gz ]; then
+ return
+ fi
+ warn "uefi-booted system, without grub-efi-amd64 package - /boot/efi will not be updated"
+
+}
+
set -- $DEB_MAINT_PARAMS
mode="${1#\'}"
mode="${mode%\'}"
@@ -228,6 +245,7 @@ case $0:$mode in
BOOT_KVERS="$(boot_kernel_list "$@")"
update_esps
disable_systemd_boot_hook
+ check_grub_efi_package
;;
*/postrm.d/*:|*/postrm.d/*:remove)
reexec_in_mountns "$@"
@@ -235,6 +253,7 @@ case $0:$mode in
BOOT_KVERS="$(boot_kernel_list)"
update_esps
disable_systemd_boot_hook
+ check_grub_efi_package
;;
esac
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH manager v2 1/1] pve7to8: check for proper grub meta-package for bootmode
2023-10-09 12:52 [pve-devel] [PATCH kernel-helper/manager v2] check for fitting grub-meta package on uefi systems Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 1/2] proxmox-boot-tool: do not exit early in kernel-hook Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed Stoiko Ivanov
@ 2023-10-09 12:52 ` Stoiko Ivanov
2023-10-11 9:38 ` Friedrich Weber
2 siblings, 1 reply; 7+ messages in thread
From: Stoiko Ivanov @ 2023-10-09 12:52 UTC (permalink / raw)
To: pve-devel
This should catch installations from our ISO on non-ZFS in uefi mode,
which won't get the updated grub efi binary installed upon upgrade,
because grub-pc is installed instead of grub-efi-amd64.
Adding this to pve7to8 should make this even more visible, than the
corresponding patch for promxox-kernel-helper (warnings printed during
regular package upgrades might be overlooked more easily than
a yellow line in the major upgrade checkscript)
The if/else order was chosen to limit the nesting level of the long
messages.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
PVE/CLI/pve7to8.pm | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
index d1a71eff..ff7825b3 100644
--- a/PVE/CLI/pve7to8.pm
+++ b/PVE/CLI/pve7to8.pm
@@ -1302,29 +1302,36 @@ sub check_time_sync {
sub check_bootloader {
log_info("Checking bootloader configuration...");
- if (!$upgraded) {
- log_skip("not yet upgraded, no need to check the presence of systemd-boot");
- return;
- }
- if (! -f "/etc/kernel/proxmox-boot-uuids") {
- log_skip("proxmox-boot-tool not used for bootloader configuration");
+ if (! -d '/sys/firmware/efi') {
+ log_skip("System booted in legacy-mode - no need for additional packages");
return;
}
- if (! -d "/sys/firmware/efi") {
- log_skip("System booted in legacy-mode - no need for systemd-boot");
- return;
- }
-
- if ( -f "/usr/share/doc/systemd-boot/changelog.Debian.gz") {
- log_pass("systemd-boot is installed");
- } else {
+ if ( -f "/etc/kernel/proxmox-boot-uuids") {
+ if (!$upgraded) {
+ log_skip("not yet upgraded, no need to check the presence of systemd-boot");
+ return;
+ }
+ if ( -f "/usr/share/doc/systemd-boot/changelog.Debian.gz") {
+ log_pass("bootloader packages installed correctly");
+ return;
+ }
log_warn(
"proxmox-boot-tool is used for bootloader configuration in uefi mode"
- . "but the separate systemd-boot package, existing in Debian Bookworm is not installed"
- . "initializing new ESPs will not work until the package is installed"
+ . " but the separate systemd-boot package, existing in Debian Bookworm is not installed"
+ . " initializing new ESPs will not work until the package is installed"
+ );
+ return;
+ } elsif ( ! -f "/usr/share/doc/grub-efi-amd64/changelog.Debian.gz" ) {
+ log_warn(
+ "System booted in uefi mode but grub-efi-amd64 meta-package not installed"
+ . " new grub versions will not be installed to /boot/efi!"
+ . " Install grub-efi-amd64."
);
+ return;
+ } else {
+ log_pass("bootloader packages installed correctly");
}
}
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH manager v2 1/1] pve7to8: check for proper grub meta-package for bootmode
2023-10-09 12:52 ` [pve-devel] [PATCH manager v2 1/1] pve7to8: check for proper grub meta-package for bootmode Stoiko Ivanov
@ 2023-10-11 9:38 ` Friedrich Weber
0 siblings, 0 replies; 7+ messages in thread
From: Friedrich Weber @ 2023-10-11 9:38 UTC (permalink / raw)
To: Proxmox VE development discussion, Stoiko Ivanov
On 09/10/2023 14:52, Stoiko Ivanov wrote:
> + } elsif ( ! -f "/usr/share/doc/grub-efi-amd64/changelog.Debian.gz" ) {
> + log_warn(
> + "System booted in uefi mode but grub-efi-amd64 meta-package not installed"
> + . " new grub versions will not be installed to /boot/efi!"
> + . " Install grub-efi-amd64."
> );
I do like the exclamation mark, but I still think some punctuation (if
not newline) between "[...] not installed" and "new grub versions [...]"
would be good. Currently, the message reads like this:
WARN: System booted in uefi mode but grub-efi-amd64 meta-package
notinstalled new grub versions will not be installed to /boot/efi! Install
grub-efi-amd64.
which is a bit hard to parse -- the following seems easier to parse
(note the extra comma):
WARN: System booted in uefi mode but grub-efi-amd64 meta-package not
installed, new grub versions will not be installed to /boot/efi! Install
grub-efi-amd64.
Sorry for obsessing over the punctuation here, but I suspect there are a
lot of UEFI-booted PVE 7 installs with LVM root, so it would be good to
reduce the potential for confusion as much as possible.
The exact phrasing aside, consider this:
Tested-by: Friedrich Weber <f.weber@proxmox.com>
Can confirm that with this patch,
* pve7to8 prints the warning on UEFI-booted system with root on LVM and
grub-pc installed
* pve7to8 does *not* print the warning on
** the same system when grub-efi-amd64 is installed
** UEFI-booted system with root on ZFS (using systemd-boot)
** legacy-booted system with root on LVM or ZFS
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed Stoiko Ivanov
@ 2023-10-11 9:39 ` Friedrich Weber
2023-10-11 10:05 ` Friedrich Weber
0 siblings, 1 reply; 7+ messages in thread
From: Friedrich Weber @ 2023-10-11 9:39 UTC (permalink / raw)
To: Proxmox VE development discussion, Stoiko Ivanov
Tested-by: Friedrich Weber <f.weber@proxmox.com>
Can confirm that with this patch,
* the warning appears after installing a new kernel on a UEFI-booted
system with root on LVM
* the warning does *not* appear after installing a new kernel on
** a UEFI-booted system with root on ZFS (using systemd-boot)
** a legacy-booted system with root on LVM or ZFS
On 09/10/2023 14:52, Stoiko Ivanov wrote:
> this part of the hook applies only to systems not using pbt for
> bootmangement.
>
> Currently our ISO installs grub-pc unconditionally - and never the
> conflicting grub-efi-amd64. Both packages are responsible for
> running grub-install (for the appropriate disks) upon an upgrade of
> grub.
>
> This results in grub currently not getting updated on uefi-booted
> systems (which do not use proxmox-boot-tool).
>
> The patch causes a warning to be printed to notify the user.
>
> Also considered putting the check+warning in d/postinst - but this way
> it will get triggered more often (upon every
> kernel-upgrade/update-initramfs, instead of only on
> proxmox-kernel-helper updates, which are less often), increasing the
> chances of being noticed.
>
> checking for the changelog-presence was chosen, over `dpkg-query` for
> the status, for consistency with the similar patch for pve7to8 (and
> potentially a small speed-gain).
>
> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> src/proxmox-boot/zz-proxmox-boot | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/src/proxmox-boot/zz-proxmox-boot b/src/proxmox-boot/zz-proxmox-boot
> index 1adc1b1..4dfa765 100755
> --- a/src/proxmox-boot/zz-proxmox-boot
> +++ b/src/proxmox-boot/zz-proxmox-boot
> @@ -215,6 +215,23 @@ disable_systemd_boot_hook() {
>
> }
>
> +check_grub_efi_package() {
> +
> + if [ -f "${ESP_LIST}" ]; then
> + return
> + fi
> +
> + if [ ! -d /sys/firmware/efi ]; then
> + return
> + fi
> +
> + if [ -f /usr/share/doc/grub-efi-amd64/changelog.Debian.gz ]; then
> + return
> + fi
> + warn "uefi-booted system, without grub-efi-amd64 package - /boot/efi will not be updated"
> +
> +}
> +
> set -- $DEB_MAINT_PARAMS
> mode="${1#\'}"
> mode="${mode%\'}"
> @@ -228,6 +245,7 @@ case $0:$mode in
> BOOT_KVERS="$(boot_kernel_list "$@")"
> update_esps
> disable_systemd_boot_hook
> + check_grub_efi_package
> ;;
> */postrm.d/*:|*/postrm.d/*:remove)
> reexec_in_mountns "$@"
> @@ -235,6 +253,7 @@ case $0:$mode in
> BOOT_KVERS="$(boot_kernel_list)"
> update_esps
> disable_systemd_boot_hook
> + check_grub_efi_package
> ;;
> esac
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed
2023-10-11 9:39 ` Friedrich Weber
@ 2023-10-11 10:05 ` Friedrich Weber
0 siblings, 0 replies; 7+ messages in thread
From: Friedrich Weber @ 2023-10-11 10:05 UTC (permalink / raw)
To: Proxmox VE development discussion, Stoiko Ivanov
On 11/10/2023 11:39, Friedrich Weber wrote:
> Can confirm that with this patch,
>
> * the warning appears after installing a new kernel on a UEFI-booted
> system with root on LVM
Just to be clear: This is if grub-pc is installed. If i install
grub-efi-amd64 instead, the warning does not appear anymore.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-11 10:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-09 12:52 [pve-devel] [PATCH kernel-helper/manager v2] check for fitting grub-meta package on uefi systems Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 1/2] proxmox-boot-tool: do not exit early in kernel-hook Stoiko Ivanov
2023-10-09 12:52 ` [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed Stoiko Ivanov
2023-10-11 9:39 ` Friedrich Weber
2023-10-11 10:05 ` Friedrich Weber
2023-10-09 12:52 ` [pve-devel] [PATCH manager v2 1/1] pve7to8: check for proper grub meta-package for bootmode Stoiko Ivanov
2023-10-11 9:38 ` Friedrich Weber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal