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

[pbs-devel] [PATCH v3 proxmox{, -backup} 0/3] 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 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 function to get boot_mode

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


proxmox-backup:

Gabriel Goller (2):
  node: status: added bootmode
  node: status: declutter kernel-version

 pbs-api-types/src/node.rs | 76 +++++++++++++++++++++++++++++++++++++--
 src/api2/node/status.rs   | 40 ++++++++++++++++-----
 www/panel/NodeInfo.js     | 29 +++++++++++++--
 3 files changed, 132 insertions(+), 13 deletions(-)


Summary over all repositories:
  5 files changed, 187 insertions(+), 13 deletions(-)

-- 
murpp v0.4.0

[pbs-devel] [PATCH v3 proxmox 1/3] sys: add function to get boot_mode

[Gabriel Goller]

Helper that returns the current boot_mode. Either EFI, BIOS, or EFI
(Secure Boot).
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 | 54 ++++++++++++++++++++++++++++++++++++
 proxmox-sys/src/lib.rs       |  1 +
 2 files changed, 55 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..6dcdf07
--- /dev/null
+++ b/proxmox-sys/src/boot_mode.rs
@@ -0,0 +1,54 @@
+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 BootModeInformation {
+    /// The BootMode is EFI/UEFI, has a SecureBoot variant
+    Efi(SecureBoot),
+    /// The BootMode is Legacy BIOS
+    Bios,
+}
+
+// Returns the current bootmode (BIOS, EFI, or EFI(Secure Boot))
+pub fn boot_mode() -> BootModeInformation {
+    lazy_static::lazy_static!(
+        static ref BOOT_MODE: Mutex<Option<BootModeInformation>> = Mutex::new(None);
+    );
+
+    let mut last = BOOT_MODE.lock().unwrap();
+    let value = last.or_else(|| {
+        if std::path::Path::new("/sys/firmware/efi").exists() {
+            // 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(BootModeInformation::Efi(SecureBoot::Disabled));
+                };
+                if buf[4..] == [1] {
+                    Some(BootModeInformation::Efi(SecureBoot::Enabled))
+                } else {
+                    Some(BootModeInformation::Efi(SecureBoot::Disabled))
+                }
+            } else {
+                Some(BootModeInformation::Efi(SecureBoot::Disabled))
+            }
+        } else {
+            Some(BootModeInformation::Bios)
+        }
+    });
+    *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

Re: [pbs-devel] [PATCH v3 proxmox 1/3] sys: add function to get boot_mode

[Wolfgang Bumiller]

