[PATCH docs/guest-common/qemu-server v5 0/9] add new pci passthrough specific hookscript phase

[PATCH docs/guest-common/qemu-server v5 0/9] 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 last qemu-server commit for why that is interesting.

changes from v4:
* fixed code style
* improved comment for ENV copying
* added cleanup commit for renaming variable
* added commit to downgrade to warn for unknown phases in example hookscript

changes from v3:
* rebase on current master
* fix `local %ENV` clearing the whole environment instead of
  just shadowing it, so the hookscript keeps the inherited environment
* split the uuid handling out of the refactoring commit
* call the hookscript once for both the mdev and the plain passthrough
  case instead of duplicating the call in both branches
* add missing `use PVE::GuestHelpers;`
* move the new phase before 'post-start' to match the actual call
  order, document that PVE_HOOK_MDEV_UUID is only set for mdevs, and
  mention what the phase is useful for
* fix typos

changes from v2:
* rebase on current master
* prefix hook script env variables with 'PVE_HOOK_'


pve-guest-common:

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

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


qemu-server:

Dominik Csapak (6):
  pci: mdev preparation: always generate local uuid first
  pci: nvidia vgpu: correct wrong comment about uuid
  pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module
  code cleanup: pci: rename 'd' variable to 'pci_device'
  pci: preparation: mdev: only generate uuid once
  pci: call hookscript for each prepared pci device

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


pve-docs:

Dominik Csapak (2):
  examples: add new hookscript phase to example hookscript
  examples: downgrade 'die' in example hookscript to 'warn'

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


Summary over all repositories:
  4 files changed, 101 insertions(+), 39 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH guest-common v5 1/9] 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.

Use the prefix 'PVE_HOOK_' so they're always in a separate namespace.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v4:
* improve ENV comment
* use better style for $params initialization

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

diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
index f8d112b..41c816f 100644
--- a/src/PVE/GuestHelpers.pm
+++ b/src/PVE/GuestHelpers.pm
@@ -115,14 +115,24 @@ 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 = {} if !defined($params);
+
     eval {
         my $hookscript = check_hookscript($conf->{hookscript});
         die $@ if $@;
 
+        # copy ENV, so we keep the inherited one before adding to it
+        local %ENV = (%ENV);
+
+        for my $key (keys $params->%*) {
+            my $new_key = "PVE_HOOK_" . uc($key);
+            $ENV{$new_key} = $params->{$key};
+        }
+
         PVE::Tools::run_command([$hookscript, $vmid, $phase]);
     };
     if (my $err = $@) {
-- 
2.47.3

[PATCH qemu-server v5 2/9] pci: mdev preparation: always generate local uuid first

[Dominik Csapak]

We still prefer the uuid from the smbios config, but this makes the
pci preparation code independent of `parse_smbios1` and we can later
move it out.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Reviewed-by: Jakob Klocker <j.klocker@proxmox.com>
Tested-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/QemuServer.pm | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 0ed75cd7..2f9eae6c 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5680,14 +5680,17 @@ sub vm_start_nolock {
 
             # 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::Mdev::generate_mdev_uuid($vmid, $index)
-                    if !defined($uuid);
+                $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
             }
         }
+
+        # 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 = $@) {
-- 
2.47.3

[PATCH qemu-server v5 3/9] pci: nvidia vgpu: correct wrong comment about uuid

[Dominik Csapak]

At the time the mdev code was written, it was believed that the uuid of
the qemu vm has to be the same as the mdev. This is obviously not true,
since we already prefer the smbios uuid, and if more than one vGPU is
given to a VM, this can't hold.

So the comment must be wrong, and the nvidia driver simply expects *a*
uuid, not the one of the mdev.

Also, it's not named NVIDIA GRID anymore, so use the current vGPU
terminology.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Reviewed-by: Jakob Klocker <j.klocker@proxmox.com>
Tested-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 2f9eae6c..9f6a4d4c 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5678,7 +5678,7 @@ sub vm_start_nolock {
             next if !$d->{mdev} && !$d->{nvidia};
             die "could not create mediated device\n" if !defined($chosen_mdev);
 
-            # nvidia grid needs the uuid of the mdev as qemu parameter
+            # nvidia vgpu needs a uuid as qemu parameter
             if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
                 $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
             }
-- 
2.47.3

[PATCH qemu-server v5 4/9] pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module

[Dominik Csapak]

no functional change intended.

Note: $conf is unused in `prepare_pci_devices` for now, but will be used
later.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Reviewed-by: Jakob Klocker <j.klocker@proxmox.com>
Tested-by: Jakob Klocker <j.klocker@proxmox.com>
---
 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 9f6a4d4c..5ac8710e 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5649,40 +5649,8 @@ 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);
-
-            # nvidia vgpu needs a uuid as qemu parameter
-            if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
-                $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
-            }
-        }
+        ($pci_reserve_list, my $uuid) =
+            PVE::QemuServer::PCI::prepare_pci_devices($conf, $vmid, $pci_devices, $start_timeout);
 
         # 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 d7dc6121..548c4720 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -714,6 +714,48 @@ 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 needs a uuid as qemu parameter
