[pbs-devel] [PATCH v4 proxmox{, -backup} 0/5] Add boot_mode, improve kernel version

[pbs-devel] [PATCH v4 proxmox{, -backup} 0/5] Add boot_mode, improve kernel version

[Gabriel Goller]

Ported the recent changes from the PVE NodeSummary (done by @Thomas) to
the PBS NodeDashboard.

It consists of:
* Adding the bootmode field, shows either Legacy BIOS, EFI, or EFI
    (Secure Boot)
* Declutter the kernel-version field and only show the release version
    and build-date.

Changes since v3:
  * Split functions into BootMode and SecureBoot status

Changes since v3:
  * Removed leftover debug print

Changes since v2:
 * Return the exact same stuff as in pve, so create a struct that holds
    mode and secureboot boolean
 * Fix indentation in js
 * Add efi spec link

Changes since v1:
 * Moved boot_mode detection to proxmox-sys
 * Added caching to boot_mode detection (lazy_static)
 * Return legacy kernel-version as well



proxmox:

Gabriel Goller (1):
  sys: add helper to get bootmode and secureboot status

 proxmox-sys/src/boot_mode.rs | 72 ++++++++++++++++++++++++++++++++++++
 proxmox-sys/src/lib.rs       |  1 +
 2 files changed, 73 insertions(+)
 create mode 100644 proxmox-sys/src/boot_mode.rs


proxmox-backup:

Gabriel Goller (4):
  node: status: added bootmode
  ui: dashboard: show the bootmode
  node: status: declutter kernel-version
  ui: dashboard: nicely display kernel version

 pbs-api-types/src/node.rs | 77 +++++++++++++++++++++++++++++++++++++--
 src/api2/node/status.rs   | 41 +++++++++++++++++----
 www/panel/NodeInfo.js     | 28 +++++++++++++-
 3 files changed, 132 insertions(+), 14 deletions(-)


Summary over all repositories:
  5 files changed, 205 insertions(+), 14 deletions(-)

-- 
murpp v0.4.0

[pbs-devel] [PATCH v4 proxmox 1/5] sys: add helper to get bootmode and secureboot status

[Gabriel Goller]

Helper that return the current boot_mode and secureboot status.
Detection works the same as in pve, we use `/sys/firmware/efi` and
the `efivars/SecureBoot-xxx..` file.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-sys/src/boot_mode.rs | 72 ++++++++++++++++++++++++++++++++++++
 proxmox-sys/src/lib.rs       |  1 +
 2 files changed, 73 insertions(+)
 create mode 100644 proxmox-sys/src/boot_mode.rs

diff --git a/proxmox-sys/src/boot_mode.rs b/proxmox-sys/src/boot_mode.rs
new file mode 100644
index 0000000..dc9d4f5
--- /dev/null
+++ b/proxmox-sys/src/boot_mode.rs
@@ -0,0 +1,72 @@
+use std::{io::Read, sync::Mutex};
+
+#[derive(Clone, Copy)]
+pub enum SecureBoot {
+    /// SecureBoot is enabled
+    Enabled,
+    /// SecureBoot is disabled
+    Disabled,
+}
+
+/// The possible BootModes
+#[derive(Clone, Copy)]
+pub enum BootMode {
+    /// The BootMode is EFI/UEFI
+    Efi,
+    /// The BootMode is Legacy BIOS
+    Bios,
+}
+
+impl BootMode {
+    /// Returns the current bootmode (BIOS or EFI)
+    pub fn query() -> BootMode {
+        lazy_static::lazy_static!(
+            static ref BOOT_MODE: Mutex<Option<BootMode>> = Mutex::new(None);
+        );
+
+        let mut last = BOOT_MODE.lock().unwrap();
+        let value = last.or_else(|| {
+            if std::path::Path::new("/sys/firmware/efi").exists() {
+                Some(BootMode::Efi)
+            } else {
+                Some(BootMode::Bios)
+            }
+        });
+        *last = value;
+        value.unwrap()
+    }
+}
+
+impl SecureBoot {
+    /// Checks if secure boot is enabled
+    pub fn query() -> SecureBoot {
+        lazy_static::lazy_static!(
+            static ref SECURE_BOOT: Mutex<Option<SecureBoot>> = Mutex::new(None);
+        );
+
+        let mut last = SECURE_BOOT.lock().unwrap();
+        let value = last.or_else(|| {
+            // Check if SecureBoot is enabled
+            // Attention: this file is not seekable!
+            // Spec: https://uefi.org/specs/UEFI/2.10/03_Boot_Manager.html?highlight=8be4d#globally-defined-variables
+            let efivar = std::fs::File::open(
+                "/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c",
+            );
+            if let Ok(mut file) = efivar {
+                let mut buf = [0; 5];
+                let Ok(_) = file.read_exact(&mut buf) else {
+                        return Some(SecureBoot::Disabled);
+                    };
+                if buf[4] == 1 {
+                    Some(SecureBoot::Enabled)
+                } else {
+                    Some(SecureBoot::Disabled)
+                }
+            } else {
+                Some(SecureBoot::Disabled)
+            }
+        });
+        *last = value;
+        value.unwrap()
+    }
+}
diff --git a/proxmox-sys/src/lib.rs b/proxmox-sys/src/lib.rs
index 7e59058..8ea7073 100644
--- a/proxmox-sys/src/lib.rs
+++ b/proxmox-sys/src/lib.rs
@@ -1,5 +1,6 @@
 use std::os::unix::ffi::OsStrExt;
 