On Mon, Nov 27, 2023 at 11:16:42AM +0100, Gabriel Goller wrote:
> Helper that returns the current boot_mode. Either EFI, BIOS, or EFI
> (Secure Boot).
> 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 | 54 ++++++++++++++++++++++++++++++++++++
>  proxmox-sys/src/lib.rs       |  1 +
>  2 files changed, 55 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..6dcdf07
> --- /dev/null
> +++ b/proxmox-sys/src/boot_mode.rs
> @@ -0,0 +1,54 @@
> +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 BootModeInformation {
> +    /// The BootMode is EFI/UEFI, has a SecureBoot variant
> +    Efi(SecureBoot),
> +    /// The BootMode is Legacy BIOS
> +    Bios,
> +}
> +
> +// Returns the current bootmode (BIOS, EFI, or EFI(Secure Boot))
> +pub fn boot_mode() -> BootModeInformation {
> +    lazy_static::lazy_static!(
> +        static ref BOOT_MODE: Mutex<Option<BootModeInformation>> = Mutex::new(None);
> +    );
> +
> +    let mut last = BOOT_MODE.lock().unwrap();
> +    let value = last.or_else(|| {
> +        if std::path::Path::new("/sys/firmware/efi").exists() {
> +            // 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(BootModeInformation::Efi(SecureBoot::Disabled));
> +                };
> +                if buf[4..] == [1] {

This doesn't need to be a range comparison, just use `buf[4] == 1`
Or rather,, should we instead use `!= 0`? 
Depending on how we want to treat a "reserved" mode...

Re: [pbs-devel] [PATCH v3 proxmox 1/3] sys: add function to get boot_mode

[Gabriel Goller]

Thanks for the review!

On 11/27/23 13:48, Wolfgang Bumiller wrote:

> On Mon, Nov 27, 2023 at 11:16:42AM +0100, Gabriel Goller wrote:
>> +// Returns the current bootmode (BIOS, EFI, or EFI(Secure Boot))
>> +pub fn boot_mode() -> BootModeInformation {
>> +    lazy_static::lazy_static!(
>> +        static ref BOOT_MODE: Mutex<Option<BootModeInformation>> = Mutex::new(None);
>> +    );
>> +
>> +    let mut last = BOOT_MODE.lock().unwrap();
>> +    let value = last.or_else(|| {
>> +        if std::path::Path::new("/sys/firmware/efi").exists() {
>> +            // 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(BootModeInformation::Efi(SecureBoot::Disabled));
>> +                };
>> +                if buf[4..] == [1] {
> This doesn't need to be a range comparison, just use `buf[4] == 1`
> Or rather,, should we instead use `!= 0`?
> Depending on how we want to treat a "reserved" mode...
Oh, right! I agree with the `buf[4] == 1`.
Hmm I think a more "conservative" matching here is better. A false
negative is better than a false positive imo.

[pbs-devel] [PATCH v3 proxmox-backup 2/3] 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 | 29 +++++++++++++++++++++++++++--
 src/api2/node/status.rs   | 28 ++++++++++++++++++++++++++--
 www/panel/NodeInfo.js     | 16 ++++++++++++++++
 3 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/pbs-api-types/src/node.rs b/pbs-api-types/src/node.rs
index 704215bb..8bcf8fd6 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,30 @@ pub struct NodeInformation {
     pub fingerprint: String,
 }
 
+
+#[api]
+#[derive(Serialize, Deserialize, Default)]
+#[serde(rename_all = "kebab-case")]
+/// The possible BootModes
+pub enum BootMode {
+    /// The BootMode is EFI/UEFI
+    Efi,
+    /// The BootMode is Legacy BIOS
+    #[default]
+    LegacyBios,
+}
+
+#[api]
+#[derive(Serialize, Deserialize, Default)]
+#[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")]
@@ -97,4 +120,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..21f1bc2c 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,25 @@ fn procfs_to_node_cpu_info(info: procfs::ProcFsCPUInfo) -> NodeCpuInformation {
     }
 }
 
+fn boot_mode_to_info(bm: boot_mode::BootModeInformation) -> BootModeInformation {
+    match bm {
+        boot_mode::BootModeInformation::Efi(secureboot) => match secureboot {
+            boot_mode::SecureBoot::Enabled => BootModeInformation {
+                mode: pbs_api_types::BootMode::Efi,
+                secureboot: true,
+            },
+            boot_mode::SecureBoot::Disabled => BootModeInformation {
+                mode: pbs_api_types::BootMode::Efi,
+                secureboot: false,
+            },
+        },
+        boot_mode::BootModeInformation::Bios => BootModeInformation {
+            mode: pbs_api_types::BootMode::LegacyBios,
+            secureboot: false,
+        },
+    }
+}
+
 #[api(
     input: {
         properties: {
@@ -79,6 +100,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::boot_mode());
+
     Ok(NodeStatus {
         memory,
         swap,
@@ -96,6 +119,7 @@ async fn get_status(
         info: NodeInformation {
             fingerprint: crate::cert_info()?.fingerprint()?,
         },
+        boot_info,
     })
 }
 
diff --git a/www/panel/NodeInfo.js b/www/panel/NodeInfo.js
index 2551c9a5..907abe3a 100644
--- a/www/panel/NodeInfo.js
+++ b/www/panel/NodeInfo.js
@@ -147,6 +147,22 @@ Ext.define('PBS.NodeInfoPanel', {
 	    textField: 'kversion',
 	    value: '',
 	},
+	{
+	    colspan: 2,
+	    title: gettext('Boot Mode'),
+	    printBar: false,
+	    textField: 'boot-info',
+	    renderer: boot => {
+                console.log(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

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

[Lukas Wagner]

On 11/27/23 11:16, Gabriel Goller wrote:
> diff --git a/www/panel/NodeInfo.js b/www/panel/NodeInfo.js
> index 2551c9a5..907abe3a 100644
> --- a/www/panel/NodeInfo.js
> +++ b/www/panel/NodeInfo.js
> @@ -147,6 +147,22 @@ Ext.define('PBS.NodeInfoPanel', {
>   	    textField: 'kversion',
>   	    value: '',
>   	},
> +	{
> +	    colspan: 2,
> +	    title: gettext('Boot Mode'),
> +	    printBar: false,
> +	    textField: 'boot-info',
> +	    renderer: boot => {
> +                console.log(boot);

Left-over debug-print? :) (also indented wrong using only spaces)

> +		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',

-- 
- Lukas

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

[Gabriel Goller]

On 11/27/23 11:39, Lukas Wagner wrote:

>
>
> On 11/27/23 11:16, Gabriel Goller wrote:
>> diff --git a/www/panel/NodeInfo.js b/www/panel/NodeInfo.js
>> index 2551c9a5..907abe3a 100644
>> --- a/www/panel/NodeInfo.js
>> +++ b/www/panel/NodeInfo.js
>> @@ -147,6 +147,22 @@ Ext.define('PBS.NodeInfoPanel', {
>>           textField: 'kversion',
>>           value: '',
>>       },
>> +    {
>> +        colspan: 2,
>> +        title: gettext('Boot Mode'),
>> +        printBar: false,
>> +        textField: 'boot-info',
>> +        renderer: boot => {
>> +                console.log(boot);
>
> Left-over debug-print? :) (also indented wrong using only spaces)
Oof, right. Submitted a new patch!
>
>> +        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',
>

[pbs-devel] [PATCH v3 proxmox-backup 3/3] 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. Extract and display the build version and kernel
release nicely.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-api-types/src/node.rs | 47 ++++++++++++++++++++++++++++++++++++++-
 src/api2/node/status.rs   | 18 +++++++--------
 www/panel/NodeInfo.js     | 13 +++++++++--
 3 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/pbs-api-types/src/node.rs b/pbs-api-types/src/node.rs
index 8bcf8fd6..6004acb3 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, Default)]
@@ -112,7 +155,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 21f1bc2c..e4723938 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::{
@@ -91,11 +90,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?;
@@ -112,7 +111,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,
diff --git a/www/panel/NodeInfo.js b/www/panel/NodeInfo.js
index 907abe3a..7170ec30 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