[pve-devel] [PATCH manager v2] api/ui: improve mdev listing for pci mappings

* [pve-devel] [PATCH manager v2] api/ui: improve mdev listing for pci mappings
@ 2024-10-30  8:59 Dominik Csapak
  2024-10-30  9:48 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Dominik Csapak @ 2024-10-30  8:59 UTC (permalink / raw)
  To: pve-devel

currently when we have a pci resource mapping, we manually check only
the available models for the first pci entry. This often works, but not
always, since one could have completely different devices in one
mapping, or with the new nvidia sysfs api we don't get the generally
available models.

To improve this, extend the 'pciid' regex to include pciids or mapping
names, and for mappings, iterate over all local pci devices in it and
extract the mdev types.

Rename also the parameter to better reflect what it is. This is OK here,
because it's a parameter that's deduced via the url path, and any
attempt to include the parameter with a name manually will result in an
error:

  duplicate parameter (already defined in URI) with conflicting values!

Since we cannot reach the api handler without giving the parameter
already via the url, there is no way to give it via name.

This also vastly simplifies the ui code, since we only have to give the
mapping to the selector instead of an (arbitrarily selected) pci id.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes since v1:
* improved commit message to tell why it's ok to rename the parameter
  here and still have backwards compatibility.

 PVE/API2/Hardware/PCI.pm     | 45 +++++++++++++++++++++++++++++-------
 www/manager6/qemu/PCIEdit.js | 12 +---------
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/PVE/API2/Hardware/PCI.pm b/PVE/API2/Hardware/PCI.pm
index a3a689bf..7135a605 100644
--- a/PVE/API2/Hardware/PCI.pm
+++ b/PVE/API2/Hardware/PCI.pm
@@ -135,7 +135,7 @@ __PACKAGE__->register_method ({
 
 __PACKAGE__->register_method ({
     name => 'pciindex',
-    path => '{pciid}',
+    path => '{pciid-or-mapping}',
     method => 'GET',
     description => "Index of available pci methods",
     permissions => {
@@ -145,9 +145,9 @@ __PACKAGE__->register_method ({
 	additionalProperties => 0,
 	properties => {
 	    node => get_standard_option('pve-node'),
-	    pciid => {
+	    'pciid-or-mapping' => {
 		type => 'string',
-		pattern => '(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F]',
+		pattern => '(?:(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F])|([a-zA-Z][a-zA-Z0-9_-]+)',
 	    },
 	},
     },
@@ -171,7 +171,7 @@ __PACKAGE__->register_method ({
 
 __PACKAGE__->register_method ({
     name => 'mdevscan',
-    path => '{pciid}/mdev',
+    path => '{pciid-or-mapping}/mdev',
     method => 'GET',
     description => "List mediated device types for given PCI device.",
     protected => 1,
@@ -183,10 +183,10 @@ __PACKAGE__->register_method ({
 	additionalProperties => 0,
 	properties => {
 	    node => get_standard_option('pve-node'),
-	    pciid => {
+	    'pciid-or-mapping' => {
 		type => 'string',
-		pattern => '(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F]',
-		description => "The PCI ID to list the mdev types for."
+		pattern => '(?:(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F])|([a-zA-Z][a-zA-Z0-9_-]+)',
+		description => "The PCI ID or mapping to list the mdev types for."
 	    },
 	},
     },
@@ -206,6 +206,12 @@ __PACKAGE__->register_method ({
 		},
 		description => {
 		    type => 'string',
+		    description => "Additional description of the type."
+		},
+		name => {
+		    type => 'string',
+		    optional => 1,
+		    description => 'A human readable name for the type.',
 		},
 	    },
 	},
@@ -213,5 +219,28 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	return PVE::SysFSTools::get_mdev_types($param->{pciid});
+	if ($param->{'pciid-or-mapping'} =~ m/^(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F]$/) {
+	    return PVE::SysFSTools::get_mdev_types($param->{'pciid-or-mapping'});
+	} else {
+	    my $mapping = $param->{'pciid-or-mapping'};
+
+	    my $types = {};
+	    my $devices = PVE::Mapping::PCI::find_on_current_node($mapping);
+	    for my $device ($devices->@*) {
+		my $id = $device->{path};
+		next if $id =~ m/;/; # mdev not supported for multifunction devices
+
+		my $device_types = PVE::SysFSTools::get_mdev_types($id);
+
+		for my $type_definition ($device_types->@*) {
+		    my $type = $type_definition->{type};
+		    if (!defined($types->{$type})) {
+			$types->{$type} = $type_definition;
+		    }
+		}
+	    }
+
+	    return [sort { $a->{type} cmp $b->{type} } values($types->%*)];
+	}
+
     }});
diff --git a/www/manager6/qemu/PCIEdit.js b/www/manager6/qemu/PCIEdit.js
index 970e81c2..7c1b7b85 100644
--- a/www/manager6/qemu/PCIEdit.js
+++ b/www/manager6/qemu/PCIEdit.js
@@ -67,17 +67,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 
 	    let path = value;
 	    if (pciDev.data.map) {
-		// find local mapping
-		for (const entry of pciDev.data.map) {
-		    let mapping = PVE.Parser.parsePropertyString(entry);
-		    if (mapping.node === pcisel.up('inputpanel').nodename) {
-			path = mapping.path.split(';')[0];
-			break;
-		    }
-		}
-		if (path.indexOf('.') === -1) {
-		    path += '.0';
-		}
+		path = pciDev.data.id;
 	    }
 
 	    if (pciDev.data.mdev) {
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 2+ messages in thread