[pve-devel] [PATCH common/qemu-server/manager v2] improve vGPU (mdev) usage for NVIDIA

[pve-devel] [PATCH common/qemu-server/manager v2] improve vGPU (mdev) usage for NVIDIA

[Dominik Csapak]

This series improves the handling of NVIDIA vGPUs by exposing the optional name
and automatically adding the uuid to the qemu process (required by NVIDIA
driver). Also adds the name to the UI for selecting a mediated devices as well
as making the dropdown larger so users can see all the relevant info.

changes from v1:
* use read_firstline instead of get_contents (to avoid the newline at the end)
* make sure only one '-uuid' parameter is given
* add patch 'cleanup pci device in more situations' (was seperate before)

pve-common:

Dominik Csapak (1):
  SysFSTools: get name from mediated device types

 src/PVE/SysFSTools.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

qemu-server:

Dominik Csapak (2):
  automatically add 'uuid' parameter when passing through NVIDIA vGPU
  cleanup pci devices in more situations

 PVE/QemuServer.pm     | 49 ++++++++++++++++++++++++++++---------------
 PVE/QemuServer/PCI.pm |  4 +++-
 2 files changed, 35 insertions(+), 18 deletions(-)

pve-manager:

Dominik Csapak (1):
  ui: improve form/MDevSelector

 www/manager6/form/MDevSelector.js | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH common v2 1/1] SysFSTools: get name from mediated device types

[Dominik Csapak]

Some vendors also provide a 'name' file here for the type, which, in case of
NVIDIA, is the official name for the vGPU type in their documentation,
so extract and return it too (if it exists).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/SysFSTools.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
index ac48f2c..b4cd5cc 100644
--- a/src/PVE/SysFSTools.pm
+++ b/src/PVE/SysFSTools.pm
@@ -172,11 +172,16 @@ sub get_mdev_types {
 	my $available = int(file_read_firstline("$type_path/available_instances"));
 	my $description = PVE::Tools::file_get_contents("$type_path/description");
 
-	push @$types, {
+	my $entry = {
 	    type => $type,
 	    description => $description,
 	    available => $available,
 	};
+
+	my $name = file_read_firstline("$type_path/name");
+	$entry->{name} = $name if defined($name);
+
+	push @$types, $entry;
     });
 
     return $types;
-- 
2.30.2

[pve-devel] applied: [PATCH common v2 1/1] SysFSTools: get name from mediated device types

[Wolfgang Bumiller]

applied, thanks

On Fri, Aug 12, 2022 at 11:29:48AM +0200, Dominik Csapak wrote:
> Some vendors also provide a 'name' file here for the type, which, in case of
> NVIDIA, is the official name for the vGPU type in their documentation,
> so extract and return it too (if it exists).
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/SysFSTools.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
> index ac48f2c..b4cd5cc 100644
> --- a/src/PVE/SysFSTools.pm
> +++ b/src/PVE/SysFSTools.pm
> @@ -172,11 +172,16 @@ sub get_mdev_types {
>  	my $available = int(file_read_firstline("$type_path/available_instances"));
>  	my $description = PVE::Tools::file_get_contents("$type_path/description");
>  
> -	push @$types, {
> +	my $entry = {
>  	    type => $type,
>  	    description => $description,
>  	    available => $available,
>  	};
> +
> +	my $name = file_read_firstline("$type_path/name");
> +	$entry->{name} = $name if defined($name);
> +
> +	push @$types, $entry;
>      });
>  
>      return $types;
> -- 
> 2.30.2

[pve-devel] [PATCH qemu-server v2 1/2] automatically add 'uuid' parameter when passing through NVIDIA vGPU

[Dominik Csapak]

When passing through an NVIDIA vGPU via mediated devices, their
software needs the qemu process to have the 'uuid' parameter set to the
one of the vGPU. Since it's currently not possible to pass through multiple
vGPUs to one VM (seems to be an NVIDIA driver limitation at the moment),
we don't have to take care about that.

Sadly, the place we do this, it does not show up in 'qm showcmd' as we
don't (want to) query the pci devices in that case, and then we don't
have a way of knowing if it's an NVIDIA card or not. But since this
is informational with QEMU anyway, i'd say we can ignore that.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm     | 9 ++++++++-
 PVE/QemuServer/PCI.pm | 4 +++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 7d9cf22..c706653 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5594,12 +5594,19 @@ sub vm_start_nolock {
     PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, $start_timeout);
 
     eval {
+	my $uuid;
 	for my $id (sort keys %$pci_devices) {
 	    my $d = $pci_devices->{$id};
 	    for my $dev ($d->{pciid}->@*) {
-		PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
+		my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
+
+		# nvidia grid needs the uuid of the mdev as qemu parameter
+		if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '10de') {
+		    $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $id);
+		}
 	    }
 	}
