From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 8558D1FF0DF for ; Thu, 05 Nov 2026 16:24:10 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 9A8BE214CD; Thu, 05 Nov 2026 16:24:06 +0100 (CET) Message-ID: Date: Thu, 5 Nov 2026 16:23:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server v3 09/19] config: define bios schema as property-string To: Christian Ludwig , pve-devel@lists.proxmox.com References: <85e63753ada8c733e10dd43b7600a749592f3ccd.1793029144.git@genua.de> Content-Language: en-US From: Fiona Ebner In-Reply-To: <85e63753ada8c733e10dd43b7600a749592f3ccd.1793029144.git@genua.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793892235698 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.294 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ITYLJ6PFJSC2G3TYTWGWOMOSDQKFGEY3 X-Message-ID-Hash: ITYLJ6PFJSC2G3TYTWGWOMOSDQKFGEY3 X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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'; > +