+pub mod boot_mode;
 pub mod command;
 #[cfg(feature = "crypt")]
 pub mod crypt;
-- 
2.39.2

[pbs-devel] [PATCH v4 proxmox-backup 2/5] node: status: added bootmode

[Gabriel Goller]

Added field that shows the bootmode of the node. The bootmode is either
Legacy Bios, EFI, or EFI (Secure Boot). To detect the mode we use the
exact same method as in pve: We check if the `/sys/firmware/efi` folder
exists, then check if the `SecureBoot-xx...` file in the `efivars`
directory has the SecureBoot flag enabled.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-api-types/src/node.rs | 30 +++++++++++++++++++++++++++---
 src/api2/node/status.rs   | 29 +++++++++++++++++++++++++++--
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/pbs-api-types/src/node.rs b/pbs-api-types/src/node.rs
index 704215bb..ab626157 100644
--- a/pbs-api-types/src/node.rs
+++ b/pbs-api-types/src/node.rs
@@ -1,9 +1,8 @@
-use serde::{Deserialize, Serialize};
 use proxmox_schema::*;
+use serde::{Deserialize, Serialize};
 
 use crate::StorageStatus;
 
-
 #[api]
 #[derive(Serialize, Deserialize, Default)]
 #[serde(rename_all = "kebab-case")]
@@ -39,6 +38,29 @@ pub struct NodeInformation {
     pub fingerprint: String,
 }
 
+
+#[api]
+#[derive(Serialize, Deserialize, Copy, Clone)]
+#[serde(rename_all = "kebab-case")]
+/// The possible BootModes
+pub enum BootMode {
+    /// The BootMode is EFI/UEFI
+    Efi,
+    /// The BootMode is Legacy BIOS
+    LegacyBios,
+}
+
+#[api]
+#[derive(Serialize, Deserialize, Clone)]
+#[serde(rename_all = "lowercase")]
+/// Holds the Bootmodes
+pub struct BootModeInformation {
+    /// The BootMode, either Efi or Bios
+    pub mode: BootMode,
+    /// SecureBoot status
+    pub secureboot: bool,
+}
+
 #[api]
 #[derive(Serialize, Deserialize, Default)]
 #[serde(rename_all = "kebab-case")]
@@ -78,7 +100,7 @@ pub struct NodeCpuInformation {
         }
     },
 )]
-#[derive(Serialize, Deserialize, Default)]
+#[derive(Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 /// The Node status
 pub struct NodeStatus {
@@ -97,4 +119,6 @@ pub struct NodeStatus {
     pub wait: f64,
     pub cpuinfo: NodeCpuInformation,
     pub info: NodeInformation,
+    /// Current boot mode
+    pub boot_info: BootModeInformation,
 }
diff --git a/src/api2/node/status.rs b/src/api2/node/status.rs
index 639d7211..17b9aff3 100644
--- a/src/api2/node/status.rs
+++ b/src/api2/node/status.rs
@@ -1,16 +1,18 @@
-use std::os::unix::prelude::OsStrExt;
+use std::os::unix::ffi::OsStrExt;
 use std::process::Command;
 
 use anyhow::{bail, format_err, Error};
 use serde_json::Value;
 
+use proxmox_sys::boot_mode;
 use proxmox_sys::linux::procfs;
 
 use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment};
 use proxmox_schema::api;
 
 use pbs_api_types::{
-    NodePowerCommand, StorageStatus, NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_POWER_MANAGEMENT,
+    BootModeInformation, NodePowerCommand, StorageStatus, NODE_SCHEMA, PRIV_SYS_AUDIT,
+    PRIV_SYS_POWER_MANAGEMENT,
 };
 
 use pbs_api_types::{
@@ -25,6 +27,26 @@ fn procfs_to_node_cpu_info(info: procfs::ProcFsCPUInfo) -> NodeCpuInformation {
     }
 }
 
+fn boot_mode_to_info(bm: boot_mode::BootMode, sb: boot_mode::SecureBoot) -> BootModeInformation {
+    use boot_mode::BootMode;
+    use boot_mode::SecureBoot;
+
+    match (bm, sb) {
+        (BootMode::Efi, SecureBoot::Enabled) => BootModeInformation {
+            mode: pbs_api_types::BootMode::Efi,
+            secureboot: true,
+        },
+        (BootMode::Efi, SecureBoot::Disabled) => BootModeInformation {
+            mode: pbs_api_types::BootMode::Efi,
+            secureboot: false,
+        },
+        (BootMode::Bios, _) => BootModeInformation {
+            mode: pbs_api_types::BootMode::LegacyBios,
+            secureboot: false,
+        },
+    }
+}
+
 #[api(
     input: {
         properties: {
@@ -79,6 +101,8 @@ async fn get_status(
 
     let disk = crate::tools::fs::fs_info_static(proxmox_lang::c_str!("/")).await?;
 
+    let boot_info = boot_mode_to_info(boot_mode::BootMode::query(), boot_mode::SecureBoot::query());
+
     Ok(NodeStatus {
         memory,
         swap,
@@ -96,6 +120,7 @@ async fn get_status(
         info: NodeInformation {
             fingerprint: crate::cert_info()?.fingerprint()?,
         },
+        boot_info,
     })
 }
 
-- 
2.39.2

[pbs-devel] [PATCH v4 proxmox-backup 3/5] ui: dashboard: show the bootmode

[Gabriel Goller]

Shows the bootmode of the instance. Options are Legacy BIOS,
EFI, or EFI(Secure Boot).

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 www/panel/NodeInfo.js | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/www/panel/NodeInfo.js b/www/panel/NodeInfo.js
index 2551c9a5..cba6d2a1 100644
--- a/www/panel/NodeInfo.js
+++ b/www/panel/NodeInfo.js
@@ -147,6 +147,21 @@ Ext.define('PBS.NodeInfoPanel', {
 	    textField: 'kversion',
 	    value: '',
 	},
+	{
+	    colspan: 2,
+	    title: gettext('Boot Mode'),
+	    printBar: false,
+	    textField: 'boot-info',
+	    renderer: boot => {
+		if (boot.mode === 'legacy-bios') {
+		    return 'Legacy BIOS';
+		} else if (boot.mode === 'efi') {
+		    return `EFI${boot.secureboot ? ' (Secure Boot)' : ''}`;
+		}
+		return Proxmox.Utils.unknownText;
+	    },
+	    value: '',
+	},
 	{
 	    xtype: 'pmxNodeInfoRepoStatus',
 	    itemId: 'repositoryStatus',
-- 
2.39.2

[pbs-devel] [PATCH v4 proxmox-backup 4/5] node: status: declutter kernel-version

[Gabriel Goller]

Return a struct with all the components of the kernel version like it
has been done in pve. Also return the legacy `kversion` to keep
backwards compat.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-api-types/src/node.rs | 47 ++++++++++++++++++++++++++++++++++++++-
 src/api2/node/status.rs   | 18 +++++++--------
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/pbs-api-types/src/node.rs b/pbs-api-types/src/node.rs
index ab626157..fd1fbe3b 100644
--- a/pbs-api-types/src/node.rs
+++ b/pbs-api-types/src/node.rs
@@ -1,3 +1,5 @@
+use std::ffi::OsStr;
+
 use proxmox_schema::*;
 use serde::{Deserialize, Serialize};
 
@@ -38,6 +40,47 @@ pub struct NodeInformation {
     pub fingerprint: String,
 }
 
+#[api]
+#[derive(Serialize, Deserialize, Default)]
+#[serde(rename_all = "lowercase")]
+/// The current kernel version (output of `uname`)
+pub struct KernelVersionInformation {
+    /// The systemname/nodename
+    pub sysname: String,
+    /// The kernel release number
+    pub release: String,
+    /// The kernel version
+    pub version: String,
+    /// The machine architecture
+    pub machine: String,
+}
+
+impl KernelVersionInformation {
+    pub fn from_ostr(sysname: &OsStr, release: &OsStr, version: &OsStr, machine: &OsStr) -> Self {
+        KernelVersionInformation {
+            sysname: sysname
+                .to_os_string()
+                .into_string()
+                .unwrap_or("".to_string()),
+            release: release
+                .to_os_string()
+                .into_string()
+                .unwrap_or("".to_string()),
+            version: version
+                .to_os_string()
+                .into_string()
+                .unwrap_or("".to_string()),
+            machine: machine
+                .to_os_string()
+                .into_string()
+                .unwrap_or("".to_string()),
+        }
+    }
+
+    pub fn get_legacy(&self) -> String {
+        format!("{} {} {}", self.sysname, self.release, self.version)
+    }
+}
 
 #[api]
 #[derive(Serialize, Deserialize, Copy, Clone)]
@@ -111,7 +154,9 @@ pub struct NodeStatus {
     pub uptime: u64,
     /// Load for 1, 5 and 15 minutes.
     pub loadavg: [f64; 3],
-    /// The current kernel version.
+    /// The current kernel version (NEW struct type).
+    pub current_kernel: KernelVersionInformation,
+    /// The current kernel version (LEGACY string type).
     pub kversion: String,
     /// Total CPU usage since last query.
     pub cpu: f64,
diff --git a/src/api2/node/status.rs b/src/api2/node/status.rs
index 17b9aff3..8b4d1638 100644
--- a/src/api2/node/status.rs
+++ b/src/api2/node/status.rs
@@ -1,4 +1,3 @@
-use std::os::unix::ffi::OsStrExt;
 use std::process::Command;
 
 use anyhow::{bail, format_err, Error};
@@ -11,8 +10,8 @@ use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment};
 use proxmox_schema::api;
 
 use pbs_api_types::{
-    BootModeInformation, NodePowerCommand, StorageStatus, NODE_SCHEMA, PRIV_SYS_AUDIT,
-    PRIV_SYS_POWER_MANAGEMENT,
+    BootModeInformation, KernelVersionInformation, NodePowerCommand, StorageStatus, NODE_SCHEMA,
+    PRIV_SYS_AUDIT, PRIV_SYS_POWER_MANAGEMENT,
 };
 
 use pbs_api_types::{
@@ -92,11 +91,11 @@ async fn get_status(
     let cpuinfo = procfs_to_node_cpu_info(cpuinfo);
 
     let uname = nix::sys::utsname::uname()?;
-    let kversion = format!(
-        "{} {} {}",
-        std::str::from_utf8(uname.sysname().as_bytes())?,
-        std::str::from_utf8(uname.release().as_bytes())?,
-        std::str::from_utf8(uname.version().as_bytes())?
+    let kernel_version = KernelVersionInformation::from_ostr(
+        uname.sysname(),
+        uname.release(),
+        uname.version(),
+        uname.machine(),
     );
 
     let disk = crate::tools::fs::fs_info_static(proxmox_lang::c_str!("/")).await?;
@@ -113,7 +112,8 @@ async fn get_status(
         },
         uptime: procfs::read_proc_uptime()?.0 as u64,
         loadavg,
-        kversion,
+        kversion: kernel_version.get_legacy(),
+        current_kernel: kernel_version,
         cpuinfo,
         cpu,
         wait,
-- 
2.39.2

[pbs-devel] [PATCH v4 proxmox-backup 5/5] ui: dashboard: nicely display kernel version

[Gabriel Goller]

Extract and display the build version and kernel
release nicely.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 www/panel/NodeInfo.js | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/www/panel/NodeInfo.js b/www/panel/NodeInfo.js
index cba6d2a1..72f97c7c 100644
--- a/www/panel/NodeInfo.js
+++ b/www/panel/NodeInfo.js
@@ -140,11 +140,20 @@ Ext.define('PBS.NodeInfoPanel', {
 	    value: '',
 	},
 	{
-	    itemId: 'kversion',
 	    colspan: 2,
 	    title: gettext('Kernel Version'),
 	    printBar: false,
-	    textField: 'kversion',
+	    // TODO: remove with next major and only use newish current-kernel textfield
+	    multiField: true,
+	    //textField: 'current-kernel',
+	    renderer: ({ data }) => {
+		if (!data['current-kernel']) {
+		    return data.kversion;
+		}
+		let kernel = data['current-kernel'];
+		let buildDate = kernel.version.match(/\((.+)\)\s*$/)[1] ?? 'unknown';
+		return `${kernel.sysname} ${kernel.release} (${buildDate})`;
+	    },
 	    value: '',
 	},
 	{
-- 
2.39.2

Re: [pbs-devel] [PATCH v4 proxmox 1/5] sys: add helper to get bootmode and secureboot status

[Wolfgang Bumiller]

On Wed, Nov 29, 2023 at 10:07:42AM +0100, Gabriel Goller wrote:
> Helper that return the current boot_mode and secureboot status.
> Detection works the same as in pve, we use `/sys/firmware/efi` and
> the `efivars/SecureBoot-xxx..` file.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  proxmox-sys/src/boot_mode.rs | 72 ++++++++++++++++++++++++++++++++++++
>  proxmox-sys/src/lib.rs       |  1 +
>  2 files changed, 73 insertions(+)
>  create mode 100644 proxmox-sys/src/boot_mode.rs
> 
> diff --git a/proxmox-sys/src/boot_mode.rs b/proxmox-sys/src/boot_mode.rs
> new file mode 100644
> index 0000000..dc9d4f5
> --- /dev/null
> +++ b/proxmox-sys/src/boot_mode.rs
> @@ -0,0 +1,72 @@
> +use std::{io::Read, sync::Mutex};
> +
> +#[derive(Clone, Copy)]

^ Maybe also + Debug + Eq + PartialEq

> +pub enum SecureBoot {
> +    /// SecureBoot is enabled
> +    Enabled,
> +    /// SecureBoot is disabled
> +    Disabled,
> +}
> +
> +/// The possible BootModes
> +#[derive(Clone, Copy)]

^ Maybe also + Debug + Eq + PartialEq

> +pub enum BootMode {
> +    /// The BootMode is EFI/UEFI
> +    Efi,
> +    /// The BootMode is Legacy BIOS
> +    Bios,
> +}
> +
> +impl BootMode {
> +    /// Returns the current bootmode (BIOS or EFI)
> +    pub fn query() -> BootMode {
> +        lazy_static::lazy_static!(
> +            static ref BOOT_MODE: Mutex<Option<BootMode>> = Mutex::new(None);
> +        );

lazy_static + Mutex = overkill.
Here we can just use std::sync::OnceLock<BootMode>.

> +
> +        let mut last = BOOT_MODE.lock().unwrap();
> +        let value = last.or_else(|| {
> +            if std::path::Path::new("/sys/firmware/efi").exists() {
> +                Some(BootMode::Efi)
> +            } else {
> +                Some(BootMode::Bios)
> +            }
> +        });
> +        *last = value;
> +        value.unwrap()
> +    }
> +}
> +
> +impl SecureBoot {
> +    /// Checks if secure boot is enabled
> +    pub fn query() -> SecureBoot {
> +        lazy_static::lazy_static!(
> +            static ref SECURE_BOOT: Mutex<Option<SecureBoot>> = Mutex::new(None);
> +        );

^ same

> +
> +        let mut last = SECURE_BOOT.lock().unwrap();
> +        let value = last.or_else(|| {
> +            // Check if SecureBoot is enabled
> +            // Attention: this file is not seekable!
> +            // Spec: https://uefi.org/specs/UEFI/2.10/03_Boot_Manager.html?highlight=8be4d#globally-defined-variables
> +            let efivar = std::fs::File::open(
> +                "/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c",
> +            );
> +            if let Ok(mut file) = efivar {
> +                let mut buf = [0; 5];
> +                let Ok(_) = file.read_exact(&mut buf) else {

Btw. we can probably shorten this chain to
    if File::open(
        "..."
    ).and_then(|file| file.read_exact(&mut buf))
    .is_ok()
        && buf[4] == 1
    {
        SecureBoot::Enabled
    } else {
        SecureBoot::Disabled
    }

but, since in the API we need to do the same thing as in PVE, we might
as well just have a From/Into<bool> and shorten this even further...

> +                        return Some(SecureBoot::Disabled);
> +                    };
> +                if buf[4] == 1 {
> +                    Some(SecureBoot::Enabled)
> +                } else {
> +                    Some(SecureBoot::Disabled)
> +                }
> +            } else {
> +                Some(SecureBoot::Disabled)
> +            }
> +        });
> +        *last = value;
> +        value.unwrap()
> +    }
> +}

Re: [pbs-devel] [PATCH v4 proxmox-backup 2/5] node: status: added bootmode

[Wolfgang Bumiller]

On Wed, Nov 29, 2023 at 10:07:43AM +0100, Gabriel Goller wrote:
> Added field that shows the bootmode of the node. The bootmode is either
> Legacy Bios, EFI, or EFI (Secure Boot). To detect the mode we use the
> exact same method as in pve: We check if the `/sys/firmware/efi` folder
> exists, then check if the `SecureBoot-xx...` file in the `efivars`
> directory has the SecureBoot flag enabled.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  pbs-api-types/src/node.rs | 30 +++++++++++++++++++++++++++---
>  src/api2/node/status.rs   | 29 +++++++++++++++++++++++++++--
>  2 files changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/pbs-api-types/src/node.rs b/pbs-api-types/src/node.rs
> index 704215bb..ab626157 100644
> --- a/pbs-api-types/src/node.rs
> +++ b/pbs-api-types/src/node.rs
> @@ -1,9 +1,8 @@
> -use serde::{Deserialize, Serialize};
>  use proxmox_schema::*;
> +use serde::{Deserialize, Serialize};
>  
>  use crate::StorageStatus;
>  
> -
>  #[api]
>  #[derive(Serialize, Deserialize, Default)]
>  #[serde(rename_all = "kebab-case")]
> @@ -39,6 +38,29 @@ pub struct NodeInformation {
>      pub fingerprint: String,
>  }
>  
> +
> +#[api]
> +#[derive(Serialize, Deserialize, Copy, Clone)]
> +#[serde(rename_all = "kebab-case")]
> +/// The possible BootModes
> +pub enum BootMode {
> +    /// The BootMode is EFI/UEFI
> +    Efi,
> +    /// The BootMode is Legacy BIOS
> +    LegacyBios,
> +}

^ Should be able to have a From<proxmox_sys::boot_mode::BootMode> here.

> +
> +#[api]
> +#[derive(Serialize, Deserialize, Clone)]
> +#[serde(rename_all = "lowercase")]
> +/// Holds the Bootmodes
> +pub struct BootModeInformation {
> +    /// The BootMode, either Efi or Bios
> +    pub mode: BootMode,
> +    /// SecureBoot status
> +    pub secureboot: bool,
> +}
> +
>  #[api]
>  #[derive(Serialize, Deserialize, Default)]
>  #[serde(rename_all = "kebab-case")]
> @@ -78,7 +100,7 @@ pub struct NodeCpuInformation {
>          }
>      },
>  )]
> -#[derive(Serialize, Deserialize, Default)]
> +#[derive(Serialize, Deserialize)]
>  #[serde(rename_all = "kebab-case")]
>  /// The Node status
>  pub struct NodeStatus {
> @@ -97,4 +119,6 @@ pub struct NodeStatus {
>      pub wait: f64,
>      pub cpuinfo: NodeCpuInformation,
>      pub info: NodeInformation,
> +    /// Current boot mode
> +    pub boot_info: BootModeInformation,
>  }
> diff --git a/src/api2/node/status.rs b/src/api2/node/status.rs
> index 639d7211..17b9aff3 100644
> --- a/src/api2/node/status.rs
> +++ b/src/api2/node/status.rs
> @@ -1,16 +1,18 @@
> -use std::os::unix::prelude::OsStrExt;
> +use std::os::unix::ffi::OsStrExt;
>  use std::process::Command;
>  
>  use anyhow::{bail, format_err, Error};
>  use serde_json::Value;
>  
> +use proxmox_sys::boot_mode;
>  use proxmox_sys::linux::procfs;
>  
>  use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment};
>  use proxmox_schema::api;
>  
>  use pbs_api_types::{
> -    NodePowerCommand, StorageStatus, NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_POWER_MANAGEMENT,
> +    BootModeInformation, NodePowerCommand, StorageStatus, NODE_SCHEMA, PRIV_SYS_AUDIT,
> +    PRIV_SYS_POWER_MANAGEMENT,
>  };
>  
>  use pbs_api_types::{
> @@ -25,6 +27,26 @@ fn procfs_to_node_cpu_info(info: procfs::ProcFsCPUInfo) -> NodeCpuInformation {
>      }
>  }
>  
> +fn boot_mode_to_info(bm: boot_mode::BootMode, sb: boot_mode::SecureBoot) -> BootModeInformation {
> +    use boot_mode::BootMode;
> +    use boot_mode::SecureBoot;
> +
> +    match (bm, sb) {

Since the info we get isn't nested anymore, we could just move the match
inside the struct building.

    BootModeInformation {
        mode: bm.into(),
        sb: sb.into() or sb == SecureBoot::Enabled with the added Eq
    }

> +            secureboot: true,
> +        },

> +        (BootMode::Efi, SecureBoot::Enabled) => BootModeInformation {
> +            mode: pbs_api_types::BootMode::Efi,
> +            secureboot: true,
> +        },
> +        (BootMode::Efi, SecureBoot::Disabled) => BootModeInformation {
> +            mode: pbs_api_types::BootMode::Efi,
> +            secureboot: false,
> +        },
> +        (BootMode::Bios, _) => BootModeInformation {
> +            mode: pbs_api_types::BootMode::LegacyBios,
> +            secureboot: false,
> +        },
> +    }
> +}
> +
>  #[api(
>      input: {
>          properties: {
> @@ -79,6 +101,8 @@ async fn get_status(
>  
>      let disk = crate::tools::fs::fs_info_static(proxmox_lang::c_str!("/")).await?;
>  
> +    let boot_info = boot_mode_to_info(boot_mode::BootMode::query(), boot_mode::SecureBoot::query());

Btw. I'm not even sure we should have the above function if we query it
all at this point anyway, this could also just be
BootModeInformation::query(), or the function above could just take no
parameters.

> +
>      Ok(NodeStatus {
>          memory,
>          swap,
> @@ -96,6 +120,7 @@ async fn get_status(
>          info: NodeInformation {
>              fingerprint: crate::cert_info()?.fingerprint()?,
>          },
> +        boot_info,
>      })
>  }
>  
> -- 
> 2.39.2

Re: [pbs-devel] [PATCH v4 proxmox-backup 4/5] node: status: declutter kernel-version

[Wolfgang Bumiller]

On Wed, Nov 29, 2023 at 10:07:45AM +0100, Gabriel Goller wrote:
> Return a struct with all the components of the kernel version like it
> has been done in pve. Also return the legacy `kversion` to keep
> backwards compat.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  pbs-api-types/src/node.rs | 47 ++++++++++++++++++++++++++++++++++++++-
>  src/api2/node/status.rs   | 18 +++++++--------
>  2 files changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/pbs-api-types/src/node.rs b/pbs-api-types/src/node.rs
> index ab626157..fd1fbe3b 100644
> --- a/pbs-api-types/src/node.rs
> +++ b/pbs-api-types/src/node.rs
> @@ -1,3 +1,5 @@
> +use std::ffi::OsStr;
> +
>  use proxmox_schema::*;
>  use serde::{Deserialize, Serialize};
>  
> @@ -38,6 +40,47 @@ pub struct NodeInformation {
>      pub fingerprint: String,
>  }
>  
> +#[api]
> +#[derive(Serialize, Deserialize, Default)]
> +#[serde(rename_all = "lowercase")]
> +/// The current kernel version (output of `uname`)
> +pub struct KernelVersionInformation {
> +    /// The systemname/nodename
> +    pub sysname: String,
> +    /// The kernel release number
> +    pub release: String,
> +    /// The kernel version
> +    pub version: String,
> +    /// The machine architecture
> +    pub machine: String,
> +}
> +
> +impl KernelVersionInformation {
> +    pub fn from_ostr(sysname: &OsStr, release: &OsStr, version: &OsStr, machine: &OsStr) -> Self {

from_ostr is a bit of a weird name for a public method taking 4
parameters.

consider a `From<&UtsName>` implementation.

> +        KernelVersionInformation {
> +            sysname: sysname
> +                .to_os_string()
> +                .into_string()
> +                .unwrap_or("".to_string()),

.unwrap_or_default(), also, whenver you think "".to_string(), type
String::new() instead ;-)

also, you first convert to an owned string - this always works, and then
do the fallible `into_string()`, so even if it's not a valid string you
clone and discard, instead, do:
    sysname
        .to_str()
        .map(String::from)
        .unwrap_or_default()

this way there's no string copying if it's not valid utf-8 becausee that
is checked first

same for the cases below

> +            release: release
> +                .to_os_string()
> +                .into_string()
> +                .unwrap_or("".to_string()),
> +            version: version
> +                .to_os_string()
> +                .into_string()
> +                .unwrap_or("".to_string()),
> +            machine: machine
> +                .to_os_string()
> +                .into_string()
> +                .unwrap_or("".to_string()),
> +        }
> +    }
> +
> +    pub fn get_legacy(&self) -> String {
> +        format!("{} {} {}", self.sysname, self.release, self.version)
> +    }
> +}
>  
>  #[api]
>  #[derive(Serialize, Deserialize, Copy, Clone)]
> @@ -111,7 +154,9 @@ pub struct NodeStatus {
>      pub uptime: u64,
>      /// Load for 1, 5 and 15 minutes.
>      pub loadavg: [f64; 3],
> -    /// The current kernel version.
> +    /// The current kernel version (NEW struct type).
> +    pub current_kernel: KernelVersionInformation,
> +    /// The current kernel version (LEGACY string type).
>      pub kversion: String,
>      /// Total CPU usage since last query.
>      pub cpu: f64,
> diff --git a/src/api2/node/status.rs b/src/api2/node/status.rs
> index 17b9aff3..8b4d1638 100644
> --- a/src/api2/node/status.rs
> +++ b/src/api2/node/status.rs
> @@ -1,4 +1,3 @@
> -use std::os::unix::ffi::OsStrExt;
>  use std::process::Command;
>  
>  use anyhow::{bail, format_err, Error};
> @@ -11,8 +10,8 @@ use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment};
>  use proxmox_schema::api;
>  
>  use pbs_api_types::{
> -    BootModeInformation, NodePowerCommand, StorageStatus, NODE_SCHEMA, PRIV_SYS_AUDIT,
> -    PRIV_SYS_POWER_MANAGEMENT,
> +    BootModeInformation, KernelVersionInformation, NodePowerCommand, StorageStatus, NODE_SCHEMA,
> +    PRIV_SYS_AUDIT, PRIV_SYS_POWER_MANAGEMENT,
>  };
>  
>  use pbs_api_types::{
> @@ -92,11 +91,11 @@ async fn get_status(
>      let cpuinfo = procfs_to_node_cpu_info(cpuinfo);
>  
>      let uname = nix::sys::utsname::uname()?;
> -    let kversion = format!(
> -        "{} {} {}",
> -        std::str::from_utf8(uname.sysname().as_bytes())?,
> -        std::str::from_utf8(uname.release().as_bytes())?,
> -        std::str::from_utf8(uname.version().as_bytes())?
> +    let kernel_version = KernelVersionInformation::from_ostr(
> +        uname.sysname(),
> +        uname.release(),
> +        uname.version(),
> +        uname.machine(),
>      );
>  
>      let disk = crate::tools::fs::fs_info_static(proxmox_lang::c_str!("/")).await?;
> @@ -113,7 +112,8 @@ async fn get_status(
>          },
>          uptime: procfs::read_proc_uptime()?.0 as u64,
>          loadavg,
> -        kversion,
> +        kversion: kernel_version.get_legacy(),
> +        current_kernel: kernel_version,
>          cpuinfo,
>          cpu,
>          wait,
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 

Re: [pbs-devel] [PATCH v4 proxmox-backup 2/5] node: status: added bootmode

[Gabriel Goller]

On 11/29/23 11:18, Wolfgang Bumiller wrote:
> On Wed, Nov 29, 2023 at 10:07:43AM +0100, Gabriel Goller wrote:
>> Added field that shows the bootmode of the node. The bootmode is either
>> Legacy Bios, EFI, or EFI (Secure Boot). To detect the mode we use the
>> exact same method as in pve: We check if the `/sys/firmware/efi` folder
>> exists, then check if the `SecureBoot-xx...` file in the `efivars`
>> directory has the SecureBoot flag enabled.
>>
>> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>> ---
>>   pbs-api-types/src/node.rs | 30 +++++++++++++++++++++++++++---
>>   src/api2/node/status.rs   | 29 +++++++++++++++++++++++++++--
>>   2 files changed, 54 insertions(+), 5 deletions(-)
>>
>> diff --git a/pbs-api-types/src/node.rs b/pbs-api-types/src/node.rs
>> index 704215bb..ab626157 100644
>> --- a/pbs-api-types/src/node.rs
>> +++ b/pbs-api-types/src/node.rs
>> @@ -1,9 +1,8 @@
>> -use serde::{Deserialize, Serialize};
>>   use proxmox_schema::*;
>> +use serde::{Deserialize, Serialize};
>>   
>>   use crate::StorageStatus;
>>   
>> -
>>   #[api]
>>   #[derive(Serialize, Deserialize, Default)]
>>   #[serde(rename_all = "kebab-case")]
>> @@ -39,6 +38,29 @@ pub struct NodeInformation {
>>       pub fingerprint: String,
>>   }
>>   
>> +
>> +#[api]
>> +#[derive(Serialize, Deserialize, Copy, Clone)]
>> +#[serde(rename_all = "kebab-case")]
>> +/// The possible BootModes
>> +pub enum BootMode {
>> +    /// The BootMode is EFI/UEFI
>> +    Efi,
>> +    /// The BootMode is Legacy BIOS
>> +    LegacyBios,
>> +}
> ^ Should be able to have a From<proxmox_sys::boot_mode::BootMode> here.
I don't know if we want to pull in `proxmox_sys` into `pbs-api-types`?

Re: [pbs-devel] [PATCH v4 proxmox-backup 4/5] node: status: declutter kernel-version

[Gabriel Goller]

On 11/29/23 11:23, Wolfgang Bumiller wrote:
> On Wed, Nov 29, 2023 at 10:07:45AM +0100, Gabriel Goller wrote:
>>   
>> +#[api]
>> +#[derive(Serialize, Deserialize, Default)]
>> +#[serde(rename_all = "lowercase")]
>> +/// The current kernel version (output of `uname`)
>> +pub struct KernelVersionInformation {
>> +    /// The systemname/nodename
>> +    pub sysname: String,
>> +    /// The kernel release number
>> +    pub release: String,
>> +    /// The kernel version
>> +    pub version: String,
>> +    /// The machine architecture
>> +    pub machine: String,
>> +}
>> +
>> +impl KernelVersionInformation {
>> +    pub fn from_ostr(sysname: &OsStr, release: &OsStr, version: &OsStr, machine: &OsStr) -> Self {
> from_ostr is a bit of a weird name for a public method taking 4
> parameters.
>
> consider a `From<&UtsName>` implementation.
Hmm I was under the impression that we don't want to pull in any more 
crates into `pbs-api-types`...
Another option I had was implement From<[&OsStr; 4] on 
KernelVersionInformation?
>> +        KernelVersionInformation {
>> +            sysname: sysname
>> +                .to_os_string()
>> +                .into_string()
>> +                .unwrap_or("".to_string()),
> .unwrap_or_default(), also, whenver you think "".to_string(), type
> String::new() instead ;-)
>
> also, you first convert to an owned string - this always works, and then
> do the fallible `into_string()`, so even if it's not a valid string you
> clone and discard, instead, do:
>      sysname
>          .to_str()
>          .map(String::from)
>          .unwrap_or_default()
>
> this way there's no string copying if it's not valid utf-8 becausee that
> is checked first
>
> same for the cases below
Ohh, thanks for the heads up!

Re: [pbs-devel] [PATCH v4 proxmox-backup 4/5] node: status: declutter kernel-version

[Wolfgang Bumiller]

On Wed, Nov 29, 2023 at 01:50:00PM +0100, Gabriel Goller wrote:
> On 11/29/23 11:23, Wolfgang Bumiller wrote:
> > On Wed, Nov 29, 2023 at 10:07:45AM +0100, Gabriel Goller wrote:
> > > +#[api]
> > > +#[derive(Serialize, Deserialize, Default)]
> > > +#[serde(rename_all = "lowercase")]
> > > +/// The current kernel version (output of `uname`)
> > > +pub struct KernelVersionInformation {
> > > +    /// The systemname/nodename
> > > +    pub sysname: String,
> > > +    /// The kernel release number
> > > +    pub release: String,
> > > +    /// The kernel version
> > > +    pub version: String,
> > > +    /// The machine architecture
> > > +    pub machine: String,
> > > +}
> > > +
> > > +impl KernelVersionInformation {
> > > +    pub fn from_ostr(sysname: &OsStr, release: &OsStr, version: &OsStr, machine: &OsStr) -> Self {
> > from_ostr is a bit of a weird name for a public method taking 4
> > parameters.
> > 
> > consider a `From<&UtsName>` implementation.
> Hmm I was under the impression that we don't want to pull in any more crates
> into `pbs-api-types`...

Sorry, that was a brainfart.
For some reason I thought we had features there (and we used to have
access to sys back when UPID type was in here...).
(Same goes for my other reply)

> Another option I had was implement From<[&OsStr; 4] on
> KernelVersionInformation?

`[&OsStr; 4]` is a bit too unspecific for a From<> impl, maybe just name
it `from_uname_parts` so it's clear what exactly to put in there.