+        if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
+            $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
+        }
+    }
+
+    return ($pci_reserve_list, $uuid);
+}
+
 sub prepare_pci_device {
     my ($vmid, $pciid, $index, $device) = @_;
 
-- 
2.47.3

[PATCH qemu-server v5 5/9] code cleanup: pci: rename 'd' variable to 'pci_device'

[Dominik Csapak]

to make it a bit cleare what it is.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v5
 src/PVE/QemuServer/PCI.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 548c4720..5cb6e91c 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -729,13 +729,13 @@ sub prepare_pci_devices {
     reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
 
     for my $id (sort keys %$pci_devices) {
-        my $d = $pci_devices->{$id};
+        my $pci_device = $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}) {
+        for my $dev ($pci_device->{ids}->@*) {
+            my $info = eval { prepare_pci_device($vmid, $dev->{id}, $index, $pci_device) };
+            if ($pci_device->{mdev} || $pci_device->{nvidia}) {
                 warn $@ if $@;
                 $chosen_mdev = $info;
                 last if $chosen_mdev; # if successful, we're done
@@ -744,7 +744,7 @@ sub prepare_pci_devices {
             }
         }
 
-        next if !$d->{mdev} && !$d->{nvidia};
+        next if !$pci_device->{mdev} && !$pci_device->{nvidia};
         die "could not create mediated device\n" if !defined($chosen_mdev);
 
         # nvidia vgpu needs a uuid as qemu parameter
-- 
2.47.3

[PATCH qemu-server v5 6/9] pci: preparation: mdev: only generate uuid once

[Dominik Csapak]

In `prepare_pci_device` we already generate a uuid for the mdev. Later,
for NVIDIA vGPU, we need the uuid again to add it to the qemu cmdline.
Instead of generating it twice, return it from `prepare_pci_device` and
reuse that if we can.

If the device uses the modern nvidia vgpu stack, we don't need to create
a mdev, so generate the fallback uuid in `prepare_pci_devices`.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Reviewed-by: Jakob Klocker <j.klocker@proxmox.com>
Tested-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/QemuServer/PCI.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 5cb6e91c..7bfe1791 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -749,7 +749,8 @@ sub prepare_pci_devices {
 
         # nvidia vgpu needs a uuid as qemu parameter
         if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
-            $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
+            $uuid = $chosen_mdev->{uuid}
+                // PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
         }
     }
 
@@ -770,6 +771,7 @@ sub prepare_pci_device {
     } elsif (my $mdev = $device->{mdev}) {
         my $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
         PVE::QemuServer::PCI::Mdev::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

[PATCH qemu-server v5 7/9] 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).

A failing hookscript aborts, and blocks the VM start.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Reviewed-by: Jakob Klocker <j.klocker@proxmox.com>
Tested-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/QemuServer/PCI.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 7bfe1791..8a7328b0 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -5,6 +5,7 @@ use strict;
 
 use IO::File;
 
+use PVE::GuestHelpers;
 use PVE::JSONSchema;
 use PVE::Mapping::PCI;
 use PVE::SysFSTools;
@@ -738,10 +739,17 @@ sub prepare_pci_devices {
             if ($pci_device->{mdev} || $pci_device->{nvidia}) {
                 warn $@ if $@;
                 $chosen_mdev = $info;
-                last if $chosen_mdev; # if successful, we're done
             } else {
                 die $@ if $@;
             }
+
+            next if !defined($info);
+
+            my $params = { id => $id, pciid => $info->{name} };
+            $params->{mdev_uuid} = $info->{uuid} if defined($info->{uuid});
+            PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-pci-prepare', 1, $params);
+
+            last if defined($chosen_mdev);
         }
 
         next if !$pci_device->{mdev} && !$pci_device->{nvidia};
-- 
2.47.3

[PATCH docs v5 8/9] 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 its
parameters with a comment.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 examples/guest-example-hookscript.pl | 29 ++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/examples/guest-example-hookscript.pl b/examples/guest-example-hookscript.pl
index 1cce2e3..02e7e7d 100755
--- a/examples/guest-example-hookscript.pl
+++ b/examples/guest-example-hookscript.pl
@@ -31,6 +31,35 @@ if ($phase eq 'pre-start') {
     # print "preparations failed, aborting."
     # exit(1);
 
+} 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 Proxmox VE 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 can be useful to do additional preparation that the Proxmox VE
+    # stack does not do, like setting vgpu_params for NVIDIA vGPU.
+    #
+    # This phase has 3 additional parameters (one optional) given via the environment:
+
+    # the id from the config, e.g. 'hostpci0'
+    my $hostpci_id = $ENV{PVE_HOOK_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{PVE_HOOK_PCIID};
+
+    # the uuid of the mediated device if it was one,
+    # e.g. '00000001-0000-0000-0000-000000008006'
+    # This is not set if the pci device is not an mdev.
+    my $mdev_uuid = $ENV{PVE_HOOK_MDEV_UUID};
+
+    print "Prepared PCI device for $hostpci_id with pciid: $pciid.\n";
+    print "It is a mediated device with UUID: $mdev_uuid\n" if $mdev_uuid;
+
 } elsif ($phase eq 'post-start') {
 
     # Second phase 'post-start' will be executed after the guest
-- 
2.47.3

[PATCH docs v5 9/9] examples: downgrade 'die' in example hookscript to 'warn'

[Dominik Csapak]

So when users copy and modify the hookscript, they only get a warning
when new phases are added.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v5
 examples/guest-example-hookscript.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/guest-example-hookscript.pl b/examples/guest-example-hookscript.pl
index 02e7e7d..853304f 100755
--- a/examples/guest-example-hookscript.pl
+++ b/examples/guest-example-hookscript.pl
@@ -84,7 +84,7 @@ if ($phase eq 'pre-start') {
     print "$vmid stopped. Doing cleanup.\n";
 
 } else {
-    die "got unknown phase '$phase'\n";
+    warn "got unknown phase '$phase'\n";
 }
 
 exit(0);
-- 
2.47.3