public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping
@ 2023-06-14  8:46 Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 1/6] enable cluster mapped USB devices for guests Dominik Csapak
                   ` (23 more replies)
  0 siblings, 24 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

this series is the remaining part to add a cluster-wide device mapping for pci and usb
devices. so that an admin can configure a device to be availble for
migration and configuring for uses that are non-root
(the existing pattern can be copied easily for other types, e.g.
markus upcoming folder sharing)

maybe the place for the docs are not optimal, if anyone has a better
suggestion, please say

i omitted the older changelogs as were getting a bit long

changes from v5:
* rebase on master
* included docs
* adapted permission checks for restore/clone to (now) existing ones
* renamed a few variables (thanks to wolfgangs feedback)
* simplified the js filterPropertyStringList helper
* added onlineHelp where appropriate

qemu-server:

Dominik Csapak (6):
  enable cluster mapped USB devices for guests
  enable cluster mapped PCI devices for guests
  check_local_resources: extend for mapped resources
  api: migrate preconditions: use new check_local_resources info
  migration: check for mapped resources
  add test for mapped pci devices

 PVE/API2/Qemu.pm                              | 119 ++++++++-
 PVE/QemuMigrate.pm                            |  23 +-
 PVE/QemuServer.pm                             | 150 ++++++++---
 PVE/QemuServer/PCI.pm                         | 243 +++++++++++++++---
 PVE/QemuServer/USB.pm                         |  27 +-
 test/MigrationTest/Shared.pm                  |  14 +
 test/cfg2cmd/q35-linux-hostpci-mapping.conf   |  17 ++
 .../q35-linux-hostpci-mapping.conf.cmd        |  36 +++
 test/cfg2cmd/q35-linux-hostpci.conf           |   2 +-
 test/cfg2cmd/q35-linux-hostpci.conf.cmd       |   2 +-
 test/run_config2command_tests.pl              |  83 ++++++
 11 files changed, 622 insertions(+), 94 deletions(-)
 create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf
 create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd


pve-manager:

Dominik Csapak (15):
  pvesh: fix parameters for proxyto_callback
  api: add resource map api endpoints for PCI and USB
  ui: parser: add helper for lists of property strings
  ui: form/USBSelector: make it more flexible with nodename
  ui: form: add PCIMapSelector
  ui: form: add USBMapSelector
  ui: qemu/PCIEdit: rework panel to add a mapped configuration
  ui: qemu/USBEdit: add 'mapped' device case
  ui: form: add MultiPCISelector
  ui: add edit window for pci mappings
  ui: add edit window for usb mappings
  ui: add ResourceMapTree
  ui: allow configuring pci and usb mapping
  ui: window/Migrate: allow mapped devices
  ui: improve permission handling for hardware

 PVE/API2/Cluster.pm                   |   8 +
 PVE/API2/Cluster/Makefile             |   5 +
 PVE/API2/Cluster/Mapping.pm           |  53 +++++
 PVE/API2/Cluster/Mapping/Makefile     |  18 ++
 PVE/API2/Cluster/Mapping/PCI.pm       | 300 ++++++++++++++++++++++++
 PVE/API2/Cluster/Mapping/USB.pm       | 295 +++++++++++++++++++++++
 PVE/API2/Hardware.pm                  |   1 -
 PVE/API2/Nodes.pm                     |   1 +
 PVE/CLI/pvesh.pm                      |  10 +-
 www/css/ext6-pve.css                  |   4 +
 www/manager6/Makefile                 |   8 +
 www/manager6/Parser.js                |   4 +
 www/manager6/StateProvider.js         |   1 +
 www/manager6/data/PermPathStore.js    |   1 +
 www/manager6/dc/Config.js             |  46 +++-
 www/manager6/dc/PCIMapView.js         | 106 +++++++++
 www/manager6/dc/USBMapView.js         |  98 ++++++++
 www/manager6/form/MultiPCISelector.js | 288 +++++++++++++++++++++++
 www/manager6/form/PCIMapSelector.js   | 112 +++++++++
 www/manager6/form/PCISelector.js      |  18 +-
 www/manager6/form/USBMapSelector.js   |  98 ++++++++
 www/manager6/form/USBSelector.js      |  33 ++-
 www/manager6/qemu/HardwareView.js     |  17 +-
 www/manager6/qemu/PCIEdit.js          | 323 +++++++++++++++++---------
 www/manager6/qemu/USBEdit.js          |  36 ++-
 www/manager6/tree/ResourceMapTree.js  | 316 +++++++++++++++++++++++++
 www/manager6/window/Migrate.js        |  52 ++++-
 www/manager6/window/PCIMapEdit.js     | 207 +++++++++++++++++
 www/manager6/window/USBMapEdit.js     | 217 +++++++++++++++++
 29 files changed, 2536 insertions(+), 140 deletions(-)
 create mode 100644 PVE/API2/Cluster/Mapping.pm
 create mode 100644 PVE/API2/Cluster/Mapping/Makefile
 create mode 100644 PVE/API2/Cluster/Mapping/PCI.pm
 create mode 100644 PVE/API2/Cluster/Mapping/USB.pm
 create mode 100644 www/manager6/dc/PCIMapView.js
 create mode 100644 www/manager6/dc/USBMapView.js
 create mode 100644 www/manager6/form/MultiPCISelector.js
 create mode 100644 www/manager6/form/PCIMapSelector.js
 create mode 100644 www/manager6/form/USBMapSelector.js
 create mode 100644 www/manager6/tree/ResourceMapTree.js
 create mode 100644 www/manager6/window/PCIMapEdit.js
 create mode 100644 www/manager6/window/USBMapEdit.js

pve-docs:

Dominik Csapak (1):
  qemu: add documentation about cluster device mapping

 qm-pci-passthrough.adoc |  8 ++++
 qm.adoc                 | 86 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)

-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server v6 1/6] enable cluster mapped USB devices for guests
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-16  7:50   ` Fabian Grünbichler
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 2/6] enable cluster mapped PCI " Dominik Csapak
                   ` (22 subsequent siblings)
  23 siblings, 1 reply; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

this patch allows configuring usb devices that are mapped via
cluster resource mapping when the user has 'Resource.Use' on the ACL
path '/resource/usb/{ID}' (in addition to the usual required vm config
privileges)

for now, this is only valid if there is exactly one mapping for the
host, since we don't track passed through usb devices yet

also checks the permission for usb devices on clone/restore

Note that this now fails for 'raw' devices when the user is not root, so
this is a breaking change!

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* move the permission check into QemuServer.pm
* add a 'check_restore_permissions' functions and refactor the checks
  into there
 PVE/API2/Qemu.pm      | 41 ++++++++++++++++++++++++++++++++++++++---
 PVE/QemuServer.pm     | 36 +++++++++++++++++++++++++++++++++---
 PVE/QemuServer/USB.pm | 27 ++++++++++++++++++++++++---
 3 files changed, 95 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index c92734a6..865edf7f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -32,6 +32,7 @@ use PVE::QemuServer::Drive;
 use PVE::QemuServer::ImportDisk;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::Machine;
+use PVE::QemuServer::USB qw(parse_usb_device);
 use PVE::QemuMigrate;
 use PVE::RPCEnvironment;
 use PVE::AccessControl;
@@ -590,8 +591,13 @@ my $check_vm_create_usb_perm = sub {
 
     foreach my $opt (keys %{$param}) {
 	next if $opt !~ m/^usb\d+$/;
+	my $entry = PVE::JSONSchema::parse_property_string('pve-qm-usb', $param->{$opt});
+	my $device = parse_usb_device($entry->{host});
 
-	if ($param->{$opt} =~ m/spice/) {
+	if ($device->{spice}) {
+	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
+	} elsif ($device->{mapped}) {
+	    $rpcenv->check_full($authuser, "/mapping/usb/$entry->{host}", ['Mapping.Use']);
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
 	} else {
 	    die "only root can set '$opt' config for real devices\n";
@@ -1719,7 +1725,12 @@ my $update_vm_api  = sub {
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
 		} elsif ($opt =~ m/^usb\d+$/) {
-		    if ($val =~ m/spice/) {
+		    my $device = PVE::JSONSchema::parse_property_string('pve-qm-usb', $val);
+		    my $host = parse_usb_device($device->{host});
+		    if ($host->{spice}) {
+			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    } elsif ($host->{mapped}) {
+			$rpcenv->check_full($authuser, "/mapping/usb/$device->{host}", ['Mapping.Use']);
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
 		    } elsif ($authuser ne 'root@pam') {
 			die "only root can delete '$opt' config for real devices\n";
@@ -1784,7 +1795,30 @@ my $update_vm_api  = sub {
 		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} elsif ($opt =~ m/^usb\d+/) {
-		    if ((!defined($conf->{$opt}) || $conf->{$opt} =~ m/spice/) && $param->{$opt} =~ m/spice/) {
+		    my $olddevice;
+		    my $oldhost;
+		    if (defined($conf->{$opt})) {
+			$olddevice = PVE::JSONSchema::parse_property_string('pve-qm-usb', $conf->{$opt});
+			$oldhost = parse_usb_device($olddevice->{host});
+		    }
+		    if (defined($oldhost)) {
+			if ($oldhost->{spice}) {
+			    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+			} elsif ($oldhost->{mapped}) {
+			    $rpcenv->check_full($authuser, "/mapping/usb/$olddevice->{host}", ['Mapping.Use']);
+			    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+			} elsif ($authuser ne 'root@pam') {
+			    die "only root can modify '$opt' config for real devices\n";
+			}
+		    }
+
+		    my $newdevice = PVE::JSONSchema::parse_property_string('pve-qm-usb', $param->{$opt});
+		    my $newhost = parse_usb_device($newdevice->{host});
+
+		    if ($newhost->{spice}) {
+			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    } elsif ($newhost->{mapped}) {
+			$rpcenv->check_full($authuser, "/mapping/usb/$newdevice->{host}", ['Mapping.Use']);
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
 		    } elsif ($authuser ne 'root@pam') {
 			die "only root can modify '$opt' config for real devices\n";
@@ -3511,6 +3545,7 @@ __PACKAGE__->register_method({
 	    my $oldconf = $snapname ? $conf->{snapshots}->{$snapname} : $conf;
 
 	    my $sharedvm = &$check_storage_access_clone($rpcenv, $authuser, $storecfg, $oldconf, $storage);
+	    PVE::QemuServer::check_mapping_access($rpcenv, $authuser, $oldconf);
 
 	    PVE::QemuServer::check_bridge_access($rpcenv, $authuser, $oldconf);
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b978ab54..ab5458c3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1095,6 +1095,8 @@ The Host USB device or port or the value 'spice'. HOSTUSBDEVICE syntax is:
 
 You can use the 'lsusb -t' command to list existing usb devices.
 
+Alternatively, you can used an ID of a mapped usb device.
+
 NOTE: This option allows direct access to host hardware. So it is no longer possible to migrate such
 machines - use with special care.
 
@@ -1111,6 +1113,8 @@ EODESCR
     },
 };
 
+PVE::JSONSchema::register_format('pve-qm-usb', $usb_fmt);
+
 my $usbdesc = {
     optional => 1,
     type => 'string', format => $usb_fmt,
@@ -2248,7 +2252,12 @@ PVE::JSONSchema::register_format('pve-qm-usb-device', \&verify_usb_device);
 sub verify_usb_device {
     my ($value, $noerr) = @_;
 
-    return $value if parse_usb_device($value);
+    my $parsed = eval { parse_usb_device($value) };
+    if (my $err = $@) {
+	die $err if !$noerr;
+	return;
+    }
+    return $value if defined($parsed);
 
     return if $noerr;
 
@@ -6527,6 +6536,27 @@ sub check_bridge_access {
     return 1;
 };
 
+sub check_mapping_access {
+   my ($rpcenv, $user, $conf) = @_;
+
+   for my $opt (keys $conf->%*) {
+       if ($opt =~ m/^usb\d+$/) {
+	   my $entry = PVE::JSONSchema::parse_property_string('pve-qm-usb', $conf->{$opt});
+	   my $device = parse_usb_device($entry->{host});
+	   if ($device->{mapped}) {
+	       $rpcenv->check_full($user, "/mapping/usb/$entry->{host}", ['Mapping.Use']);
+	   } elsif (!$device->{spice}) {
+	       die "only root can set '$opt' config for real devices\n";
+	   }
+       }
+   }
+};
+
+sub check_restore_permissions {
+    my ($rpcenv, $user, $conf) = @_;
+    check_bridge_access($rpcenv, $user, $conf);
+    check_mapping_access($rpcenv, $user, $conf);
+}
 # vzdump restore implementaion
 
 sub tar_archive_read_firstfile {
@@ -7171,7 +7201,7 @@ sub restore_proxmox_backup_archive {
     }
 
     my $new_conf = $restore_merge_config->($conffile, $new_conf_raw, $options->{override_conf});
-    check_bridge_access($rpcenv, $user, $new_conf);
+    check_restore_permissions($rpcenv, $user, $new_conf);
     PVE::QemuConfig->write_config($vmid, $new_conf);
 
     eval { rescan($vmid, 1); };
@@ -7485,7 +7515,7 @@ sub restore_vma_archive {
     }
 
     my $new_conf = $restore_merge_config->($conffile, $new_conf_raw, $opts->{override_conf});
-    check_bridge_access($rpcenv, $user, $new_conf);
+    check_restore_permissions($rpcenv, $user, $new_conf);
     PVE::QemuConfig->write_config($vmid, $new_conf);
 
     eval { rescan($vmid, 1); };
diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index 686461cc..d6b4c531 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -6,6 +6,7 @@ use PVE::QemuServer::PCI qw(print_pci_addr);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Helpers qw(min_version windows_version);
 use PVE::JSONSchema;
+use PVE::Mapping::USB;
 use base 'Exporter';
 
 our @EXPORT_OK = qw(
@@ -17,7 +18,7 @@ get_usb_devices
 my $OLD_MAX_USB = 5;
 
 sub parse_usb_device {
-    my ($value) = @_;
+    my ($value, $checkMap) = @_;
 
     return if !$value;
 
@@ -31,7 +32,27 @@ sub parse_usb_device {
     } elsif ($value =~ m/^spice$/i) {
 	$res->{spice} = 1;
     } else {
-	return;
+	# we have no ordinary usb device, must be a mapping
+	my $devices = PVE::Mapping::USB::find_on_current_node($value);
+	if ($checkMap) {
+	    die "USB device mapping not found for '$value'\n" if !$devices || !scalar($devices->@*);
+	    die "More than one USB mapping per host not supported\n" if scalar($devices->@*) > 1;
+	    eval {
+		PVE::Mapping::USB::assert_valid($value, $devices->[0]);
+	    };
+	    if (my $err = $@) {
+		die "USB Mapping invalid (hardware probably changed): $err\n";
+	    }
+	    my $device = $devices->[0];
+
+	    if ($device->{path}) {
+		$res = parse_usb_device($device->{path});
+	    } else {
+		$res = parse_usb_device($device->{id});
+	    }
+	}
+
+	$res->{mapped} = 1;
     }
 
     return $res;
@@ -111,7 +132,7 @@ sub get_usb_devices {
 	my $port = $use_qemu_xhci ? $i + 1 : undef;
 
 	if (defined($d->{host})) {
-	    my $hostdevice = parse_usb_device($d->{host});
+	    my $hostdevice = parse_usb_device($d->{host}, 1);
 	    $hostdevice->{usb3} = $d->{usb3};
 	    if ($hostdevice->{spice}) {
 		# usb redir support for spice
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server v6 2/6] enable cluster mapped PCI devices for guests
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 1/6] enable cluster mapped USB devices for guests Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-16  7:49   ` Fabian Grünbichler
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 3/6] check_local_resources: extend for mapped resources Dominik Csapak
                   ` (21 subsequent siblings)
  23 siblings, 1 reply; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

this patch allows configuring pci devices that are mapped via cluster
resource mapping when the user has 'Resource.Use' on the ACL path
'/mapping/pci/{ID}' (in  addition to the usual required vm config
privileges)

When given multiple mappings in the config, we use them as alternatives
for the passthrough, and will select the first free one on startup.
It is using our regular pci reservation mechanism for regular devices and
we introduce a selection mechanism for mediated devices.

A few changes to the inner workings were required to make this work well:
* parse_hostpci now returns a different structure where we have a list
  of lists (first level is for the different alternatives and second
  level is for the different devices that should be passed through
  together)
* factor out the 'parse_hostpci_devices' which parses each device from
  the config and does some precondition checks
* reserve_pci_usage now behaves slightly different when trying to
  reserve an device with the same VMID that's already reserved for,
  since for checking which alternative we can use, we already must
  reserve one (this means that qm showcmd can actually reserve devices,
  albeit only for up to 10 seconds)
* configuring a mediated device on a multifunction device is not
  supported anymore, and results in failure to start (previously, it
  just chose the first device to do it). This is a breaking change
* configuring a single pci device twice on different hostpci slots now
  fails during commandline generation instead on qemu start, so we had
  to adapt one test where this occurred (it could never have worked
  anyway)

Add the permission checks also to clone/restore. This fails now for non
root users for raw devices, this is a breaking change!

Fixes #3574: Improve SR-IOV usability
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* move permission check for clone/restore to QemuServer.pm
 PVE/API2/Qemu.pm                        |  54 +++++-
 PVE/QemuServer.pm                       |  71 ++++---
 PVE/QemuServer/PCI.pm                   | 243 ++++++++++++++++++++----
 test/cfg2cmd/q35-linux-hostpci.conf     |   2 +-
 test/cfg2cmd/q35-linux-hostpci.conf.cmd |   2 +-
 test/run_config2command_tests.pl        |   1 +
 6 files changed, 299 insertions(+), 74 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 865edf7f..4c3c53d9 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -32,6 +32,7 @@ use PVE::QemuServer::Drive;
 use PVE::QemuServer::ImportDisk;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::Machine;
+use PVE::QemuServer::PCI;
 use PVE::QemuServer::USB qw(parse_usb_device);
 use PVE::QemuMigrate;
 use PVE::RPCEnvironment;
@@ -607,6 +608,26 @@ my $check_vm_create_usb_perm = sub {
     return 1;
 };
 
+my $check_vm_create_hostpci_perm = sub {
+    my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
+
+    return 1 if $authuser eq 'root@pam';
+
+    foreach my $opt (keys %{$param}) {
+	next if $opt !~ m/^hostpci\d+$/;
+
+	my $device = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $param->{$opt});
+	if ($device->{host} !~ m/:/) {
+	    $rpcenv->check_full($authuser, "/mapping/pci/$device->{host}", ['Mapping.Use']);
+	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
+	} else {
+	    die "only root can set '$opt' config for non-mapped devices\n";
+	}
+    }
+
+    return 1;
+};
+
 my $check_vm_modify_config_perm = sub {
     my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
 
@@ -617,7 +638,7 @@ my $check_vm_modify_config_perm = sub {
 	# else, as there the permission can be value dependend
 	next if PVE::QemuServer::is_valid_drivename($opt);
 	next if $opt eq 'cdrom';
-	next if $opt =~ m/^(?:unused|serial|usb)\d+$/;
+	next if $opt =~ m/^(?:unused|serial|usb|hostpci)\d+$/;
 	next if $opt eq 'tags';
 
 
@@ -646,7 +667,7 @@ my $check_vm_modify_config_perm = sub {
 	    # also needs privileges on the storage, that will be checked later
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk', 'VM.PowerMgmt' ]);
 	} else {
-	    # catches hostpci\d+, args, lock, etc.
+	    # catches args, lock, etc.
 	    # new options will be checked here
 	    die "only root can set '$opt' config\n";
 	}
@@ -885,6 +906,8 @@ __PACKAGE__->register_method({
 
 	    &$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, $param);
 	    &$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, $param);
+	    &$check_vm_create_hostpci_perm($rpcenv, $authuser, $vmid, $pool, $param);
+
 	    PVE::QemuServer::check_bridge_access($rpcenv, $authuser, $param);
 	    &$check_cpu_model_access($rpcenv, $authuser, $param);
 
@@ -1737,6 +1760,16 @@ my $update_vm_api  = sub {
 		    }
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
+		} elsif ($opt =~ m/^hostpci\d+$/) {
+		    my $olddevice = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $val);
+		    if ($olddevice->{host} !~ m/:/) {
+			$rpcenv->check_full($authuser, "/mapping/pci/$olddevice->{host}", ['Mapping.Use']);
+			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    } elsif ($authuser ne 'root@pam') {
+			die "only root can set '$opt' config for non-mapped devices\n";
+		    }
+		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
+		    PVE::QemuConfig->write_config($vmid, $conf);
 		} elsif ($opt eq 'tags') {
 		    assert_tag_permissions($vmid, $val, '', $rpcenv, $authuser);
 		    delete $conf->{$opt};
@@ -1823,6 +1856,23 @@ my $update_vm_api  = sub {
 		    } elsif ($authuser ne 'root@pam') {
 			die "only root can modify '$opt' config for real devices\n";
 		    }
+
+		    $conf->{pending}->{$opt} = $param->{$opt};
+		} elsif ($opt =~ m/^hostpci\d+$/) {
+		    my $olddevice;
+		    if (defined($conf->{$opt})) {
+			$olddevice = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $conf->{$opt});
+		    }
+		    my $newdevice = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $param->{$opt});
+		    if ((!defined($olddevice) || $olddevice->{host} !~ m/:/) && $newdevice->{host} !~ m/:/) {
+			if (defined($olddevice)) {
+			    $rpcenv->check_full($authuser, "/mapping/pci/$olddevice->{host}", ['Mapping.Use']);
+			}
+			$rpcenv->check_full($authuser, "/mapping/pci/$newdevice->{host}", ['Mapping.Use']);
+			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    } elsif ($authuser ne 'root@pam') {
+			die "only root can set '$opt' config for non-mapped devices\n";
+		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} elsif ($opt eq 'tags') {
 		    assert_tag_permissions($vmid, $conf->{$opt}, $param->{$opt}, $rpcenv, $authuser);
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ab5458c3..4fb8d1c2 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3783,8 +3783,8 @@ sub config_to_command {
     my $bootorder = device_bootorder($conf);
 
     # host pci device passthrough
-    my ($kvm_off, $gpu_passthrough, $legacy_igd) = PVE::QemuServer::PCI::print_hostpci_devices(
-	$vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder);
+    my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) = PVE::QemuServer::PCI::print_hostpci_devices(
+	$vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $machine_type, $bootorder);
 
     # usb devices
     my $usb_dev_features = {};
@@ -4203,7 +4203,7 @@ sub config_to_command {
 	push @$cmd, @$aa;
     }
 
-    return wantarray ? ($cmd, $vollist, $spice_port) : $cmd;
+    return wantarray ? ($cmd, $vollist, $spice_port, $pci_devices) : $cmd;
 }
 
 sub check_rng_source {
@@ -5762,7 +5762,7 @@ sub vm_start_nolock {
 	print "Resuming suspended VM\n";
     }
 
-    my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid,
+    my ($cmd, $vollist, $spice_port, $pci_devices) = config_to_command($storecfg, $vmid,
 	$conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'});
 
     my $migration_ip;
@@ -5847,38 +5847,44 @@ sub vm_start_nolock {
 
     my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $resume);
 
-    my $pci_devices = {}; # host pci devices
-    for (my $i = 0; $i < $PVE::QemuServer::PCI::MAX_HOSTPCI_DEVICES; $i++)  {
-	my $dev = $conf->{"hostpci$i"} or next;
-	$pci_devices->{$i} = parse_hostpci($dev);
+    my $pci_reserve_list = [];
+    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}->@*;
     }
 
-    # do not reserve pciid for mediated devices, sysfs will error out for duplicate assignment
-    my $real_pci_devices = [ grep { !(defined($_->{mdev}) && scalar($_->{pciid}->@*) == 1) } values $pci_devices->%* ];
-
-    # map to a flat list of pci ids
-    my $pci_id_list = [ map { $_->{id} } map { $_->{pciid}->@* } $real_pci_devices->@* ];
-
     # reserve all PCI IDs before actually doing anything with them
-    PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, $start_timeout);
+    PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
 
     eval {
 	my $uuid;
 	for my $id (sort keys %$pci_devices) {
 	    my $d = $pci_devices->{$id};
-	    for my $dev ($d->{pciid}->@*) {
-		my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
-
-		# nvidia grid needs the qemu parameter '-uuid' set
-		# use smbios uuid or mdev uuid as fallback for that
-		if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '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, $id) if !defined($uuid);
+	    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->{mdev}) };
+		if ($d->{mdev}) {
+		    warn $@ if $@;
+		    $chosen_mdev = $info;
+		    last if $chosen_mdev; # if successful, we're done
+		} else {
+		    die $@ if $@;
 		}
 	    }
+
+	    next if !$d->{mdev};
+	    die "could not create mediated device\n" if !defined($chosen_mdev);
+
+	    # 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);
+	    }
 	}
 	push @$cmd, '-uuid', $uuid if defined($uuid);
     };
@@ -5992,7 +5998,7 @@ sub vm_start_nolock {
 
     # re-reserve all PCI IDs now that we can know the actual VM PID
     my $pid = PVE::QemuServer::Helpers::vm_running_locally($vmid);
-    eval { PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, undef, $pid) };
+    eval { PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, undef, $pid) };
     warn $@ if $@;
 
     if (defined($res->{migrate})) {
@@ -6187,9 +6193,7 @@ sub cleanup_pci_devices {
 
 	    # some nvidia vgpu driver versions want to clean the mdevs up themselves, and error
 	    # out when we do it first. so wait for 10 seconds and then try it
-	    my $pciid = $d->{pciid}->[0]->{id};
-	    my $info = PVE::SysFSTools::pci_device_info("$pciid");
-	    if ($info->{vendor} eq '10de') {
+	    if ($d->{ids}->[0]->[0]->{vendor} =~ m/^(0x)?10de$/) {
 		sleep 10;
 	    }
 
@@ -6548,6 +6552,13 @@ sub check_mapping_access {
 	   } elsif (!$device->{spice}) {
 	       die "only root can set '$opt' config for real devices\n";
 	   }
+       } elsif ($opt =~ m/^hostpci\d+$/) {
+	   my $device = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $conf->{$opt});
+	   if ($device->{host} !~ m/:/) {
+	       $rpcenv->check_full($user, "/mapping/pci/$device->{host}", ['Mapping.Use']);
+	   } else {
+	       die "only root can set '$opt' config for non-mapped devices\n";
+	   }
        }
    }
 };
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index a18b9747..5a5cbcca 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -4,6 +4,7 @@ use warnings;
 use strict;
 
 use PVE::JSONSchema;
+use PVE::Mapping::PCI;
 use PVE::SysFSTools;
 use PVE::Tools;
 
@@ -23,8 +24,8 @@ my $hostpci_fmt = {
     host => {
 	default_key => 1,
 	type => 'string',
-	pattern => qr/$PCIRE(;$PCIRE)*/,
-	format_description => 'HOSTPCIID[;HOSTPCIID2...]',
+	pattern => qr/(:?$PCIRE(;$PCIRE)*)|(:?$PVE::JSONSchema::CONFIGID_RE)/,
+	format_description => 'HOSTPCIID[;HOSTPCIID2...] or configured mapping id',
 	description => <<EODESCR,
 Host PCI device pass through. The PCI ID of a host's PCI device or a list
 of PCI virtual functions of the host. HOSTPCIID syntax is:
@@ -32,6 +33,8 @@ of PCI virtual functions of the host. HOSTPCIID syntax is:
 'bus:dev.func' (hexadecimal numbers)
 
 You can us the 'lspci' command to list existing PCI devices.
+
+Alternatively use the ID of a mapped pci device.
 EODESCR
     },
     rombar => {
@@ -376,6 +379,32 @@ sub print_pcie_root_port {
     return $res;
 }
 
+# returns the parsed pci config but parses the 'host' part into
+# a list if lists into the 'id' property like this:
+#
+# {
+#   mdev => 1,
+#   rombar => ...
+#   ...
+#   ids => [
+#       # this contains a list of alternative devices,
+#       [
+#           # which are itself lists of ids for one multifunction device
+#           {
+#               id => "0000:00:00.0",
+#               vendor => "...",
+#           },
+#           {
+#               id => "0000:00:00.1",
+#               vendor => "...",
+#           },
+#       ],
+#       [
+#           ...
+#       ],
+#       ...
+#   ],
+# }
 sub parse_hostpci {
     my ($value) = @_;
 
@@ -383,31 +412,167 @@ sub parse_hostpci {
 
     my $res = PVE::JSONSchema::parse_property_string($hostpci_fmt, $value);
 
-    my @idlist = split(/;/, $res->{host});
+    my $alternatives = [];
+    if ($res->{host} !~ m/:/) {
+	# we have no ordinary pci id, must be a mapping
+	my $devices = PVE::Mapping::PCI::find_on_current_node($res->{host});
+	die "PCI device mapping not found for '$res->{host}'\n" if !$devices || !scalar($devices->@*);
+
+	for my $device ($devices->@*) {
+	    eval { PVE::Mapping::PCI::assert_valid($res->{host}, $device) };
+	    die "PCI device mapping invalid (hardware probably changed): $@\n" if $@;
+	    push $alternatives->@*, [split(/;/, $device->{path})];
+	}
+    } else {
+	push $alternatives->@*, [split(/;/, $res->{host})];
+    }
     delete $res->{host};
-    foreach my $id (@idlist) {
-	my $devs = PVE::SysFSTools::lspci($id);
-	die "no PCI device found for '$id'\n" if !scalar(@$devs);
-	push @{$res->{pciid}}, @$devs;
+
+    $res->{ids} = [];
+    for my $alternative ($alternatives->@*) {
+	my $ids = [];
+	foreach my $id ($alternative->@*) {
+	    my $devs = PVE::SysFSTools::lspci($id);
+	    die "no PCI device found for '$id'\n" if !scalar($devs->@*);
+	    push $ids->@*, @$devs;
+	}
+	if (scalar($ids->@*) > 1) {
+	    $res->{'has-multifunction'} = 1;
+	    die "cannot use mediated device with multifunction device\n" if $res->{mdev};
+	}
+	push $res->{ids}->@*, $ids;
     }
+
     return $res;
 }
 
+# parses all hostpci devices from a config and does some sanity checks
+# returns a hash like this:
+# {
+#     hostpci0 => {
+#         # hash from parse_hostpci function
+#     },
+#     hostpci1 => { ... },
+#     ...
+# }
+sub parse_hostpci_devices {
+    my ($conf) = @_;
+
+    my $q35 = PVE::QemuServer::Machine::machine_type_is_q35($conf);
+    my $legacy_igd = 0;
+
+    my $parsed_devices = {};
+    for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
+	my $id = "hostpci$i";
+	my $d = parse_hostpci($conf->{$id});
+	next if !$d;
+
+	# check syntax
+	die "q35 machine model is not enabled" if !$q35 && $d->{pcie};
+
+	if ($d->{'legacy-igd'}) {
+	    die "only one device can be assigned in legacy-igd mode\n"
+		if $legacy_igd;
+	    $legacy_igd = 1;
+
+	    die "legacy IGD assignment requires VGA mode to be 'none'\n"
+		if !defined($conf->{'vga'}) || $conf->{'vga'} ne 'none';
+	    die "legacy IGD assignment requires rombar to be enabled\n"
+		if defined($d->{rombar}) && !$d->{rombar};
+	    die "legacy IGD assignment is not compatible with x-vga\n"
+		if $d->{'x-vga'};
+	    die "legacy IGD assignment is not compatible with mdev\n"
+		if $d->{mdev};
+	    die "legacy IGD assignment is not compatible with q35\n"
+		if $q35;
+	    die "legacy IGD assignment is not compatible with multifunction devices\n"
+		if $d->{'has-multifunction'};
+	    die "legacy IGD assignment is not compatible with alternate devices\n"
+		if scalar($d->{ids}->@*) > 1;
+	    # check first device for valid id
+	    die "legacy IGD assignment only works for devices on host bus 00:02.0\n"
+		if $d->{ids}->[0]->[0]->{id} !~ m/02\.0$/;
+	}
+
+	$parsed_devices->{$id} = $d;
+    }
+
+    return $parsed_devices;
+}
+
+# takes the hash returned by parse_hostpci_devices and for all non mdev gpus,
+# selects one of the given alternatives by trying to reserve it
+#
+# mdev devices must be chosen later when we actually allocate it, but we
+# flatten the inner list since there can only be one device per alternative anyway
+my sub choose_hostpci_devices {
+    my ($devices, $vmid) = @_;
+
+    my $used = {};
+
+    my $add_used_device = sub {
+	my ($devices) = @_;
+	for my $used_device ($devices->@*) {
+	    my $used_id = $used_device->{id};
+	    die "device '$used_id' assigned more than once\n" if $used->{$used_id};
+	    $used->{$used_id} = 1;
+	}
+    };
+
+    for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
+	my $device = $devices->{"hostpci$i"};
+	next if !$device;
+
+	if ($device->{mdev}) {
+	    $device->{ids} = [ map { $_->[0] } $device->{ids}->@* ];
+	    next;
+	}
+
+	if (scalar($device->{ids}->@* == 1)) {
+	    # we only have one alternative, use that
+	    $device->{ids} = $device->{ids}->[0];
+	    $add_used_device->($device->{ids});
+	    next;
+	}
+
+	my $found = 0;
+	for my $alternative ($device->{ids}->@*) {
+	    my $ids = [map { $_->{id} } @$alternative];
+
+	    next if grep { defined($used->{$_}) } @$ids; # already used
+	    eval { reserve_pci_usage($ids, $vmid, 10, undef) };
+	    next if $@;
+
+	    # found one that is not used or reserved
+	    $add_used_device->($alternative);
+	    $device->{ids} = $alternative;
+	    $found = 1;
+	    last;
+	}
+	die "could not find a free device for 'hostpci$i'\n" if !$found;
+    }
+
+    return $devices;
+}
+
 sub print_hostpci_devices {
-    my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder) = @_;
+    my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $machine_type, $bootorder) = @_;
 
     my $kvm_off = 0;
     my $gpu_passthrough = 0;
     my $legacy_igd = 0;
 
     my $pciaddr;
+    my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
+
     for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
 	my $id = "hostpci$i";
-	my $d = parse_hostpci($conf->{$id});
+	my $d = $pci_devices->{$id};
 	next if !$d;
 
+	$legacy_igd = 1 if $d->{'legacy-igd'};
+
 	if (my $pcie = $d->{pcie}) {
-	    die "q35 machine model is not enabled" if !$q35;
 	    # win7 wants to have the pcie devices directly on the pcie bus
 	    # instead of in the root port
 	    if ($winversion == 7) {
@@ -425,29 +590,8 @@ sub print_hostpci_devices {
 	    $pciaddr = print_pci_addr($pci_name, $bridges, $arch, $machine_type);
 	}
 
-	my $pcidevices = $d->{pciid};
-	my $multifunction = @$pcidevices > 1;
-
-	if ($d->{'legacy-igd'}) {
-	    die "only one device can be assigned in legacy-igd mode\n"
-		if $legacy_igd;
-	    $legacy_igd = 1;
-
-	    die "legacy IGD assignment requires VGA mode to be 'none'\n"
-		if !defined($conf->{'vga'}) || $conf->{'vga'} ne 'none';
-	    die "legacy IGD assignment requires rombar to be enabled\n"
-		if defined($d->{rombar}) && !$d->{rombar};
-	    die "legacy IGD assignment is not compatible with x-vga\n"
-		if $d->{'x-vga'};
-	    die "legacy IGD assignment is not compatible with mdev\n"
-		if $d->{mdev};
-	    die "legacy IGD assignment is not compatible with q35\n"
-		if $q35;
-	    die "legacy IGD assignment is not compatible with multifunction devices\n"
-		if $multifunction;
-	    die "legacy IGD assignment only works for devices on host bus 00:02.0\n"
-		if $pcidevices->[0]->{id} !~ m/02\.0$/;
-	}
+	my $num_devices = scalar($d->{ids}->@*);
+	my $multifunction = $num_devices > 1 && !$d->{mdev};
 
 	my $xvga = '';
 	if ($d->{'x-vga'}) {
@@ -458,15 +602,13 @@ sub print_hostpci_devices {
 	}
 
 	my $sysfspath;
-	if ($d->{mdev} && scalar(@$pcidevices) == 1) {
+	if ($d->{mdev}) {
 	    my $uuid = generate_mdev_uuid($vmid, $i);
 	    $sysfspath = "/sys/bus/mdev/devices/$uuid";
-	} elsif ($d->{mdev}) {
-	    warn "ignoring mediated device '$id' with multifunction device\n";
 	}
 
-	my $j = 0;
-	foreach my $pcidevice (@$pcidevices) {
+	for (my $j = 0; $j < $num_devices; $j++) {
+	    my $pcidevice = $d->{ids}->[$j];
 	    my $devicestr = "vfio-pci";
 
 	    if ($sysfspath) {
@@ -489,12 +631,13 @@ sub print_hostpci_devices {
 		}
 	    }
 
+
 	    push @$devices, '-device', $devicestr;
-	    $j++;
+	    last if $d->{mdev};
 	}
     }
 
-    return ($kvm_off, $gpu_passthrough, $legacy_igd);
+    return ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices);
 }
 
 sub prepare_pci_device {
@@ -596,6 +739,26 @@ sub reserve_pci_usage {
 			warn "leftover PCI reservation found for $id, lets take it...\n";
 		    }
 		}
+	    } elsif ($reservation) {
+		# already reserved by the same vmid
+		if (my $reserved_time = $reservation->{time}) {
+		    if (defined($timeout)) {
+			# use the longer timeout
+			my $old_timeout = $reservation->{time} - 5 - $ctime;
+			$timeout = $old_timeout if $old_timeout > $timeout;
+		    }
+		} elsif (my $reserved_pid = $reservation->{pid}) {
+		    my $running_pid = PVE::QemuServer::Helpers::vm_running_locally($reservation->{vmid});
+		    if (defined($running_pid) && $running_pid == $reservation->{pid}) {
+			if (defined($pid)) {
+			    die "PCI device '$id' already in use by running VMID '$reservation->{vmid}'\n";
+			} elsif (defined($timeout)) {
+			    # ignore timeout reservation for running vms, can happen with e.g.
+			    # qm showcmd
+			    return;
+			}
+		    }
+		}
 	    }
 
 	    $reservation_list->{$id} = { vmid => $vmid };
diff --git a/test/cfg2cmd/q35-linux-hostpci.conf b/test/cfg2cmd/q35-linux-hostpci.conf
index 749f9839..7290120a 100644
--- a/test/cfg2cmd/q35-linux-hostpci.conf
+++ b/test/cfg2cmd/q35-linux-hostpci.conf
@@ -8,7 +8,7 @@ hostpci1: d0:13.0,pcie=1
 hostpci2: 00:f4.0
 hostpci3: d0:15.1,pcie=1
 hostpci4: d0:17.0,pcie=1,rombar=0
-hostpci7: d0:15.1,pcie=1
+hostpci7: d0:15.2,pcie=1
 machine: q35
 memory: 512
 net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
index 0774047d..0fedbd2c 100644
--- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd
+++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
@@ -32,7 +32,7 @@
   -device 'pcie-root-port,id=ich9-pcie-port-5,addr=10.0,x-speed=16,x-width=32,multifunction=on,bus=pcie.0,port=5,chassis=5' \
   -device 'vfio-pci,host=0000:d0:17.0,id=hostpci4,bus=ich9-pcie-port-5,addr=0x0,rombar=0' \
   -device 'pcie-root-port,id=ich9-pcie-port-8,addr=10.3,x-speed=16,x-width=32,multifunction=on,bus=pcie.0,port=8,chassis=8' \
-  -device 'vfio-pci,host=0000:d0:15.1,id=hostpci7,bus=ich9-pcie-port-8,addr=0x0' \
+  -device 'vfio-pci,host=0000:d0:15.2,id=hostpci7,bus=ich9-pcie-port-8,addr=0x0' \
   -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
   -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
   -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index c4966902..dda44d99 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -81,6 +81,7 @@ my $pci_devs = [
     "0000:0f:f2.0",
     "0000:d0:13.0",
     "0000:d0:15.1",
+    "0000:d0:15.2",
     "0000:d0:17.0",
     "0000:f0:42.0",
     "0000:f0:43.0",
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server v6 3/6] check_local_resources: extend for mapped resources
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 1/6] enable cluster mapped USB devices for guests Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 2/6] enable cluster mapped PCI " Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 4/6] api: migrate preconditions: use new check_local_resources info Dominik Csapak
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

by adding them to their own list, saving the nodes where
they are not allowed, and return those on 'wantarray' so we don't break
existing callers that don't expect it.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* renamed hash to missing_mappings_by_node
 PVE/QemuServer.pm            | 43 ++++++++++++++++++++++++++++++++++--
 test/MigrationTest/Shared.pm | 14 ++++++++++++
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 4fb8d1c2..8d868072 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -34,6 +34,8 @@ use PVE::DataCenterConfig;
 use PVE::Exception qw(raise raise_param_exc);
 use PVE::Format qw(render_duration render_bytes);
 use PVE::GuestHelpers qw(safe_string_ne safe_num_ne safe_boolean_ne);
+use PVE::Mapping::PCI;
+use PVE::Mapping::USB;
 use PVE::INotify;
 use PVE::JSONSchema qw(get_standard_option parse_property_string);
 use PVE::ProcFSTools;
@@ -2709,6 +2711,28 @@ sub check_local_resources {
     my ($conf, $noerr) = @_;
 
     my @loc_res = ();
+    my $mapped_res = [];
+
+    my $nodelist = PVE::Cluster::get_nodelist();
+    my $pci_map = PVE::Mapping::PCI::config();
+    my $usb_map = PVE::Mapping::USB::config();
+
+    my $missing_mappings_by_node = { map { $_ => [] } @$nodelist };
+
+    my $add_missing_mapping = sub {
+	my ($type, $key, $id) = @_;
+	for my $node (@$nodelist) {
+	    my $entry;
+	    if ($type eq 'pci') {
+		$entry = PVE::Mapping::PCI::get_node_mapping($pci_map, $id, $node);
+	    } elsif ($type eq 'usb') {
+		$entry = PVE::Mapping::USB::get_node_mapping($usb_map, $id, $node);
+	    }
+	    if (!scalar($entry->@*)) {
+		push @{$missing_mappings_by_node->{$node}}, $key;
+	    }
+	}
+    };
 
     push @loc_res, "hostusb" if $conf->{hostusb}; # old syntax
     push @loc_res, "hostpci" if $conf->{hostpci}; # old syntax
@@ -2716,7 +2740,22 @@ sub check_local_resources {
     push @loc_res, "ivshmem" if $conf->{ivshmem};
 
     foreach my $k (keys %$conf) {
-	next if $k =~ m/^usb/ && ($conf->{$k} =~ m/^spice(?![^,])/);
+	if ($k =~ m/^usb/) {
+	    my $entry = parse_property_string($usb_fmt, $conf->{$k});
+	    my $usb = PVE::QemuServer::USB::parse_usb_device($entry->{host});
+	    next if $usb->{spice};
+	    if ($usb->{mapped}) {
+		$add_missing_mapping->('usb', $k, $entry->{host});
+		push @$mapped_res, $k;
+	    }
+	}
+	if ($k =~ m/^hostpci/) {
+	    my $entry = parse_property_string('pve-qm-hostpci', $conf->{$k});
+	    if ($entry->{host} !~ m/:/) {
+		$add_missing_mapping->('pci', $k, $entry->{host});
+		push @$mapped_res, $k;
+	    }
+	}
 	# sockets are safe: they will recreated be on the target side post-migrate
 	next if $k =~ m/^serial/ && ($conf->{$k} eq 'socket');
 	push @loc_res, $k if $k =~ m/^(usb|hostpci|serial|parallel)\d+$/;
@@ -2724,7 +2763,7 @@ sub check_local_resources {
 
     die "VM uses local resources\n" if scalar @loc_res && !$noerr;
 
-    return \@loc_res;
+    return wantarray ? (\@loc_res, $mapped_res, $missing_mappings_by_node) : \@loc_res;
 }
 
 # check if used storages are available on all nodes (use by migrate)
diff --git a/test/MigrationTest/Shared.pm b/test/MigrationTest/Shared.pm
index bd4d20c0..aa7203d1 100644
--- a/test/MigrationTest/Shared.pm
+++ b/test/MigrationTest/Shared.pm
@@ -76,6 +76,20 @@ $cluster_module->mock(
     },
 );
 
+our $mapping_usb_module = Test::MockModule->new("PVE::Mapping::USB");
+$mapping_usb_module->mock(
+    config => sub {
+	return {};
+    },
+);
+
+our $mapping_pci_module = Test::MockModule->new("PVE::Mapping::PCI");
+$mapping_pci_module->mock(
+    config => sub {
+	return {};
+    },
+);
+
 our $ha_config_module = Test::MockModule->new("PVE::HA::Config");
 $ha_config_module->mock(
     vm_is_ha_managed => sub {
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server v6 4/6] api: migrate preconditions: use new check_local_resources info
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (2 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 3/6] check_local_resources: extend for mapped resources Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 5/6] migration: check for mapped resources Dominik Csapak
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

for offline migration, limit the allowed nodes to the ones where the
mapped resources are available

this adds new info to the api call namely the 'mapped-resources' list,
as well as the 'unavailable-resources' info in the 'not_allowed_nodes'
object

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* renamed new return values with '-' instead of '_'
* added 'mapped-resources' to the return value description
* pulled out the $missing_mappings variable for reuse
 PVE/API2/Qemu.pm | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 4c3c53d9..030bb272 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4317,7 +4317,11 @@ __PACKAGE__->register_method({
 	    local_resources => {
 		type => 'array',
 		description => "List local resources e.g. pci, usb"
-	    }
+	    },
+	    'mapped-resources' => {
+		type => 'array',
+		description => "List of mapped resources e.g. pci, usb"
+	    },
 	},
     },
     code => sub {
@@ -4346,7 +4350,11 @@ __PACKAGE__->register_method({
 
 	$res->{running} = PVE::QemuServer::check_running($vmid) ? 1:0;
 
-	# if vm is not running, return target nodes where local storage is available
+	my ($local_resources, $mapped_resources, $missing_mappings_by_node) =
+	    PVE::QemuServer::check_local_resources($vmconf, 1);
+	delete $missing_mappings_by_node->{$localnode};
+
+	# if vm is not running, return target nodes where local storage/mapped devices are available
 	# for offline migration
 	if (!$res->{running}) {
 	    $res->{allowed_nodes} = [];
@@ -4354,7 +4362,13 @@ __PACKAGE__->register_method({
 	    delete $checked_nodes->{$localnode};
 
 	    foreach my $node (keys %$checked_nodes) {
-		if (!defined $checked_nodes->{$node}->{unavailable_storages}) {
+		my $missing_mappings = $missing_mappings_by_node->{$node};
+		if (scalar($missing_mappings->@*)) {
+		    $checked_nodes->{$node}->{'unavailable-resources'} = $missing_mappings;
+		    next;
+		}
+
+		if (!defined($checked_nodes->{$node}->{unavailable_storages})) {
 		    push @{$res->{allowed_nodes}}, $node;
 		}
 
@@ -4362,13 +4376,11 @@ __PACKAGE__->register_method({
 	    $res->{not_allowed_nodes} = $checked_nodes;
 	}
 
-
 	my $local_disks = &$check_vm_disks_local($storecfg, $vmconf, $vmid);
 	$res->{local_disks} = [ values %$local_disks ];;
 
-	my $local_resources =  PVE::QemuServer::check_local_resources($vmconf, 1);
-
 	$res->{local_resources} = $local_resources;
+	$res->{'mapped-resources'} = $mapped_resources;
 
 	return $res;
 
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server v6 5/6] migration: check for mapped resources
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (3 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 4/6] api: migrate preconditions: use new check_local_resources info Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 6/6] add test for mapped pci devices Dominik Csapak
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

they can only be migrated to nodes where there exists a mapping and if
the migration is done offline

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
chnages from v5:
* renamed to $missing_mappings_by_node and $missing_mappings
 PVE/QemuMigrate.pm | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 09cc1d83..3975ae41 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -210,15 +210,32 @@ sub prepare {
 	$self->{vm_was_paused} = 1 if PVE::QemuServer::vm_is_paused($vmid);
     }
 
-    my $loc_res = PVE::QemuServer::check_local_resources($conf, 1);
-    if (scalar @$loc_res) {
+    my ($loc_res, $mapped_res, $missing_mappings_by_node) = PVE::QemuServer::check_local_resources($conf, 1);
+    my $blocking_resources = [];
+    for my $res ($loc_res->@*) {
+	if (!grep($res, $mapped_res->@*)) {
+	    push $blocking_resources->@*, $res;
+	}
+    }
+    if (scalar($blocking_resources->@*)) {
 	if ($self->{running} || !$self->{opts}->{force}) {
-	    die "can't migrate VM which uses local devices: " . join(", ", @$loc_res) . "\n";
+	    die "can't migrate VM which uses local devices: " . join(", ", $blocking_resources->@*) . "\n";
 	} else {
 	    $self->log('info', "migrating VM which uses local devices");
 	}
     }
 
+    if (scalar($mapped_res->@*)) {
+	my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
+	if ($running) {
+	    die "can't migrate running VM which uses mapped devices: " . join(", ", $mapped_res->@*) . "\n";
+	} elsif (scalar($missing_mappings->@*)) {
+	    die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
+	} else {
+	    $self->log('info', "migrating VM which uses mapped local devices");
+	}
+    }
+
     my $vollist = PVE::QemuServer::get_vm_volumes($conf);
 
     my $storages = {};
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server v6 6/6] add test for mapped pci devices
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (4 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 5/6] migration: check for mapped resources Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 01/15] pvesh: fix parameters for proxyto_callback Dominik Csapak
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 test/cfg2cmd/q35-linux-hostpci-mapping.conf   | 17 ++++
 .../q35-linux-hostpci-mapping.conf.cmd        | 36 ++++++++
 test/run_config2command_tests.pl              | 82 +++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf
 create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd

diff --git a/test/cfg2cmd/q35-linux-hostpci-mapping.conf b/test/cfg2cmd/q35-linux-hostpci-mapping.conf
new file mode 100644
index 00000000..2402cf2a
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-mapping.conf
@@ -0,0 +1,17 @@
+# TEST: Config with q35, NUMA, hostpci mapping passthrough, EFI & Linux
+bios: ovmf
+bootdisk: scsi0
+cores: 1
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+hostpci0: someNic
+hostpci1: someGpu,mdev=some-model
+hostpci2: someNic
+machine: q35
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+numa: 1
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+sockets: 2
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
new file mode 100644
index 00000000..814a9021
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
@@ -0,0 +1,36 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name 'vm8006,debug-threads=on' \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+  -drive 'if=pflash,unit=1,id=drive-efidisk0,format=qcow2,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
+  -smp '2,sockets=2,cores=1,maxcpus=2' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -object 'memory-backend-ram,id=ram-node0,size=256M' \
+  -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \
+  -object 'memory-backend-ram,id=ram-node1,size=256M' \
+  -numa 'node,nodeid=1,cpus=1,memdev=ram-node1' \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+  -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+  -device 'vfio-pci,host=0000:07:10.0,id=hostpci0,bus=pci.0,addr=0x10' \
+  -device 'vfio-pci,sysfsdev=/sys/bus/mdev/devices/00000001-0000-0000-0000-000000008006,id=hostpci1,bus=pci.0,addr=0x11' \
+  -device 'vfio-pci,host=0000:07:10.4,id=hostpci2,bus=pci.0,addr=0x1b' \
+  -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=1024,bootindex=300' \
+  -machine 'type=q35+pve0'
diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index dda44d99..08718549 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -87,8 +87,37 @@ my $pci_devs = [
     "0000:f0:43.0",
     "0000:f0:43.1",
     "1234:f0:43.1",
+    "0000:01:00.4",
+    "0000:01:00.5",
+    "0000:01:00.6",
+    "0000:07:10.0",
+    "0000:07:10.1",
+    "0000:07:10.4",
 ];
 
+my $pci_map_config = {
+    ids => {
+	someGpu => {
+	    type => 'pci',
+	    map => [
+		'node=localhost,path=0000:01:00.4,id=10de:2231,iommugroup=1',
+		'node=localhost,path=0000:01:00.5,id=10de:2231,iommugroup=1',
+		'node=localhost,path=0000:01:00.6,id=10de:2231,iommugroup=1',
+	    ],
+	},
+	someNic => {
+	    type => 'pci',
+	    map => [
+		'node=localhost,path=0000:07:10.0,id=8086:1520,iommugroup=2',
+		'node=localhost,path=0000:07:10.1,id=8086:1520,iommugroup=2',
+		'node=localhost,path=0000:07:10.4,id=8086:1520,iommugroup=2',
+	    ],
+	},
+    },
+};
+
+my $usb_map_config = {},
+
 my $current_test; # = {
 #   description => 'Test description', # if available
 #   qemu_version => '2.12',
@@ -275,6 +304,28 @@ $pve_common_sysfstools->mock(
 	    } sort @$pci_devs
 	];
     },
+    pci_device_info => sub {
+	my ($path, $noerr) = @_;
+
+	if ($path =~ m/^0000:01:00/) {
+	    return {
+		mdev => 1,
+		iommugroup => 1,
+		mdev => 1,
+		vendor => "0x10de",
+		device => "0x2231",
+	    };
+	} elsif ($path =~ m/^0000:07:10/) {
+	    return {
+		iommugroup => 2,
+		mdev => 0,
+		vendor => "0x8086",
+		device => "0x1520",
+	    };
+	} else {
+	    return {};
+	}
+    },
 );
 
 my $qemu_monitor_module;
@@ -303,6 +354,37 @@ $qemu_monitor_module->mock(
 );
 $qemu_monitor_module->mock('qmp_cmd', \&qmp_cmd);
 
+my $mapping_usb_module = Test::MockModule->new("PVE::Mapping::USB");
+$mapping_usb_module->mock(
+    config => sub {
+	return $usb_map_config;
+    },
+);
+
+my $mapping_pci_module = Test::MockModule->new("PVE::Mapping::PCI");
+$mapping_pci_module->mock(
+    config => sub {
+	return $pci_map_config;
+    },
+);
+
+my $pci_module = Test::MockModule->new("PVE::QemuServer::PCI");
+$pci_module->mock(
+    reserve_pci_usage => sub {
+	my ($ids, $vmid, $timeout, $pid, $dryrun) = @_;
+
+	$ids = [$ids] if !ref($ids);
+
+	for my $id (@$ids) {
+	    if ($id eq "0000:07:10.1") {
+		die "reserved";
+	    }
+	}
+
+	return undef;
+    },
+);
+
 sub diff($$) {
     my ($a, $b) = @_;
     return if $a eq $b;
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 01/15] pvesh: fix parameters for proxyto_callback
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (5 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 6/6] add test for mapped pci devices Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-16  9:27   ` [pve-devel] applied: " Wolfgang Bumiller
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 02/15] api: add resource map api endpoints for PCI and USB Dominik Csapak
                   ` (16 subsequent siblings)
  23 siblings, 1 reply; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

in pve-http-server the proxyto_callback always has a complete list of
parameters, not only the ones in the url, so adapt the implementation
here to do the same

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/CLI/pvesh.pm | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/PVE/CLI/pvesh.pm b/PVE/CLI/pvesh.pm
index 9acf292a..28e2518d 100755
--- a/PVE/CLI/pvesh.pm
+++ b/PVE/CLI/pvesh.pm
@@ -82,13 +82,15 @@ my $method_map = {
 };
 
 sub check_proxyto {
-    my ($info, $uri_param) = @_;
+    my ($info, $uri_param, $params) = @_;
 
     my $rpcenv = PVE::RPCEnvironment->get();
 
+    my $all_params = { %$uri_param, %$params };
+
     if ($info->{proxyto} || $info->{proxyto_callback}) {
 	my $node = PVE::API2Tools::resolve_proxyto(
-	    $rpcenv, $info->{proxyto_callback}, $info->{proxyto}, $uri_param);
+	    $rpcenv, $info->{proxyto_callback}, $info->{proxyto}, $all_params);
 
 	if ($node ne 'localhost' && ($node ne PVE::INotify::nodename())) {
 	    die "proxy loop detected - aborting\n" if $disable_proxy;
@@ -301,7 +303,7 @@ sub call_api_method {
     }
 
     my $data;
-    my ($node, $remip) = check_proxyto($info, $uri_param);
+    my ($node, $remip) = check_proxyto($info, $uri_param, $param);
     if ($node) {
 	$data = proxy_handler($node, $remip, $path, $cmd, $param);
     } else {
@@ -345,7 +347,7 @@ __PACKAGE__->register_method ({
 
 	my $res;
 
-	my ($node, $remip) = check_proxyto($info, $uri_param);
+	my ($node, $remip) = check_proxyto($info, $uri_param, $param);
 	if ($node) {
 	    $res = proxy_handler($node, $remip, $path, 'ls', $param);
 	} else {
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 02/15] api: add resource map api endpoints for PCI and USB
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (6 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 01/15] pvesh: fix parameters for proxyto_callback Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-16  7:50   ` Fabian Grünbichler
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 03/15] ui: parser: add helper for lists of property strings Dominik Csapak
                   ` (15 subsequent siblings)
  23 siblings, 1 reply; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

this adds the typical section config crud API calls for
USB and PCI resource mapping to /cluster/resource/{TYPE}

the only special thing that this series does is the list call
for both has a special 'check-node' parameter that uses the
'proxyto_callback' to reroute the api call to the given node
so that it can check the validity of the mapping for that node

in the future when we e.g. broadcast the lspci output via pmxcfs
we drop the proxyto_callback and directly use the info from
pmxcfs (or we drop the parameter and always check all nodes)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Cluster.pm               |   8 +
 PVE/API2/Cluster/Makefile         |   5 +
 PVE/API2/Cluster/Mapping.pm       |  53 ++++++
 PVE/API2/Cluster/Mapping/Makefile |  18 ++
 PVE/API2/Cluster/Mapping/PCI.pm   | 300 ++++++++++++++++++++++++++++++
 PVE/API2/Cluster/Mapping/USB.pm   | 295 +++++++++++++++++++++++++++++
 PVE/API2/Hardware.pm              |   1 -
 PVE/API2/Nodes.pm                 |   1 +
 8 files changed, 680 insertions(+), 1 deletion(-)
 create mode 100644 PVE/API2/Cluster/Mapping.pm
 create mode 100644 PVE/API2/Cluster/Mapping/Makefile
 create mode 100644 PVE/API2/Cluster/Mapping/PCI.pm
 create mode 100644 PVE/API2/Cluster/Mapping/USB.pm

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index c1637af9..bf409fbe 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -26,6 +26,7 @@ use PVE::API2::ACMEPlugin;
 use PVE::API2::Backup;
 use PVE::API2::Cluster::BackupInfo;
 use PVE::API2::Cluster::Ceph;
+use PVE::API2::Cluster::Mapping;
 use PVE::API2::Cluster::Jobs;
 use PVE::API2::Cluster::MetricServer;
 use PVE::API2::ClusterConfig;
@@ -90,6 +91,12 @@ __PACKAGE__->register_method ({
     subclass => "PVE::API2::Cluster::Jobs",
     path => 'jobs',
 });
+
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Cluster::Mapping",
+    path => 'mapping',
+});
+
 if ($have_sdn) {
     __PACKAGE__->register_method ({
        subclass => "PVE::API2::Network::SDN",
@@ -145,6 +152,7 @@ __PACKAGE__->register_method ({
 	    { name => 'options' },
 	    { name => 'replication' },
 	    { name => 'resources' },
+	    { name => 'resource' },
 	    { name => 'status' },
 	    { name => 'tasks' },
 	];
diff --git a/PVE/API2/Cluster/Makefile b/PVE/API2/Cluster/Makefile
index 5c92e4f6..0c52a241 100644
--- a/PVE/API2/Cluster/Makefile
+++ b/PVE/API2/Cluster/Makefile
@@ -1,10 +1,13 @@
 include ../../../defines.mk
 
+SUBDIRS=Mapping
+
 # for node independent, cluster-wide applicable, API endpoints
 # ensure we do not conflict with files shipped by pve-cluster!!
 PERLSOURCE= 			\
 	BackupInfo.pm		\
 	MetricServer.pm		\
+	Mapping.pm		\
 	Jobs.pm			\
 	Ceph.pm
 
@@ -13,8 +16,10 @@ all:
 .PHONY: clean
 clean:
 	rm -rf *~
+	set -e && for i in ${SUBDIRS}; do ${MAKE} -C $$i $@; done
 
 .PHONY: install
 install: $(PERLSOURCE)
 	install -d $(PERLLIBDIR)/PVE/API2/Cluster
 	install -m 0644 $(PERLSOURCE) $(PERLLIBDIR)/PVE/API2/Cluster
+	set -e && for i in $(SUBDIRS); do $(MAKE) -C $$i $@; done
diff --git a/PVE/API2/Cluster/Mapping.pm b/PVE/API2/Cluster/Mapping.pm
new file mode 100644
index 00000000..01fa986b
--- /dev/null
+++ b/PVE/API2/Cluster/Mapping.pm
@@ -0,0 +1,53 @@
+package PVE::API2::Cluster::Mapping;
+
+use strict;
+use warnings;
+
+use PVE::RESTHandler;
+
+use PVE::API2::Cluster::Mapping::PCI;
+use PVE::API2::Cluster::Mapping::USB;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Cluster::Mapping::PCI",
+    path => 'pci',
+});
+
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Cluster::Mapping::USB",
+    path => 'usb',
+});
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => "List resource types.",
+    permissions => {
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $result = [
+	    { name => 'pci' },
+	    { name => 'usb' },
+	];
+
+	return $result;
+    }});
+
+1;
diff --git a/PVE/API2/Cluster/Mapping/Makefile b/PVE/API2/Cluster/Mapping/Makefile
new file mode 100644
index 00000000..e7345ab4
--- /dev/null
+++ b/PVE/API2/Cluster/Mapping/Makefile
@@ -0,0 +1,18 @@
+include ../../../../defines.mk
+
+# for node independent, cluster-wide applicable, API endpoints
+# ensure we do not conflict with files shipped by pve-cluster!!
+PERLSOURCE= 	\
+	PCI.pm	\
+	USB.pm
+
+all:
+
+.PHONY: clean
+clean:
+	rm -rf *~
+
+.PHONY: install
+install: ${PERLSOURCE}
+	install -d ${PERLLIBDIR}/PVE/API2/Cluster/Mapping
+	install -m 0644 ${PERLSOURCE} ${PERLLIBDIR}/PVE/API2/Cluster/Mapping
diff --git a/PVE/API2/Cluster/Mapping/PCI.pm b/PVE/API2/Cluster/Mapping/PCI.pm
new file mode 100644
index 00000000..9fe20bea
--- /dev/null
+++ b/PVE/API2/Cluster/Mapping/PCI.pm
@@ -0,0 +1,300 @@
+package PVE::API2::Cluster::Mapping::PCI;
+
+use strict;
+use warnings;
+
+use Storable qw(dclone);
+
+use PVE::Cluster qw(cfs_lock_file);
+use PVE::Mapping::PCI;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Tools qw(extract_param);
+
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    # only proxy if we give the 'check-node' parameter
+    proxyto_callback => sub {
+	my ($rpcenv, $proxyto, $param) = @_;
+	return $param->{'check-node'} // 'localhost';
+    },
+    description => "List PCI Hardware Mapping",
+    permissions => {
+	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or".
+	    " 'Mapping.Audit' permissions on '/mapping/pci/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    'check-node' => get_standard_option('pve-node', {
+		description => "If given, checks the configurations on the given node for ".
+		    "correctness, and adds relevant errors to the devices.",
+		optional => 1,
+	    }),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => {
+		id => {
+		    type => 'string',
+		    description => "The logical ID of the mapping."
+		},
+		map => {
+		    type => 'array',
+		    description => "The entries of the mapping.",
+		    items => {
+			type => 'string',
+			description => "A mapping for a node.",
+		    },
+		},
+		description => {
+		    type => 'string',
+		    description => "A description of the logical mapping.",
+		},
+		error => {
+		    description => "A list of errors when 'check_node' is given.",
+		    items => {
+			type => 'object',
+			properties => {
+			    severity => {
+				type => "string",
+				description => "The severity of the error",
+			    },
+			    message => {
+				type => "string",
+				description => "The message of the error",
+			    },
+			},
+		    }
+		},
+	    },
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $node = $param->{'check-node'};
+
+	die "Wrong node to check\n"
+	    if defined($node) && $node ne 'localhost' && $node ne PVE::INotify::nodename();
+
+	my $cfg = PVE::Mapping::PCI::config();
+
+	my $res = [];
+
+	my $privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit'];
+
+	for my $id (keys $cfg->{ids}->%*) {
+	    next if !$rpcenv->check_full($authuser, "/mapping/pci/$id", $privs, 1, 1);
+	    next if !$cfg->{ids}->{$id};
+
+	    my $entry = dclone($cfg->{ids}->{$id});
+	    $entry->{id} = $id;
+	    $entry->{digest} = $cfg->{digest};
+
+	    if (defined($node)) {
+		$entry->{errors} = [];
+		if (my $mappings = PVE::Mapping::PCI::get_node_mapping($cfg, $id, $node)) {
+		    if (!scalar($mappings->@*)) {
+			push $entry->{errors}->@*, {
+			    severity => 'warning',
+			    message => "No mapping for node $node.",
+			};
+		    }
+		    for my $mapping ($mappings->@*) {
+			eval {
+			    PVE::Mapping::PCI::assert_valid($id, $mapping);
+			};
+			if (my $err = $@) {
+			    push $entry->{errors}->@*, {
+				severity => 'error',
+				message => "Invalid configuration: $err",
+			    };
+			}
+		    }
+		}
+	    }
+
+	    push @$res, $entry;
+	}
+
+	return $res;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'get',
+    protected => 1,
+    path => '{id}',
+    method => 'GET',
+    description => "Get PCI Mapping.",
+    permissions => {
+	check =>['or',
+	    ['perm', '/mapping/pci/{name}', ['Mapping.Use']],
+	    ['perm', '/mapping/pci/{name}', ['Mapping.Modify']],
+	],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    id => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'object' },
+    code => sub {
+	my ($param) = @_;
+
+	my $cfg = PVE::Mapping::PCI::config();
+	my $id = $param->{id};
+
+	my $entry = $cfg->{ids}->{$id};
+	die "mapping '$param->{id}' not found\n" if !defined($entry);
+
+	my $data = dclone($entry);
+
+	$data->{digest} = $cfg->{digest};
+
+	return $data;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'create',
+    protected => 1,
+    path => '',
+    method => 'POST',
+    description => "Create a new hardware mapping.",
+    permissions => {
+	check => ['perm', '/mapping/pci/{name}', ['Mapping.Modify']],
+    },
+    # todo parameters
+    parameters => PVE::Mapping::PCI->createSchema(1),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $id = extract_param($param, 'id');
+
+	my $plugin = PVE::Mapping::PCI->lookup('pci');
+	my $opts = $plugin->check_config($id, $param, 1, 1);
+
+	PVE::Mapping::PCI::lock_pci_config(sub {
+	    my $cfg = PVE::Mapping::PCI::config();
+
+	    die "pci ID '$id' already defined\n" if defined($cfg->{ids}->{$id});
+
+	    $cfg->{ids}->{$id} = $opts;
+
+	    PVE::Mapping::PCI::write_pci_config($cfg);
+
+	}, "create hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'update',
+    protected => 1,
+    path => '{id}',
+    method => 'PUT',
+    description => "Update a hardware mapping.",
+    permissions => {
+	check => ['perm', '/mapping/pci/{id}', ['Mapping.Modify']],
+    },
+    parameters => PVE::Mapping::PCI->updateSchema(),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+	my $id = extract_param($param, 'id');
+
+	if ($delete) {
+	    $delete = [ PVE::Tools::split_list($delete) ];
+	}
+
+	PVE::Mapping::PCI::lock_pci_config(sub {
+	    my $cfg = PVE::Mapping::PCI::config();
+
+	    PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest);
+
+	    die "pci ID '$id' does not exist\n" if !defined($cfg->{ids}->{$id});
+
+	    my $plugin = PVE::Mapping::PCI->lookup('pci');
+	    my $opts = $plugin->check_config($id, $param, 1, 1);
+
+	    my $data = $cfg->{ids}->{$id};
+
+	    my $options = $plugin->private()->{options}->{pci};
+	    PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete);
+
+	    $data->{$_} = $opts->{$_} for keys $opts->%*;
+
+	    PVE::Mapping::PCI::write_pci_config($cfg);
+
+	}, "update hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete',
+    protected => 1,
+    path => '{id}',
+    method => 'DELETE',
+    description => "Remove Hardware Mapping.",
+    permissions => {
+	check => [ 'perm', '/mapping/pci/{id}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    id => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $id = $param->{id};
+
+	PVE::Mapping::PCI::lock_pci_config(sub {
+	    my $cfg = PVE::Mapping::PCI::config();
+
+	    if ($cfg->{ids}->{$id}) {
+		delete $cfg->{ids}->{$id};
+	    }
+
+	    PVE::Mapping::PCI::write_pci_config($cfg);
+
+	}, "delete pci mapping failed");
+
+	return;
+    }
+});
+
+1;
diff --git a/PVE/API2/Cluster/Mapping/USB.pm b/PVE/API2/Cluster/Mapping/USB.pm
new file mode 100644
index 00000000..40f5e8f2
--- /dev/null
+++ b/PVE/API2/Cluster/Mapping/USB.pm
@@ -0,0 +1,295 @@
+package PVE::API2::Cluster::Mapping::USB;
+
+use strict;
+use warnings;
+
+use Storable qw(dclone);
+
+use PVE::Cluster qw(cfs_lock_file);
+use PVE::Mapping::USB;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Tools qw(extract_param);
+
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => "List USB Hardware Mappings",
+    permissions => {
+	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or".
+	    " 'Mapping.Audit' permissions on '/mapping/usb/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    'check-node' => get_standard_option('pve-node', {
+		description => "If given, checks the configurations on the given node for ".
+		    "correctness, and adds relevant errors to the devices.",
+		optional => 1,
+	    }),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => {
+		id => {
+		    type => 'string',
+		    description => "The logical ID of the mapping."
+		},
+		map => {
+		    type => 'array',
+		    description => "The entries of the mapping.",
+		    items => {
+			type => 'string',
+			description => "A mapping for a node.",
+		    },
+		},
+		description => {
+		    type => 'string',
+		    description => "A description of the logical mapping.",
+		},
+		error => {
+		    description => "A list of errors when 'check_node' is given.",
+		    items => {
+			type => 'object',
+			properties => {
+			    severity => {
+				type => "string",
+				description => "The severity of the error",
+			    },
+			    message => {
+				type => "string",
+				description => "The message of the error",
+			    },
+			},
+		    }
+		},
+	    },
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $node = $param->{'check-node'};
+
+	die "Wrong node to check\n"
+	    if defined($node) && $node ne 'localhost' && $node ne PVE::INotify::nodename();
+
+	my $cfg = PVE::Mapping::USB::config();
+
+	my $res = [];
+
+	my $privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit'];
+
+	for my $id (keys $cfg->{ids}->%*) {
+	    next if !$rpcenv->check_full($authuser, "/mapping/usb/$id", $privs, 1, 1);
+	    next if !$cfg->{ids}->{$id};
+
+	    my $entry = dclone($cfg->{ids}->{$id});
+	    $entry->{id} = $id;
+	    $entry->{digest} = $cfg->{digest};
+
+	    if (defined($node)) {
+		$entry->{errors} = [];
+		if (my $mappings = PVE::Mapping::USB::get_node_mapping($cfg, $id, $node)) {
+		    if (!scalar($mappings->@*)) {
+			push $entry->{errors}->@*, {
+			    severity => 'warning',
+			    message => "No mapping for node $node.",
+			};
+		    }
+		    for my $mapping ($mappings->@*) {
+			eval {
+			    PVE::Mapping::USB::assert_valid($id, $mapping);
+			};
+			if (my $err = $@) {
+			    push $entry->{errors}->@*, {
+				severity => 'error',
+				message => "Invalid configuration: $err",
+			    };
+			}
+		    }
+		}
+	    }
+
+	    push @$res, $entry;
+	}
+
+	return $res;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'get',
+    protected => 1,
+    path => '{id}',
+    method => 'GET',
+    description => "Get USB Mapping.",
+    permissions => {
+	check =>['or',
+	    ['perm', '/mapping/usb/{id}', ['Mapping.Use']],
+	    ['perm', '/mapping/usb/{id}', ['Mapping.Modify']],
+	],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    id => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'object' },
+    code => sub {
+	my ($param) = @_;
+
+	my $cfg = PVE::Mapping::USB::config();
+	my $id = $param->{id};
+
+	my $entry = $cfg->{ids}->{$id};
+	die "mapping '$param->{id}' not found\n" if !defined($entry);
+
+	my $data = dclone($entry);
+
+	$data->{digest} = $cfg->{digest};
+
+	return $data;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'create',
+    protected => 1,
+    path => '',
+    method => 'POST',
+    description => "Create a new hardware mapping.",
+    permissions => {
+	check => ['perm', '/mapping/usb/{name}', ['Mapping.Modify']],
+    },
+    # todo parameters
+    parameters => PVE::Mapping::USB->createSchema(1),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $id = extract_param($param, 'id');
+
+	my $plugin = PVE::Mapping::USB->lookup('usb');
+	my $opts = $plugin->check_config($id, $param, 1, 1);
+
+	PVE::Mapping::USB::lock_usb_config(sub {
+	    my $cfg = PVE::Mapping::USB::config();
+
+	    die "usb ID '$id' already defined\n" if defined($cfg->{ids}->{$id});
+
+	    $cfg->{ids}->{$id} = $opts;
+
+	    PVE::Mapping::USB::write_usb_config($cfg);
+
+	}, "create hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'update',
+    protected => 1,
+    path => '{id}',
+    method => 'PUT',
+    description => "Update a hardware mapping.",
+    permissions => {
+	check => ['perm', '/mapping/usb/{id}', ['Mapping.Modify']],
+    },
+    parameters => PVE::Mapping::USB->updateSchema(),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+	my $id = extract_param($param, 'id');
+
+	if ($delete) {
+	    $delete = [ PVE::Tools::split_list($delete) ];
+	}
+
+	PVE::Mapping::USB::lock_usb_config(sub {
+	    my $cfg = PVE::Mapping::USB::config();
+
+	    PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest);
+
+	    die "usb ID '$id' does not exist\n" if !defined($cfg->{ids}->{$id});
+
+	    my $plugin = PVE::Mapping::USB->lookup('usb');
+	    my $opts = $plugin->check_config($id, $param, 1, 1);
+
+	    my $data = $cfg->{ids}->{$id};
+
+	    my $options = $plugin->private()->{options}->{usb};
+	    PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete);
+
+	    $data->{$_} = $opts->{$_} for keys $opts->%*;
+
+	    PVE::Mapping::USB::write_usb_config($cfg);
+
+	}, "update hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete',
+    protected => 1,
+    path => '{id}',
+    method => 'DELETE',
+    description => "Remove Hardware Mapping.",
+    permissions => {
+	check => [ 'perm', '/mapping/usb/{id}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    id => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $id = $param->{id};
+
+	PVE::Mapping::USB::lock_usb_config(sub {
+	    my $cfg = PVE::Mapping::USB::config();
+
+	    if ($cfg->{ids}->{$id}) {
+		delete $cfg->{ids}->{$id};
+	    }
+
+	    PVE::Mapping::USB::write_usb_config($cfg);
+
+	}, "delete usb mapping failed");
+
+	return;
+    }
+});
+
+1;
diff --git a/PVE/API2/Hardware.pm b/PVE/API2/Hardware.pm
index f59bfbe0..1c6fd8f5 100644
--- a/PVE/API2/Hardware.pm
+++ b/PVE/API2/Hardware.pm
@@ -21,7 +21,6 @@ __PACKAGE__->register_method ({
     path => 'usb',
 });
 
-
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 9269694d..026d89f2 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -279,6 +279,7 @@ __PACKAGE__->register_method ({
 	    { name => 'query-url-metadata' },
 	    { name => 'replication' },
 	    { name => 'report' },
+	    { name => 'resource-map' },
 	    { name => 'rrd' }, # fixme: remove?
 	    { name => 'rrddata' },# fixme: remove?
 	    { name => 'scan' },
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 03/15] ui: parser: add helper for lists of property strings
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (7 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 02/15] api: add resource map api endpoints for PCI and USB Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 04/15] ui: form/USBSelector: make it more flexible with nodename Dominik Csapak
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

namely the filtering while preserving the original string,
it's just one line, but having a shorthand for it still makes it a bit
nicer

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* removed list parser (was unused)
* changed the filter helper to a simple 'filter' call
  (the double map before/after was unnecessary)
 www/manager6/Parser.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/www/manager6/Parser.js b/www/manager6/Parser.js
index c3772d3b..bc6a4338 100644
--- a/www/manager6/Parser.js
+++ b/www/manager6/Parser.js
@@ -604,5 +604,9 @@ Ext.define('PVE.Parser', {
 	});
 	return [res, extradata];
     },
+
+    filterPropertyStringList: function(list, filterFn, defaultKey) {
+	return list.filter((entry) => filterFn(PVE.Parser.parsePropertyString(entry, defaultKey)));
+    },
 },
 });
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 04/15] ui: form/USBSelector: make it more flexible with nodename
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (8 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 03/15] ui: parser: add helper for lists of property strings Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 05/15] ui: form: add PCIMapSelector Dominik Csapak
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

similar to the pciselector, make it accept a plain nodename,
or no node at all and provide a setNodename function

to keep backwards compatibility, also check pveSelNode for the nodename

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/USBSelector.js | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/www/manager6/form/USBSelector.js b/www/manager6/form/USBSelector.js
index a67c8765..011778d7 100644
--- a/www/manager6/form/USBSelector.js
+++ b/www/manager6/form/USBSelector.js
@@ -23,25 +23,39 @@ Ext.define('PVE.form.USBSelector', {
 	return gettext("Invalid Value");
     },
 
-    initComponent: function() {
+    setNodename: function(nodename) {
 	var me = this;
 
-	var nodename = me.pveSelNode.data.node;
+	if (!nodename || me.nodename === nodename) {
+	    return;
+	}
+
+	me.nodename = nodename;
+
+	me.store.setProxy({
+	    type: 'proxmox',
+	    url: `/api2/json/nodes/${me.nodename}/hardware/usb`,
+	});
+
+	me.store.load();
+    },
+
+    initComponent: function() {
+	var me = this;
 
-	if (!nodename) {
-	    throw "no nodename specified";
+	if (me.pveSelNode) {
+	    me.nodename = me.pveSelNode.data.node;
 	}
 
+	var nodename = me.nodename;
+	me.nodename = undefined;
+
 	if (me.type !== 'device' && me.type !== 'port') {
 	    throw "no valid type specified";
 	}
 
 	let store = new Ext.data.Store({
 	    model: `pve-usb-${me.type}`,
-	    proxy: {
-		type: 'proxmox',
-		url: `/api2/json/nodes/${nodename}/hardware/usb`,
-	    },
 	    filters: [
 		({ data }) => !!data.usbpath && !!data.prodid && String(data.class) !== "9",
 	    ],
@@ -99,7 +113,7 @@ Ext.define('PVE.form.USBSelector', {
 
 	me.callParent();
 
-	store.load();
+	me.setNodename(nodename);
     },
 
 }, function() {
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 05/15] ui: form: add PCIMapSelector
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (9 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 04/15] ui: form/USBSelector: make it more flexible with nodename Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 06/15] ui: form: add USBMapSelector Dominik Csapak
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

akin to the PCISelector, but uses the api for mapped devices

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Makefile               |   1 +
 www/manager6/form/PCIMapSelector.js | 112 ++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)
 create mode 100644 www/manager6/form/PCIMapSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 9b6dd13b..8de983aa 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -49,6 +49,7 @@ JSSRC= 							\
 	form/NetworkCardSelector.js			\
 	form/NodeSelector.js				\
 	form/PCISelector.js				\
+	form/PCIMapSelector.js				\
 	form/PermPathSelector.js			\
 	form/PoolSelector.js				\
 	form/PreallocationSelector.js			\
diff --git a/www/manager6/form/PCIMapSelector.js b/www/manager6/form/PCIMapSelector.js
new file mode 100644
index 00000000..3ded65dc
--- /dev/null
+++ b/www/manager6/form/PCIMapSelector.js
@@ -0,0 +1,112 @@
+Ext.define('pve-mapped-pci-model', {
+    extend: 'Ext.data.Model',
+
+    fields: ['id', 'path', 'vendor', 'device', 'iommugroup', 'mdev', 'description', 'map'],
+    idProperty: 'id',
+});
+
+Ext.define('PVE.form.PCIMapSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    xtype: 'pvePCIMapSelector',
+
+    store: {
+	model: 'pve-mapped-pci-model',
+	filterOnLoad: true,
+	sorters: [
+	    {
+		property: 'id',
+		direction: 'ASC',
+	    },
+	],
+    },
+
+    autoSelect: false,
+    valueField: 'id',
+    displayField: 'id',
+
+    // can contain a load callback for the store
+    // useful to determine the state of the IOMMU
+    onLoadCallBack: undefined,
+
+    listConfig: {
+	width: 800,
+	columns: [
+	    {
+		header: gettext('ID'),
+		dataIndex: 'id',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Description'),
+		dataIndex: 'description',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Status'),
+		dataIndex: 'errors',
+		renderer: function(value) {
+		    let me = this;
+
+		    if (!Ext.isArray(value) || !value?.length) {
+			return `<i class="fa fa-check-circle good"></i> ${gettext('Mapping OK')}`;
+		    }
+
+		    let errors = [];
+
+		    value.forEach((error) => {
+			let iconCls;
+			switch (error?.severity) {
+			    case 'warning':
+				iconCls = 'fa-exclamation-circle warning';
+				break;
+			    case 'error':
+				iconCls = 'fa-times-circle critical';
+				break;
+			}
+
+			let message = error?.message;
+			let icon = `<i class="fa ${iconCls}"></i>`;
+			if (iconCls !== undefined) {
+			    errors.push(`${icon} ${message}`);
+			}
+		    });
+
+		    return errors.join('<br>');
+		},
+		flex: 3,
+	    },
+	],
+    },
+
+    setNodename: function(nodename) {
+	var me = this;
+
+	if (!nodename || me.nodename === nodename) {
+	    return;
+	}
+
+	me.nodename = nodename;
+
+	me.store.setProxy({
+	    type: 'proxmox',
+	    url: `/api2/json/cluster/mapping/pci?check-node=${nodename}`,
+	});
+
+	me.store.load();
+    },
+
+    initComponent: function() {
+	var me = this;
+
+	var nodename = me.nodename;
+	me.nodename = undefined;
+
+        me.callParent();
+
+	if (me.onLoadCallBack !== undefined) {
+	    me.mon(me.getStore(), 'load', me.onLoadCallBack);
+	}
+
+	me.setNodename(nodename);
+    },
+});
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 06/15] ui: form: add USBMapSelector
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (10 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 05/15] ui: form: add PCIMapSelector Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 07/15] ui: qemu/PCIEdit: rework panel to add a mapped configuration Dominik Csapak
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

similar to PCIMapSelector

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Makefile               |  1 +
 www/manager6/form/USBMapSelector.js | 98 +++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)
 create mode 100644 www/manager6/form/USBMapSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 8de983aa..40a60639 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -69,6 +69,7 @@ JSSRC= 							\
 	form/TFASelector.js				\
 	form/TokenSelector.js 				\
 	form/USBSelector.js				\
+	form/USBMapSelector.js				\
 	form/UserSelector.js				\
 	form/VLanField.js				\
 	form/VMCPUFlagSelector.js			\
diff --git a/www/manager6/form/USBMapSelector.js b/www/manager6/form/USBMapSelector.js
new file mode 100644
index 00000000..990ef30f
--- /dev/null
+++ b/www/manager6/form/USBMapSelector.js
@@ -0,0 +1,98 @@
+Ext.define('PVE.form.USBMapSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: 'widget.pveUSBMapSelector',
+
+    store: {
+	fields: ['name', 'vendor', 'device', 'path'],
+	filterOnLoad: true,
+	sorters: [
+	    {
+		property: 'name',
+		direction: 'ASC',
+	    },
+	],
+    },
+
+    allowBlank: false,
+    autoSelect: false,
+    displayField: 'id',
+    valueField: 'id',
+
+    listConfig: {
+	width: 800,
+	columns: [
+	    {
+		header: gettext('Name'),
+		dataIndex: 'id',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Status'),
+		dataIndex: 'errors',
+		flex: 2,
+		renderer: function(value) {
+		    let me = this;
+
+		    if (!Ext.isArray(value) || !value?.length) {
+			return `<i class="fa fa-check-circle good"></i> ${gettext('Mapping OK')}`;
+		    }
+
+		    let errors = [];
+
+		    value.forEach((error) => {
+			let iconCls;
+			switch (error?.severity) {
+			    case 'warning':
+				iconCls = 'fa-exclamation-circle warning';
+				break;
+			    case 'error':
+				iconCls = 'fa-times-circle critical';
+				break;
+			}
+
+			let message = error?.message;
+			let icon = `<i class="fa ${iconCls}"></i>`;
+			if (iconCls !== undefined) {
+			    errors.push(`${icon} ${message}`);
+			}
+		    });
+
+		    return errors.join('<br>');
+		},
+	    },
+	    {
+		header: gettext('Comment'),
+		dataIndex: 'description',
+		flex: 1,
+	    },
+	],
+    },
+
+    setNodename: function(nodename) {
+	var me = this;
+
+	if (!nodename || me.nodename === nodename) {
+	    return;
+	}
+
+	me.nodename = nodename;
+
+	me.store.setProxy({
+	    type: 'proxmox',
+	    url: `/api2/json/cluster/mapping/usb?check-node=${nodename}`,
+	});
+
+	me.store.load();
+    },
+
+    initComponent: function() {
+	var me = this;
+
+	var nodename = me.nodename;
+	me.nodename = undefined;
+
+        me.callParent();
+
+	me.setNodename(nodename);
+    },
+});
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 07/15] ui: qemu/PCIEdit: rework panel to add a mapped configuration
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (11 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 06/15] ui: form: add USBMapSelector Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 08/15] ui: qemu/USBEdit: add 'mapped' device case Dominik Csapak
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

reworks the panel to use a controller, so that we can easily
add the selector for mapped pci devices

shows now a selection between 'raw' and 'mapped' devices, where
'raw' ones work like before, and 'mapped' ones take the values
form the hardware map config

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/PCIEdit.js | 323 ++++++++++++++++++++++++-----------
 1 file changed, 219 insertions(+), 104 deletions(-)

diff --git a/www/manager6/qemu/PCIEdit.js b/www/manager6/qemu/PCIEdit.js
index 2f67aece..d98f410d 100644
--- a/www/manager6/qemu/PCIEdit.js
+++ b/www/manager6/qemu/PCIEdit.js
@@ -3,71 +3,164 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 
     onlineHelp: 'qm_pci_passthrough_vm_config',
 
-    setVMConfig: function(vmconfig) {
-	var me = this;
-	me.vmconfig = vmconfig;
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	setVMConfig: function(vmconfig) {
+	    let me = this;
+	    let view = me.getView();
+	    me.vmconfig = vmconfig;
 
-	var hostpci = me.vmconfig[me.confid] || '';
+	    let hostpci = me.vmconfig[view.confid] || '';
 
-	var values = PVE.Parser.parsePropertyString(hostpci, 'host');
-	if (values.host) {
-	    if (!values.host.match(/^[0-9a-f]{4}:/i)) { // add optional domain
-		values.host = "0000:" + values.host;
+	    let values = PVE.Parser.parsePropertyString(hostpci, 'host');
+	    if (values.host) {
+		if (values.host.includes(':')) {
+		    values.type = 'raw';
+		    if (!values.host.match(/^[0-9a-f]{4}:/i)) { // add optional domain
+			values.host = "0000:" + values.host;
+		    }
+		    if (values.host.length < 11) { // 0000:00:00 format not 0000:00:00.0
+			values.host += ".0";
+			values.multifunction = true;
+		    }
+		} else {
+		    values.hostmapped = values.host;
+		    delete values.host;
+		    values.type = 'mapped';
+		}
 	    }
-	    if (values.host.length < 11) { // 0000:00:00 format not 0000:00:00.0
-		values.host += ".0";
-		values.multifunction = true;
+
+	    values['x-vga'] = PVE.Parser.parseBoolean(values['x-vga'], 0);
+	    values.pcie = PVE.Parser.parseBoolean(values.pcie, 0);
+	    values.rombar = PVE.Parser.parseBoolean(values.rombar, 1);
+
+	    view.setValues(values);
+	    if (!me.vmconfig.machine || me.vmconfig.machine.indexOf('q35') === -1) {
+		// machine is not set to some variant of q35, so we disable pcie
+		let pcie = me.lookup('pcie');
+		pcie.setDisabled(true);
+		pcie.setBoxLabel(gettext('Q35 only'));
 	    }
-	}
 
-	values['x-vga'] = PVE.Parser.parseBoolean(values['x-vga'], 0);
-	values.pcie = PVE.Parser.parseBoolean(values.pcie, 0);
-	values.rombar = PVE.Parser.parseBoolean(values.rombar, 1);
+	    if (values.romfile) {
+		me.lookup('romfile').setVisible(true);
+	    }
+	},
 
-	me.setValues(values);
-	if (!me.vmconfig.machine || me.vmconfig.machine.indexOf('q35') === -1) {
-	    // machine is not set to some variant of q35, so we disable pcie
-	    var pcie = me.down('field[name=pcie]');
-	    pcie.setDisabled(true);
-	    pcie.setBoxLabel(gettext('Q35 only'));
-	}
+	selectorEnable: function(selector) {
+	    let me = this;
+	    me.pciDevChange(selector, selector.getValue());
+	},
 
-	if (values.romfile) {
-	    me.down('field[name=romfile]').setVisible(true);
-	}
-    },
+	pciDevChange: function(pcisel, value) {
+	    let me = this;
+	    let mdevfield = me.lookup('mdev');
+	    if (!value) {
+		if (!pcisel.isDisabled()) {
+		    mdevfield.setDisabled(true);
+		}
+		return;
+	    }
+	    let pciDev = pcisel.getStore().getById(value);
 
-    onGetValues: function(values) {
-	let me = this;
-	if (!me.confid) {
-	    for (let i = 0; i < PVE.Utils.hardware_counts.hostpci; i++) {
-		if (!me.vmconfig['hostpci' + i.toString()]) {
-		    me.confid = 'hostpci' + i.toString();
-		    break;
+	    mdevfield.setDisabled(!pciDev || !pciDev.data.mdev);
+	    if (!pciDev) {
+		return;
+	    }
+
+	    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';
 		}
 	    }
-	    // FIXME: what if no confid was found??
-	}
-	values.host.replace(/^0000:/, ''); // remove optional '0000' domain
 
-	if (values.multifunction) {
-	    values.host = values.host.substring(0, values.host.indexOf('.')); // skip the '.X'
-	    delete values.multifunction;
-	}
+	    if (pciDev.data.mdev) {
+		mdevfield.setPciID(path);
+	    }
+	    if (pcisel.reference === 'selector') {
+		let iommu = pciDev.data.iommugroup;
+		if (iommu === -1) {
+		    return;
+		}
+		// try to find out if there are more devices in that iommu group
+		let id = path.substring(0, 5); // 00:00
+		let count = 0;
+		pcisel.getStore().each(({ data }) => {
+		    if (data.iommugroup === iommu && data.id.substring(0, 5) !== id) {
+			count++;
+			return false;
+		    }
+		    return true;
+		});
+		me.lookup('group_warning').setVisible(count > 0);
+	    }
+	},
 
-	if (values.rombar) {
-	    delete values.rombar;
-	} else {
-	    values.rombar = 0;
-	}
+	onGetValues: function(values) {
+	    let me = this;
+	    let view = me.getView();
+	    if (!view.confid) {
+		for (let i = 0; i < PVE.Utils.hardware_counts.hostpci; i++) {
+		    if (!me.vmconfig['hostpci' + i.toString()]) {
+			view.confid = 'hostpci' + i.toString();
+			break;
+		    }
+		}
+		// FIXME: what if no confid was found??
+	    }
 
-	if (!values.romfile) {
-	    delete values.romfile;
-	}
+	    if (values.hostmapped) {
+		values.host = values.hostmapped;
+		delete values.hostmapped;
+	    } else {
+		values.host.replace(/^0000:/, ''); // remove optional '0000' domain
+
+		if (values.multifunction) {
+		    values.host = values.host.substring(0, values.host.indexOf('.')); // skip the '.X'
+		    delete values.multifunction;
+		}
+	    }
+
+	    if (values.rombar) {
+		delete values.rombar;
+	    } else {
+		values.rombar = 0;
+	    }
+
+	    if (!values.romfile) {
+		delete values.romfile;
+	    }
+
+	    delete values.type;
+
+	    let ret = {};
+	    ret[view.confid] = PVE.Parser.printPropertyString(values, 'host');
+	    return ret;
+	},
+    },
 
-	let ret = {};
-	ret[me.confid] = PVE.Parser.printPropertyString(values, 'host');
-	return ret;
+    viewModel: {
+	data: {
+	    isMapped: true,
+	},
+    },
+
+    setVMConfig: function(vmconfig) {
+	return this.getController().setVMConfig(vmconfig);
+    },
+
+    onGetValues: function(values) {
+	return this.getController().onGetValues(values);
     },
 
     initComponent: function() {
@@ -78,78 +171,97 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 	    throw "no node name specified";
 	}
 
+	me.columnT = [
+	    {
+		xtype: 'displayfield',
+		reference: 'iommu_warning',
+		hidden: true,
+		columnWidth: 1,
+		padding: '0 0 10 0',
+		value: 'No IOMMU detected, please activate it.' +
+		'See Documentation for further information.',
+		userCls: 'pmx-hint',
+	    },
+	    {
+		xtype: 'displayfield',
+		reference: 'group_warning',
+		hidden: true,
+		columnWidth: 1,
+		padding: '0 0 10 0',
+		itemId: 'iommuwarning',
+		value: 'The selected Device is not in a seperate IOMMU group, make sure this is intended.',
+		userCls: 'pmx-hint',
+	    },
+	];
+
 	me.column1 = [
+	    {
+		xtype: 'radiofield',
+		name: 'type',
+		inputValue: 'mapped',
+		boxLabel: gettext('Mapped Device'),
+		bind: {
+		    value: '{isMapped}',
+		},
+	    },
+	    {
+		xtype: 'pvePCIMapSelector',
+		fieldLabel: gettext('Device'),
+		reference: 'mapped_selector',
+		name: 'hostmapped',
+		labelAlign: 'right',
+		nodename: me.nodename,
+		allowBlank: false,
+		bind: {
+		    disabled: '{!isMapped}',
+		},
+		listeners: {
+		    change: 'pciDevChange',
+		    enable: 'selectorEnable',
+		},
+	    },
+	    {
+		xtype: 'radiofield',
+		name: 'type',
+		inputValue: 'raw',
+		checked: true,
+		boxLabel: gettext('Raw Device'),
+	    },
 	    {
 		xtype: 'pvePCISelector',
 		fieldLabel: gettext('Device'),
 		name: 'host',
+		reference: 'selector',
 		nodename: me.nodename,
+		labelAlign: 'right',
 		allowBlank: false,
+		disabled: true,
+		bind: {
+		    disabled: '{isMapped}',
+		},
 		onLoadCallBack: function(store, records, success) {
 		    if (!success || !records.length) {
 			return;
 		    }
-		    if (records.every((val) => val.data.iommugroup === -1)) { // no IOMMU groups
-			let warning = Ext.create('Ext.form.field.Display', {
-			    columnWidth: 1,
-			    padding: '0 0 10 0',
-			    value: 'No IOMMU detected, please activate it.' +
-				   'See Documentation for further information.',
-			    userCls: 'pmx-hint',
-			});
-			me.items.insert(0, warning);
-			me.updateLayout(); // insert does not trigger that
-		    }
+		    me.lookup('iommu_warning').setVisible(
+			records.every((val) => val.data.iommugroup === -1),
+		    );
 		},
 		listeners: {
-		    change: function(pcisel, value) {
-			if (!value) {
-			    return;
-			}
-			let pciDev = pcisel.getStore().getById(value);
-			let mdevfield = me.down('field[name=mdev]');
-			mdevfield.setDisabled(!pciDev || !pciDev.data.mdev);
-			if (!pciDev) {
-			    return;
-			}
-			if (pciDev.data.mdev) {
-			    mdevfield.setPciID(value);
-			}
-			let iommu = pciDev.data.iommugroup;
-			if (iommu === -1) {
-			    return;
-			}
-			// try to find out if there are more devices in that iommu group
-			let id = pciDev.data.id.substring(0, 5); // 00:00
-			let count = 0;
-			pcisel.getStore().each(({ data }) => {
-			    if (data.iommugroup === iommu && data.id.substring(0, 5) !== id) {
-				count++;
-				return false;
-			    }
-			    return true;
-			});
-			let warning = me.down('#iommuwarning');
-			if (count && !warning) {
-			    warning = Ext.create('Ext.form.field.Display', {
-				columnWidth: 1,
-				padding: '0 0 10 0',
-				itemId: 'iommuwarning',
-				value: 'The selected Device is not in a seperate IOMMU group, make sure this is intended.',
-				userCls: 'pmx-hint',
-			    });
-			    me.items.insert(0, warning);
-			    me.updateLayout(); // insert does not trigger that
-			} else if (!count && warning) {
-			    me.remove(warning);
-			}
-		    },
+		    change: 'pciDevChange',
+		    enable: 'selectorEnable',
 		},
 	    },
 	    {
 		xtype: 'proxmoxcheckbox',
 		fieldLabel: gettext('All Functions'),
+		reference: 'all_functions',
+		disabled: true,
+		labelAlign: 'right',
 		name: 'multifunction',
+		bind: {
+		    disabled: '{isMapped}',
+		},
 	    },
 	];
 
@@ -157,6 +269,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 	    {
 		xtype: 'pveMDevSelector',
 		name: 'mdev',
+		reference: 'mdev',
 		disabled: true,
 		fieldLabel: gettext('MDev Type'),
 		nodename: me.nodename,
@@ -188,6 +301,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 		submitValue: true,
 		hidden: true,
 		fieldLabel: 'ROM-File',
+		reference: 'romfile',
 		name: 'romfile',
 	    },
 	    {
@@ -214,6 +328,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 	    {
 		xtype: 'proxmoxcheckbox',
 		fieldLabel: 'PCI-Express',
+		reference: 'pcie',
 		name: 'pcie',
 	    },
 	    {
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 08/15] ui: qemu/USBEdit: add 'mapped' device case
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (12 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 07/15] ui: qemu/PCIEdit: rework panel to add a mapped configuration Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 09/15] ui: form: add MultiPCISelector Dominik Csapak
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

to be able to select 'mapped' usb devices

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/USBEdit.js | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/www/manager6/qemu/USBEdit.js b/www/manager6/qemu/USBEdit.js
index fe51d186..cfcdd31f 100644
--- a/www/manager6/qemu/USBEdit.js
+++ b/www/manager6/qemu/USBEdit.js
@@ -5,6 +5,15 @@ Ext.define('PVE.qemu.USBInputPanel', {
     autoComplete: false,
     onlineHelp: 'qm_usb_passthrough',
 
+    cbindData: function(initialConfig) {
+	let me = this;
+	if (!me.pveSelNode) {
+	    throw "no pveSelNode given";
+	}
+
+	return { nodename: me.pveSelNode.data.node };
+    },
+
     viewModel: {
 	data: {},
     },
@@ -36,6 +45,7 @@ Ext.define('PVE.qemu.USBInputPanel', {
 	    case 'spice':
 		val = 'spice';
 		break;
+	    case 'mapped':
 	    case 'hostdevice':
 	    case 'port':
 		val = 'host=' + values[type];
@@ -66,6 +76,23 @@ Ext.define('PVE.qemu.USBInputPanel', {
 		    submitValue: false,
 		    checked: true,
 		},
+		{
+		    name: 'usb',
+		    inputValue: 'mapped',
+		    boxLabel: gettext('Use mapped Device'),
+		    reference: 'mapped',
+		    submitValue: false,
+		},
+		{
+		    xtype: 'pveUSBMapSelector',
+		    disabled: true,
+		    name: 'mapped',
+		    cbind: { nodename: '{nodename}' },
+		    bind: { disabled: '{!mapped.checked}' },
+		    allowBlank: false,
+		    fieldLabel: gettext('Choose Device'),
+		    labelAlign: 'right',
+		},
 		{
 		    name: 'usb',
 		    inputValue: 'hostdevice',
@@ -150,7 +177,7 @@ Ext.define('PVE.qemu.USBEdit', {
 		}
 
 		var data = response.result.data[me.confid].split(',');
-		var port, hostdevice, usb3 = false;
+		var port, hostdevice, mapped, usb3 = false;
 		var type = 'spice';
 
 		for (let i = 0; i < data.length; i++) {
@@ -162,6 +189,12 @@ Ext.define('PVE.qemu.USBEdit', {
 			port = data[i];
 			port = port.replace('host=', '');
 			type = 'port';
+		    } else if (/^(host=)?[a-zA-Z0-9\-_]+$/.test(data[i])) {
+			if (data[i] !== 'spice') {
+			    mapped = data[i];
+			    mapped = mapped.replace('host=', '');
+			    type = 'mapped';
+			}
 		    }
 
 		    if (/^usb3=(1|on|true)$/.test(data[i])) {
@@ -173,6 +206,7 @@ Ext.define('PVE.qemu.USBEdit', {
 		    hostdevice: hostdevice,
 		    port: port,
 		    usb3: usb3,
+		    mapped,
 		};
 
 		ipanel.setValues(values);
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 09/15] ui: form: add MultiPCISelector
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (13 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 08/15] ui: qemu/USBEdit: add 'mapped' device case Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 10/15] ui: add edit window for pci mappings Dominik Csapak
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

this is a grid field for selecting multiple pci devices at once, like we
need for the mapped pci ui. There we want to be able to select multiple
devices such that one gets selected automatically

we can select a whole slot here, but that disables selecting the
individual functions of that device.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/css/ext6-pve.css                  |   4 +
 www/manager6/Makefile                 |   1 +
 www/manager6/form/MultiPCISelector.js | 288 ++++++++++++++++++++++++++
 3 files changed, 293 insertions(+)
 create mode 100644 www/manager6/form/MultiPCISelector.js

diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index a9ead5d3..3af64255 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -700,3 +700,7 @@ table.osds td:first-of-type {
     cursor: pointer;
     padding-left: 2px;
 }
+
+.x-grid-item .x-item-disabled {
+    opacity: 0.3;
+}
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 40a60639..e534cecd 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -46,6 +46,7 @@ JSSRC= 							\
 	form/IPRefSelector.js				\
 	form/MDevSelector.js				\
 	form/MemoryField.js				\
+	form/MultiPCISelector.js			\
 	form/NetworkCardSelector.js			\
 	form/NodeSelector.js				\
 	form/PCISelector.js				\
diff --git a/www/manager6/form/MultiPCISelector.js b/www/manager6/form/MultiPCISelector.js
new file mode 100644
index 00000000..99f9d50b
--- /dev/null
+++ b/www/manager6/form/MultiPCISelector.js
@@ -0,0 +1,288 @@
+Ext.define('PVE.form.MultiPCISelector', {
+    extend: 'Ext.grid.Panel',
+    alias: 'widget.pveMultiPCISelector',
+
+    emptyText: gettext('No Devices found'),
+
+    mixins: {
+	field: 'Ext.form.field.Field',
+    },
+
+    getValue: function() {
+	let me = this;
+	return me.value ?? [];
+    },
+
+    getSubmitData: function() {
+	let me = this;
+	let res = {};
+	res[me.name] = me.getValue();
+	return res;
+    },
+
+    setValue: function(value) {
+	let me = this;
+
+	value ??= [];
+
+	me.updateSelectedDevices(value);
+
+	return me.mixins.field.setValue.call(me, value);
+    },
+
+    getErrors: function() {
+	let me = this;
+
+	let errorCls = ['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid'];
+
+	if (me.getValue().length < 1) {
+	    let error = gettext("Must choose at least one device");
+	    me.addCls(errorCls);
+	    me.getActionEl()?.dom.setAttribute('data-errorqtip', error);
+
+	    return [error];
+	}
+
+	me.removeCls(errorCls);
+	me.getActionEl()?.dom.setAttribute('data-errorqtip', "");
+
+	return [];
+    },
+
+    viewConfig: {
+	getRowClass: function(record) {
+	    if (record.data.disabled === true) {
+		return 'x-item-disabled';
+	    }
+	    return '';
+	},
+    },
+
+    updateSelectedDevices: function(value = []) {
+	let me = this;
+
+	let recs = [];
+	let store = me.getStore();
+
+	for (const map of value) {
+	    let parsed = PVE.Parser.parsePropertyString(map);
+	    if (parsed.node !== me.nodename) {
+		continue;
+	    }
+
+	    let rec = store.getById(parsed.path);
+	    if (rec) {
+		recs.push(rec);
+	    }
+	}
+
+	me.suspendEvent('change');
+	me.setSelection([]);
+	me.setSelection(recs);
+	me.resumeEvent('change');
+    },
+
+    setNodename: function(nodename) {
+	let me = this;
+
+	if (!nodename || me.nodename === nodename) {
+	    return;
+	}
+
+	me.nodename = nodename;
+
+	me.getStore().setProxy({
+	    type: 'proxmox',
+	    url: '/api2/json/nodes/' + me.nodename + '/hardware/pci?pci-class-blacklist=',
+	});
+
+	me.setSelection([]);
+
+	me.getStore().load({
+	    callback: (recs, op, success) => me.addSlotRecords(recs, op, success),
+	});
+    },
+
+    setMdev: function(mdev) {
+	let me = this;
+	if (mdev) {
+	    me.getStore().addFilter({
+		id: 'mdev-filter',
+		property: 'mdev',
+		value: '1',
+		operator: '=',
+	    });
+	} else {
+	    me.getStore().removeFilter('mdev-filter');
+	}
+    },
+
+    // adds the virtual 'slot' records (e.g. '0000:01:00') to the store
+    addSlotRecords: function(records, _op, success) {
+	let me = this;
+	if (!success) {
+	    return;
+	}
+
+	let slots = {};
+	records.forEach((rec) => {
+	    let slotname = rec.data.id.slice(0, -2); // remove function
+	    rec.set('slot', slotname);
+	    if (slots[slotname] !== undefined) {
+		slots[slotname].count++;
+		return;
+	    }
+
+	    slots[slotname] = {
+		count: 1,
+	    };
+
+	    if (rec.data.id.endsWith('.0')) {
+		slots[slotname].device = rec.data;
+	    }
+	});
+
+	let store = me.getStore();
+
+	for (const [slot, { count, device }] of Object.entries(slots)) {
+	    if (count === 1) {
+		continue;
+	    }
+	    store.add(Ext.apply({}, {
+		id: slot,
+		mdev: undefined,
+		device_name: gettext('Pass through all functions as one device'),
+	    }, device));
+	}
+
+	me.updateSelectedDevices(me.value);
+    },
+
+    selectionChange: function(_grid, selection) {
+	let me = this;
+
+	let ids = {};
+	selection
+	    .filter(rec => rec.data.id.indexOf('.') === -1)
+	    .forEach((rec) => { ids[rec.data.id] = true; });
+
+	let to_disable = [];
+
+	me.getStore().each(rec => {
+	    let id = rec.data.id;
+	    rec.set('disabled', false);
+	    if (id.indexOf('.') === -1) {
+		return;
+	    }
+	    let slot = id.slice(0, -2); // remove function
+
+	    if (ids[slot]) {
+		to_disable.push(rec);
+		rec.set('disabled', true);
+	    }
+	});
+
+	me.suspendEvent('selectionchange');
+	me.getSelectionModel().deselect(to_disable);
+	me.resumeEvent('selectionchange');
+
+	me.value = me.getSelection().map((rec) => {
+	    let res = {
+		path: rec.data.id,
+		node: me.nodename,
+		id: `${rec.data.vendor}:${rec.data.device}`.replace(/0x/g, ''),
+		'subsystem-id': `${rec.data.subsystem_vendor}:${rec.data.subsystem_device}`.replace(/0x/g, ''),
+	    };
+
+	    if (rec.data.iommugroup !== -1) {
+		res.iommugroup = rec.data.iommugroup;
+	    }
+
+	    return PVE.Parser.printPropertyString(res);
+	});
+	me.checkChange();
+    },
+
+    selModel: {
+	type: 'checkboxmodel',
+	mode: 'SIMPLE',
+    },
+
+    columns: [
+	{
+	    header: 'ID',
+	    dataIndex: 'id',
+	    width: 150,
+	},
+	{
+	    header: gettext('IOMMU Group'),
+	    dataIndex: 'iommugroup',
+	    renderer: (v, _md, rec) => rec.data.slot === rec.data.id ? '' : v === -1 ? '-' : v,
+	    width: 50,
+	},
+	{
+	    header: gettext('Vendor'),
+	    dataIndex: 'vendor_name',
+	    flex: 3,
+	},
+	{
+	    header: gettext('Device'),
+	    dataIndex: 'device_name',
+	    flex: 6,
+	},
+	{
+	    header: gettext('Mediated Devices'),
+	    dataIndex: 'mdev',
+	    flex: 1,
+	    renderer: function(val) {
+		return Proxmox.Utils.format_boolean(!!val);
+	    },
+	},
+    ],
+
+    listeners: {
+	selectionchange: function() {
+	    this.selectionChange(...arguments);
+	},
+    },
+
+    store: {
+	fields: [
+	    'id', 'vendor_name', 'device_name', 'vendor', 'device', 'iommugroup', 'mdev',
+	    'subsystem_vendor', 'subsystem_device', 'disabled',
+	    {
+		name: 'subsystem-vendor',
+		calculate: function(data) {
+		    return data.subsystem_vendor;
+		},
+	    },
+	    {
+		name: 'subsystem-device',
+		calculate: function(data) {
+		    return data.subsystem_device;
+		},
+	    },
+	],
+	sorters: [
+	    {
+		property: 'id',
+		direction: 'ASC',
+	    },
+	],
+    },
+
+    initComponent: function() {
+	let me = this;
+
+	let nodename = me.nodename;
+	me.nodename = undefined;
+
+	me.callParent();
+
+	Proxmox.Utils.monStoreErrors(me, me.getStore(), true);
+
+	me.setNodename(nodename);
+
+	me.initField();
+    },
+});
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 10/15] ui: add edit window for pci mappings
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (14 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 09/15] ui: form: add MultiPCISelector Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 11/15] ui: add edit window for usb mappings Dominik Csapak
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

This contains the window to edit a PCI mapping for a single host.
It is designed to work in 3 modes:

* without an id and a nodename: for new mappings
* with an id but without nodename: for adding new host mappings to an
  existing one
* with id and nodename: when editing an existing host mapping

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* add onlineHelp
 www/manager6/Makefile             |   1 +
 www/manager6/form/PCISelector.js  |  17 ++-
 www/manager6/window/PCIMapEdit.js | 207 ++++++++++++++++++++++++++++++
 3 files changed, 224 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/window/PCIMapEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index e534cecd..98a5b9a1 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -123,6 +123,7 @@ JSSRC= 							\
 	window/Wizard.js				\
 	window/GuestDiskReassign.js				\
 	window/TreeSettingsEdit.js			\
+	window/PCIEdit.js				\
 	ha/Fencing.js					\
 	ha/GroupEdit.js					\
 	ha/GroupSelector.js				\
diff --git a/www/manager6/form/PCISelector.js b/www/manager6/form/PCISelector.js
index 4e0a778f..39e111f0 100644
--- a/www/manager6/form/PCISelector.js
+++ b/www/manager6/form/PCISelector.js
@@ -3,7 +3,22 @@ Ext.define('PVE.form.PCISelector', {
     xtype: 'pvePCISelector',
 
     store: {
-	fields: ['id', 'vendor_name', 'device_name', 'vendor', 'device', 'iommugroup', 'mdev'],
+	fields: [
+	    'id', 'vendor_name', 'device_name', 'vendor', 'device', 'iommugroup', 'mdev',
+	    'subsystem_vendor', 'subsystem_device',
+	    {
+		name: 'subsystem-vendor',
+		calculate: function(data) {
+		    return data.subsystem_vendor;
+		},
+	    },
+	    {
+		name: 'subsystem-device',
+		calculate: function(data) {
+		    return data.subsystem_device;
+		},
+	    },
+	],
 	filterOnLoad: true,
 	sorters: [
 	    {
diff --git a/www/manager6/window/PCIMapEdit.js b/www/manager6/window/PCIMapEdit.js
new file mode 100644
index 00000000..1572f442
--- /dev/null
+++ b/www/manager6/window/PCIMapEdit.js
@@ -0,0 +1,207 @@
+Ext.define('PVE.window.PCIMapEditWindow', {
+    extend: 'Proxmox.window.Edit',
+
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    width: 800,
+
+    subject: gettext('PCI mapping'),
+
+    onlineHelp: 'resource_mapping',
+
+    method: 'POST',
+
+    cbindData: function(initialConfig) {
+	let me = this;
+	me.isCreate = !me.name;
+	me.method = me.isCreate ? 'POST' : 'PUT';
+	return {
+	    name: me.name,
+	    nodename: me.nodename,
+	};
+    },
+
+    submitUrl: function(_url, data) {
+	let me = this;
+	let name = me.isCreate ? '' : me.name;
+	return `/cluster/mapping/pci/${name}`;
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	onGetValues: function(values) {
+	    let me = this;
+	    let view = me.getView();
+	    if (view.method === "POST") {
+		delete me.digest;
+	    }
+
+	    if (values.iommugroup === -1) {
+		delete values.iommugroup;
+	    }
+
+	    let nodename = values.node ?? view.nodename;
+	    delete values.node;
+	    if (me.originalMap) {
+		let otherMaps = PVE.Parser
+		    .filterPropertyStringList(me.originalMap, (e) => e.node !== nodename);
+		if (otherMaps.length) {
+		    values.map = values.map.concat(otherMaps);
+		}
+	    }
+
+	    return values;
+	},
+
+	onSetValues: function(values) {
+	    let me = this;
+	    let view = me.getView();
+	    me.originalMap = [...values.map];
+	    values.map = PVE.Parser.filterPropertyStringList(values.map, (e) => e.node === view.nodename);
+	    return values;
+	},
+
+	checkIommu: function(store, records, success) {
+	    let me = this;
+	    if (!success || !records.length) {
+		return;
+	    }
+	    me.lookup('iommu_warning').setVisible(
+		records.every((val) => val.data.iommugroup === -1),
+	    );
+	},
+
+	mdevChange: function(mdevField, value) {
+	    this.lookup('pciselector').setMdev(value);
+	},
+
+	nodeChange: function(_field, value) {
+	    this.lookup('pciselector').setNodename(value);
+	},
+
+	control: {
+	    'field[name=mdev]': {
+		change: 'mdevChange',
+	    },
+	    'pveNodeSelector': {
+		change: 'nodeChange',
+	    },
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+	    onGetValues: function(values) {
+		return this.up('window').getController().onGetValues(values);
+	    },
+
+	    onSetValues: function(values) {
+		return this.up('window').getController().onSetValues(values);
+	    },
+
+	    columnT: [
+		{
+		    xtype: 'displayfield',
+		    reference: 'iommu_warning',
+		    hidden: true,
+		    columnWidth: 1,
+		    padding: '0 0 10 0',
+		    value: 'No IOMMU detected, please activate it.' +
+		    'See Documentation for further information.',
+		    userCls: 'pmx-hint',
+		},
+		{
+		    xtype: 'displayfield',
+		    reference: 'multiple_warning',
+		    hidden: true,
+		    columnWidth: 1,
+		    padding: '0 0 10 0',
+		    value: 'When multiple devices are selected, the first free one will be chosen' +
+			' on guest start.',
+		    userCls: 'pmx-hint',
+		},
+		{
+		    xtype: 'displayfield',
+		    reference: 'group_warning',
+		    hidden: true,
+		    columnWidth: 1,
+		    padding: '0 0 10 0',
+		    itemId: 'iommuwarning',
+		    value: 'The selected Device is not in a seperate IOMMU group, make sure this is intended.',
+		    userCls: 'pmx-hint',
+		},
+	    ],
+
+	    column1: [
+		{
+		    xtype: 'pmxDisplayEditField',
+		    fieldLabel: gettext('Name'),
+		    labelWidth: 120,
+		    cbind: {
+			editable: '{!name}',
+			value: '{name}',
+			submitValue: '{isCreate}',
+		    },
+		    name: 'id',
+		    allowBlank: false,
+		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    fieldLabel: gettext('Mediated Devices'),
+		    labelWidth: 120,
+		    reference: 'mdev',
+		    name: 'mdev',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+
+	    column2: [
+		{
+		    xtype: 'pmxDisplayEditField',
+		    fieldLabel: gettext('Node'),
+		    labelWidth: 120,
+		    name: 'node',
+		    editConfig: {
+			xtype: 'pveNodeSelector',
+		    },
+		    cbind: {
+			editable: '{!nodename}',
+			value: '{nodename}',
+		    },
+		    allowBlank: false,
+		},
+	    ],
+
+	    columnB: [
+		{
+		    xtype: 'pveMultiPCISelector',
+		    fieldLabel: gettext('Device'),
+		    labelWidth: 120,
+		    height: 300,
+		    reference: 'pciselector',
+		    name: 'map',
+		    cbind: {
+			nodename: '{nodename}',
+		    },
+		    allowBlank: false,
+		    onLoadCallBack: 'checkIommu',
+		    margin: '0 0 10 0',
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    fieldLabel: gettext('Comment'),
+		    labelWidth: 120,
+		    submitValue: true,
+		    name: 'description',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+	},
+    ],
+});
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 11/15] ui: add edit window for usb mappings
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (15 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 10/15] ui: add edit window for pci mappings Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 12/15] ui: add ResourceMapTree Dominik Csapak
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

very similar to the PCIMapEdit window, but we only ever allow one
mapping per host

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* add onlineHelp
 www/manager6/Makefile             |   3 +-
 www/manager6/window/USBMapEdit.js | 217 ++++++++++++++++++++++++++++++
 2 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/window/USBMapEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 98a5b9a1..99ebc4dc 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -123,7 +123,8 @@ JSSRC= 							\
 	window/Wizard.js				\
 	window/GuestDiskReassign.js				\
 	window/TreeSettingsEdit.js			\
-	window/PCIEdit.js				\
+	window/PCIMapEdit.js				\
+	window/USBMapEdit.js				\
 	ha/Fencing.js					\
 	ha/GroupEdit.js					\
 	ha/GroupSelector.js				\
diff --git a/www/manager6/window/USBMapEdit.js b/www/manager6/window/USBMapEdit.js
new file mode 100644
index 00000000..80f8e785
--- /dev/null
+++ b/www/manager6/window/USBMapEdit.js
@@ -0,0 +1,217 @@
+Ext.define('PVE.window.USBMapEditWindow', {
+    extend: 'Proxmox.window.Edit',
+
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    cbindData: function(initialConfig) {
+	let me = this;
+	me.isCreate = !me.name;
+	me.method = me.isCreate ? 'POST' : 'PUT';
+	return {
+	    name: me.name,
+	    nodename: me.nodename,
+	};
+    },
+
+    submitUrl: function(_url, data) {
+	let me = this;
+	let name = me.isCreate ? '' : me.name;
+	return `/cluster/mapping/usb/${name}`;
+    },
+
+    title: gettext('Add USB mapping'),
+
+    onlineHelp: 'resource_mapping',
+
+    method: 'POST',
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	onGetValues: function(values) {
+	    let me = this;
+	    let view = me.getView();
+	    values.node ??= view.nodename;
+
+	    let type = me.getView().down('radiofield').getGroupValue();
+	    let name = values.name;
+	    let description = values.description;
+	    delete values.description;
+	    delete values.name;
+
+	    if (type === 'path') {
+		let usbsel = me.lookup(type);
+		let usbDev = usbsel.getStore().findRecord('usbid', values[type], 0, false, true, true);
+
+		if (!usbDev) {
+		    return {};
+		}
+		values.id = `${usbDev.data.vendid}:${usbDev.data.prodid}`;
+	    }
+
+	    let map = [];
+	    if (me.originalMap) {
+		map = PVE.Parser.filterPropertyStringList(me.originalMap, (e) => e.node !== values.node);
+	    }
+	    map.push(PVE.Parser.printPropertyString(values));
+
+	    values = {
+		map,
+		description,
+	    };
+
+	    if (view.isCreate) {
+		values.id = name;
+	    }
+
+	    return values;
+	},
+
+	onSetValues: function(values) {
+	    let me = this;
+	    let view = me.getView();
+	    me.originalMap = [...values.map];
+	    PVE.Parser.filterPropertyStringList(values.map, (e) => {
+		if (e.node === view.nodename) {
+		    values = e;
+		}
+		return false;
+	    });
+
+	    if (values.path) {
+		values.usb = 'path';
+	    }
+
+	    return values;
+	},
+
+	modeChange: function(field, value) {
+	    let me = this;
+	    let type = field.inputValue;
+	    let usbsel = me.lookup(type);
+	    usbsel.setDisabled(!value);
+	},
+
+	nodeChange: function(_field, value) {
+	    this.lookup('id').setNodename(value);
+	    this.lookup('path').setNodename(value);
+	},
+
+
+	init: function(view) {
+	    let me = this;
+
+	    if (!view.nodename) {
+		//throw "no nodename given";
+	    }
+	},
+
+	control: {
+	    'radiofield': {
+		change: 'modeChange',
+	    },
+	    'pveNodeSelector': {
+		change: 'nodeChange',
+	    },
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+	    onGetValues: function(values) {
+		return this.up('window').getController().onGetValues(values);
+	    },
+
+	    onSetValues: function(values) {
+		return this.up('window').getController().onSetValues(values);
+	    },
+
+	    column1: [
+		{
+		    xtype: 'pmxDisplayEditField',
+		    fieldLabel: gettext('Name'),
+		    cbind: {
+			editable: '{!name}',
+			value: '{name}',
+			submitValue: '{isCreate}',
+		    },
+		    name: 'name',
+		    allowBlank: false,
+		},
+		{
+		    xtype: 'pmxDisplayEditField',
+		    fieldLabel: gettext('Node'),
+		    name: 'node',
+		    editConfig: {
+			xtype: 'pveNodeSelector',
+		    },
+		    cbind: {
+			editable: '{!nodename}',
+			value: '{nodename}',
+		    },
+		    allowBlank: false,
+		},
+	    ],
+
+	    column2: [
+		{
+		    xtype: 'fieldcontainer',
+		    defaultType: 'radiofield',
+		    layout: 'fit',
+		    items: [
+			{
+			    name: 'usb',
+			    inputValue: 'id',
+			    checked: true,
+			    boxLabel: gettext('Use USB Vendor/Device ID'),
+			    submitValue: false,
+			},
+			{
+			    xtype: 'pveUSBSelector',
+			    type: 'device',
+			    reference: 'id',
+			    name: 'id',
+			    cbind: {
+				nodename: '{nodename}',
+			    },
+			    editable: true,
+			    allowBlank: false,
+			    fieldLabel: gettext('Choose Device'),
+			    labelAlign: 'right',
+			},
+			{
+			    name: 'usb',
+			    inputValue: 'path',
+			    boxLabel: gettext('Use USB Port'),
+			    submitValue: false,
+			},
+			{
+			    xtype: 'pveUSBSelector',
+			    disabled: true,
+			    name: 'path',
+			    reference: 'path',
+			    cbind: {
+				nodename: '{nodename}',
+			    },
+			    editable: true,
+			    type: 'port',
+			    allowBlank: false,
+			    fieldLabel: gettext('Choose Port'),
+			    labelAlign: 'right',
+			},
+		    ],
+		},
+	    ],
+
+	    columnB: [
+		{
+		    xtype: 'proxmoxtextfield',
+		    fieldLabel: gettext('Comment'),
+		    submitValue: true,
+		    name: 'description',
+		},
+	    ],
+	},
+    ],
+});
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 12/15] ui: add ResourceMapTree
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (16 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 11/15] ui: add edit window for usb mappings Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 13/15] ui: allow configuring pci and usb mapping Dominik Csapak
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

this will be the base class for trees for the individual mapping types,
e.g. pci and usb mapping.

there are a few things to configure, but the overall code sharing is
still significant, and should work out fine for future mapping types

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Makefile                |   1 +
 www/manager6/tree/ResourceMapTree.js | 316 +++++++++++++++++++++++++++
 2 files changed, 317 insertions(+)
 create mode 100644 www/manager6/tree/ResourceMapTree.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 99ebc4dc..0cb922d6 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -101,6 +101,7 @@ JSSRC= 							\
 	panel/MultiDiskEdit.js				\
 	tree/ResourceTree.js				\
 	tree/SnapshotTree.js				\
+	tree/ResourceMapTree.js				\
 	window/Backup.js				\
 	window/BackupConfig.js				\
 	window/BulkAction.js				\
diff --git a/www/manager6/tree/ResourceMapTree.js b/www/manager6/tree/ResourceMapTree.js
new file mode 100644
index 00000000..df50b63a
--- /dev/null
+++ b/www/manager6/tree/ResourceMapTree.js
@@ -0,0 +1,316 @@
+Ext.define('PVE.tree.ResourceMapTree', {
+    extend: 'Ext.tree.Panel',
+    alias: 'widget.pveResourceMapTree',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    rootVisible: false,
+
+    emptyText: gettext('No Mapping found'),
+
+    // will be opened on edit
+    editWindowClass: undefined,
+
+    // The base url of the resource
+    baseUrl: undefined,
+
+    // icon class to show on the entries
+    mapIconCls: undefined,
+
+    // if given, should be a function that takes a nodename and returns
+    // the url for getting the data to check the status
+    getStatusCheckUrl: undefined,
+
+    // the result of above api call and the nodename is passed and can set the status
+    checkValidity: undefined,
+
+    // the property that denotes a single map entry for a node
+    entryIdProperty: undefined,
+
+    cbindData: function(initialConfig) {
+	let me = this;
+	const caps = Ext.state.Manager.get('GuiCap');
+	me.canConfigure = !!caps.mapping['Mapping.Modify'];
+
+	return {};
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	addMapping: function() {
+	    let me = this;
+	    let view = me.getView();
+	    Ext.create(view.editWindowClass, {
+		url: view.baseUrl,
+		autoShow: true,
+		listeners: {
+		    destroy: () => me.load(),
+		},
+	    });
+	},
+
+	addHost: function() {
+	    let me = this;
+	    me.edit(false);
+	},
+
+	edit: function(includeNodename = true) {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (!selection || !selection.length) {
+		return;
+	    }
+	    let rec = selection[0];
+	    if (!view.canConfigure || (rec.data.type === 'entry' && includeNodename)) {
+		return;
+	    }
+
+	    Ext.create(view.editWindowClass, {
+		url: `${view.baseUrl}/${rec.data.name}`,
+		autoShow: true,
+		autoLoad: true,
+		nodename: includeNodename ? rec.data.node : undefined,
+		name: rec.data.name,
+		listeners: {
+		    destroy: () => me.load(),
+		},
+	    });
+	},
+
+	remove: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (!selection || !selection.length) {
+		return;
+	    }
+
+	    let data = selection[0].data;
+	    let url = `${view.baseUrl}/${data.name}`;
+	    let method = 'PUT';
+	    let params = {
+		digest: me.lookup[data.name].digest,
+	    };
+	    let map = me.lookup[data.name].map;
+	    switch (data.type) {
+		case 'entry':
+		    method = 'DELETE';
+		    params = undefined;
+		    break;
+		case 'node':
+		    params.map = PVE.Parser.filterPropertyStringList(map, (e) => e.node !== data.node);
+		    break;
+		case 'map':
+		    params.map = PVE.Parser.filterPropertyStringList(map, (e) =>
+			Object.entries(e).some(([key, value]) => data[key] !== value));
+		    break;
+		default:
+		    throw "invalid type";
+	    }
+	    if (!params?.map.length) {
+		method = 'DELETE';
+		params = undefined;
+	    }
+	    Proxmox.Utils.API2Request({
+		url,
+		method,
+		params,
+		success: function() {
+		    me.load();
+		},
+	    });
+	},
+
+	load: function() {
+	    let me = this;
+	    let view = me.getView();
+	    Proxmox.Utils.API2Request({
+		url: view.baseUrl,
+		method: 'GET',
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+		success: function({ result: { data } }) {
+		    let lookup = {};
+		    data.forEach((entry) => {
+			lookup[entry.id] = Ext.apply({}, entry);
+			entry.iconCls = 'fa fa-fw fa-folder-o';
+			entry.name = entry.id;
+			entry.text = entry.id;
+			entry.type = 'entry';
+
+			let nodes = {};
+			for (const map of entry.map) {
+			    let parsed = PVE.Parser.parsePropertyString(map);
+			    parsed.iconCls = view.mapIconCls;
+			    parsed.leaf = true;
+			    parsed.name = entry.id;
+			    parsed.text = parsed[view.entryIdProperty];
+			    parsed.type = 'map';
+
+			    if (nodes[parsed.node] === undefined) {
+				nodes[parsed.node] = {
+				    children: [],
+				    expanded: true,
+				    iconCls: 'fa fa-fw fa-building-o',
+				    leaf: false,
+				    name: entry.id,
+				    node: parsed.node,
+				    text: parsed.node,
+				    type: 'node',
+				};
+			    }
+			    nodes[parsed.node].children.push(parsed);
+			}
+			delete entry.id;
+			entry.children = Object.values(nodes);
+			entry.leaf = entry.children.length === 0;
+		    });
+		    me.lookup = lookup;
+		    if (view.getStatusCheckUrl !== undefined && view.checkValidity !== undefined) {
+			me.loadStatusData();
+		    }
+		    view.setRootNode({
+			children: data,
+		    });
+		    let root = view.getRootNode();
+		    root.expand();
+		    root.childNodes.forEach(node => node.expand());
+		},
+	    });
+	},
+
+	nodeLoadingState: {},
+
+	loadStatusData: function() {
+	    let me = this;
+	    let view = me.getView();
+	    PVE.data.ResourceStore.getNodes().forEach(({ node }) => {
+		me.nodeLoadingState[node] = true;
+		let url = view.getStatusCheckUrl(node);
+		Proxmox.Utils.API2Request({
+		    url,
+		    method: 'GET',
+		    failure: function(response) {
+			me.nodeLoadingState[node] = false;
+			view.getRootNode()?.cascade(function(rec) {
+			    if (rec.data.node !== node) {
+				return;
+			    }
+
+			    rec.set('valid', 0);
+			    rec.set('errmsg', response.htmlStatus);
+			    rec.commit();
+			});
+		    },
+		    success: function({ result: { data } }) {
+			me.nodeLoadingState[node] = false;
+			view.checkValidity(data, node);
+		    },
+		});
+	    });
+	},
+
+	renderStatus: function(value, _metadata, record) {
+	    let me = this;
+	    if (record.data.type !== 'map') {
+		return '';
+	    }
+	    let iconCls;
+	    let status;
+	    if (value === undefined) {
+		if (me.nodeLoadingState[record.data.node]) {
+		    iconCls = 'fa-spinner fa-spin';
+		    status = gettext('Loading...');
+		} else {
+		    iconCls = 'fa-question-circle';
+		    status = gettext('Unknown Node');
+		}
+	    } else {
+		let state = value ? 'good' : 'critical';
+		iconCls = PVE.Utils.get_health_icon(state, true);
+		status = value ? gettext("OK") : record.data.errmsg || Proxmox.Utils.unknownText;
+	    }
+	    return `<i class="fa ${iconCls}"></i> ${status}`;
+	},
+
+	init: function(view) {
+	    let me = this;
+
+	    ['editWindowClass', 'baseUrl', 'mapIconCls', 'entryIdProperty'].forEach((property) => {
+		if (view[property] === undefined) {
+		    throw `No ${property} defined`;
+		}
+	    });
+
+	    me.load();
+	},
+    },
+
+    store: {
+	sorters: 'text',
+	data: {},
+    },
+
+
+    tbar: [
+	{
+	    text: gettext('Add mapping'),
+	    handler: 'addMapping',
+	    cbind: {
+		disabled: '{!canConfigure}',
+	    },
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('New Host mapping'),
+	    disabled: true,
+	    parentXType: 'treepanel',
+	    enableFn: function(_rec) {
+		return this.up('treepanel').canConfigure;
+	    },
+	    handler: 'addHost',
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Edit'),
+	    disabled: true,
+	    parentXType: 'treepanel',
+	    enableFn: function(rec) {
+		return rec && rec.data.type !== 'entry' && this.up('treepanel').canConfigure;
+	    },
+	    handler: 'edit',
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    parentXType: 'treepanel',
+	    handler: 'remove',
+	    disabled: true,
+	    text: gettext('Remove'),
+	    enableFn: function(rec) {
+		return rec && this.up('treepanel').canConfigure;
+	    },
+	    confirmMsg: function(rec) {
+		let msg, id;
+		let view = this.up('treepanel');
+		switch (rec.data.type) {
+		    case 'entry':
+			msg = gettext("Are you sure you want to remove '{0}'");
+			return Ext.String.format(msg, rec.data.name);
+		    case 'node':
+			msg = gettext("Are you sure you want to remove '{0}' entries for '{1}'");
+			return Ext.String.format(msg, rec.data.node, rec.data.name);
+		    case 'map':
+			msg = gettext("Are you sure you want to remove '{0}' on '{1}' for '{2}'");
+			id = rec.data[view.entryIdProperty];
+			return Ext.String.format(msg, id, rec.data.node, rec.data.name);
+		    default:
+			throw "invalid type";
+		}
+	    },
+	},
+    ],
+
+    listeners: {
+	itemdblclick: 'edit',
+    },
+});
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 13/15] ui: allow configuring pci and usb mapping
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (17 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 12/15] ui: add ResourceMapTree Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 14/15] ui: window/Migrate: allow mapped devices Dominik Csapak
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

uses the new ResourceMapTree to add the CRUD interfaces for the
mappings.

We add both of them into a single panel, since the datacenter menu
already has many entries, and without a proper summary for the group, we
cannot really put them in a category

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v5:
* add onlineHelp
 www/manager6/Makefile         |   2 +
 www/manager6/StateProvider.js |   1 +
 www/manager6/dc/Config.js     |  46 ++++++++++++++-
 www/manager6/dc/PCIMapView.js | 106 ++++++++++++++++++++++++++++++++++
 www/manager6/dc/USBMapView.js |  98 +++++++++++++++++++++++++++++++
 5 files changed, 251 insertions(+), 2 deletions(-)
 create mode 100644 www/manager6/dc/PCIMapView.js
 create mode 100644 www/manager6/dc/USBMapView.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 0cb922d6..2d884f4a 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -174,6 +174,8 @@ JSSRC= 							\
 	dc/UserTagAccessEdit.js				\
 	dc/RegisteredTagsEdit.js			\
 	dc/RealmSyncJob.js				\
+	dc/PCIMapView.js				\
+	dc/USBMapView.js				\
 	lxc/CmdMenu.js					\
 	lxc/Config.js					\
 	lxc/CreateWizard.js				\
diff --git a/www/manager6/StateProvider.js b/www/manager6/StateProvider.js
index fafbb112..d62d33b2 100644
--- a/www/manager6/StateProvider.js
+++ b/www/manager6/StateProvider.js
@@ -47,6 +47,7 @@ Ext.define('PVE.StateProvider', {
     hprefix: 'v1',
 
     compDict: {
+	'resource-map': 55,
         tfa: 54,
 	sdn: 53,
 	cloudinit: 52,
diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index 72a9bec1..a3c6e06a 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -262,8 +262,50 @@ Ext.define('PVE.dc.Config', {
 		iconCls: 'fa fa-bar-chart',
 		itemId: 'metricservers',
 		onlineHelp: 'external_metric_server',
-	    },
-	    {
+	    });
+	}
+
+	if (caps.mapping['Mapping.Audit'] ||
+	    caps.mapping['Mapping.Use'] ||
+	    caps.mapping['Mapping.Modify']) {
+	    me.items.push(
+		{
+		    xtype: 'container',
+		    onlineHelp: 'resource_mapping',
+		    title: gettext('Resource Mappings'),
+		    itemId: 'resources',
+		    iconCls: 'fa fa-folder-o',
+		    layout: {
+			type: 'vbox',
+			align: 'stretch',
+			multi: true,
+		    },
+		    scrollable: true,
+		    defaults: {
+			collapsible: true,
+			animCollapse: false,
+			margin: '7 10 3 10',
+		    },
+		    items: [
+			{
+			    collapsible: true,
+			    xtype: 'pveDcPCIMapView',
+			    title: gettext('PCI Devices'),
+			    flex: 1,
+			},
+			{
+			    collapsible: true,
+			    xtype: 'pveDcUSBMapView',
+			    title: gettext('USB Devices'),
+			    flex: 1,
+			},
+		    ],
+		},
+	    );
+	}
+
+	if (caps.dc['Sys.Audit']) {
+	    me.items.push({
 		xtype: 'pveDcSupport',
 		title: gettext('Support'),
 		itemId: 'support',
diff --git a/www/manager6/dc/PCIMapView.js b/www/manager6/dc/PCIMapView.js
new file mode 100644
index 00000000..3efa19d8
--- /dev/null
+++ b/www/manager6/dc/PCIMapView.js
@@ -0,0 +1,106 @@
+Ext.define('pve-resource-pci-tree', {
+    extend: 'Ext.data.Model',
+    idProperty: 'internalId',
+    fields: ['type', 'text', 'path', 'id', 'subsystem-id', 'iommugroup', 'description', 'digest'],
+});
+
+Ext.define('PVE.dc.PCIMapView', {
+    extend: 'PVE.tree.ResourceMapTree',
+    alias: 'widget.pveDcPCIMapView',
+
+    editWindowClass: 'PVE.window.PCIMapEditWindow',
+    baseUrl: '/cluster/mapping/pci',
+    mapIconCls: 'pve-itype-icon-pci',
+    getStatusCheckUrl: (node) => `/nodes/${node}/hardware/pci?pci-class-blacklist=`,
+    entryIdProperty: 'path',
+
+    checkValidity: function(data, node) {
+	let me = this;
+	let ids = {};
+	data.forEach((entry) => {
+	    ids[entry.id] = entry;
+	});
+	me.getRootNode()?.cascade(function(rec) {
+	    if (rec.data.node !== node || rec.data.type !== 'map') {
+		return;
+	    }
+
+	    let id = rec.data.path;
+	    if (!id.match(/\.\d$/)) {
+		id += '.0';
+	    }
+	    let device = ids[id];
+	    if (!device) {
+		rec.set('valid', 0);
+		rec.set('errmsg', Ext.String.format(gettext("Cannot find PCI id {0}"), id));
+		rec.commit();
+		return;
+	    }
+
+
+	    let deviceId = `${device.vendor}:${device.device}`.replace(/0x/g, '');
+	    let subId = `${device.subsystem_vendor}:${device.subsystem_device}`.replace(/0x/g, '');
+
+	    let toCheck = {
+		id: deviceId,
+		'subsystem-id': subId,
+		iommugroup: device.iommugroup !== -1 ? device.iommugroup : undefined,
+	    };
+
+	    let valid = 1;
+	    let errors = [];
+	    let errText = gettext("Configuration for {0} not correct ('{1}' != '{2}')");
+	    for (const [key, validValue] of Object.entries(toCheck)) {
+		if (`${rec.data[key]}` !== `${validValue}`) {
+		    errors.push(Ext.String.format(errText, key, rec.data[key] ?? '', validValue));
+		    valid = 0;
+		}
+	    }
+
+	    rec.set('valid', valid);
+	    rec.set('errmsg', errors.join('<br>'));
+	    rec.commit();
+	});
+    },
+
+    store: {
+	sorters: 'text',
+	model: 'pve-resource-pci-tree',
+	data: {},
+    },
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    text: gettext('ID/Node/Path'),
+	    dataIndex: 'text',
+	    width: 200,
+	},
+	{
+	    text: gettext('Vendor/Device'),
+	    dataIndex: 'id',
+	},
+	{
+	    text: gettext('Subsystem Vendor/Device'),
+	    dataIndex: 'subsystem-id',
+	},
+	{
+	    text: gettext('IOMMU group'),
+	    dataIndex: 'iommugroup',
+	},
+	{
+	    header: gettext('Status'),
+	    dataIndex: 'valid',
+	    flex: 1,
+	    renderer: 'renderStatus',
+	},
+	{
+	    header: gettext('Comment'),
+	    dataIndex: 'description',
+	    renderer: function(value, _meta, record) {
+		return value ?? record.data.comment;
+	    },
+	    flex: 1,
+	},
+    ],
+});
diff --git a/www/manager6/dc/USBMapView.js b/www/manager6/dc/USBMapView.js
new file mode 100644
index 00000000..953e2425
--- /dev/null
+++ b/www/manager6/dc/USBMapView.js
@@ -0,0 +1,98 @@
+Ext.define('pve-resource-usb-tree', {
+    extend: 'Ext.data.Model',
+    idProperty: 'internalId',
+    fields: ['type', 'text', 'path', 'id', 'description', 'digest'],
+});
+
+Ext.define('PVE.dc.USBMapView', {
+    extend: 'PVE.tree.ResourceMapTree',
+    alias: 'widget.pveDcUSBMapView',
+
+    editWindowClass: 'PVE.window.USBMapEditWindow',
+    baseUrl: '/cluster/mapping/usb',
+    mapIconCls: 'fa fa-usb',
+    getStatusCheckUrl: (node) => `/nodes/${node}/hardware/usb`,
+    entryIdProperty: 'id',
+
+    checkValidity: function(data, node) {
+	let me = this;
+	let ids = {};
+	let paths = {};
+	data.forEach((entry) => {
+	    ids[`${entry.vendid}:${entry.prodid}`] = entry;
+	    paths[`${entry.busnum}-${entry.usbpath}`] = entry;
+	});
+	me.getRootNode()?.cascade(function(rec) {
+	    if (rec.data.node !== node || rec.data.type !== 'map') {
+		return;
+	    }
+
+	    let device;
+	    if (rec.data.path) {
+		device = paths[rec.data.path];
+	    }
+	    device ??= ids[rec.data.id];
+
+	    if (!device) {
+		rec.set('valid', 0);
+		rec.set('errmsg', Ext.String.format(gettext("Cannot find USB device {0}"), rec.data.id));
+		rec.commit();
+		return;
+	    }
+
+
+	    let deviceId = `${device.vendid}:${device.prodid}`.replace(/0x/g, '');
+
+	    let toCheck = {
+		id: deviceId,
+	    };
+
+	    let valid = 1;
+	    let errors = [];
+	    let errText = gettext("Configuration for {0} not correct ('{1}' != '{2}')");
+	    for (const [key, validValue] of Object.entries(toCheck)) {
+		if (rec.data[key] !== validValue) {
+		    errors.push(Ext.String.format(errText, key, rec.data[key] ?? '', validValue));
+		    valid = 0;
+		}
+	    }
+
+	    rec.set('valid', valid);
+	    rec.set('errmsg', errors.join('<br>'));
+	    rec.commit();
+	});
+    },
+
+    store: {
+	sorters: 'text',
+	model: 'pve-resource-usb-tree',
+	data: {},
+    },
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    text: gettext('ID/Node/Vendor&Device'),
+	    dataIndex: 'text',
+	    width: 200,
+	},
+	{
+	    text: gettext('Path'),
+	    dataIndex: 'path',
+	},
+	{
+	    header: gettext('Status'),
+	    dataIndex: 'valid',
+	    flex: 1,
+	    renderer: 'renderStatus',
+	},
+	{
+	    header: gettext('Comment'),
+	    dataIndex: 'description',
+	    renderer: function(value, _meta, record) {
+		return value ?? record.data.comment;
+	    },
+	    flex: 1,
+	},
+    ],
+});
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 14/15] ui: window/Migrate: allow mapped devices
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (18 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 13/15] ui: allow configuring pci and usb mapping Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 15/15] ui: improve permission handling for hardware Dominik Csapak
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

if the migration is an offline migration and when the mapping on
the target node exists, otherwise not

this does not change the behaviour for 'raw' devices in the config
those can still be forced to be migrated, like before

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/Migrate.js | 52 +++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index 1c23abb3..c310342d 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -219,36 +219,68 @@ Ext.define('PVE.window.Migrate', {
 		let target = me.lookup('pveNodeSelector').value;
 		if (target.length && !migrateStats.allowed_nodes.includes(target)) {
 		    let disallowed = migrateStats.not_allowed_nodes[target];
-		    let missingStorages = disallowed.unavailable_storages.join(', ');
+		    if (disallowed.unavailable_storages !== undefined) {
+			let missingStorages = disallowed.unavailable_storages.join(', ');
 
-		    migration.possible = false;
-		    migration.preconditions.push({
-			text: 'Storage (' + missingStorages + ') not available on selected target. ' +
-			  'Start VM to use live storage migration or select other target node',
-			severity: 'error',
-		    });
+			migration.possible = false;
+			migration.preconditions.push({
+			    text: 'Storage (' + missingStorages + ') not available on selected target. ' +
+			      'Start VM to use live storage migration or select other target node',
+			    severity: 'error',
+			});
+		    }
+
+		    if (disallowed['unavailable-resources'] !== undefined) {
+			let unavailableResources = disallowed['unavailable-resources'].join(', ');
+
+			migration.possible = false;
+			migration.preconditions.push({
+			    text: 'Mapped Resources (' + unavailableResources + ') not available on selected target. ',
+			    severity: 'error',
+			});
+		    }
 		}
 	    }
 
-	    if (migrateStats.local_resources.length) {
+	    let blockingResources = [];
+	    let mappedResources = migrateStats['mapped-resources'] ?? [];
+
+	    for (const res of migrateStats.local_resources) {
+		if (mappedResources.indexOf(res) === -1) {
+		    blockingResources.push(res);
+		}
+	    }
+
+	    if (blockingResources.length) {
 		migration.hasLocalResources = true;
 		if (!migration.overwriteLocalResourceCheck || vm.get('running')) {
 		    migration.possible = false;
 		    migration.preconditions.push({
 			text: Ext.String.format('Can\'t migrate VM with local resources: {0}',
-			migrateStats.local_resources.join(', ')),
+			blockingResources.join(', ')),
 			severity: 'error',
 		    });
 		} else {
 		    migration.preconditions.push({
 			text: Ext.String.format('Migrate VM with local resources: {0}. ' +
 			'This might fail if resources aren\'t available on the target node.',
-			migrateStats.local_resources.join(', ')),
+			blockingResources.join(', ')),
 			severity: 'warning',
 		    });
 		}
 	    }
 
+	    if (mappedResources && mappedResources.length) {
+		if (vm.get('running')) {
+		    migration.possible = false;
+		    migration.preconditions.push({
+			text: Ext.String.format('Can\'t migrate running VM with mapped resources: {0}',
+			mappedResources.join(', ')),
+			severity: 'error',
+		    });
+		}
+	    }
+
 	    if (migrateStats.local_disks.length) {
 		migrateStats.local_disks.forEach(function(disk) {
 		    if (disk.cdrom && disk.cdrom === 1) {
-- 
2.30.2





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

* [pve-devel] [PATCH manager v6 15/15] ui: improve permission handling for hardware
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (19 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 14/15] ui: window/Migrate: allow mapped devices Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14  8:46 ` [pve-devel] [PATCH docs v6 1/1] qemu: add documentation about cluster device mapping Dominik Csapak
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