+	push @$cmd, '-uuid', $uuid if defined($uuid);
     };
     if (my $err = $@) {
 	eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 23fe508..3d0e70e 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -253,7 +253,7 @@ sub get_pci_addr_map {
     return $pci_addr_map;
 }
 
-my sub generate_mdev_uuid {
+sub generate_mdev_uuid {
     my ($vmid, $index) = @_;
     return sprintf("%08d-0000-0000-0000-%012d", $index, $vmid);
 }
@@ -514,6 +514,8 @@ sub prepare_pci_device {
 	die "can't reset PCI device '$pciid'\n"
 	    if $info->{has_fl_reset} && !PVE::SysFSTools::pci_dev_reset($info);
     }
+
+    return $info;
 }
 
 my $RUNDIR = '/run/qemu-server';
-- 
2.30.2

[pve-devel] applied: [PATCH qemu-server v2 1/2] automatically add 'uuid' parameter when passing through NVIDIA vGPU

[Wolfgang Bumiller]

applied this one, left out patch 2 after some off-list discussion

On Fri, Aug 12, 2022 at 11:29:49AM +0200, Dominik Csapak wrote:
> When passing through an NVIDIA vGPU via mediated devices, their
> software needs the qemu process to have the 'uuid' parameter set to the
> one of the vGPU. Since it's currently not possible to pass through multiple
> vGPUs to one VM (seems to be an NVIDIA driver limitation at the moment),
> we don't have to take care about that.
> 
> Sadly, the place we do this, it does not show up in 'qm showcmd' as we
> don't (want to) query the pci devices in that case, and then we don't
> have a way of knowing if it's an NVIDIA card or not. But since this
> is informational with QEMU anyway, i'd say we can ignore that.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/QemuServer.pm     | 9 ++++++++-
>  PVE/QemuServer/PCI.pm | 4 +++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 7d9cf22..c706653 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5594,12 +5594,19 @@ sub vm_start_nolock {
>      PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, $start_timeout);
>  
>      eval {
> +	my $uuid;
>  	for my $id (sort keys %$pci_devices) {
>  	    my $d = $pci_devices->{$id};
>  	    for my $dev ($d->{pciid}->@*) {
> -		PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
> +		my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
> +
> +		# nvidia grid needs the uuid of the mdev as qemu parameter
> +		if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '10de') {
> +		    $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $id);
> +		}
>  	    }
>  	}
> +	push @$cmd, '-uuid', $uuid if defined($uuid);
>      };
>      if (my $err = $@) {
>  	eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 23fe508..3d0e70e 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -253,7 +253,7 @@ sub get_pci_addr_map {
>      return $pci_addr_map;
>  }
>  
> -my sub generate_mdev_uuid {
> +sub generate_mdev_uuid {
>      my ($vmid, $index) = @_;
>      return sprintf("%08d-0000-0000-0000-%012d", $index, $vmid);
>  }
> @@ -514,6 +514,8 @@ sub prepare_pci_device {
>  	die "can't reset PCI device '$pciid'\n"
>  	    if $info->{has_fl_reset} && !PVE::SysFSTools::pci_dev_reset($info);
>      }
> +
> +    return $info;
>  }
>  
>  my $RUNDIR = '/run/qemu-server';
> -- 
> 2.30.2

Re: [pve-devel] applied: [PATCH qemu-server v2 1/2] automatically add 'uuid' parameter when passing through NVIDIA vGPU

[DERUMIER, Alexandre]

Hi,
sorry to be late,
I  justh ave tested this patch, this is working fine with an nvidia 
quadro P2200

(gui improvement is ok too)

Le 12/08/22 à 14:04, Wolfgang Bumiller a écrit :
> applied this one, left out patch 2 after some off-list discussion
>
> On Fri, Aug 12, 2022 at 11:29:49AM +0200, Dominik Csapak wrote:
>> When passing through an NVIDIA vGPU via mediated devices, their
>> software needs the qemu process to have the 'uuid' parameter set to the
>> one of the vGPU. Since it's currently not possible to pass through multiple
>> vGPUs to one VM (seems to be an NVIDIA driver limitation at the moment),
>> we don't have to take care about that.
>>
>> Sadly, the place we do this, it does not show up in 'qm showcmd' as we
>> don't (want to) query the pci devices in that case, and then we don't
>> have a way of knowing if it's an NVIDIA card or not. But since this
>> is informational with QEMU anyway, i'd say we can ignore that.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   PVE/QemuServer.pm     | 9 ++++++++-
>>   PVE/QemuServer/PCI.pm | 4 +++-
>>   2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 7d9cf22..c706653 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -5594,12 +5594,19 @@ sub vm_start_nolock {
>>       PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, $start_timeout);
>>   
>>       eval {
>> +	my $uuid;
>>   	for my $id (sort keys %$pci_devices) {
>>   	    my $d = $pci_devices->{$id};
>>   	    for my $dev ($d->{pciid}->@*) {
>> -		PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
>> +		my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
>> +
>> +		# nvidia grid needs the uuid of the mdev as qemu parameter
>> +		if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '10de') {
>> +		    $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $id);
>> +		}
>>   	    }
>>   	}
>> +	push @$cmd, '-uuid', $uuid if defined($uuid);
>>       };
>>       if (my $err = $@) {
>>   	eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
>> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
>> index 23fe508..3d0e70e 100644
>> --- a/PVE/QemuServer/PCI.pm
>> +++ b/PVE/QemuServer/PCI.pm
>> @@ -253,7 +253,7 @@ sub get_pci_addr_map {
>>       return $pci_addr_map;
>>   }
>>   
>> -my sub generate_mdev_uuid {
>> +sub generate_mdev_uuid {
>>       my ($vmid, $index) = @_;
>>       return sprintf("%08d-0000-0000-0000-%012d", $index, $vmid);
>>   }
>> @@ -514,6 +514,8 @@ sub prepare_pci_device {
>>   	die "can't reset PCI device '$pciid'\n"
>>   	    if $info->{has_fl_reset} && !PVE::SysFSTools::pci_dev_reset($info);
>>       }
>> +
>> +    return $info;
>>   }
>>   
>>   my $RUNDIR = '/run/qemu-server';
>> -- 
>> 2.30.2
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=SXhidDdiNE5weDlFTk1JSNLbEhnZz0r5xBqHo4FRwGw&r=MDl4M3AxU2RhSHpiVVJtMCSRIuBWP0MkbUjJ6GlgifyacKKqSZKspIsU9TEo6BbAsvT3b6rPVY4h-jFc7ynSsw&f=ZmttOUJBaHM0cVJhc0pLb7wCBKCwQBbVKZco1R6kyD6TKeVHMsDU0aWnpDb0rpOe-IAuBpS0N9wZvCfoM2C-1Q&u=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel&k=AKCY
>

[pve-devel] [PATCH qemu-server v2 2/2] cleanup pci devices in more situations

[Dominik Csapak]

if the preparing of pci devices or the start of the vm fails, we need
to cleanup the pci devices (reservations *and* mdevs), or else
it might happen that there are leftovers which must be manually removed.

to include also mdevs now, refactor the cleanup code from 'vm_stop_cleanup'
into it's own function, and call that instead of only 'remove_pci_reservation'

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c706653..5435410 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5609,7 +5609,7 @@ sub vm_start_nolock {
 	push @$cmd, '-uuid', $uuid if defined($uuid);
     };
     if (my $err = $@) {
-	eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
+	eval { cleanup_pci_devices($vmid, $conf) };
 	warn $@ if $@;
 	die $err;
     }
@@ -5705,7 +5705,9 @@ sub vm_start_nolock {
     if (my $err = $@) {
 	# deactivate volumes if start fails
 	eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); };
-	eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
+	warn $@ if $@;
+	eval { cleanup_pci_devices($vmid, $conf) };
+	warn $@ if $@;
 
 	die "start failed: $err";
     }
