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 9F5181FF09F for ; Thu, 03 Sep 2026 10:53:16 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 71E5621590; Thu, 03 Sep 2026 10:53:16 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Thu, 03 Sep 2026 10:53:12 +0200 Message-Id: Subject: Re: [PATCH qemu-server v4 4/7] pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module From: "Jakob Klocker" To: "Dominik Csapak" , Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.20.0 References: <20260825135502.3971930-1-d.csapak@proxmox.com> <20260825135502.3971930-5-d.csapak@proxmox.com> In-Reply-To: <20260825135502.3971930-5-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788425590634 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.658 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: CY2IXA4YHZZ2XVGIWVFS6YQHS4XKRUMK X-Message-ID-Hash: CY2IXA4YHZZ2XVGIWVFS6YQHS4XKRUMK X-MailFrom: j.klocker@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: nit inline On Tue Aug 25, 2026 at 3:54 PM CEST, Dominik Csapak wrote: > no functional change intended. > > Note: $conf is unused in `prepare_pci_devices` for now, but will be used > later. > > Signed-off-by: Dominik Csapak > --- > src/PVE/QemuServer.pm | 36 ++------------------------------- > src/PVE/QemuServer/PCI.pm | 42 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+), 34 deletions(-) > > diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm > index 9c4c13b9..3eeea871 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -5697,40 +5697,8 @@ sub vm_start_nolock { > =20 > push $cmd->@*, $state_cmdline->@*; > =20 > - for my $device (values $pci_devices->%*) { > - next if $device->{mdev}; # we don't reserve for mdev devices > - push $pci_reserve_list->@*, map { $_->{id} } $device->{ids}-= >@*; > - } > - > - # reserve all PCI IDs before actually doing anything with them > - PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid= , $start_timeout); > - > - my $uuid; > - for my $id (sort keys %$pci_devices) { > - my $d =3D $pci_devices->{$id}; > - my ($index) =3D ($id =3D~ m/^hostpci(\d+)$/); > - > - my $chosen_mdev; > - for my $dev ($d->{ids}->@*) { > - my $info =3D > - eval { PVE::QemuServer::PCI::prepare_pci_device($vmi= d, $dev->{id}, $index, $d) }; > - if ($d->{mdev} || $d->{nvidia}) { > - warn $@ if $@; > - $chosen_mdev =3D $info; > - last if $chosen_mdev; # if successful, we're done > - } else { > - die $@ if $@; > - } > - } > - > - next if !$d->{mdev} && !$d->{nvidia}; > - die "could not create mediated device\n" if !defined($chosen= _mdev); > - > - # nvidia vgpu needs a uuid as qemu parameter > - if (!defined($uuid) && $chosen_mdev->{vendor} =3D~ m/^(0x)?1= 0de$/) { > - $uuid =3D PVE::QemuServer::PCI::Mdev::generate_mdev_uuid= ($vmid, $index); > - } > - } > + ($pci_reserve_list, my $uuid) =3D > + PVE::QemuServer::PCI::prepare_pci_devices($conf, $vmid, $pci= _devices, $start_timeout); > =20 > # uuid for nvidia vgpu > # prefer the smbios1 uuid if we have and need it > diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm > index 0b67943c..f7167dcc 100644 > --- a/src/PVE/QemuServer/PCI.pm > +++ b/src/PVE/QemuServer/PCI.pm > @@ -704,6 +704,48 @@ sub print_hostpci_devices { > return ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices); > } > =20 > +sub prepare_pci_devices { > + my ($conf, $vmid, $pci_devices, $start_timeout) =3D @_; > + > + my $pci_reserve_list =3D []; > + my $uuid; > + > + for my $device (values $pci_devices->%*) { > + next if $device->{mdev}; # we don't reserve for mdev devices > + push $pci_reserve_list->@*, map { $_->{id} } $device->{ids}->@*; > + } > + > + # reserve all PCI IDs before actually doing anything with them > + reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout); > + > + for my $id (sort keys %$pci_devices) { > + my $d =3D $pci_devices->{$id}; Since this code is being moved anyway, it might be a good opportunity to give $d a more descriptive name. Since $device is already in use, maybe $pci_device? > + my ($index) =3D ($id =3D~ m/^hostpci(\d+)$/); > + > + my $chosen_mdev; > + for my $dev ($d->{ids}->@*) { > + my $info =3D eval { prepare_pci_device($vmid, $dev->{id}, $i= ndex, $d) }; > + if ($d->{mdev} || $d->{nvidia}) { > + warn $@ if $@; > + $chosen_mdev =3D $info; > + last if $chosen_mdev; # if successful, we're done > + } else { > + die $@ if $@; > + } > + } > + > + next if !$d->{mdev} && !$d->{nvidia}; > + die "could not create mediated device\n" if !defined($chosen_mde= v); > + > + # nvidia vgpu needs a uuid as qemu parameter > + if (!defined($uuid) && $chosen_mdev->{vendor} =3D~ m/^(0x)?10de$= /) { > + $uuid =3D PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vm= id, $index); > + } > + } > + > + return ($pci_reserve_list, $uuid); > +} > + > sub prepare_pci_device { > my ($vmid, $pciid, $index, $device) =3D @_; > =20