From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 52D181FF0DF for ; Thu, 05 Nov 2026 16:24:02 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A4B98214E4; Thu, 05 Nov 2026 16:23:58 +0100 (CET) Message-ID: Date: Thu, 5 Nov 2026 16:23:54 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server v3 11/19] api: check the custom firmware of the bios option To: Christian Ludwig , pve-devel@lists.proxmox.com References: Content-Language: en-US From: Fiona Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793892234301 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: ZQW2YKXDLBAT2BUCDVPRFFCQGEAJHWI4 X-Message-ID-Hash: ZQW2YKXDLBAT2BUCDVPRFFCQGEAJHWI4 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: Am 27.10.26 um 11:14 AM schrieb Christian Ludwig: > When the bios option is set via the POST/PUT {vmid}/config endpoints, > check that the user may access the custom firmware image, that it is an > efi-firmware volume, and that the BIOS type is 'ovmf'. Like for ISO > images, whether the image is usable on the node is only checked when the > VM is started. The option keeps requiring the VM.Config.Options > permission. > > A cloned VM references the same firmware image, like an ISO image, so a > clone is only considered shared if the image is on a shared storage. > > Report custom firmware images in the local disks of the migration > precondition check, so that clients can tell why a migration is not > possible. > > Signed-off-by: Christian Ludwig > --- > src/PVE/API2/Qemu.pm | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm > index 922c6599..3338edc2 100644 > --- a/src/PVE/API2/Qemu.pm > +++ b/src/PVE/API2/Qemu.pm > @@ -29,6 +29,7 @@ use PVE::GuestImport; > use PVE::QemuConfig; > use PVE::QemuServer; > use PVE::QemuServer::Agent; > +use PVE::QemuServer::BIOS; > use PVE::QemuServer::Blockdev; > use PVE::QemuServer::BlockJob; > use PVE::QemuServer::Cloudinit; > @@ -107,6 +108,20 @@ my $foreach_volume_with_alloc = sub { > } > }; > > +my $check_bios_param = sub { Style nit: please use "my sub check_bios_param {" > + my ($param) = @_; > + > + return if !defined($param->{bios}); > + > + eval { > + PVE::QemuServer::BIOS::check_bios(PVE::QemuServer::BIOS::parse_bios($param->{bios})); > + }; > + if (my $err = $@) { > + chomp($err); > + raise_param_exc({ bios => $err }); > + } > +}; > + > my $check_drive_param = sub { > my ($param, $storecfg, $extra_checks) = @_; > > @@ -277,6 +292,13 @@ my $check_storage_access = sub { > "/storage/$settings->{vmstatestorage}", > ['Datastore.AllocateSpace'], > ) if defined($settings->{vmstatestorage}); > + > + if (defined($settings->{bios})) { > + my $firmware = PVE::QemuServer::BIOS::parse_bios($settings->{bios})->{firmware}; > + PVE::Storage::check_volume_access( > + $rpcenv, $authuser, $storecfg, $vmid, $firmware, 'efi-firmware', > + ) if defined($firmware); Style nit: multiline post-if should be avoided. > + } > }; > > my $check_storage_access_clone = sub { > @@ -319,6 +341,13 @@ my $check_storage_access_clone = sub { > $rpcenv->check($authuser, "/storage/$conf->{vmstatestorage}", ['Datastore.AllocateSpace']) > if defined($conf->{vmstatestorage}); > > + # like ISO images, a custom firmware image is simply referenced by the clone > + if (my $firmware = PVE::QemuServer::BIOS::get_bios_firmware($conf)) { > + my ($sid) = PVE::Storage::parse_volume_id($firmware); > + my $scfg = PVE::Storage::storage_config($storecfg, $sid); > + $sharedvm = 0 if !$scfg->{shared}; > + } > + > return $sharedvm; > }; > > @@ -1365,6 +1394,7 @@ __PACKAGE__->register_method({ > &$check_cpu_model_access($rpcenv, $authuser, $param); > > $check_drive_param->($param, $storecfg); > + $check_bios_param->($param); > > PVE::QemuServer::Network::add_random_macs($param); > } > @@ -2203,6 +2233,8 @@ my $update_vm_api = sub { > $rpcenv, $authuser, $storecfg, $vmid, $param, undef, $extraction_storage, > ); > > + $check_bios_param->($param); > + > PVE::QemuServer::check_bridge_access($rpcenv, $authuser, $param); > > my $load_and_check_config = sub { > @@ -5348,6 +5380,11 @@ __PACKAGE__->register_method({ > type => 'boolean', > description => 'True if the disk is a cdrom.', > }, > + is_firmware => { > + type => 'boolean', > + description => 'True if the volume is a custom firmware image.', > + optional => 1, > + }, > is_unused => { > type => 'boolean', > description => 'True if the disk is unused.',