Re: [PATCH qemu-server v3 09/19] config: define bios schema as property-string

[Fiona Ebner <f.ebner@proxmox.com>]

The qemu-server patches need a rebase. More comments below:

Am 27.10.26 um 11:14 AM schrieb Christian Ludwig:
> Convert the bios parameter to a property-string and use the BIOS type as
> the default key for backward compatibility. This allows adding further
> BIOS related options, like a custom firmware image, as sub-properties.
> 
> Add helpers to parse and print the property-string and to query the
> BIOS type, and use the latter everywhere the bios option was compared
> directly. No functional change intended.
> 
> Signed-off-by: Christian Ludwig <christian_ludwig@genua.de>
> ---
>  src/PVE/QemuConfig.pm           |  3 ++-
>  src/PVE/QemuServer.pm           | 15 ++++++-----
>  src/PVE/QemuServer/BIOS.pm      | 48 +++++++++++++++++++++++++++++++++
>  src/PVE/QemuServer/CPUConfig.pm |  5 ++--
>  src/PVE/QemuServer/Makefile     |  1 +
>  src/PVE/QemuServer/PCI.pm       |  3 ++-
>  6 files changed, 64 insertions(+), 11 deletions(-)
>  create mode 100644 src/PVE/QemuServer/BIOS.pm
> 
> diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
> index 2ef6c075..d83a474c 100644
> --- a/src/PVE/QemuConfig.pm
> +++ b/src/PVE/QemuConfig.pm
> @@ -10,6 +10,7 @@ use PVE::INotify;
>  use PVE::JSONSchema;
>  use PVE::QemuMigrate::Helpers;
>  use PVE::QemuServer::Agent;
> +use PVE::QemuServer::BIOS;
>  use PVE::QemuServer::Blockdev;
>  use PVE::QemuServer::CPUConfig;
>  use PVE::QemuServer::Drive;
> @@ -192,7 +193,7 @@ sub get_backup_volumes {
>          my $reason = "backup=";
>          $reason .= defined($drive->{backup}) ? 'no' : 'yes';
>  
> -        if ($key =~ m/^efidisk/ && (!defined($conf->{bios}) || $conf->{bios} ne 'ovmf')) {
> +        if ($key =~ m/^efidisk/ && PVE::QemuServer::BIOS::get_bios_type($conf) ne 'ovmf') {

Minor style nit: could import the function like in other places for
easier readability in the conditional

>              $included = 0;
>              $reason = "efidisk but no OVMF BIOS";
>          }
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index 74772fdd..c288ba22 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -53,6 +53,7 @@ use PVE::QemuConfig;
>  use PVE::QemuConfig::NoWrite;
>  use PVE::QemuMigrate::Helpers;
>  use PVE::QemuServer::Agent qw(get_qga_key parse_guest_agent qga_check_running);
> +use PVE::QemuServer::BIOS qw(get_bios_type);
>  use PVE::QemuServer::Blockdev;
>  use PVE::QemuServer::BlockJob;
>  use PVE::QemuServer::Cfg2Cmd;
> @@ -659,7 +660,7 @@ EODESCR
>      bios => {
>          optional => 1,
>          type => 'string',
> -        enum => [qw(seabios ovmf)],
> +        format => 'pve-qm-bios',
>          description => "Select BIOS implementation.",
>          default => 'seabios',

The default should be moved to the new 'type' property inside.

>      },
> @@ -1535,7 +1536,7 @@ sub print_vga_device {
>  
>      my $edidoff = "";
>      if ($type eq 'VGA' && windows_version($conf->{ostype})) {
> -        $edidoff = ",edid=off" if (!defined($conf->{bios}) || $conf->{bios} ne 'ovmf');
> +        $edidoff = ",edid=off" if get_bios_type($conf) ne 'ovmf';
>      }
>  
>      my $q35 = PVE::QemuServer::Machine::machine_type_is_q35($conf);
> @@ -3075,7 +3076,7 @@ my sub should_disable_smm {
>      return if $machine =~ m/^virt/; # there is no smm flag that could be disabled
>  
>      return
> -        (!defined($conf->{bios}) || $conf->{bios} eq 'seabios')
> +        get_bios_type($conf) eq 'seabios'
>          && $vga->{type}
>          && $vga->{type} =~ m/^(serial\d+|none)$/;
>  }
> @@ -3258,7 +3259,7 @@ sub config_to_command {
>          push @$cmd, '-smbios', "type=1" . $smbios_string;
>      }
>  
> -    if ($conf->{bios} && $conf->{bios} eq 'ovmf') {
> +    if (get_bios_type($conf) eq 'ovmf') {
>          die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n"
>              if !$forcecpu && get_cpu_bitness($conf->{cpu}, $arch) == 32;
>  
> @@ -3771,10 +3772,10 @@ sub config_to_command {
>      }
>  
>      if ($conf->{'amd-sev'}) {
> -        push @$devices, '-object', get_amd_sev_object($conf->{'amd-sev'}, $conf->{bios});
> +        push @$devices, '-object', get_amd_sev_object($conf->{'amd-sev'}, get_bios_type($conf));
>          push @$machineFlags, 'confidential-guest-support=sev0';
>      } elsif ($conf->{'intel-tdx'}) {
> -        my $tdx_object = get_intel_tdx_object($conf->{'intel-tdx'}, $conf->{bios});
> +        my $tdx_object = get_intel_tdx_object($conf->{'intel-tdx'}, get_bios_type($conf));
>          push @$devices, '-object', to_json($tdx_object, { canonical => 1 });
>          push @$machineFlags, 'confidential-guest-support=tdx0';
>          push @$machineFlags, 'kernel_irqchip=split';
> @@ -5631,7 +5632,7 @@ sub vm_start_nolock {
>          my $storage_hints = generate_storage_hints($conf, 1);
>          PVE::Storage::activate_volumes($storecfg, $vollist, undef, $storage_hints);
>  
> -        check_efi_vars($storecfg, $vmid, $conf) if $conf->{bios} && $conf->{bios} eq 'ovmf';
> +        check_efi_vars($storecfg, $vmid, $conf) if get_bios_type($conf) eq 'ovmf';
>  
>          PVE::QemuServer::Drive::warn_about_virtio_win_issues_in_config($conf);
>  
> diff --git a/src/PVE/QemuServer/BIOS.pm b/src/PVE/QemuServer/BIOS.pm
> new file mode 100644
> index 00000000..4e1a3936
> --- /dev/null
> +++ b/src/PVE/QemuServer/BIOS.pm
> @@ -0,0 +1,48 @@
> +package PVE::QemuServer::BIOS;
> +
> +use strict;
> +use warnings;

Nit: please use v5.36; and subroutine signatures for new modules.

> +
> +use PVE::JSONSchema qw(parse_property_string print_property_string);
> +
> +use base 'Exporter';
> +