@@ -5870,6 +5872,25 @@ sub get_vm_volumes {
     return $vollist;
 }
 
+sub cleanup_pci_devices {
+    my ($vmid, $conf) = @_;
+
+    my $ids = [];
+    foreach my $key (keys %$conf) {
+	next if $key !~ m/^hostpci(\d+)$/;
+	my $hostpciindex = $1;
+	my $d = parse_hostpci($conf->{$key});
+	my $uuid = PVE::SysFSTools::generate_mdev_uuid($vmid, $hostpciindex);
+
+	foreach my $pci (@{$d->{pciid}}) {
+	    my $pciid = $pci->{id};
+	    push @$ids, $pci->{id};
+	    PVE::SysFSTools::pci_cleanup_mdev_device($pciid, $uuid);
+	}
+    }
+    PVE::QemuServer::PCI::remove_pci_reservation($ids);
+}
+
 sub vm_stop_cleanup {
     my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes) = @_;
 
@@ -5901,20 +5922,7 @@ sub vm_stop_cleanup {
 	    unlink '/dev/shm/pve-shm-' . ($ivshmem->{name} // $vmid);
 	}
 
-	my $ids = [];
-	foreach my $key (keys %$conf) {
-	    next if $key !~ m/^hostpci(\d+)$/;
-	    my $hostpciindex = $1;
-	    my $d = parse_hostpci($conf->{$key});
-	    my $uuid = PVE::SysFSTools::generate_mdev_uuid($vmid, $hostpciindex);
-
-	    foreach my $pci (@{$d->{pciid}}) {
-		my $pciid = $pci->{id};
-		push @$ids, $pci->{id};
-		PVE::SysFSTools::pci_cleanup_mdev_device($pciid, $uuid);
-	    }
-	}
-	PVE::QemuServer::PCI::remove_pci_reservation($ids);
+	cleanup_pci_devices($vmid, $conf);
 
 	vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
     };
-- 
2.30.2

[pve-devel] [PATCH manager v2 1/1] ui: improve form/MDevSelector

[Dominik Csapak]

by
* showing the (optional) name in front of the type
* making the 'availble' column a bit narrower
* enabling 'cellWrap' for the description
* making the dropdown a bit wider (so all the information can fit)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/MDevSelector.js | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/www/manager6/form/MDevSelector.js b/www/manager6/form/MDevSelector.js
index 8ee73c0c..0a157813 100644
--- a/www/manager6/form/MDevSelector.js
+++ b/www/manager6/form/MDevSelector.js
@@ -16,21 +16,29 @@ Ext.define('PVE.form.MDevSelector', {
     valueField: 'type',
     displayField: 'type',
     listConfig: {
+	width: 550,
 	columns: [
 	    {
 		header: gettext('Type'),
 		dataIndex: 'type',
+		renderer: function(value, md, rec) {
+		    if (rec.data.name !== undefined) {
+			return `${rec.data.name} (${value})`;
+		    }
+		    return value;
+		},
 		flex: 1,
 	    },
 	    {
-		header: gettext('Available'),
+		header: gettext('Avail.'),
 		dataIndex: 'available',
-		width: 80,
+		width: 60,
 	    },
 	    {
 		header: gettext('Description'),
 		dataIndex: 'description',
 		flex: 1,
+		cellWrap: true,
 		renderer: function(value) {
 		    if (!value) {
 			return '';
-- 
2.30.2

[pve-devel] applied: [PATCH manager v2 1/1] ui: improve form/MDevSelector

[Wolfgang Bumiller]

applied, thanks

On Fri, Aug 12, 2022 at 11:29:51AM +0200, Dominik Csapak wrote:
> by
> * showing the (optional) name in front of the type
> * making the 'availble' column a bit narrower
> * enabling 'cellWrap' for the description
> * making the dropdown a bit wider (so all the information can fit)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/manager6/form/MDevSelector.js | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/form/MDevSelector.js b/www/manager6/form/MDevSelector.js
> index 8ee73c0c..0a157813 100644
> --- a/www/manager6/form/MDevSelector.js
> +++ b/www/manager6/form/MDevSelector.js
> @@ -16,21 +16,29 @@ Ext.define('PVE.form.MDevSelector', {
>      valueField: 'type',
>      displayField: 'type',
>      listConfig: {
> +	width: 550,
>  	columns: [
>  	    {
>  		header: gettext('Type'),
>  		dataIndex: 'type',
> +		renderer: function(value, md, rec) {
> +		    if (rec.data.name !== undefined) {
> +			return `${rec.data.name} (${value})`;
> +		    }
> +		    return value;
> +		},
>  		flex: 1,
>  	    },
>  	    {
> -		header: gettext('Available'),
> +		header: gettext('Avail.'),
>  		dataIndex: 'available',
> -		width: 80,
> +		width: 60,
>  	    },
>  	    {
>  		header: gettext('Description'),
>  		dataIndex: 'description',
>  		flex: 1,
> +		cellWrap: true,
>  		renderer: function(value) {
>  		    if (!value) {
>  			return '';
> -- 
> 2.30.2