[pbs-devel] [PATCH proxmox-backup v2 0/3] pbs3to4: rework bootloader checks

[pbs-devel] [PATCH proxmox-backup v2 0/3] pbs3to4: rework bootloader checks

[Stoiko Ivanov]

v1->v2:
Shannon caught a logic-error (wrong negation) in the check if systemd-boot
is actually used, and a useless temporary variable. additionally patch 2/3
needed a rustfmt - huge thx!

supersedes:
https://lore.proxmox.com/pbs-devel/20250818194026.840749-1-s.ivanov@proxmox.com/T/#t

Stoiko Ivanov (3):
  pbs3to4: fix logic error and typo in log message
  pbs3to4: use boolean variable for systemd-boot installation state
  pbs3to4: bootloader: only allow systemd-boot before upgrade when used

 src/bin/pbs3to4.rs | 61 ++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v2 1/3] pbs3to4: fix logic error and typo in log message

[Stoiko Ivanov]

/usr/share/doc/systemd-boot/changelog.Debian.gz existing indicates
that system-boot is installed (the `!` was in error).

additionally fix a typo in the log message.

Fixes:  94cc9903 ("bin: pbs3to4: adapt boot-loader checks to trixie")
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/bin/pbs3to4.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/pbs3to4.rs b/src/bin/pbs3to4.rs
index 2e6ea933..5df960a3 100644
--- a/src/bin/pbs3to4.rs
+++ b/src/bin/pbs3to4.rs
@@ -207,7 +207,7 @@ impl Checker {
             .log_info("Checking bootloader configuration...")?;
 
         if !Path::new("/sys/firmware/efi").is_dir() {
-            if !Path::new("/usr/share/doc/systemd-boot/changelog.Debian.gz").is_file() {
+            if Path::new("/usr/share/doc/systemd-boot/changelog.Debian.gz").is_file() {
                 self.output.log_info(
                     "systemd-boot package installed on legacy-boot system is not \
                     necessary, consider removing it",
@@ -215,7 +215,7 @@ impl Checker {
                 return Ok(());
             }
             self.output
-                .log_skip("System booted in legacy-mode - no need for additional pacckages.")?;
+                .log_skip("System booted in legacy-mode - no need for additional packages.")?;
             return Ok(());
         }
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v2 2/3] pbs3to4: use boolean variable for systemd-boot installation state

[Stoiko Ivanov]

while it's not saving too much run-time wise (each branch stats the
file only once), it's a bit easier to read.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/bin/pbs3to4.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/bin/pbs3to4.rs b/src/bin/pbs3to4.rs
index 5df960a3..3c3a020f 100644
--- a/src/bin/pbs3to4.rs
+++ b/src/bin/pbs3to4.rs
@@ -206,8 +206,11 @@ impl Checker {
         self.output
             .log_info("Checking bootloader configuration...")?;
 
+        let sd_boot_installed =
+            Path::new("/usr/share/doc/systemd-boot/changelog.Debian.gz").is_file();
+
         if !Path::new("/sys/firmware/efi").is_dir() {
-            if Path::new("/usr/share/doc/systemd-boot/changelog.Debian.gz").is_file() {
+            if sd_boot_installed {
                 self.output.log_info(
                     "systemd-boot package installed on legacy-boot system is not \
                     necessary, consider removing it",
@@ -227,7 +230,7 @@ impl Checker {
                     .log_skip("not yet upgraded, systemd-boot still needed for bootctl")?;
                 return Ok(());
             }
-            if Path::new("/usr/share/doc/systemd-boot/changelog.Debian.gz").is_file() {
+            if sd_boot_installed {
                 self.output.log_fail(
                     "systemd-boot meta-package installed. This will cause issues on upgrades of \
                     boot-related packages.\n\
@@ -237,7 +240,7 @@ impl Checker {
                 return Ok(());
             }
         } else {
-            if Path::new("/usr/share/doc/systemd-boot/changelog.Debian.gz").is_file() {
+            if sd_boot_installed {
                 self.output.log_fail(
                     "systemd-boot meta-package installed. This will cause problems on upgrades of other \
                     boot-related packages.\n\
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v2 3/3] pbs3to4: bootloader: only allow systemd-boot before upgrade when used

[Stoiko Ivanov]

This carries over the changes from pve-manager:
7e168453 ("pve8to9: only allow systemd-boot when it is actually used before upgrade")

additionally it pushes the check for systemd-boot being present to the
bottom (there is an early return for the single case which is not
problematic (used by p-b-t on a system still on bookworm) - and logs a
failure with a link to the upgrade guide in all cases.
The previous suggestion of installing systemd-boot-tools and
systemd-boot-efi explicitly is not fitting when systemd-boot is not
really used for booting (mostly: secure-boot enabled p-b-t setups)

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/bin/pbs3to4.rs | 54 +++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/src/bin/pbs3to4.rs b/src/bin/pbs3to4.rs
index 3c3a020f..4b432beb 100644
--- a/src/bin/pbs3to4.rs
+++ b/src/bin/pbs3to4.rs
@@ -211,7 +211,7 @@ impl Checker {
 
         if !Path::new("/sys/firmware/efi").is_dir() {
             if sd_boot_installed {
-                self.output.log_info(
+                self.output.log_warn(
                     "systemd-boot package installed on legacy-boot system is not \
                     necessary, consider removing it",
                 )?;
@@ -226,29 +226,21 @@ impl Checker {
         if Path::new("/etc/kernel/proxmox-boot-uuids").is_file() {
             // PBS packages version check needs to be run before
             if !self.upgraded {
-                self.output
-                    .log_skip("not yet upgraded, systemd-boot still needed for bootctl")?;
-                return Ok(());
-            }
-            if sd_boot_installed {
-                self.output.log_fail(
-                    "systemd-boot meta-package installed. This will cause issues on upgrades of \
-                    boot-related packages.\n\
-                    Install 'systemd-boot-efi' and 'systemd-boot-tools' explicitly and remove \
-                    'systemd-boot'",
-                )?;
-                return Ok(());
+                let output = std::process::Command::new("proxmox-boot-tool")
+                    .arg("status")
+                    .output()
+                    .map_err(|err| {
+                        format_err!("failed to retrieve proxmox-boot-tool status - {err}")
+                    })?;
+                let re = Regex::new(r"configured with:.* (uefi|systemd-boot) \(versions:")
+                    .expect("failed to proxmox-boot-tool status");
+                if re.is_match(std::str::from_utf8(&output.stdout)?) {
+                    self.output
+                        .log_skip("not yet upgraded, systemd-boot still needed for bootctl")?;
+                    return Ok(());
+                }
             }
         } else {
-            if sd_boot_installed {
-                self.output.log_fail(
-                    "systemd-boot meta-package installed. This will cause problems on upgrades of other \
-                    boot-related packages.\n\
-                    Remove the 'systemd-boot' package.\n\
-                    See https://pbs.proxmox.com/wiki/Upgrade_from_3_to_4#sd-boot-warning for more information."
-                )?;
-                boot_ok = false;
-            }
             if !Path::new("/usr/share/doc/grub-efi-amd64/changelog.Debian.gz").is_file() {
                 self.output.log_warn(
                     "System booted in uefi mode but grub-efi-amd64 meta-package not installed, \
@@ -277,12 +269,20 @@ impl Checker {
                     boot_ok = false;
                 }
             }
-            if boot_ok {
-                self.output
-                    .log_pass("bootloader packages installed correctly")?;
-            }
         }
-
+        if sd_boot_installed {
+            self.output.log_fail(
+                "systemd-boot meta-package installed. This will cause problems on upgrades of other \
+                boot-related packages.\n\
+                Remove the 'systemd-boot' package.\n\
+                See https://pbs.proxmox.com/wiki/Upgrade_from_3_to_4#sd-boot-warning for more information."
+            )?;
+            boot_ok = false;
+        }
+        if boot_ok {
+            self.output
+                .log_pass("bootloader packages installed correctly")?;
+        }
         Ok(())
     }
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup v2 0/3] pbs3to4: rework bootloader checks

[Shannon Sterz]

On Thu Aug 21, 2025 at 4:17 PM CEST, Stoiko Ivanov wrote:
> v1->v2:
> Shannon caught a logic-error (wrong negation) in the check if systemd-boot
> is actually used, and a useless temporary variable. additionally patch 2/3
> needed a rustfmt - huge thx!
>
> supersedes:
> https://lore.proxmox.com/pbs-devel/20250818194026.840749-1-s.ivanov@proxmox.com/T/#t
>
> Stoiko Ivanov (3):
>   pbs3to4: fix logic error and typo in log message
>   pbs3to4: use boolean variable for systemd-boot installation state
>   pbs3to4: bootloader: only allow systemd-boot before upgrade when used
>
>  src/bin/pbs3to4.rs | 61 ++++++++++++++++++++++++----------------------
>  1 file changed, 32 insertions(+), 29 deletions(-)

looks good to me, tested this series on the following set ups:

- legacy-mode boot -> outputs `SKIP: System booted in legacy-mode - no
  need for additional packages`
- proxmox-boot-tool + grub but without secure boot -> outputs `FAIL:
  systemd-boot meta-package installed. This will cause problems [..]`
- zfs + systemd-boot (no secure boot, but uefi) -> outputs `SKIP: not
  yet upgraded, systemd-boot still needed for bootctl`

so consider this:

Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
Tested-by: Shannon Sterz <s.sterz@proxmox.com>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH proxmox-backup v2 0/3] pbs3to4: rework bootloader checks

[Thomas Lamprecht]

On Thu, 21 Aug 2025 16:17:16 +0200, Stoiko Ivanov wrote:
> v1->v2:
> Shannon caught a logic-error (wrong negation) in the check if systemd-boot
> is actually used, and a useless temporary variable. additionally patch 2/3
> needed a rustfmt - huge thx!
> 
> supersedes:
> https://lore.proxmox.com/pbs-devel/20250818194026.840749-1-s.ivanov@proxmox.com/T/#t
> 
> [...]

Applied, thanks!

[1/3] pbs3to4: fix logic error and typo in log message
      commit: e7beb64391f5c6b48bb468b38bd9476706eabfd3
[2/3] pbs3to4: use boolean variable for systemd-boot installation state
      commit: 2a36f7016c1ff06d5ed004500f8679d3e2852a56
[3/3] pbs3to4: bootloader: only allow systemd-boot before upgrade when used
      commit: 847d39c353a250e3085be1e8a8979e36b63336e3


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel