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 D7D9A1FF0B2 for ; Mon, 24 Aug 2026 16:38:33 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 0AD012166A; Mon, 24 Aug 2026 16:38:14 +0200 (CEST) Message-ID: <4210ce86-02eb-41ae-828d-9f08b6074c6b@proxmox.com> Date: Mon, 24 Aug 2026 16:38:10 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server v3 2/2] pci: call hookscript for each prepared pci device To: Dominik Csapak , pve-devel@lists.proxmox.com References: <20260227120728.2152303-1-d.csapak@proxmox.com> <20260227120728.2152303-4-d.csapak@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260227120728.2152303-4-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787582260217 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.171 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) KAM_MAILER 2 Automated Mailer Tag Left in Email 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: TQB7NMUFABPMKDJZ64NCOLT6A6JKVDFH X-Message-ID-Hash: TQB7NMUFABPMKDJZ64NCOLT6A6JKVDFH 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.02.26 um 1:07 PM schrieb Dominik Csapak: > There are situations where a user might want to do extra things > for a passed through PCI device after it has been prepared/created (e.g. > in case of vGPU/mdev) but before the actual QEMU process is started. > > Two examples are (both are used with NVIDIA vGPUs): > * setting 'vgpu_params' such as removing the frame-rate-limiter > * setting the gpu_instance_id for MIG devices > > So instead of creating (nvidia-specific) interfaces for these, give a > user the ability to do it themselves via the hookscript as a first step. > > Call it for each prepared device, so that we can give the hookscript the > 'hostpciX' id, and the used uuid (in case of mdevs) or the pci id (in > case of regular or modern vGPU passthrough). > > Include the generated mdev uuid in the return value of > `prepare_pci_device`, to avoid having to generate that multiple times. > With that we can get rid of one extra generation here too. Could you please split out this last part into its own change? > > Signed-off-by: Dominik Csapak > --- > src/PVE/QemuServer/PCI.pm | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm > index f778c60f..5d9c7ab2 100644 > --- a/src/PVE/QemuServer/PCI.pm > +++ b/src/PVE/QemuServer/PCI.pm > @@ -761,9 +761,28 @@ sub prepare_pci_devices { > if ($d->{mdev} || $d->{nvidia}) { > warn $@ if $@; > $chosen_mdev = $info; > - last if $chosen_mdev; # if successful, we're done > + if (defined($chosen_mdev)) { > + my $params = { id => $id, pciid => $chosen_mdev->{name} }; > + $params->{mdev_uuid} = $chosen_mdev->{uuid}; > + PVE::GuestHelpers::exec_hookscript( > + $conf, $vmid, 'post-pci-prepare', 1, $params, > + ); > + last; > + } > } else { > die $@ if $@; > + if (defined($info)) { > + PVE::GuestHelpers::exec_hookscript( > + $conf, > + $vmid, > + 'post-pci-prepare', > + 1, > + { > + id => $id, > + pciid => $info->{name}, > + }, > + ); > + } > } Nit: If I'm not missing anything, $info is set if and only if there is no error. So we could restructure the code here as follows: > diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm > index 63d8c135..48b80e73 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -5714,12 +5714,17 @@ sub vm_start_nolock { > for my $dev ($d->{ids}->@*) { > my $info = > eval { PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $index, $d) }; > + if (my $err = $@) { > + die $err if !$d->{mdev} && !$d->{nvidia}; > + warn $err; > + next; > + } > + > + # hookscript (and stuff from bug #7711 ;)) here > + > if ($d->{mdev} || $d->{nvidia}) { > - warn $@ if $@; > $chosen_mdev = $info; > - last if $chosen_mdev; # if successful, we're done > - } else { > - die $@ if $@; > + last; > } > } > Then the error handling is tight near the prepare call and we can easily get away with just one exec_hookscript() call (just inject the additional param for the mdev||nvidia case. > } > > @@ -774,7 +793,7 @@ sub prepare_pci_devices { > # that here, so returnt any mdev uuid to signal we want one and as a fallback, > # in case there is not smbios uuid > if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) { > - $uuid = generate_mdev_uuid($vmid, $index) if !defined($uuid); > + $uuid = $chosen_mdev->{uuid} if !defined($uuid); > } > } > > @@ -795,6 +814,7 @@ sub prepare_pci_device { > } elsif (my $mdev = $device->{mdev}) { > my $uuid = generate_mdev_uuid($vmid, $index); > PVE::SysFSTools::pci_create_mdev_device($pciid, $uuid, $mdev); > + $info->{uuid} = $uuid; > } else { > die "can't unbind/bind PCI group to VFIO '$pciid'\n" > if !PVE::SysFSTools::pci_dev_group_bind_to_vfio($pciid);