qemu/HardwareView:

with the new Hardware privileges, we want to adapt a few places where
we now allow to show the add/edit window with those permissions.

form/{PCI,USB}Selector:

increase the minHeight property of the PCI/USBSelector, so that
the user can see the error message if he has not enough permissions.

data/PermPathStore:

add '/hardware' to the list of acl paths

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/data/PermPathStore.js |  1 +
 www/manager6/form/PCISelector.js   |  1 +
 www/manager6/form/USBSelector.js   |  1 +
 www/manager6/qemu/HardwareView.js  | 17 +++++++++--------
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/www/manager6/data/PermPathStore.js b/www/manager6/data/PermPathStore.js
index cf702c03..c3ac7f0e 100644
--- a/www/manager6/data/PermPathStore.js
+++ b/www/manager6/data/PermPathStore.js
@@ -8,6 +8,7 @@ Ext.define('PVE.data.PermPathStore', {
 	{ 'value': '/access' },
 	{ 'value': '/access/groups' },
 	{ 'value': '/access/realm' },
+	{ 'value': '/mapping' },
 	{ 'value': '/nodes' },
 	{ 'value': '/pool' },
 	{ 'value': '/sdn/zones' },
diff --git a/www/manager6/form/PCISelector.js b/www/manager6/form/PCISelector.js
index 39e111f0..a6e697a4 100644
--- a/www/manager6/form/PCISelector.js
+++ b/www/manager6/form/PCISelector.js
@@ -37,6 +37,7 @@ Ext.define('PVE.form.PCISelector', {
     onLoadCallBack: undefined,
 
     listConfig: {
+	minHeight: 80,
 	width: 800,
 	columns: [
 	    {
diff --git a/www/manager6/form/USBSelector.js b/www/manager6/form/USBSelector.js
index 011778d7..975b2646 100644
--- a/www/manager6/form/USBSelector.js
+++ b/www/manager6/form/USBSelector.js
@@ -71,6 +71,7 @@ Ext.define('PVE.form.USBSelector', {
 	    store: store,
 	    emptyText: emptyText,
 	    listConfig: {
+		minHeight: 80,
 		width: 520,
 		columns: [
 		    {
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index af35a980..5b33b1e2 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -259,8 +259,8 @@ Ext.define('PVE.qemu.HardwareView', {
 		group: 25,
 		order: i,
 		iconCls: 'usb',
-		editor: caps.nodes['Sys.Console'] ? 'PVE.qemu.USBEdit' : undefined,
-		never_delete: !caps.nodes['Sys.Console'],
+		editor: caps.nodes['Sys.Console'] || caps.mapping['Mapping.Use'] ? 'PVE.qemu.USBEdit' : undefined,
+		never_delete: !caps.nodes['Sys.Console'] && !caps.mapping['Mapping.Use'],
 		header: gettext('USB Device') + ' (' + confid + ')',
 	    };
 	}
@@ -270,8 +270,8 @@ Ext.define('PVE.qemu.HardwareView', {
 		group: 30,
 		order: i,
 		tdCls: 'pve-itype-icon-pci',
-		never_delete: !caps.nodes['Sys.Console'],
-		editor: caps.nodes['Sys.Console'] ? 'PVE.qemu.PCIEdit' : undefined,
+		never_delete: !caps.nodes['Sys.Console'] && !caps.mapping['Mapping.Use'],
+		editor: caps.nodes['Sys.Console'] || caps.mapping['Mapping.Use'] ? 'PVE.qemu.PCIEdit' : undefined,
 		header: gettext('PCI Device') + ' (' + confid + ')',
 	    };
 	}
@@ -577,14 +577,15 @@ Ext.define('PVE.qemu.HardwareView', {
 
 	    // heuristic only for disabling some stuff, the backend has the final word.
 	    const noSysConsolePerm = !caps.nodes['Sys.Console'];
+	    const noHWPerm = !caps.nodes['Sys.Console'] && !caps.mapping['Mapping.Use'];
 	    const noVMConfigHWTypePerm = !caps.vms['VM.Config.HWType'];
 	    const noVMConfigNetPerm = !caps.vms['VM.Config.Network'];
 	    const noVMConfigDiskPerm = !caps.vms['VM.Config.Disk'];
 	    const noVMConfigCDROMPerm = !caps.vms['VM.Config.CDROM'];
 	    const noVMConfigCloudinitPerm = !caps.vms['VM.Config.Cloudinit'];
 
-	    me.down('#addUsb').setDisabled(noSysConsolePerm || isAtUsbLimit());
-	    me.down('#addPci').setDisabled(noSysConsolePerm || isAtLimit('hostpci'));
+	    me.down('#addUsb').setDisabled(noHWPerm || isAtUsbLimit());
+	    me.down('#addPci').setDisabled(noHWPerm || isAtLimit('hostpci'));
 	    me.down('#addAudio').setDisabled(noVMConfigHWTypePerm || isAtLimit('audio'));
 	    me.down('#addSerial').setDisabled(noVMConfigHWTypePerm || isAtLimit('serial'));
 	    me.down('#addNet').setDisabled(noVMConfigNetPerm || isAtLimit('net'));
@@ -697,14 +698,14 @@ Ext.define('PVE.qemu.HardwareView', {
 				text: gettext('USB Device'),
 				itemId: 'addUsb',
 				iconCls: 'fa fa-fw fa-usb black',
-				disabled: !caps.nodes['Sys.Console'],
+				disabled: !caps.nodes['Sys.Console'] && !caps.mapping['Mapping.Use'],
 				handler: editorFactory('USBEdit'),
 			    },
 			    {
 				text: gettext('PCI Device'),
 				itemId: 'addPci',
 				iconCls: 'pve-itype-icon-pci',
-				disabled: !caps.nodes['Sys.Console'],
+				disabled: !caps.nodes['Sys.Console'] && !caps.mapping['Mapping.Use'],
 				handler: editorFactory('PCIEdit'),
 			    },
 			    {
-- 
2.30.2





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

* [pve-devel] [PATCH docs v6 1/1] qemu: add documentation about cluster device mapping
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (20 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 15/15] ui: improve permission handling for hardware Dominik Csapak
@ 2023-06-14  8:46 ` Dominik Csapak
  2023-06-14 12:01 ` [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Markus Frank
  2023-06-16  7:51 ` Fabian Grünbichler
  23 siblings, 0 replies; 29+ messages in thread
From: Dominik Csapak @ 2023-06-14  8:46 UTC (permalink / raw)
  To: pve-devel

explain why someone would want it, how to configure and which privileges
are necessary

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v6
 qm-pci-passthrough.adoc |  8 ++++
 qm.adoc                 | 86 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)

diff --git a/qm-pci-passthrough.adoc b/qm-pci-passthrough.adoc
index df6cf21..b90a0b9 100644
--- a/qm-pci-passthrough.adoc
+++ b/qm-pci-passthrough.adoc
@@ -400,6 +400,14 @@ Example configuration with an `Intel GVT-g vGPU` (`Intel Skylake 6700k`):
 With this set, {pve} automatically creates such a device on VM start, and
 cleans it up again when the VM stops.
 
+Use in Clusters
+~~~~~~~~~~~~~~~
+
+It is also possible to map devices on a cluster level, so that they can be
+properly used with HA and hardware changes are detected and non root users
+can configure them. See xref:resource_mapping[Resource Mapping]
+for details on that.
+
 ifdef::wiki[]
 
 See Also
diff --git a/qm.adoc b/qm.adoc
index c6dc652..53f6450 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -753,6 +753,10 @@ if you use a SPICE client which supports it. If you add a SPICE USB port
 to your VM, you can passthrough a USB device from where your SPICE client is,
 directly to the VM (for example an input device or hardware dongle).
 
+It is also possible to map devices on a cluster level, so that they can be
+properly used with HA and hardware changes are detected and non root users
+can configure them. See xref:resource_mapping[Resource Mapping]
+for details on that.
 
 [[qm_bios_and_uefi]]
 BIOS and UEFI
@@ -1511,6 +1515,88 @@ chosen, the first of:
 3. The first non-shared storage from any VM disk.
 4. The storage `local` as a fallback.
 
+[[resource_mapping]]
+Resource Mapping
+~~~~~~~~~~~~~~~~
+
+When using or referencing local resources (e.g. address of a pci device), using
+the raw address or id is sometimes problematic, for example:
+
+* when using HA, a different device with the same id or path may exist on the
+  target node, and if one is not careful when assigning such guests to HA
+  groups, the wrong device could be used, breaking configurations.
+
+* changing hardware can change ids and paths, so one would have to check all
+  assigned devices and see if the path or id is still correct.
+
+To handle this better, one can define cluster wide resource mappings, such that
+a resource has a cluster unique, user selected identifier which can correspond
+to different devices on different hosts. With this, HA won't start a guest with
+a wrong device, and hardware changes can be detected.
+
+Creating such a mapping can be done with the {pve} web GUI under `Datacenter`
+in the relevant tab in the `Resource Mappings` category, or on the cli with
+
+----
+# pvesh create /cluster/mapping/TYPE OPTIONS
+----
+
+Where `TYPE` is the hardware type (currently either `pci` or `usb`) and
+`OPTIONS` are the device mappings and other configuration parameters.
+
+Note that the options must include a map property with all identifying
+properties of that hardware, so that it's possible to verify the hardware did
+not change and the correct device is passed through.
+
+For example to add a PCI device as `device1` with the path `0000:01:00.0` that
+has the device id `0001` and the vendor id `0002` on the node `node1`, and
+`0000:02:00.0` on `node2` you can add it with:
+
+----
+# pvesh create /cluster/mapping/pci --id device1 \
+ --map node=node1,path=0000:01:00.0,id=0002:0001 \
+ --map node=node2,path=0000:02:00.0,id=0002:0001
+----
+
+You must repeat the `map` parameter for each node where that device should have
+a mapping.
+
+Using the GUI makes this much easier, as the correct properties are
+automatically picked up and sent to the API.
+
+It's also possible for PCI devices to provide multiple devices per node with
+multiple map properties for the nodes. If such a device is assigned to a guest,
+the first free one will be used when the guest is started. The order of the
+paths given is also the order in which they are tried, so arbitrary allocation
+policies can be implemented.
+
+This is useful for devices with SR-IOV, since some times it is not important
+which exact virtual function is passed through.
+
+You can assign such a device to a guest either with the GUI or with
+
+----
+# qm set ID -hostpci0 NAME
+----
+
+for PCI devices, or
+
+----
+# qm set ID -usb0 NAME
+----
+
+for USB devices.
+
+Where `ID` is the guests id and `NAME` is the chosen name for the created
+mapping. All usual options for passing through the devices are allowed, such as
+`mdev`.
+
+To create mappings `Mapping.Modify` on `/mapping/TYPE/NAME` is necessary
+(where `TYPE` is the device type and `NAME` is the name of the mapping).
+
+To use these mappings, `Mapping.Use` on `/mapping/TYPE/NAME` is necessary (in
+addition to the normal guest privileges to edit the configuration).
+
 Managing Virtual Machines with `qm`
 ------------------------------------
 
-- 
2.30.2





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

* Re: [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (21 preceding siblings ...)
  2023-06-14  8:46 ` [pve-devel] [PATCH docs v6 1/1] qemu: add documentation about cluster device mapping Dominik Csapak
@ 2023-06-14 12:01 ` Markus Frank
  2023-06-16  7:51 ` Fabian Grünbichler
  23 siblings, 0 replies; 29+ messages in thread
From: Markus Frank @ 2023-06-14 12:01 UTC (permalink / raw)
  To: Dominik Csapak, pve-devel

Tested with my viommu patches installed on my host so that I can use iommu inside
a VM and pass though virtual devices to Level-2 VMs.

I created a pve8 vm and a pve8 2-node vm cluster.

I used the Web UI for this testing but made a quick test with pvesh on the
API (/cluster/mapping/pci).

PCI:
* Passed through single PCI devices and groups (Pass through all functions as
one device). works fine

USB/PCI:
* made a mapping for 2 nodes and started a vm using that mapping on each node.
works fine

provoke errors:
* "PCI device map for 'mapping1' not found" is displayed when starting a VM
if a map was added that has no configuration for this node.
Also displays a warning when adding a map that has no configuration.
* could only migrate if PCI/USB Mapping is preset on the target node
* logically could not live migrate with PCI/USB Devices assigned.

Error handling also works fine

There is only one thing I noticed:
* When assigning multiple devices to a node, not all of them are passed through,
just one. There is a hint field (multiple_warning) which is not visible
right now.

Tested-by: Markus Frank <m.frank@proxmox.com>

On 6/14/23 10:46, Dominik Csapak wrote:
> this series is the remaining part to add a cluster-wide device mapping for pci and usb
> devices. so that an admin can configure a device to be availble for
> migration and configuring for uses that are non-root
> (the existing pattern can be copied easily for other types, e.g.
> markus upcoming folder sharing)




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

* Re: [pve-devel] [PATCH qemu-server v6 2/6] enable cluster mapped PCI devices for guests
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 2/6] enable cluster mapped PCI " Dominik Csapak
@ 2023-06-16  7:49   ` Fabian Grünbichler
  0 siblings, 0 replies; 29+ messages in thread
From: Fabian Grünbichler @ 2023-06-16  7:49 UTC (permalink / raw)
  To: Proxmox VE development discussion

On June 14, 2023 10:46 am, Dominik Csapak wrote:
> this patch allows configuring pci devices that are mapped via cluster
> resource mapping when the user has 'Resource.Use' on the ACL path
> '/mapping/pci/{ID}' (in  addition to the usual required vm config
> privileges)
> 
> When given multiple mappings in the config, we use them as alternatives
> for the passthrough, and will select the first free one on startup.
> It is using our regular pci reservation mechanism for regular devices and
> we introduce a selection mechanism for mediated devices.
> 
> A few changes to the inner workings were required to make this work well:
> * parse_hostpci now returns a different structure where we have a list
>   of lists (first level is for the different alternatives and second
>   level is for the different devices that should be passed through
>   together)
> * factor out the 'parse_hostpci_devices' which parses each device from
>   the config and does some precondition checks
> * reserve_pci_usage now behaves slightly different when trying to
>   reserve an device with the same VMID that's already reserved for,
>   since for checking which alternative we can use, we already must
>   reserve one (this means that qm showcmd can actually reserve devices,
>   albeit only for up to 10 seconds)
> * configuring a mediated device on a multifunction device is not
>   supported anymore, and results in failure to start (previously, it
>   just chose the first device to do it). This is a breaking change
> * configuring a single pci device twice on different hostpci slots now
>   fails during commandline generation instead on qemu start, so we had
>   to adapt one test where this occurred (it could never have worked
>   anyway)
> 
> Add the permission checks also to clone/restore. This fails now for non
> root users for raw devices, this is a breaking change!
> 
> Fixes #3574: Improve SR-IOV usability
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v5:
> * move permission check for clone/restore to QemuServer.pm
>  PVE/API2/Qemu.pm                        |  54 +++++-
>  PVE/QemuServer.pm                       |  71 ++++---
>  PVE/QemuServer/PCI.pm                   | 243 ++++++++++++++++++++----
>  test/cfg2cmd/q35-linux-hostpci.conf     |   2 +-
>  test/cfg2cmd/q35-linux-hostpci.conf.cmd |   2 +-
>  test/run_config2command_tests.pl        |   1 +
>  6 files changed, 299 insertions(+), 74 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 865edf7f..4c3c53d9 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -32,6 +32,7 @@ use PVE::QemuServer::Drive;
>  use PVE::QemuServer::ImportDisk;
>  use PVE::QemuServer::Monitor qw(mon_cmd);
>  use PVE::QemuServer::Machine;
> +use PVE::QemuServer::PCI;
>  use PVE::QemuServer::USB qw(parse_usb_device);
>  use PVE::QemuMigrate;
>  use PVE::RPCEnvironment;
> @@ -607,6 +608,26 @@ my $check_vm_create_usb_perm = sub {
>      return 1;
>  };
>  
> +my $check_vm_create_hostpci_perm = sub {
> +    my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
> +
> +    return 1 if $authuser eq 'root@pam';
> +
> +    foreach my $opt (keys %{$param}) {
> +	next if $opt !~ m/^hostpci\d+$/;
> +
> +	my $device = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $param->{$opt});
> +	if ($device->{host} !~ m/:/) {
> +	    $rpcenv->check_full($authuser, "/mapping/pci/$device->{host}", ['Mapping.Use']);
> +	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> +	} else {
> +	    die "only root can set '$opt' config for non-mapped devices\n";
> +	}
> +    }
> +
> +    return 1;
> +};
> +
>  my $check_vm_modify_config_perm = sub {
>      my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
>  
> @@ -617,7 +638,7 @@ my $check_vm_modify_config_perm = sub {
>  	# else, as there the permission can be value dependend
>  	next if PVE::QemuServer::is_valid_drivename($opt);
>  	next if $opt eq 'cdrom';
> -	next if $opt =~ m/^(?:unused|serial|usb)\d+$/;
> +	next if $opt =~ m/^(?:unused|serial|usb|hostpci)\d+$/;
>  	next if $opt eq 'tags';
>  
>  
> @@ -646,7 +667,7 @@ my $check_vm_modify_config_perm = sub {
>  	    # also needs privileges on the storage, that will be checked later
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk', 'VM.PowerMgmt' ]);
>  	} else {
> -	    # catches hostpci\d+, args, lock, etc.
> +	    # catches args, lock, etc.
>  	    # new options will be checked here
>  	    die "only root can set '$opt' config\n";
>  	}
> @@ -885,6 +906,8 @@ __PACKAGE__->register_method({
>  
>  	    &$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, $param);
>  	    &$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, $param);
> +	    &$check_vm_create_hostpci_perm($rpcenv, $authuser, $vmid, $pool, $param);
> +
>  	    PVE::QemuServer::check_bridge_access($rpcenv, $authuser, $param);
>  	    &$check_cpu_model_access($rpcenv, $authuser, $param);
>  
> @@ -1737,6 +1760,16 @@ my $update_vm_api  = sub {
>  		    }
>  		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
> +		} elsif ($opt =~ m/^hostpci\d+$/) {
> +		    my $olddevice = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $val);
> +		    if ($olddevice->{host} !~ m/:/) {
> +			$rpcenv->check_full($authuser, "/mapping/pci/$olddevice->{host}", ['Mapping.Use']);
> +			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    } elsif ($authuser ne 'root@pam') {
> +			die "only root can set '$opt' config for non-mapped devices\n";
> +		    }

this part

> +		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
> +		    PVE::QemuConfig->write_config($vmid, $conf);
>  		} elsif ($opt eq 'tags') {
>  		    assert_tag_permissions($vmid, $val, '', $rpcenv, $authuser);
>  		    delete $conf->{$opt};
> @@ -1823,6 +1856,23 @@ my $update_vm_api  = sub {
>  		    } elsif ($authuser ne 'root@pam') {
>  			die "only root can modify '$opt' config for real devices\n";
>  		    }
> +
> +		    $conf->{pending}->{$opt} = $param->{$opt};
> +		} elsif ($opt =~ m/^hostpci\d+$/) {
> +		    my $olddevice;
> +		    if (defined($conf->{$opt})) {
> +			$olddevice = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $conf->{$opt});
> +		    }
> +		    my $newdevice = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $param->{$opt});
> +		    if ((!defined($olddevice) || $olddevice->{host} !~ m/:/) && $newdevice->{host} !~ m/:/) {
> +			if (defined($olddevice)) {
> +			    $rpcenv->check_full($authuser, "/mapping/pci/$olddevice->{host}", ['Mapping.Use']);
> +			}
> +			$rpcenv->check_full($authuser, "/mapping/pci/$newdevice->{host}", ['Mapping.Use']);
> +			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);

and this part could be the same, with the latter being two calls to the
same helper, which would be easier to read..

it checks:
  VM.Config.HWType
  access to the old value if there is one (root if raw, mapping if mapping)
  access to the new value (root if raw, mapping if mapping)

doing

0. (optional defined guard for existing value)
1. parse old
2. call check helper
3. parse new
4. call check helper

with the helper doing

if raw require root else check mapping && VM.Config.HWType

is way more readable and should have the same result, and only requires
one place to change should we change the format/checks in the future.

keep in mind that ACL results are cached, so doing the exact same check
twice in a row means the second one is almost free.

> +		    } elsif ($authuser ne 'root@pam') {
> +			die "only root can set '$opt' config for non-mapped devices\n";
> +		    }
>  		    $conf->{pending}->{$opt} = $param->{$opt};
>  		} elsif ($opt eq 'tags') {
>  		    assert_tag_permissions($vmid, $conf->{$opt}, $param->{$opt}, $rpcenv, $authuser);
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index ab5458c3..4fb8d1c2 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3783,8 +3783,8 @@ sub config_to_command {
>      my $bootorder = device_bootorder($conf);
>  
>      # host pci device passthrough
> -    my ($kvm_off, $gpu_passthrough, $legacy_igd) = PVE::QemuServer::PCI::print_hostpci_devices(
> -	$vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder);
> +    my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) = PVE::QemuServer::PCI::print_hostpci_devices(
> +	$vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $machine_type, $bootorder);
>  
>      # usb devices
>      my $usb_dev_features = {};
> @@ -4203,7 +4203,7 @@ sub config_to_command {
>  	push @$cmd, @$aa;
>      }
>  
> -    return wantarray ? ($cmd, $vollist, $spice_port) : $cmd;
> +    return wantarray ? ($cmd, $vollist, $spice_port, $pci_devices) : $cmd;
>  }
>  
>  sub check_rng_source {
> @@ -5762,7 +5762,7 @@ sub vm_start_nolock {
>  	print "Resuming suspended VM\n";
>      }
>  
> -    my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid,
> +    my ($cmd, $vollist, $spice_port, $pci_devices) = config_to_command($storecfg, $vmid,
>  	$conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'});
>  
>      my $migration_ip;
> @@ -5847,38 +5847,44 @@ sub vm_start_nolock {
>  
>      my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $resume);
>  
> -    my $pci_devices = {}; # host pci devices
> -    for (my $i = 0; $i < $PVE::QemuServer::PCI::MAX_HOSTPCI_DEVICES; $i++)  {
> -	my $dev = $conf->{"hostpci$i"} or next;
> -	$pci_devices->{$i} = parse_hostpci($dev);
> +    my $pci_reserve_list = [];
> +    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}->@*;
>      }
>  
> -    # do not reserve pciid for mediated devices, sysfs will error out for duplicate assignment
> -    my $real_pci_devices = [ grep { !(defined($_->{mdev}) && scalar($_->{pciid}->@*) == 1) } values $pci_devices->%* ];
> -
> -    # map to a flat list of pci ids
> -    my $pci_id_list = [ map { $_->{id} } map { $_->{pciid}->@* } $real_pci_devices->@* ];
> -
>      # reserve all PCI IDs before actually doing anything with them
> -    PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, $start_timeout);
> +    PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
>  
>      eval {
>  	my $uuid;
>  	for my $id (sort keys %$pci_devices) {
>  	    my $d = $pci_devices->{$id};
> -	    for my $dev ($d->{pciid}->@*) {
> -		my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
> -
> -		# nvidia grid needs the qemu parameter '-uuid' set
> -		# use smbios uuid or mdev uuid as fallback for that
> -		if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '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, $id) if !defined($uuid);
> +	    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->{mdev}) };
> +		if ($d->{mdev}) {
> +		    warn $@ if $@;
> +		    $chosen_mdev = $info;
> +		    last if $chosen_mdev; # if successful, we're done
> +		} else {
> +		    die $@ if $@;
>  		}
>  	    }
> +
> +	    next if !$d->{mdev};
> +	    die "could not create mediated device\n" if !defined($chosen_mdev);
> +
> +	    # 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);
> +	    }
>  	}
>  	push @$cmd, '-uuid', $uuid if defined($uuid);
>      };
> @@ -5992,7 +5998,7 @@ sub vm_start_nolock {
>  
>      # re-reserve all PCI IDs now that we can know the actual VM PID
>      my $pid = PVE::QemuServer::Helpers::vm_running_locally($vmid);
> -    eval { PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, undef, $pid) };
> +    eval { PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, undef, $pid) };
>      warn $@ if $@;
>  
>      if (defined($res->{migrate})) {
> @@ -6187,9 +6193,7 @@ sub cleanup_pci_devices {
>  
>  	    # some nvidia vgpu driver versions want to clean the mdevs up themselves, and error
>  	    # out when we do it first. so wait for 10 seconds and then try it
> -	    my $pciid = $d->{pciid}->[0]->{id};
> -	    my $info = PVE::SysFSTools::pci_device_info("$pciid");
> -	    if ($info->{vendor} eq '10de') {
> +	    if ($d->{ids}->[0]->[0]->{vendor} =~ m/^(0x)?10de$/) {
>  		sleep 10;
>  	    }
>  
> @@ -6548,6 +6552,13 @@ sub check_mapping_access {
>  	   } elsif (!$device->{spice}) {
>  	       die "only root can set '$opt' config for real devices\n";
>  	   }
> +       } elsif ($opt =~ m/^hostpci\d+$/) {
> +	   my $device = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $conf->{$opt});
> +	   if ($device->{host} !~ m/:/) {
> +	       $rpcenv->check_full($user, "/mapping/pci/$device->{host}", ['Mapping.Use']);
> +	   } else {
> +	       die "only root can set '$opt' config for non-mapped devices\n";
> +	   }
>         }
>     }
>  };
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index a18b9747..5a5cbcca 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -4,6 +4,7 @@ use warnings;
>  use strict;
>  
>  use PVE::JSONSchema;
> +use PVE::Mapping::PCI;
>  use PVE::SysFSTools;
>  use PVE::Tools;
>  
> @@ -23,8 +24,8 @@ my $hostpci_fmt = {
>      host => {
>  	default_key => 1,
>  	type => 'string',
> -	pattern => qr/$PCIRE(;$PCIRE)*/,
> -	format_description => 'HOSTPCIID[;HOSTPCIID2...]',
> +	pattern => qr/(:?$PCIRE(;$PCIRE)*)|(:?$PVE::JSONSchema::CONFIGID_RE)/,
> +	format_description => 'HOSTPCIID[;HOSTPCIID2...] or configured mapping id',
>  	description => <<EODESCR,
>  Host PCI device pass through. The PCI ID of a host's PCI device or a list
>  of PCI virtual functions of the host. HOSTPCIID syntax is:
> @@ -32,6 +33,8 @@ of PCI virtual functions of the host. HOSTPCIID syntax is:
>  'bus:dev.func' (hexadecimal numbers)
>  
>  You can us the 'lspci' command to list existing PCI devices.
> +
> +Alternatively use the ID of a mapped pci device.
>  EODESCR
>      },
>      rombar => {
> @@ -376,6 +379,32 @@ sub print_pcie_root_port {
>      return $res;
>  }
>  
> +# returns the parsed pci config but parses the 'host' part into
> +# a list if lists into the 'id' property like this:
> +#
> +# {
> +#   mdev => 1,
> +#   rombar => ...
> +#   ...
> +#   ids => [
> +#       # this contains a list of alternative devices,
> +#       [
> +#           # which are itself lists of ids for one multifunction device
> +#           {
> +#               id => "0000:00:00.0",
> +#               vendor => "...",
> +#           },
> +#           {
> +#               id => "0000:00:00.1",
> +#               vendor => "...",
> +#           },
> +#       ],
> +#       [
> +#           ...
> +#       ],
> +#       ...
> +#   ],
> +# }
>  sub parse_hostpci {
>      my ($value) = @_;
>  
> @@ -383,31 +412,167 @@ sub parse_hostpci {
>  
>      my $res = PVE::JSONSchema::parse_property_string($hostpci_fmt, $value);
>  
> -    my @idlist = split(/;/, $res->{host});
> +    my $alternatives = [];
> +    if ($res->{host} !~ m/:/) {
> +	# we have no ordinary pci id, must be a mapping
> +	my $devices = PVE::Mapping::PCI::find_on_current_node($res->{host});
> +	die "PCI device mapping not found for '$res->{host}'\n" if !$devices || !scalar($devices->@*);
> +
> +	for my $device ($devices->@*) {
> +	    eval { PVE::Mapping::PCI::assert_valid($res->{host}, $device) };
> +	    die "PCI device mapping invalid (hardware probably changed): $@\n" if $@;
> +	    push $alternatives->@*, [split(/;/, $device->{path})];
> +	}
> +    } else {
> +	push $alternatives->@*, [split(/;/, $res->{host})];
> +    }
>      delete $res->{host};
> -    foreach my $id (@idlist) {
> -	my $devs = PVE::SysFSTools::lspci($id);
> -	die "no PCI device found for '$id'\n" if !scalar(@$devs);
> -	push @{$res->{pciid}}, @$devs;
> +
> +    $res->{ids} = [];
> +    for my $alternative ($alternatives->@*) {
> +	my $ids = [];
> +	foreach my $id ($alternative->@*) {
> +	    my $devs = PVE::SysFSTools::lspci($id);
> +	    die "no PCI device found for '$id'\n" if !scalar($devs->@*);
> +	    push $ids->@*, @$devs;
> +	}
> +	if (scalar($ids->@*) > 1) {
> +	    $res->{'has-multifunction'} = 1;
> +	    die "cannot use mediated device with multifunction device\n" if $res->{mdev};
> +	}
> +	push $res->{ids}->@*, $ids;
>      }
> +
>      return $res;
>  }
>  
> +# parses all hostpci devices from a config and does some sanity checks
> +# returns a hash like this:
> +# {
> +#     hostpci0 => {
> +#         # hash from parse_hostpci function
> +#     },
> +#     hostpci1 => { ... },
> +#     ...
> +# }
> +sub parse_hostpci_devices {
> +    my ($conf) = @_;
> +
> +    my $q35 = PVE::QemuServer::Machine::machine_type_is_q35($conf);
> +    my $legacy_igd = 0;
> +
> +    my $parsed_devices = {};
> +    for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
> +	my $id = "hostpci$i";
> +	my $d = parse_hostpci($conf->{$id});
> +	next if !$d;
> +
> +	# check syntax
> +	die "q35 machine model is not enabled" if !$q35 && $d->{pcie};
> +
> +	if ($d->{'legacy-igd'}) {
> +	    die "only one device can be assigned in legacy-igd mode\n"
> +		if $legacy_igd;
> +	    $legacy_igd = 1;
> +
> +	    die "legacy IGD assignment requires VGA mode to be 'none'\n"
> +		if !defined($conf->{'vga'}) || $conf->{'vga'} ne 'none';
> +	    die "legacy IGD assignment requires rombar to be enabled\n"
> +		if defined($d->{rombar}) && !$d->{rombar};
> +	    die "legacy IGD assignment is not compatible with x-vga\n"
> +		if $d->{'x-vga'};
> +	    die "legacy IGD assignment is not compatible with mdev\n"
> +		if $d->{mdev};
> +	    die "legacy IGD assignment is not compatible with q35\n"
> +		if $q35;
> +	    die "legacy IGD assignment is not compatible with multifunction devices\n"
> +		if $d->{'has-multifunction'};
> +	    die "legacy IGD assignment is not compatible with alternate devices\n"
> +		if scalar($d->{ids}->@*) > 1;
> +	    # check first device for valid id
> +	    die "legacy IGD assignment only works for devices on host bus 00:02.0\n"
> +		if $d->{ids}->[0]->[0]->{id} !~ m/02\.0$/;
> +	}
> +
> +	$parsed_devices->{$id} = $d;
> +    }
> +
> +    return $parsed_devices;
> +}
> +
> +# takes the hash returned by parse_hostpci_devices and for all non mdev gpus,
> +# selects one of the given alternatives by trying to reserve it
> +#
> +# mdev devices must be chosen later when we actually allocate it, but we
> +# flatten the inner list since there can only be one device per alternative anyway
> +my sub choose_hostpci_devices {
> +    my ($devices, $vmid) = @_;
> +
> +    my $used = {};
> +
> +    my $add_used_device = sub {
> +	my ($devices) = @_;
> +	for my $used_device ($devices->@*) {
> +	    my $used_id = $used_device->{id};
> +	    die "device '$used_id' assigned more than once\n" if $used->{$used_id};
> +	    $used->{$used_id} = 1;
> +	}
> +    };
> +
> +    for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
> +	my $device = $devices->{"hostpci$i"};
> +	next if !$device;
> +
> +	if ($device->{mdev}) {
> +	    $device->{ids} = [ map { $_->[0] } $device->{ids}->@* ];
> +	    next;
> +	}
> +
> +	if (scalar($device->{ids}->@* == 1)) {
> +	    # we only have one alternative, use that
> +	    $device->{ids} = $device->{ids}->[0];
> +	    $add_used_device->($device->{ids});
> +	    next;
> +	}
> +
> +	my $found = 0;
> +	for my $alternative ($device->{ids}->@*) {
> +	    my $ids = [map { $_->{id} } @$alternative];
> +
> +	    next if grep { defined($used->{$_}) } @$ids; # already used
> +	    eval { reserve_pci_usage($ids, $vmid, 10, undef) };
> +	    next if $@;
> +
> +	    # found one that is not used or reserved
> +	    $add_used_device->($alternative);
> +	    $device->{ids} = $alternative;
> +	    $found = 1;
> +	    last;
> +	}
> +	die "could not find a free device for 'hostpci$i'\n" if !$found;
> +    }
> +
> +    return $devices;
> +}
> +
>  sub print_hostpci_devices {
> -    my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder) = @_;
> +    my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $machine_type, $bootorder) = @_;
>  
>      my $kvm_off = 0;
>      my $gpu_passthrough = 0;
>      my $legacy_igd = 0;
>  
>      my $pciaddr;
> +    my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
> +
>      for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
>  	my $id = "hostpci$i";
> -	my $d = parse_hostpci($conf->{$id});
> +	my $d = $pci_devices->{$id};
>  	next if !$d;
>  
> +	$legacy_igd = 1 if $d->{'legacy-igd'};
> +
>  	if (my $pcie = $d->{pcie}) {
> -	    die "q35 machine model is not enabled" if !$q35;
>  	    # win7 wants to have the pcie devices directly on the pcie bus
>  	    # instead of in the root port
>  	    if ($winversion == 7) {
> @@ -425,29 +590,8 @@ sub print_hostpci_devices {
>  	    $pciaddr = print_pci_addr($pci_name, $bridges, $arch, $machine_type);
>  	}
>  
> -	my $pcidevices = $d->{pciid};
> -	my $multifunction = @$pcidevices > 1;
> -
> -	if ($d->{'legacy-igd'}) {
> -	    die "only one device can be assigned in legacy-igd mode\n"
> -		if $legacy_igd;
> -	    $legacy_igd = 1;
> -
> -	    die "legacy IGD assignment requires VGA mode to be 'none'\n"
> -		if !defined($conf->{'vga'}) || $conf->{'vga'} ne 'none';
> -	    die "legacy IGD assignment requires rombar to be enabled\n"
> -		if defined($d->{rombar}) && !$d->{rombar};
> -	    die "legacy IGD assignment is not compatible with x-vga\n"
> -		if $d->{'x-vga'};
> -	    die "legacy IGD assignment is not compatible with mdev\n"
> -		if $d->{mdev};
> -	    die "legacy IGD assignment is not compatible with q35\n"
> -		if $q35;
> -	    die "legacy IGD assignment is not compatible with multifunction devices\n"
> -		if $multifunction;
> -	    die "legacy IGD assignment only works for devices on host bus 00:02.0\n"
> -		if $pcidevices->[0]->{id} !~ m/02\.0$/;
> -	}
> +	my $num_devices = scalar($d->{ids}->@*);
> +	my $multifunction = $num_devices > 1 && !$d->{mdev};
>  
>  	my $xvga = '';
>  	if ($d->{'x-vga'}) {
> @@ -458,15 +602,13 @@ sub print_hostpci_devices {
>  	}
>  
>  	my $sysfspath;
> -	if ($d->{mdev} && scalar(@$pcidevices) == 1) {
> +	if ($d->{mdev}) {
>  	    my $uuid = generate_mdev_uuid($vmid, $i);
>  	    $sysfspath = "/sys/bus/mdev/devices/$uuid";
> -	} elsif ($d->{mdev}) {
> -	    warn "ignoring mediated device '$id' with multifunction device\n";
>  	}
>  
> -	my $j = 0;
> -	foreach my $pcidevice (@$pcidevices) {
> +	for (my $j = 0; $j < $num_devices; $j++) {
> +	    my $pcidevice = $d->{ids}->[$j];
>  	    my $devicestr = "vfio-pci";
>  
>  	    if ($sysfspath) {
> @@ -489,12 +631,13 @@ sub print_hostpci_devices {
>  		}
>  	    }
>  
> +
>  	    push @$devices, '-device', $devicestr;
> -	    $j++;
> +	    last if $d->{mdev};
>  	}
>      }
>  
> -    return ($kvm_off, $gpu_passthrough, $legacy_igd);
> +    return ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices);
>  }
>  
>  sub prepare_pci_device {
> @@ -596,6 +739,26 @@ sub reserve_pci_usage {
>  			warn "leftover PCI reservation found for $id, lets take it...\n";
>  		    }
>  		}
> +	    } elsif ($reservation) {
> +		# already reserved by the same vmid
> +		if (my $reserved_time = $reservation->{time}) {
> +		    if (defined($timeout)) {
> +			# use the longer timeout
> +			my $old_timeout = $reservation->{time} - 5 - $ctime;
> +			$timeout = $old_timeout if $old_timeout > $timeout;
> +		    }
> +		} elsif (my $reserved_pid = $reservation->{pid}) {
> +		    my $running_pid = PVE::QemuServer::Helpers::vm_running_locally($reservation->{vmid});
> +		    if (defined($running_pid) && $running_pid == $reservation->{pid}) {
> +			if (defined($pid)) {
> +			    die "PCI device '$id' already in use by running VMID '$reservation->{vmid}'\n";
> +			} elsif (defined($timeout)) {
> +			    # ignore timeout reservation for running vms, can happen with e.g.
> +			    # qm showcmd
> +			    return;
> +			}
> +		    }
> +		}
>  	    }
>  
>  	    $reservation_list->{$id} = { vmid => $vmid };
> diff --git a/test/cfg2cmd/q35-linux-hostpci.conf b/test/cfg2cmd/q35-linux-hostpci.conf
> index 749f9839..7290120a 100644
> --- a/test/cfg2cmd/q35-linux-hostpci.conf
> +++ b/test/cfg2cmd/q35-linux-hostpci.conf
> @@ -8,7 +8,7 @@ hostpci1: d0:13.0,pcie=1
>  hostpci2: 00:f4.0
>  hostpci3: d0:15.1,pcie=1
>  hostpci4: d0:17.0,pcie=1,rombar=0
> -hostpci7: d0:15.1,pcie=1
> +hostpci7: d0:15.2,pcie=1
>  machine: q35
>  memory: 512
>  net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
> diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> index 0774047d..0fedbd2c 100644
> --- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> +++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> @@ -32,7 +32,7 @@
>    -device 'pcie-root-port,id=ich9-pcie-port-5,addr=10.0,x-speed=16,x-width=32,multifunction=on,bus=pcie.0,port=5,chassis=5' \
>    -device 'vfio-pci,host=0000:d0:17.0,id=hostpci4,bus=ich9-pcie-port-5,addr=0x0,rombar=0' \
>    -device 'pcie-root-port,id=ich9-pcie-port-8,addr=10.3,x-speed=16,x-width=32,multifunction=on,bus=pcie.0,port=8,chassis=8' \
> -  -device 'vfio-pci,host=0000:d0:15.1,id=hostpci7,bus=ich9-pcie-port-8,addr=0x0' \
> +  -device 'vfio-pci,host=0000:d0:15.2,id=hostpci7,bus=ich9-pcie-port-8,addr=0x0' \
>    -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
>    -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
>    -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
> diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
> index c4966902..dda44d99 100755
> --- a/test/run_config2command_tests.pl
> +++ b/test/run_config2command_tests.pl
> @@ -81,6 +81,7 @@ my $pci_devs = [
>      "0000:0f:f2.0",
>      "0000:d0:13.0",
>      "0000:d0:15.1",
> +    "0000:d0:15.2",
>      "0000:d0:17.0",
>      "0000:f0:42.0",
>      "0000:f0:43.0",
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




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

* Re: [pve-devel] [PATCH qemu-server v6 1/6] enable cluster mapped USB devices for guests
  2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 1/6] enable cluster mapped USB devices for guests Dominik Csapak
@ 2023-06-16  7:50   ` Fabian Grünbichler
  0 siblings, 0 replies; 29+ messages in thread
From: Fabian Grünbichler @ 2023-06-16  7:50 UTC (permalink / raw)
  To: Proxmox VE development discussion

On June 14, 2023 10:46 am, Dominik Csapak wrote:
> this patch allows configuring usb devices that are mapped via
> cluster resource mapping when the user has 'Resource.Use' on the ACL
> path '/resource/usb/{ID}' (in addition to the usual required vm config
> privileges)
> 
> for now, this is only valid if there is exactly one mapping for the
> host, since we don't track passed through usb devices yet
> 
> also checks the permission for usb devices on clone/restore
> 
> Note that this now fails for 'raw' devices when the user is not root, so
> this is a breaking change!
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v5:
> * move the permission check into QemuServer.pm
> * add a 'check_restore_permissions' functions and refactor the checks
>   into there
>  PVE/API2/Qemu.pm      | 41 ++++++++++++++++++++++++++++++++++++++---
>  PVE/QemuServer.pm     | 36 +++++++++++++++++++++++++++++++++---
>  PVE/QemuServer/USB.pm | 27 ++++++++++++++++++++++++---
>  3 files changed, 95 insertions(+), 9 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index c92734a6..865edf7f 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -32,6 +32,7 @@ use PVE::QemuServer::Drive;
>  use PVE::QemuServer::ImportDisk;
>  use PVE::QemuServer::Monitor qw(mon_cmd);
>  use PVE::QemuServer::Machine;
> +use PVE::QemuServer::USB qw(parse_usb_device);
>  use PVE::QemuMigrate;
>  use PVE::RPCEnvironment;
>  use PVE::AccessControl;
> @@ -590,8 +591,13 @@ my $check_vm_create_usb_perm = sub {
>  
>      foreach my $opt (keys %{$param}) {
>  	next if $opt !~ m/^usb\d+$/;
> +	my $entry = PVE::JSONSchema::parse_property_string('pve-qm-usb', $param->{$opt});
> +	my $device = parse_usb_device($entry->{host});
>  
> -	if ($param->{$opt} =~ m/spice/) {
> +	if ($device->{spice}) {
> +	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> +	} elsif ($device->{mapped}) {
> +	    $rpcenv->check_full($authuser, "/mapping/usb/$entry->{host}", ['Mapping.Use']);
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
>  	} else {
>  	    die "only root can set '$opt' config for real devices\n";

this part here
> @@ -1719,7 +1725,12 @@ my $update_vm_api  = sub {
>  		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
>  		} elsif ($opt =~ m/^usb\d+$/) {
> -		    if ($val =~ m/spice/) {
> +		    my $device = PVE::JSONSchema::parse_property_string('pve-qm-usb', $val);
> +		    my $host = parse_usb_device($device->{host});
> +		    if ($host->{spice}) {
> +			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    } elsif ($host->{mapped}) {
> +			$rpcenv->check_full($authuser, "/mapping/usb/$device->{host}", ['Mapping.Use']);
>  			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
>  		    } elsif ($authuser ne 'root@pam') {
>  			die "only root can delete '$opt' config for real devices\n";

this part here (modulo variable names, especially confusing since
$device is used in both!)

> @@ -1784,7 +1795,30 @@ my $update_vm_api  = sub {
>  		    }
>  		    $conf->{pending}->{$opt} = $param->{$opt};
>  		} elsif ($opt =~ m/^usb\d+/) {
> -		    if ((!defined($conf->{$opt}) || $conf->{$opt} =~ m/spice/) && $param->{$opt} =~ m/spice/) {
> +		    my $olddevice;
> +		    my $oldhost;
> +		    if (defined($conf->{$opt})) {
> +			$olddevice = PVE::JSONSchema::parse_property_string('pve-qm-usb', $conf->{$opt});
> +			$oldhost = parse_usb_device($olddevice->{host});
> +		    }
> +		    if (defined($oldhost)) {
> +			if ($oldhost->{spice}) {
> +			    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +			} elsif ($oldhost->{mapped}) {
> +			    $rpcenv->check_full($authuser, "/mapping/usb/$olddevice->{host}", ['Mapping.Use']);
> +			    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +			} elsif ($authuser ne 'root@pam') {
> +			    die "only root can modify '$opt' config for real devices\n";
> +			}

this part here

> +		    }
> +
> +		    my $newdevice = PVE::JSONSchema::parse_property_string('pve-qm-usb', $param->{$opt});
> +		    my $newhost = parse_usb_device($newdevice->{host});
> +
> +		    if ($newhost->{spice}) {
> +			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    } elsif ($newhost->{mapped}) {
> +			$rpcenv->check_full($authuser, "/mapping/usb/$newdevice->{host}", ['Mapping.Use']);
>  			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
>  		    } elsif ($authuser ne 'root@pam') {
>  			die "only root can modify '$opt' config for real devices\n";

and this part here

are all identical except for some variations in variable names. this
screams "single helper" to me..

it also might make sense to structure it differently?

1. always check VM.Config.HWType (explicitly or implicitly done in all branches
atm)
2. if mapped, check mapping
3. if not, die if not root

I know you discussed with Thomas to split the config format a bit more
here to make it less overloaded, which might make this part easier to
understand as well :)

> @@ -3511,6 +3545,7 @@ __PACKAGE__->register_method({
>  	    my $oldconf = $snapname ? $conf->{snapshots}->{$snapname} : $conf;
>  
>  	    my $sharedvm = &$check_storage_access_clone($rpcenv, $authuser, $storecfg, $oldconf, $storage);
> +	    PVE::QemuServer::check_mapping_access($rpcenv, $authuser, $oldconf);
>  
>  	    PVE::QemuServer::check_bridge_access($rpcenv, $authuser, $oldconf);
>  
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index b978ab54..ab5458c3 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -1095,6 +1095,8 @@ The Host USB device or port or the value 'spice'. HOSTUSBDEVICE syntax is:
>  
>  You can use the 'lsusb -t' command to list existing usb devices.
>  
> +Alternatively, you can used an ID of a mapped usb device.
> +
>  NOTE: This option allows direct access to host hardware. So it is no longer possible to migrate such
>  machines - use with special care.
>  
> @@ -1111,6 +1113,8 @@ EODESCR
>      },
>  };
>  
> +PVE::JSONSchema::register_format('pve-qm-usb', $usb_fmt);
> +
>  my $usbdesc = {
>      optional => 1,
>      type => 'string', format => $usb_fmt,
> @@ -2248,7 +2252,12 @@ PVE::JSONSchema::register_format('pve-qm-usb-device', \&verify_usb_device);
>  sub verify_usb_device {
>      my ($value, $noerr) = @_;
>  
> -    return $value if parse_usb_device($value);
> +    my $parsed = eval { parse_usb_device($value) };
> +    if (my $err = $@) {
> +	die $err if !$noerr;
> +	return;
> +    }
> +    return $value if defined($parsed);
>  
>      return if $noerr;
>  
> @@ -6527,6 +6536,27 @@ sub check_bridge_access {
>      return 1;
>  };
>  
> +sub check_mapping_access {
> +   my ($rpcenv, $user, $conf) = @_;
> +
> +   for my $opt (keys $conf->%*) {
> +       if ($opt =~ m/^usb\d+$/) {
> +	   my $entry = PVE::JSONSchema::parse_property_string('pve-qm-usb', $conf->{$opt});
> +	   my $device = parse_usb_device($entry->{host});
> +	   if ($device->{mapped}) {
> +	       $rpcenv->check_full($user, "/mapping/usb/$entry->{host}", ['Mapping.Use']);
> +	   } elsif (!$device->{spice}) {
> +	       die "only root can set '$opt' config for real devices\n";
> +	   }
> +       }
> +   }
> +};
> +
> +sub check_restore_permissions {
> +    my ($rpcenv, $user, $conf) = @_;
> +    check_bridge_access($rpcenv, $user, $conf);
> +    check_mapping_access($rpcenv, $user, $conf);
> +}

might want a FIXME comment there to improve this w.r.t. checking before
disk allocation if possible?

e.g., the helper could take old and optionally new config (not possible
for STDIN, but then we are root anyway so the checks should also never
fail ;)), and do the checks on the "overlay". then we could call it
twice:
- with old config and optionally extracted config before starting the
  restore (early abort!)
- with the merged config after the restore (final check with the real
  config we write out)

>  # vzdump restore implementaion
>  
>  sub tar_archive_read_firstfile {
> @@ -7171,7 +7201,7 @@ sub restore_proxmox_backup_archive {
>      }
>  
>      my $new_conf = $restore_merge_config->($conffile, $new_conf_raw, $options->{override_conf});
> -    check_bridge_access($rpcenv, $user, $new_conf);
> +    check_restore_permissions($rpcenv, $user, $new_conf);
>      PVE::QemuConfig->write_config($vmid, $new_conf);
>  
>      eval { rescan($vmid, 1); };
> @@ -7485,7 +7515,7 @@ sub restore_vma_archive {
>      }
>  
>      my $new_conf = $restore_merge_config->($conffile, $new_conf_raw, $opts->{override_conf});
> -    check_bridge_access($rpcenv, $user, $new_conf);
> +    check_restore_permissions($rpcenv, $user, $new_conf);
>      PVE::QemuConfig->write_config($vmid, $new_conf);
>  
>      eval { rescan($vmid, 1); };
> diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
> index 686461cc..d6b4c531 100644
> --- a/PVE/QemuServer/USB.pm
> +++ b/PVE/QemuServer/USB.pm
> @@ -6,6 +6,7 @@ use PVE::QemuServer::PCI qw(print_pci_addr);
>  use PVE::QemuServer::Machine;
>  use PVE::QemuServer::Helpers qw(min_version windows_version);
>  use PVE::JSONSchema;
> +use PVE::Mapping::USB;
>  use base 'Exporter';
>  
>  our @EXPORT_OK = qw(
> @@ -17,7 +18,7 @@ get_usb_devices
>  my $OLD_MAX_USB = 5;
>  
>  sub parse_usb_device {
> -    my ($value) = @_;
> +    my ($value, $checkMap) = @_;
>  
>      return if !$value;
>  
> @@ -31,7 +32,27 @@ sub parse_usb_device {
>      } elsif ($value =~ m/^spice$/i) {
>  	$res->{spice} = 1;
>      } else {
> -	return;
> +	# we have no ordinary usb device, must be a mapping
> +	my $devices = PVE::Mapping::USB::find_on_current_node($value);
> +	if ($checkMap) {
> +	    die "USB device mapping not found for '$value'\n" if !$devices || !scalar($devices->@*);
> +	    die "More than one USB mapping per host not supported\n" if scalar($devices->@*) > 1;
> +	    eval {
> +		PVE::Mapping::USB::assert_valid($value, $devices->[0]);
> +	    };
> +	    if (my $err = $@) {
> +		die "USB Mapping invalid (hardware probably changed): $err\n";
> +	    }
> +	    my $device = $devices->[0];
> +
> +	    if ($device->{path}) {
> +		$res = parse_usb_device($device->{path});
> +	    } else {
> +		$res = parse_usb_device($device->{id});
> +	    }
> +	}
> +
> +	$res->{mapped} = 1;
>      }
>  
>      return $res;
> @@ -111,7 +132,7 @@ sub get_usb_devices {
>  	my $port = $use_qemu_xhci ? $i + 1 : undef;
>  
>  	if (defined($d->{host})) {
> -	    my $hostdevice = parse_usb_device($d->{host});
> +	    my $hostdevice = parse_usb_device($d->{host}, 1);
>  	    $hostdevice->{usb3} = $d->{usb3};
>  	    if ($hostdevice->{spice}) {
>  		# usb redir support for spice
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




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

* Re: [pve-devel] [PATCH manager v6 02/15] api: add resource map api endpoints for PCI and USB
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 02/15] api: add resource map api endpoints for PCI and USB Dominik Csapak
@ 2023-06-16  7:50   ` Fabian Grünbichler
  0 siblings, 0 replies; 29+ messages in thread
From: Fabian Grünbichler @ 2023-06-16  7:50 UTC (permalink / raw)
  To: Proxmox VE development discussion

On June 14, 2023 10:46 am, Dominik Csapak wrote:
> this adds the typical section config crud API calls for
> USB and PCI resource mapping to /cluster/resource/{TYPE}
> 
> the only special thing that this series does is the list call
> for both has a special 'check-node' parameter that uses the
> 'proxyto_callback' to reroute the api call to the given node
> so that it can check the validity of the mapping for that node
> 
> in the future when we e.g. broadcast the lspci output via pmxcfs
> we drop the proxyto_callback and directly use the info from
> pmxcfs (or we drop the parameter and always check all nodes)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/API2/Cluster.pm               |   8 +
>  PVE/API2/Cluster/Makefile         |   5 +
>  PVE/API2/Cluster/Mapping.pm       |  53 ++++++
>  PVE/API2/Cluster/Mapping/Makefile |  18 ++
>  PVE/API2/Cluster/Mapping/PCI.pm   | 300 ++++++++++++++++++++++++++++++
>  PVE/API2/Cluster/Mapping/USB.pm   | 295 +++++++++++++++++++++++++++++
>  PVE/API2/Hardware.pm              |   1 -
>  PVE/API2/Nodes.pm                 |   1 +
>  8 files changed, 680 insertions(+), 1 deletion(-)
>  create mode 100644 PVE/API2/Cluster/Mapping.pm
>  create mode 100644 PVE/API2/Cluster/Mapping/Makefile
>  create mode 100644 PVE/API2/Cluster/Mapping/PCI.pm
>  create mode 100644 PVE/API2/Cluster/Mapping/USB.pm

[..]

> diff --git a/PVE/API2/Cluster/Mapping/PCI.pm b/PVE/API2/Cluster/Mapping/PCI.pm
> new file mode 100644
> index 00000000..9fe20bea
> --- /dev/null
> +++ b/PVE/API2/Cluster/Mapping/PCI.pm
> @@ -0,0 +1,300 @@
> +package PVE::API2::Cluster::Mapping::PCI;
> +
> +use strict;
> +use warnings;
> +
> +use Storable qw(dclone);
> +
> +use PVE::Cluster qw(cfs_lock_file);
> +use PVE::Mapping::PCI;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::Tools qw(extract_param);
> +
> +use PVE::RESTHandler;
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> +    name => 'index',
> +    path => '',
> +    method => 'GET',
> +    # only proxy if we give the 'check-node' parameter
> +    proxyto_callback => sub {
> +       my ($rpcenv, $proxyto, $param) = @_;
> +       return $param->{'check-node'} // 'localhost';
> +    },
> +    description => "List PCI Hardware Mapping",
> +    permissions => {
> +       description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or".
> +           " 'Mapping.Audit' permissions on '/mapping/pci/<name>'.",

nit: the schema/parameters call it 'id', not 'name', repeated a few
times below..

if I create a mapping and then query this API endpoint with pvesh, I get
a wrong result:

$ pvesh ls /cluster/mapping/usb --output-format json
Use of uninitialized value $c in concatenation (.) or string at /usr/share/perl5/PVE/CLI/pvesh.pm line 364.
[{"capabilities":"Dr-c-","name":null}]

not sure whether the issue is with pvesh or here, but could be related
to

> +       user => 'all',
> +    },
> +    parameters => {
> +       additionalProperties => 0,
> +       properties => {
> +           'check-node' => get_standard_option('pve-node', {
> +               description => "If given, checks the configurations on the given node for ".
> +                   "correctness, and adds relevant errors to the devices.",
> +               optional => 1,
> +           }),
> +       },
> +    },
> +    returns => {
> +       type => 'array',
> +       items => {
> +           type => "object",
> +           properties => {
> +               id => {
> +                   type => 'string',
> +                   description => "The logical ID of the mapping."
> +               },
> +               map => {
> +                   type => 'array',
> +                   description => "The entries of the mapping.",
> +                   items => {
> +                       type => 'string',
> +                       description => "A mapping for a node.",
> +                   },
> +               },
> +               description => {
> +                   type => 'string',
> +                   description => "A description of the logical mapping.",
> +               },
> +               error => {
> +                   description => "A list of errors when 'check_node' is given.",
> +                   items => {
> +                       type => 'object',
> +                       properties => {
> +                           severity => {
> +                               type => "string",
> +                               description => "The severity of the error",
> +                           },
> +                           message => {
> +                               type => "string",
> +                               description => "The message of the error",
> +                           },
> +                       },
> +                   }
> +               },
> +           },
> +       },
> +       links => [ { rel => 'child', href => "{name}" } ],

this part here, where it tries to link children using 'name', although
the return value only contains 'id' as property..

> +    },
> +    code => sub {
> +       my ($param) = @_;
> +
> +       my $rpcenv = PVE::RPCEnvironment::get();
> +       my $authuser = $rpcenv->get_user();
> +       my $node = $param->{'check-node'};
> +
> +       die "Wrong node to check\n"
> +           if defined($node) && $node ne 'localhost' && $node ne PVE::INotify::nodename();
> +
> +       my $cfg = PVE::Mapping::PCI::config();
> +
> +       my $res = [];
> +
> +       my $privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit'];
> +
> +       for my $id (keys $cfg->{ids}->%*) {
> +           next if !$rpcenv->check_full($authuser, "/mapping/pci/$id", $privs, 1, 1);
> +           next if !$cfg->{ids}->{$id};
> +
> +           my $entry = dclone($cfg->{ids}->{$id});
> +           $entry->{id} = $id;
> +           $entry->{digest} = $cfg->{digest};
> +
> +           if (defined($node)) {
> +               $entry->{errors} = [];
> +               if (my $mappings = PVE::Mapping::PCI::get_node_mapping($cfg, $id, $node)) {
> +                   if (!scalar($mappings->@*)) {
> +                       push $entry->{errors}->@*, {
> +                           severity => 'warning',
> +                           message => "No mapping for node $node.",
> +                       };
> +                   }
> +                   for my $mapping ($mappings->@*) {
> +                       eval {
> +                           PVE::Mapping::PCI::assert_valid($id, $mapping);
> +                       };
> +                       if (my $err = $@) {
> +                           push $entry->{errors}->@*, {
> +                               severity => 'error',
> +                               message => "Invalid configuration: $err",
> +                           };
> +                       }
> +                   }
> +               }
> +           }
> +
> +           push @$res, $entry;

this adds the full entry to the returned value, and the permission check
allows it with Mappings.*

> +       }
> +
> +       return $res;
> +    },
> +});
> +
> +__PACKAGE__->register_method ({
> +    name => 'get',
> +    protected => 1,
> +    path => '{id}',
> +    method => 'GET',
> +    description => "Get PCI Mapping.",
> +    permissions => {
> +       check =>['or',
> +           ['perm', '/mapping/pci/{name}', ['Mapping.Use']],
> +           ['perm', '/mapping/pci/{name}', ['Mapping.Modify']],

but this here doesn't allow Mapping.Audit?

either the index call needs to return a limited view, or this should be
allowed for Audit as well I think?

> +       ],
> +    },
> +    parameters => {
> +       additionalProperties => 0,
> +       properties => {
> +           id => {
> +               type => 'string',
> +               format => 'pve-configid',
> +           },
> +       }
> +    },
> +    returns => { type => 'object' },
> +    code => sub {
> +       my ($param) = @_;
> +
> +       my $cfg = PVE::Mapping::PCI::config();
> +       my $id = $param->{id};
> +
> +       my $entry = $cfg->{ids}->{$id};
> +       die "mapping '$param->{id}' not found\n" if !defined($entry);
> +
> +       my $data = dclone($entry);
> +
> +       $data->{digest} = $cfg->{digest};
> +
> +       return $data;
> +    }});
> +

> +__PACKAGE__->register_method ({
> +    name => 'create',
> +    protected => 1,
> +    path => '',
> +    method => 'POST',
> +    description => "Create a new hardware mapping.",
> +    permissions => {
> +	check => ['perm', '/mapping/pci/{name}', ['Mapping.Modify']],

we usually use the higher level path for creating (/mapping/pci) -
although the priv here is 'Modify' and not 'Allocate', we are still
creating a new entry ;)

> +    },
> +    # todo parameters

? ;)

> +    parameters => PVE::Mapping::PCI->createSchema(1),
> +    returns => {
> +	type => 'null',
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $id = extract_param($param, 'id');
> +
> +	my $plugin = PVE::Mapping::PCI->lookup('pci');
> +	my $opts = $plugin->check_config($id, $param, 1, 1);
> +
> +	PVE::Mapping::PCI::lock_pci_config(sub {
> +	    my $cfg = PVE::Mapping::PCI::config();
> +
> +	    die "pci ID '$id' already defined\n" if defined($cfg->{ids}->{$id});
> +
> +	    $cfg->{ids}->{$id} = $opts;
> +
> +	    PVE::Mapping::PCI::write_pci_config($cfg);
> +
> +	}, "create hardware mapping failed");
> +
> +	return;
> +    },
> +});
> +
> +__PACKAGE__->register_method ({
> +    name => 'update',
> +    protected => 1,
> +    path => '{id}',
> +    method => 'PUT',
> +    description => "Update a hardware mapping.",
> +    permissions => {
> +	check => ['perm', '/mapping/pci/{id}', ['Mapping.Modify']],

this one is in line with our usual scheme

> +    },
> +    parameters => PVE::Mapping::PCI->updateSchema(),
> +    returns => {
> +	type => 'null',
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $digest = extract_param($param, 'digest');
> +	my $delete = extract_param($param, 'delete');
> +	my $id = extract_param($param, 'id');
> +
> +	if ($delete) {
> +	    $delete = [ PVE::Tools::split_list($delete) ];
> +	}
> +
> +	PVE::Mapping::PCI::lock_pci_config(sub {
> +	    my $cfg = PVE::Mapping::PCI::config();
> +
> +	    PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest);
> +
> +	    die "pci ID '$id' does not exist\n" if !defined($cfg->{ids}->{$id});
> +
> +	    my $plugin = PVE::Mapping::PCI->lookup('pci');
> +	    my $opts = $plugin->check_config($id, $param, 1, 1);
> +
> +	    my $data = $cfg->{ids}->{$id};
> +
> +	    my $options = $plugin->private()->{options}->{pci};
> +	    PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete);
> +
> +	    $data->{$_} = $opts->{$_} for keys $opts->%*;
> +
> +	    PVE::Mapping::PCI::write_pci_config($cfg);
> +
> +	}, "update hardware mapping failed");
> +
> +	return;
> +    },
> +});
> +
> +__PACKAGE__->register_method ({
> +    name => 'delete',
> +    protected => 1,
> +    path => '{id}',
> +    method => 'DELETE',
> +    description => "Remove Hardware Mapping.",
> +    permissions => {
> +	check => [ 'perm', '/mapping/pci/{id}', ['Mapping.Modify']],

this one should be changed if the create one is changed - they should
match..

> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    id => {
> +		type => 'string',
> +		format => 'pve-configid',
> +	    },
> +	}
> +    },
> +    returns => { type => 'null' },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $id = $param->{id};
> +
> +	PVE::Mapping::PCI::lock_pci_config(sub {
> +	    my $cfg = PVE::Mapping::PCI::config();
> +
> +	    if ($cfg->{ids}->{$id}) {
> +		delete $cfg->{ids}->{$id};
> +	    }
> +
> +	    PVE::Mapping::PCI::write_pci_config($cfg);
> +
> +	}, "delete pci mapping failed");
> +
> +	return;
> +    }
> +});
> +
> +1;
> diff --git a/PVE/API2/Cluster/Mapping/USB.pm b/PVE/API2/Cluster/Mapping/USB.pm

and pretty much everything mentioned above applies to this one as well
;)

> [..]




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

* Re: [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping
  2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
                   ` (22 preceding siblings ...)
  2023-06-14 12:01 ` [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Markus Frank
@ 2023-06-16  7:51 ` Fabian Grünbichler
  23 siblings, 0 replies; 29+ messages in thread
From: Fabian Grünbichler @ 2023-06-16  7:51 UTC (permalink / raw)
  To: Proxmox VE development discussion

On June 14, 2023 10:46 am, Dominik Csapak wrote:
> this series is the remaining part to add a cluster-wide device mapping for pci and usb
> devices. so that an admin can configure a device to be availble for
> migration and configuring for uses that are non-root
> (the existing pattern can be copied easily for other types, e.g.
> markus upcoming folder sharing)
> 
> maybe the place for the docs are not optimal, if anyone has a better
> suggestion, please say
> 
> i omitted the older changelogs as were getting a bit long

did a sweep over the ACL/permission related parts, mostly looking good,
see replies to individual patches.

the manager patches require docs (new reference for online help)




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

* [pve-devel] applied: [PATCH manager v6 01/15] pvesh: fix parameters for proxyto_callback
  2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 01/15] pvesh: fix parameters for proxyto_callback Dominik Csapak
@ 2023-06-16  9:27   ` Wolfgang Bumiller
  0 siblings, 0 replies; 29+ messages in thread
From: Wolfgang Bumiller @ 2023-06-16  9:27 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: pve-devel

applied this one

On Wed, Jun 14, 2023 at 10:46:07AM +0200, Dominik Csapak wrote:
> in pve-http-server the proxyto_callback always has a complete list of
> parameters, not only the ones in the url, so adapt the implementation
> here to do the same
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/CLI/pvesh.pm | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/CLI/pvesh.pm b/PVE/CLI/pvesh.pm
> index 9acf292a..28e2518d 100755
> --- a/PVE/CLI/pvesh.pm
> +++ b/PVE/CLI/pvesh.pm
> @@ -82,13 +82,15 @@ my $method_map = {
>  };
>  
>  sub check_proxyto {
> -    my ($info, $uri_param) = @_;
> +    my ($info, $uri_param, $params) = @_;
>  
>      my $rpcenv = PVE::RPCEnvironment->get();
>  
> +    my $all_params = { %$uri_param, %$params };
> +
>      if ($info->{proxyto} || $info->{proxyto_callback}) {
>  	my $node = PVE::API2Tools::resolve_proxyto(
> -	    $rpcenv, $info->{proxyto_callback}, $info->{proxyto}, $uri_param);
> +	    $rpcenv, $info->{proxyto_callback}, $info->{proxyto}, $all_params);
>  
>  	if ($node ne 'localhost' && ($node ne PVE::INotify::nodename())) {
>  	    die "proxy loop detected - aborting\n" if $disable_proxy;
> @@ -301,7 +303,7 @@ sub call_api_method {
>      }
>  
>      my $data;
> -    my ($node, $remip) = check_proxyto($info, $uri_param);
> +    my ($node, $remip) = check_proxyto($info, $uri_param, $param);
>      if ($node) {
>  	$data = proxy_handler($node, $remip, $path, $cmd, $param);
>      } else {
> @@ -345,7 +347,7 @@ __PACKAGE__->register_method ({
>  
>  	my $res;
>  
> -	my ($node, $remip) = check_proxyto($info, $uri_param);
> +	my ($node, $remip) = check_proxyto($info, $uri_param, $param);
>  	if ($node) {
>  	    $res = proxy_handler($node, $remip, $path, 'ls', $param);
>  	} else {
> -- 
> 2.30.2




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

end of thread, other threads:[~2023-06-16  9:28 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-14  8:46 [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 1/6] enable cluster mapped USB devices for guests Dominik Csapak
2023-06-16  7:50   ` Fabian Grünbichler
2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 2/6] enable cluster mapped PCI " Dominik Csapak
2023-06-16  7:49   ` Fabian Grünbichler
2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 3/6] check_local_resources: extend for mapped resources Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 4/6] api: migrate preconditions: use new check_local_resources info Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 5/6] migration: check for mapped resources Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH qemu-server v6 6/6] add test for mapped pci devices Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 01/15] pvesh: fix parameters for proxyto_callback Dominik Csapak
2023-06-16  9:27   ` [pve-devel] applied: " Wolfgang Bumiller
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 02/15] api: add resource map api endpoints for PCI and USB Dominik Csapak
2023-06-16  7:50   ` Fabian Grünbichler
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 03/15] ui: parser: add helper for lists of property strings Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 04/15] ui: form/USBSelector: make it more flexible with nodename Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 05/15] ui: form: add PCIMapSelector Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 06/15] ui: form: add USBMapSelector Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 07/15] ui: qemu/PCIEdit: rework panel to add a mapped configuration Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 08/15] ui: qemu/USBEdit: add 'mapped' device case Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 09/15] ui: form: add MultiPCISelector Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 10/15] ui: add edit window for pci mappings Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 11/15] ui: add edit window for usb mappings Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 12/15] ui: add ResourceMapTree Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 13/15] ui: allow configuring pci and usb mapping Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 14/15] ui: window/Migrate: allow mapped devices Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH manager v6 15/15] ui: improve permission handling for hardware Dominik Csapak
2023-06-14  8:46 ` [pve-devel] [PATCH docs v6 1/1] qemu: add documentation about cluster device mapping Dominik Csapak
2023-06-14 12:01 ` [pve-devel] [PATCH qemu-server/manger/docs v6] cluster mapping Markus Frank
2023-06-16  7:51 ` Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal