[pve-devel] [PATCH docs/guest-common/qemu-server v2 0/4] add new pci passthrough specific hookscript phase

[pve-devel] [PATCH docs/guest-common/qemu-server v2 0/4] add new pci passthrough specific hookscript phase

[Dominik Csapak]

this series adds a new phase to the guest hookscript that is called for each
passed throug pci device after it's prepared, but before the qemu process is
started.

See the second qemu-server commit for why that is interesting.

pve-guest-common:

Dominik Csapak (1):
  helpers: exec hookscript: add optional parameters

 src/PVE/GuestHelpers.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)


qemu-server:

Dominik Csapak (2):
  pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module
  pci: call hookscript for each prepared pci device

 src/PVE/QemuServer.pm     | 45 +++++----------------------
 src/PVE/QemuServer/PCI.pm | 65 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 37 deletions(-)


pve-docs:

Dominik Csapak (1):
  examples: add new hookscript phase to example hookscript

 examples/guest-example-hookscript.pl | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)


Summary over all repositories:
  4 files changed, 107 insertions(+), 38 deletions(-)

-- 
Generated by git-murpp 0.8.1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH guest-common v2 1/1] helpers: exec hookscript: add optional parameters

[Dominik Csapak]

sometimes we may want to call the hookscript with additional parameters
in some phases, e.g. we want to call it for each pci device that was
prepared before starting with the correct uuid or pci id.

Add these new parameters to the environment instead of the positional
parameters of the hookscript, since that is more future proof and we get
a key/value pair instead of just the position.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* use a hash instead of a list for the parameters, and give them to the
  hookscript via the environment instead of positional parameters, like
  we do for the vzdump hookscript

 src/PVE/GuestHelpers.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
index f8d112b..b4122e6 100644
--- a/src/PVE/GuestHelpers.pm
+++ b/src/PVE/GuestHelpers.pm
@@ -115,14 +115,22 @@ sub check_hookscript {
 }
 
 sub exec_hookscript {
-    my ($conf, $vmid, $phase, $stop_on_error) = @_;
+    my ($conf, $vmid, $phase, $stop_on_error, $params) = @_;
 
     return if !$conf->{hookscript};
 
+    $params //= {};
+
     eval {
         my $hookscript = check_hookscript($conf->{hookscript});
         die $@ if $@;
 
+        local %ENV;
+
+        for my $key (keys $params->%*) {
+            $ENV{ uc($key) } = $params->{$key};
+        }
+
         PVE::Tools::run_command([$hookscript, $vmid, $phase]);
     };
     if (my $err = $@) {
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v2 1/2] pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module

[Dominik Csapak]

only slight change to the logic here was to return a uuid from an mdev
in any case if we need it, and overwrite it in vm_start_nolock with
the one from smbios1 if possible, instead of doing it the other way
round.

This was necessary since we don't have access to parse_smbios1 in the
PCI module.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v2
 src/PVE/QemuServer.pm     | 45 +++++++--------------------------------
 src/PVE/QemuServer/PCI.pm | 45 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 7a0a2606..6c9fd22a 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5633,45 +5633,16 @@ sub vm_start_nolock {
 
         push $cmd->@*, $state_cmdline->@*;
 
-        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 = $pci_devices->{$id};
-            my ($index) = ($id =~ m/^hostpci(\d+)$/);
-
-            my $chosen_mdev;
-            for my $dev ($d->{ids}->@*) {
-                my $info =
-                    eval { PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $index, $d) };
-                if ($d->{mdev} || $d->{nvidia}) {
-                    warn $@ if $@;
-                    $chosen_mdev = $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);
+        ($pci_reserve_list, my $uuid) =
+            PVE::QemuServer::PCI::prepare_pci_devices($conf, $vmid, $pci_devices, $start_timeout);
 
-            # nvidia grid needs the uuid of the mdev as qemu parameter
-            if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
-                if (defined($conf->{smbios1})) {
-                    my $smbios_conf = parse_smbios1($conf->{smbios1});
-                    $uuid = $smbios_conf->{uuid} if defined($smbios_conf->{uuid});
-                }
-                $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $index)
-                    if !defined($uuid);
-            }
+        # uuid for nvidia vgpu
+        # prefer the smbios1 uuid if we have and need it
+        if (defined($uuid) && defined($conf->{smbios1})) {
+            my $smbios_conf = parse_smbios1($conf->{smbios1});
+            $uuid = $smbios_conf->{uuid} if defined($smbios_conf->{uuid});
         }
+
         push @$cmd, '-uuid', $uuid if defined($uuid);
     };
     if (my $err = $@) {
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index c9cf8de0..f778c60f 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -736,6 +736,51 @@ sub print_hostpci_devices {
     return ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices);
 }
 
+sub prepare_pci_devices {
+    my ($conf, $vmid, $pci_devices, $start_timeout) = @_;
+
+    my $pci_reserve_list = [];
+    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 = $pci_devices->{$id};
+        my ($index) = ($id =~ m/^hostpci(\d+)$/);
+
+        my $chosen_mdev;
+        for my $dev ($d->{ids}->@*) {
+            my $info =
+                eval { prepare_pci_device($vmid, $dev->{id}, $index, $d) };
+            if ($d->{mdev} || $d->{nvidia}) {
+                warn $@ if $@;
+                $chosen_mdev = $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 need a uuid parameter, we want the smbios one but we can't parse
+        # 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);
+        }
+    }
+
+    return ($pci_reserve_list, $uuid);
+}
+
 sub prepare_pci_device {
     my ($vmid, $pciid, $index, $device) = @_;
 
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v2 2/2] pci: call hookscript for each prepared pci device

[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.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* adapt to new location and uuid code from previous patch
* use a hash for the parameters now, and always give the pciid, even
  if we have a uuid

 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},
+                        },
+                    );
+                }
             }
         }
 
@@ -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);
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs v2 1/1] examples: add new hookscript phase to example hookscript

[Dominik Csapak]

qemu-server has a new phase 'post-pci-prepare' that is called for vms
with pci passthrough for each device prepared.

add that to the example hookscript and explain when it's called and it's
parameters with a comment.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* adapt to the changes from positional parameters to environment variables

 examples/guest-example-hookscript.pl | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/examples/guest-example-hookscript.pl b/examples/guest-example-hookscript.pl
index 1cce2e3..a5a50d5 100755
--- a/examples/guest-example-hookscript.pl
+++ b/examples/guest-example-hookscript.pl
@@ -38,6 +38,31 @@ if ($phase eq 'pre-start') {
 
     print "$vmid started successfully.\n";
 
+} elsif ($phase eq 'post-pci-prepare') {
+
+    # Only called for virtual machines, not containers.
+    #
+    # This phase will be called for each pci device that is passed through,
+    # after it was prepared by the PVE stack. In other words when either
+    # * the mdev/vGPU was created
+    # * the driver was changed to vfio-pci and the device was reset
+    #
+    # This phase has 3 additional parameters given via the environment
+
+    # the id from the config, e.g. 'hostpci0'
+    my $hostpci_x = $ENV{ID};
+
+    # the pciid of the passed through device or the underlying device in case of an mdev/vGPU
+    # e.g. '0000:01:00.0'
+    my $pciid = $ENV{PCIID};
+
+    # the uuid of the mediated device if it was one,
+    # e.g. '00000001-0000-0000-0000-000000008006'
+    my $mdev_uuid = $ENV{MDEV_UUID};
+
+    print "Prepared PCI device for $hostpci_x with pciid: $pciid.\n";
+    print "It is a mediated device with UUID: $mdev_uuid\n" if $mdev_uuid;
+
 } elsif ($phase eq 'pre-stop') {
 
     # Third phase 'pre-stop' will be executed before stopping the guest
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel