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

[Gabriel Goller <g.goller@proxmox.com>]

Thanks for the review!

On 11/27/23 14:08, Wolfgang Bumiller wrote:
> On Mon, Nov 27, 2023 at 11:52:36AM +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 {
> In addition to my v3 reply with the range comparison: please avoid more
> free-standing functions for this type of stuff.
>
> Instead, this could be BootModeInformation::query().
>
> Also, are we sure we want this a a nested enum if we separate it in
> `pbs-api-types` later? Maybe these should just stay separate things,
> queried via BootMode::query() + SecureBoot::query()
> (A bit shorter with the `Information` suffix dropped there...)
Oh, that's actually not a bad idea.
I would have one `BootMode::query()` function that just checks if the 
`/sys/firmare/efi`
dir exists and return `Bios` or `Efi` + another function 
`SecureBoot::query()` that
returns `Enabled` or `Disabled` if the secureboot is enabled (checking 
the SecureBoot-xxx efivar).