Re: [PATCH qemu-server v3 2/2] pci: call hookscript for each prepared pci device

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

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 <d.csapak@proxmox.com>
> ---
>  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);