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

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

Thanks for the review!

On 11/27/23 14:10, Wolfgang Bumiller wrote:
> On Mon, Nov 27, 2023 at 11:52:37AM +0100, Gabriel Goller wrote:
>> +
>> +#[api]
>> +#[derive(Serialize, Deserialize, Default)]
> And Clone + Copy
Agree
>> +#[serde(rename_all = "kebab-case")]
>> +/// The possible BootModes
>> +pub enum BootMode {
>> +    /// The BootMode is EFI/UEFI
>> +    Efi,
>> +    /// The BootMode is Legacy BIOS
>> +    #[default]
> ^ do we *need* Default on this type? And why is Bios the default?
Removed it. Was enabled on the `NodeStatus` struct and cascaded down, 
but afaik we can remove it
on the `NodeStatus` struct as well and get rid of it.
>
>> +    LegacyBios,
>> +}
>> +
>> +#[api]
>> +#[derive(Serialize, Deserialize, Default)]
> And Clone
yep
>> +fn boot_mode_to_info(bm: boot_mode::BootModeInformation) -> BootModeInformation {
> I'd prefer the match to be a single level by doing
>
>      use boot_mode::{BootModeIntofmation::*, SecureBoot};
>      use pbs_api_types::BootMode;
>      match bm {
>          Efi(SecureBoot::Enabled) => BootModeInformation {
>              ...
>              mode: BootMode::Efi,
>              secureboot: true,
>          },
>          Efi(SecureBoot::Disabled) => BootModeInformation {
>              ...
>              mode: BootMode::Efi,
>              secureboot: false,
>          },
>          Bios => BootModeInformation {
>              ...
>              mode: BootMode::LegacyBios,
>              secureboot: false,
>          },
>      }
>
> IMO that's easier to follow.
Good point!