[pve-devel] [PATCH many] add cluster-wide hardware device mapping

[pve-devel] [PATCH many] add cluster-wide hardware device mapping

[Dominik Csapak]

this series aims 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

built-in are some additional safety checks in contrast to current
passthrough, e.g. if pci addresses shift, with the mapping
we can detect that and prevent a vm to boot with the wrong device
(in most cases, there are some edge cases when one has multiple
of the same device, e.g. the same gpu, that we cannot detect)

a few pain points that are probably worth discussing/thinking about:
(i did not really get feedback on my last RFC on this)
* the config format
    i changed to a json backed config, since it makes handling it much
    easier (since we have a id -> nodenames -> mapping relation that
    we cannot easily represent with a section config). some
    (small) parts are written from scratch (update/createSchema for
    instance) but we would have to do that anyway

    if wanted i can make the section config work, but it makes the
    handling quite a big uglier (for example, we have name the usb/pci
    properties differently because the section config cannnot have
    different formats for different sections)

* getting the cluster wide info
    the configuring of mappings is all done via node specific api paths,
    but i created a cluster wide api path that returns the overall
    structure for easy consumption from the gui. to get the remaining
    data from the other nodes, i let the gui make an api call
    for each node.

    alternatively we could distribute the necessary info via pmxcfs,
    but we'd have to broadcast basically the whole pci listing for all
    nodes in a relatively short interval, only for it to be extremly
    seldomly used (when looking at the cluster wide hardware
    mappings...)

* some minor things that can be improved are how the gui looks/behaves:
    - 'add new' and 'add mapping' are probably to similar, but i did
      not come up with really better alternatives
    - i find the tree of entry -> node-mappings nice, but there may be
      an even better representation?
    - position in cluster menu is probably not optimal
      (but where to put it?)

changes from the rfc:
* new cluster wide gui instead of node-local one (removed that, since
  it's not necessary when we have a cluster-wide one)
* uses json instead of a section config
* api is quite different overall, i split the type into its own level
  for configuring, similar to what we do in pbs
  (e.g. /nodes/NODENAME/hardware/mapping/usb/)
* fixed quite some bugs the rfc had
* added patch for handling the gui with limited permissions better
* added a 'comment' field for mappings

dependencies are ofc:

    manager depends on qemu-server,pve-access-control,pve-common
    qemu-server depends on pve-access-control,pve-common
    pve-common depends on pve-cluster

pve-cluster:

Dominik Csapak (1):
  add nodes/hardware-map.conf

 data/PVE/Cluster.pm | 1 +
 data/src/status.c   | 1 +
 2 files changed, 2 insertions(+)

pve-access-control:

Dominik Csapak (2):
  PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
  PVE/RPCEnvironment: add helper for checking hw permissions

 src/PVE/AccessControl.pm  | 13 +++++++++++++
 src/PVE/RPCEnvironment.pm |  9 +++++++++
 2 files changed, 22 insertions(+)

pve-common:

Dominik Csapak (1):
  add PVE/HardwareMap

 src/Makefile           |   1 +
 src/PVE/HardwareMap.pm | 363 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 364 insertions(+)
 create mode 100644 src/PVE/HardwareMap.pm

qemu-server:

Dominik Csapak (7):
  PVE/QemuServer: allow mapped usb devices in config
  PVE/QemuServer: allow mapped pci deviced in config
  PVE/API2/Qemu: add permission checks for mapped usb devices
  PVE/API2/Qemu: add permission checks for mapped pci devices
  PVE/QemuServer: extend 'check_local_resources' for mapped resources
  PVE/API2/Qemu: migrate preconditions: use new check_local_resources
    info
  PVE/QemuMigrate: check for mapped resources on migration

 PVE/API2/Qemu.pm      | 108 ++++++++++++++++++++++++++++++++++++++----
 PVE/QemuMigrate.pm    |  13 ++++-
 PVE/QemuServer.pm     |  38 ++++++++++++++-
 PVE/QemuServer/PCI.pm |  20 +++++++-
 PVE/QemuServer/USB.pm |  21 +++++++-
 5 files changed, 185 insertions(+), 15 deletions(-)

pve-manager:

Dominik Csapak (12):
  PVE/API2/Hardware: add Mapping.pm
  PVE/API2/Cluster: add Hardware mapping list api call
  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: add window/PCIEdit: edit window for pci mappings
  ui: add window/USBEdit: edit window for usb mappings
  ui: add dc/HardwareView: a CRUD interface for hardware mapping
  ui: window/Migrate: allow mapped devices
  ui: improve permission handling for hardware

 PVE/API2/Cluster.pm                 |   8 +
 PVE/API2/Cluster/Hardware.pm        | 117 +++++
 PVE/API2/Cluster/Makefile           |   1 +
 PVE/API2/Hardware.pm                |   6 +
 PVE/API2/Hardware/Makefile          |   1 +
 PVE/API2/Hardware/Mapping.pm        | 708 ++++++++++++++++++++++++++++
 www/manager6/Makefile               |   5 +
 www/manager6/data/PermPathStore.js  |   1 +
 www/manager6/dc/Config.js           |  18 +-
 www/manager6/dc/HardwareView.js     | 314 ++++++++++++
 www/manager6/form/PCIMapSelector.js |  95 ++++
 www/manager6/form/PCISelector.js    |  18 +-
 www/manager6/form/USBMapSelector.js |  73 +++
 www/manager6/form/USBSelector.js    |  33 +-
 www/manager6/qemu/HardwareView.js   |  17 +-
 www/manager6/qemu/PCIEdit.js        | 231 ++++++---
 www/manager6/qemu/USBEdit.js        |  34 +-
 www/manager6/window/Migrate.js      |  37 +-
 www/manager6/window/PCIEdit.js      | 323 +++++++++++++
 www/manager6/window/USBEdit.js      | 248 ++++++++++
 20 files changed, 2185 insertions(+), 103 deletions(-)
 create mode 100644 PVE/API2/Cluster/Hardware.pm
 create mode 100644 PVE/API2/Hardware/Mapping.pm
 create mode 100644 www/manager6/dc/HardwareView.js
 create mode 100644 www/manager6/form/PCIMapSelector.js
 create mode 100644 www/manager6/form/USBMapSelector.js
 create mode 100644 www/manager6/window/PCIEdit.js
 create mode 100644 www/manager6/window/USBEdit.js

-- 
2.30.2

[pve-devel] [PATCH cluster 1/1] add nodes/hardware-map.conf

[Dominik Csapak]

to PVE/Cluster.pm
and status.c

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 data/PVE/Cluster.pm | 1 +
 data/src/status.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
index abcc46d..5dfc6b2 100644
--- a/data/PVE/Cluster.pm
+++ b/data/PVE/Cluster.pm
@@ -76,6 +76,7 @@ my $observed = {
     'sdn/dns.cfg' => 1,
     'sdn/.running-config' => 1,
     'virtual-guest/cpu-models.conf' => 1,
+    'nodes/hardware-map.conf' => 1,
 };
 
 sub prepare_observed_file_basedirs {
diff --git a/data/src/status.c b/data/src/status.c
index 9bceaeb..f46f71d 100644
--- a/data/src/status.c
+++ b/data/src/status.c
@@ -106,6 +106,7 @@ static memdb_change_t memdb_change_array[] = {
 	{ .path = "sdn/dns.cfg" },
 	{ .path = "sdn/.running-config" },
 	{ .path = "virtual-guest/cpu-models.conf" },
+	{ .path = "nodes/hardware-map.conf" },
 };
 
 static GMutex mutex;
-- 
2.30.2

[pve-devel] [PATCH access-control 1/2] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths

[Dominik Csapak]

so that we can assign privileges on hardware level

this will generate a new role (PVEHardwareAdmin)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/AccessControl.pm  | 13 +++++++++++++
 src/PVE/RPCEnvironment.pm |  1 +
 2 files changed, 14 insertions(+)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 91b3aff..5fde663 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1085,6 +1085,17 @@ my $privgroups = {
 	    'Pool.Audit',
 	],
     },
+    Hardware => {
+	root => [
+	    'Hardware.Configure', # create/edit mappings
+	],
+	admin => [
+	    'Hardware.Use',
+	],
+	audit => [
+	    'Hardware.Audit',
+	],
+    },
 };
 
 my $valid_privs = {};
@@ -1214,6 +1225,8 @@ sub check_path {
 	|/storage/[[:alnum:]\.\-\_]+
 	|/vms
 	|/vms/[1-9][0-9]{2,}
+	|/hardware
+	|/hardware/[[:alnum:]\.\-\_]+
     )$!xs;
 }
 
diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
index 0ee2346..7c37c6e 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -187,6 +187,7 @@ sub compute_api_permission {
 	nodes => qr/Sys\.|Permissions\.Modify/,
 	sdn => qr/SDN\.|Permissions\.Modify/,
 	dc => qr/Sys\.Audit|SDN\./,
+	hardware => qr/Hardware\.|Permissiions\.Modify/,
     };
     map { $res->{$_} = {} } keys %$priv_re_map;
 
-- 
2.30.2

[pve-devel] [PATCH access-control 2/2] PVE/RPCEnvironment: add helper for checking hw permissions

[Dominik Csapak]

like check_vm_perm, etc.

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

diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
index 7c37c6e..c1b712d 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -356,6 +356,14 @@ sub check_vm_perm {
     return $self->check_full($user, "/vms/$vmid", $privs, $any, $noerr);
 };
 
+sub check_hw_perm {
+    my ($self, $user, $id, $privs, $any, $noerr) = @_;
+
+    my $cfg = $self->{user_cfg};
+
+    return $self->check_full($user, "/hardware/$id", $privs, $any, $noerr);
+}
+
 sub is_group_member {
     my ($self, $group, $user) = @_;
 
-- 
2.30.2

[pve-devel] [PATCH common 1/1] add PVE/HardwareMap

[Dominik Csapak]

this adds functionality for the hardwaremap config (as json)
the format of the config is like this:

{
    usb => {
	name => {
	    nodename1 => { /* mapping object */ },
	    nodename2 => { /* mapping object */ }
	}
    },
    pci => {
	/* same as above */
    },
    digest => "<DIGEST-STRING>"
}

it also adds some helpers for the api schema & asserting that the
device mappings are valid (by checking the saved properties
against the ones found on the current available devices)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/Makefile           |   1 +
 src/PVE/HardwareMap.pm | 363 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 364 insertions(+)
 create mode 100644 src/PVE/HardwareMap.pm

diff --git a/src/Makefile b/src/Makefile
index 13de6c6..8527704 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -17,6 +17,7 @@ LIB_SOURCES = \
 	Daemon.pm \
 	Exception.pm \
 	Format.pm \
+	HardwareMap.pm \
 	INotify.pm \
 	JSONSchema.pm \
 	LDAP.pm \
diff --git a/src/PVE/HardwareMap.pm b/src/PVE/HardwareMap.pm
new file mode 100644
index 0000000..1b94abc
--- /dev/null
+++ b/src/PVE/HardwareMap.pm
@@ -0,0 +1,363 @@
+package PVE::HardwareMap;
+
+use strict;
+use warnings;
+
+use Digest::SHA;
+use JSON;
+use Storable qw(dclone);
+
+use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
+use PVE::INotify;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::SysFSTools;
+
+use base qw(Exporter);
+
+our @EXPORT_OK = qw(find_device_on_current_node);
+
+my $FILENAME = "nodes/hardware-map.conf";
+cfs_register_file($FILENAME, \&read_hardware_map, \&write_hardware_map);
+
+# a mapping format per type
+my $format = {
+    usb => {
+	vendor => {
+	    description => "The vendor ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	},
+	device => {
+	    description => "The device ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	},
+	'subsystem-vendor' => {
+	    description => "The subsystem vendor ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	    optional => 1,
+	},
+	'subsystem-device' => {
+	    description => "The subsystem device ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	    optional => 1,
+	},
+	path => {
+	    description => "The path to the usb device.",
+	    type => 'string',
+	    optional => 1,
+	    pattern => qr/^(\d+)\-(\d+(\.\d+)*)$/,
+	},
+	comment => {
+	    description => "Description.",
+	    type => 'string',
+	    optional => 1,
+	    maxLength => 4096,
+	},
+    },
+    pci => {
+	vendor => {
+	    description => "The vendor ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	},
+	device => {
+	    description => "The device ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	},
+	'subsystem-vendor' => {
+	    description => "The subsystem vendor ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	    optional => 1,
+	},
+	'subsystem-device' => {
+	    description => "The subsystem device ID",
+	    type => 'string',
+	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
+	    optional => 1,
+	},
+	path => {
+	    description => "The path to the device. If the function is omitted, the whole device is mapped. In that case use the attrubes of the first device.",
+	    type => 'string',
+	    pattern => "(?:[a-f0-9]{4,}:)?[a-f0-9]{2}:[a-f0-9]{2}(?:\.[a-f0-9])?",
+	},
+	mdev => {
+	    description => "The Device supports mediated devices.",
+	    type => 'boolean',
+	    optional => 1,
+	    default => 0,
+	},
+	iommugroup => {
+	    type => 'integer',
+	    description => "The IOMMU group in which the device is in.",
+	    optional => 1,
+	},
+	comment => {
+	    description => "Description.",
+	    type => 'string',
+	    optional => 1,
+	    maxLength => 4096,
+	},
+    },
+};
+
+my $name_format = {
+    description => "The custom name for the device",
+    type => 'string',
+    format => 'pve-configid',
+};
+
+sub find_device_on_current_node {
+    my ($type, $id) = @_;
+
+    my $cfg = config();
+    my $node = PVE::INotify::nodename();
+
+    return undef if !defined($cfg->{$type}->{$id}) || !defined($cfg->{$type}->{$id}->{$node});
+    return $cfg->{$type}->{$id}->{$node};
+}
+
+sub config {
+    return cfs_read_file($FILENAME);
+}
+
+sub lock_config {
+    my ($code, $errmsg) = @_;
+
+    cfs_lock_file($FILENAME, undef, $code);
+    if (my $err = $@) {
+	$errmsg ? die "$errmsg: $err" : die $err;
+    }
+}
+
+sub write_config {
+    my ($cfg) = @_;
+
+    cfs_write_file($FILENAME, $cfg);
+}
+
+sub check_prop {
+    my ($schema, $key, $value) = @_;
+    my $errors = {};
+    PVE::JSONSchema::check_prop($value, $schema, '', $errors);
+    if (scalar(keys %$errors)) {
+	die "$errors->{$key}\n" if $errors->{$key};
+	die "$errors->{_root}\n" if $errors->{_root};
+	die "unknown error\n";
+    }
+}
+
+sub check_config {
+    my ($cfg) = @_;
+
+    for my $type (keys %$format) {
+	my $type_cfg = $cfg->{$type};
+	my $type_format = $format->{$type};
+
+	for my $name (keys %$type_cfg) {
+	    check_prop($name_format, 'name', $name);
+
+	    for my $node (keys $type_cfg->{$name}->%*) {
+		check_prop(get_standard_option('pve-node'), 'node', $node);
+		my $entry = $type_cfg->{$name}->{$node};
+
+		# check required props
+		for my $prop  (keys %$type_format) {
+		    next if $type_format->{$prop}->{optional};
+		    die "missing property '$prop' for $type entry '$name'\n"
+			if !defined($entry->{$prop});
+		}
+
+		for my $prop (keys %$entry) {
+		    check_prop($type_format->{$prop}, $prop, $entry->{$prop});
+		}
+	    }
+	}
+    }
+}
+
+sub read_hardware_map {
+    my ($filename, $raw)  = @_;
+
+    my $digest = Digest::SHA::sha1_hex($raw);
+
+    if (!defined($raw) || $raw eq '') {
+	return {
+	    digest => $digest,
+	};
+    }
+
+    my $cfg = from_json($raw);
+    check_config($cfg);
+    $cfg->{digest} = $digest;
+
+    return $cfg;
+}
+
+sub write_hardware_map {
+    my ($filename, $cfg) = @_;
+
+    check_config($cfg);
+
+    return to_json($cfg);
+}
+
+my $pci_valid = sub {
+    my ($cfg) = @_;
+
+    my $path = $cfg->{path} // '';
+
+    if ($path !~ m/\.[a-f0-9]/i) {
+	# whole device, add .0 (must exist)
+	$path = "$path.0";
+    }
+
+    my $info = PVE::SysFSTools::pci_device_info($path, 1);
+    die "pci device '$path' not found\n" if !defined($info);
+
+    my $correct_props = {
+	vendor => $info->{vendor},
+	device => $info->{device},
+	'subsystem-vendor' => $info->{'subsystem_vendor'},
+	'subsystem-device' => $info->{'subsystem_device'},
+	mdev => $info->{mdev},
+	iommugroup => $info->{iommugroup},
+    };
+
+    for my $prop (sort keys %$correct_props) {
+	next if !defined($correct_props->{$prop}) && !defined($cfg->{$prop});
+	die "no '$prop' for device '$path'\n"
+	    if defined($correct_props->{$prop}) && !defined($cfg->{$prop});
+	die "'$prop' configured but should not be\n"
+	    if !defined($correct_props->{$prop}) && defined($cfg->{$prop});
+
+	my $correct_prop = $correct_props->{$prop};
+	$correct_prop =~ s/^0x//;
+	my $configured_prop = $cfg->{$prop};
+	$configured_prop =~ s/^0x//;
+
+	die "'$prop' does not match for '$cfg->{name}' ($correct_prop != $configured_prop)\n"
+	    if $correct_prop ne $configured_prop;
+    }
+
+    return 1;
+};
+
+my $usb_valid = sub {
+    my ($cfg) = @_;
+
+    my $name = $cfg->{name};
+    my $vendor = $cfg->{vendor};
+    my $device = $cfg->{device};
+
+    my $usb_list = PVE::SysFSTools::scan_usb();
+
+    my $info;
+    if (my $path = $cfg->{path}) {
+	for my $dev (@$usb_list) {
+	    next if !$dev->{usbpath} || !$dev->{busnum};
+	    my $usbpath = "$dev->{busnum}-$dev->{usbpath}";
+	    next if $usbpath ne $path;
+	    $info = $dev;
+	}
+	die "usb device '$path' not found\n" if !defined($info);
+
+	die "'vendor' does not match for '$name'\n"
+	    if $info->{vendid} ne $cfg->{vendor};
+	die "'device' does not match for '$name'\n"
+	    if $info->{prodid} ne $cfg->{device};
+    } else {
+	for my $dev (@$usb_list) {
+	    next if $dev->{vendid} ne $vendor;
+	    next if $dev->{prodid} ne $device;
+	    $info = $dev;
+	}
+	die "usb device '$vendor:$device' not found\n" if !defined($info);
+    }
+
+    return 1;
+};
+
+sub assert_device_valid {
+    my ($type, $cfg) = @_;
+
+    if ($type eq 'usb') {
+	return $usb_valid->($cfg);
+    } elsif ($type eq 'pci') {
+	return $pci_valid->($cfg);
+    }
+
+    die "invalid type $type\n";
+}
+
+sub createSchema {
+    my ($type) = @_;
+
+    my $schema = {};
+
+    $schema->{name} = $name_format;
+    $schema->{node} = get_standard_option('pve-node');
+
+    for my $opt (sort keys $format->{$type}->%*) {
+	$schema->{$opt} = $format->{$type}->{$opt};
+    }
+
+    return {
+	additionalProperties => 0,
+	properties => $schema,
+    };
+
+}
+
+sub updateSchema {
+    my ($type) = @_;
+
+    my $schema = {};
+
+    $schema->{name} = $name_format;
+    $schema->{node} = get_standard_option('pve-node');
+
+    my $deletable = [];
+
+    for my $opt (sort keys $format->{$type}->%*) {
+	$schema->{$opt} = dclone($format->{$type}->{$opt});
+	$schema->{$opt}->{optional} = 1;
+	if ($format->{$type}->{$opt}->{optional}) {
+	    push @$deletable, $opt;
+	}
+    }
+
+    my $deletable_pattern = join('|', @$deletable);
+
+    $schema->{delete} = {
+	type => 'string', format => 'pve-configid-list',
+	description => "A list of settings you want to delete.",
+	maxLength => 4096,
+	optional => 1,
+    };
+
+    $schema->{digest} = get_standard_option('pve-config-digest');
+
+    return {
+	additionalProperties => 0,
+	properties => $schema,
+    };
+}
+
+sub options {
+    my ($type) = @_;
+
+    my $opts = {};
+
+    for my $opt (sort keys $format->{$type}->%*) {
+	$opts->{$opt}->{optional} = $format->{$type}->{$opt}->{optional};
+    }
+
+    return $opts;
+}
+
+1;
-- 
2.30.2

[pve-devel] [PATCH qemu-server 1/7] PVE/QemuServer: allow mapped usb devices in config

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm     |  2 ++
 PVE/QemuServer/USB.pm | 21 ++++++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 7d9cf22..a6ca80d 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1069,6 +1069,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.
 
diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index 3c8da2c..279078a 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use PVE::QemuServer::PCI qw(print_pci_addr);
 use PVE::JSONSchema;
+use PVE::HardwareMap;
 use base 'Exporter';
 
 our @EXPORT_OK = qw(
@@ -27,7 +28,25 @@ 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 $device = PVE::HardwareMap::find_device_on_current_node('usb', $value);
+	return undef if !defined($device);
+	eval {
+	    PVE::HardwareMap::assert_device_valid('usb', $device);
+	};
+	if (my $err = $@) {
+	    warn "USB Mapping invalid (hardware probably changed): $err\n";
+	    return;
+	}
+
+	if ($device->{path}) {
+	    $res = parse_usb_device($device->{path});
+	} else {
+	    $res->{vendorid} = $device->{vendor};
+	    $res->{productid} = $device->{device};
+	}
+
+	$res->{mapped} = 1;
     }
 
     return $res;
-- 
2.30.2

[pve-devel] [PATCH qemu-server 2/7] PVE/QemuServer: allow mapped pci deviced in config

[Dominik Csapak]

and get the correct pci device during parsing

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

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 23fe508..1ff4bce 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -4,6 +4,7 @@ use warnings;
 use strict;
 
 use PVE::JSONSchema;
+use PVE::HardwareMap;
 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 => {
@@ -383,6 +386,19 @@ sub parse_hostpci {
 
     my $res = PVE::JSONSchema::parse_property_string($hostpci_fmt, $value);
 
+    if ($res->{host} !~ m/:/) {
+	# we have no ordinary pci id, must be a mapping
+	my $device = PVE::HardwareMap::find_device_on_current_node('pci', $res->{host});
+	die "PCI device mapping not found for '$res->{host}'\n" if !defined($device);
+	eval {
+	    PVE::HardwareMap::assert_device_valid('pci', $device);
+	};
+	if (my $err = $@) {
+	    die "PCI device mapping invalid (hardware probably changed): $err\n";
+	}
+	$res->{host} = $device->{path};
+    }
+
     my @idlist = split(/;/, $res->{host});
     delete $res->{host};
     foreach my $id (@idlist) {
-- 
2.30.2

[pve-devel] [PATCH qemu-server 3/7] PVE/API2/Qemu: add permission checks for mapped usb devices

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 99b426e..aa7ddea 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -26,6 +26,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;
@@ -567,8 +568,12 @@ my $check_vm_create_usb_perm = sub {
 
     foreach my $opt (keys %{$param}) {
 	next if $opt !~ m/^usb\d+$/;
+	my $device = parse_usb_device($param->{$opt});
 
-	if ($param->{$opt} =~ m/spice/) {
+	if ($device->{spice}) {
+	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
+	} elsif ($device->{mapped}) {
+	    $rpcenv->check_hw_perm($authuser, $device->{host}, ['Hardware.Use']);
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
 	} else {
 	    die "only root can set '$opt' config for real devices\n";
@@ -1552,7 +1557,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::QemuServer::USB::parse_usb_device($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_hw_perm($authuser, $device->{host}, ['Hardware.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";
@@ -1613,7 +1623,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::QemuServer::USB::parse_usb_device($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_hw_perm($authuser, $olddevice->{host}, ['Hardware.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::QemuServer::USB::parse_usb_device($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_hw_perm($authuser, $newdevice->{host}, ['Hardware.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";
-- 
2.30.2

[pve-devel] [PATCH qemu-server 4/7] PVE/API2/Qemu: add permission checks for mapped pci devices

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm | 54 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index aa7ddea..a8029c2 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -26,6 +26,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;
@@ -583,6 +584,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_hw_perm($authuser, $device->{host}, ['Hardware.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) = @_;
 
@@ -593,7 +614,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+$/;
 
 
 	if ($cpuoptions->{$opt} || $opt =~ m/^numa\d+$/) {
@@ -621,7 +642,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";
 	}
@@ -856,6 +877,7 @@ __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);
 
 	    &$check_cpu_model_access($rpcenv, $authuser, $param);
 
@@ -1569,6 +1591,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_hw_perm($authuser, $olddevice->{host}, ['Hardware.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);
 		} else {
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
@@ -1651,6 +1683,24 @@ 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_hw_perm($authuser, $olddevice->{host}, ['Hardware.Use']);
+			}
+			$rpcenv->check_hw_perm($authuser, $newdevice->{host}, ['Hardware.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::QemuServer::PCI::parse_hostpci($param->{$opt});
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} else {
 		    $conf->{pending}->{$opt} = $param->{$opt};
-- 
2.30.2

[pve-devel] [PATCH qemu-server 5/7] PVE/QemuServer: extend 'check_local_resources' for mapped resources

[Dominik Csapak]

by adding them to their own list, saving the nodes where
they are not allowed, and return those on 'wantarray'

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

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a6ca80d..ea7e213 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2617,6 +2617,21 @@ sub check_local_resources {
     my ($conf, $noerr) = @_;
 
     my @loc_res = ();
+    my $mapped_res = [];
+
+    my $nodelist = PVE::Cluster::get_nodelist();
+    my $hw_map = PVE::HardwareMap::config();
+
+    my $not_allowed_nodes = { map { $_ => [] } @$nodelist };
+
+    my $add_not_allowed_nodes = sub {
+	my ($type, $key, $id) = @_;
+	for my $node (@$nodelist) {
+	    if (!defined($id) || !defined($hw_map->{$type}->{$id}->{$node})) {
+		push @{$not_allowed_nodes->{$node}}, $key;
+	    }
+	}
+    };
 
     push @loc_res, "hostusb" if $conf->{hostusb}; # old syntax
     push @loc_res, "hostpci" if $conf->{hostpci}; # old syntax
@@ -2624,7 +2639,24 @@ 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_not_allowed_nodes->('usb', $k, $entry->{host});
+		push @$mapped_res, $k;
+		next;
+	    }
+	}
+	if ($k =~ m/^hostpci/) {
+	    my $entry = parse_property_string('pve-qm-hostpci', $conf->{$k});
+	    if ($entry->{host} !~ m/:/) {
+		$add_not_allowed_nodes->('pci', $k, $entry->{host});
+		push @$mapped_res, $k;
+		next;
+	    }
+	}
 	# 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+$/;
@@ -2632,7 +2664,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, $not_allowed_nodes) : \@loc_res;
 }
 
 # check if used storages are available on all nodes (use by migrate)
-- 
2.30.2

[pve-devel] [PATCH qemu-server 6/7] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info

[Dominik Csapak]

restrict the nodes also for mapped devices, and return them in their
own property

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index a8029c2..72c8755 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4158,6 +4158,10 @@ __PACKAGE__->register_method({
 
 	$res->{running} = PVE::QemuServer::check_running($vmid) ? 1:0;
 
+	my ($local_resources, $mapped_resources, $not_allowed_nodes) =
+	    PVE::QemuServer::check_local_resources($vmconf, 1);
+	delete $not_allowed_nodes->{$localnode};
+
 	# if vm is not running, return target nodes where local storage is available
 	# for offline migration
 	if (!$res->{running}) {
@@ -4166,7 +4170,12 @@ __PACKAGE__->register_method({
 	    delete $checked_nodes->{$localnode};
 
 	    foreach my $node (keys %$checked_nodes) {
-		if (!defined $checked_nodes->{$node}->{unavailable_storages}) {
+		if (scalar(@{$not_allowed_nodes->{$node}})) {
+		    $checked_nodes->{$node}->{unavailable_resources} = $not_allowed_nodes->{$node};
+		    next;
+		}
+
+		if (!defined($checked_nodes->{$node}->{unavailable_storages})) {
 		    push @{$res->{allowed_nodes}}, $node;
 		}
 
@@ -4174,13 +4183,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

[pve-devel] [PATCH qemu-server 7/7] PVE/QemuMigrate: check for mapped resources on migration

[Dominik Csapak]

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>
---
 PVE/QemuMigrate.pm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index d52dc8d..d40beac 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -162,7 +162,7 @@ sub prepare {
 	$self->{vm_was_paused} = 1 if PVE::QemuServer::vm_is_paused($vmid);
     }
 
-    my $loc_res = PVE::QemuServer::check_local_resources($conf, 1);
+    my ($loc_res, $mapped_res, $not_allowed_nodes) = PVE::QemuServer::check_local_resources($conf, 1);
     if (scalar @$loc_res) {
 	if ($self->{running} || !$self->{opts}->{force}) {
 	    die "can't migrate VM which uses local devices: " . join(", ", @$loc_res) . "\n";
@@ -171,6 +171,17 @@ sub prepare {
 	}
     }
 
+    if (scalar @$mapped_res) {
+	my $not_available = $not_allowed_nodes->{$self->{node}};
+	if ($self->{running}) {
+	    die "can't migrate running VM which uses mapped devices: " . join(", ", @$mapped_res) . "\n";
+	} elsif (scalar @$not_available) {
+	    die "can't migrate to '$self->{node}': missing mapped devices" . join(", ", @$not_available) . "\n";
+	} else {
+	    $self->log('info', "migrating VM which uses mapped local devices");
+	}
+    }
+
     my $vollist = PVE::QemuServer::get_vm_volumes($conf);
     foreach my $volid (@$vollist) {
 	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
-- 
2.30.2

[pve-devel] [PATCH manager 01/12] PVE/API2/Hardware: add Mapping.pm

[Dominik Csapak]

adds the basic api calls to list/get/create/update/delete device
mappings

these api calls are only per node, so it only affects
the node specific mapping (thought consistency checks are
done for the whole config, e.g if an id exists already on another
node with a different type)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Hardware.pm         |   6 +
 PVE/API2/Hardware/Makefile   |   1 +
 PVE/API2/Hardware/Mapping.pm | 708 +++++++++++++++++++++++++++++++++++
 3 files changed, 715 insertions(+)
 create mode 100644 PVE/API2/Hardware/Mapping.pm

diff --git a/PVE/API2/Hardware.pm b/PVE/API2/Hardware.pm
index f59bfbe0..ab7b5e63 100644
--- a/PVE/API2/Hardware.pm
+++ b/PVE/API2/Hardware.pm
@@ -8,6 +8,7 @@ use PVE::RESTHandler;
 
 use PVE::API2::Hardware::PCI;
 use PVE::API2::Hardware::USB;
+use PVE::API2::Hardware::Mapping;
 
 use base qw(PVE::RESTHandler);
 
@@ -21,6 +22,10 @@ __PACKAGE__->register_method ({
     path => 'usb',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Hardware::Mapping",
+    path => "mapping",
+});
 
 __PACKAGE__->register_method ({
     name => 'index',
@@ -50,6 +55,7 @@ __PACKAGE__->register_method ({
 	my $res = [
 	    { type => 'pci' },
 	    { type => 'usb' },
+	    { type => 'mapping' },
 	];
 
 	return $res;
diff --git a/PVE/API2/Hardware/Makefile b/PVE/API2/Hardware/Makefile
index d27d2201..9f5f3231 100644
--- a/PVE/API2/Hardware/Makefile
+++ b/PVE/API2/Hardware/Makefile
@@ -3,6 +3,7 @@ include ../../../defines.mk
 PERLSOURCE=			\
 	PCI.pm			\
 	USB.pm			\
+	Mapping.pm			\
 
 all:
 
diff --git a/PVE/API2/Hardware/Mapping.pm b/PVE/API2/Hardware/Mapping.pm
new file mode 100644
index 00000000..72a46ffe
--- /dev/null
+++ b/PVE/API2/Hardware/Mapping.pm
@@ -0,0 +1,708 @@
+package PVE::API2::Hardware::Mapping::USB;
+
+use strict;
+use warnings;
+
+use Storable qw(dclone);
+
+use PVE::Cluster qw(cfs_lock_file);
+use PVE::HardwareMap;
+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 => "USB Hardware Mapping",
+    permissions => {
+	description => "Only lists entries where you have 'Hardware.Audit', 'Hardware.Use', 'Hardware.Configure' permissions on '/hardware/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => { name => { type => 'string'} },
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $node = $param->{node};
+
+	my $cfg = PVE::HardwareMap::config();
+
+	my $res = [];
+
+	my $privs = ['Hardware.Audit', 'Hardware.Use', 'Hardware.Configure'];
+
+	for my $id (keys $cfg->{usb}->%*) {
+
+	    next if !defined($cfg->{usb}->{$id}->{$node});
+	    next if !$rpcenv->check_hw_perm($authuser, $id, $privs, 1, 1);
+
+	    my $entry = dclone($cfg->{usb}->{$id}->{$node});
+	    $entry->{name} = $id;
+	    $entry->{node} = $node;
+	    $entry->{type} = 'usb';
+
+	    eval {
+		PVE::HardwareMap::assert_device_valid('usb', $entry);
+	    };
+	    if (my $err = $@) {
+		$entry->{valid} = 0;
+		$entry->{errmsg} = "$err";
+	    } else {
+		$entry->{valid} = 1;
+	    }
+
+	    push @$res, $entry;
+	}
+
+	return $res;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'get',
+    protected => 1,
+    proxyto => 'node',
+    path => '{name}',
+    method => 'GET',
+    description => "GET Hardware Mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Audit']],
+		    ['perm', '/hardware/{name}', ['Hardware.Audit']],
+		 ],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	    node => get_standard_option('pve-node'),
+	}
+    },
+    returns => { type => 'object' },
+    code => sub {
+	my ($param) = @_;
+
+	my $cfg = PVE::HardwareMap::config();
+	my $name = $param->{name};
+	my $node = $param->{node};
+
+	die "mapping '$param->{name}' not found on '$param->{node}'\n"
+	    if !defined($cfg->{usb}->{$name}) || !defined($cfg->{usb}->{$name}->{$node});
+
+	my $data = dclone($cfg->{usb}->{$name}->{$node});
+
+	eval {
+	    PVE::HardwareMap::assert_device_valid('usb', $data);
+	};
+	if (my $err = $@) {
+	    $data->{valid} = 0;
+	    $data->{errmsg} = "$err";
+	} else {
+	    $data->{valid} = 1;
+	}
+
+	$data->{digest} = $cfg->{digest};
+
+	return $data;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'create',
+    protected => 1,
+    proxyto => 'node',
+    path => '',
+    method => 'POST',
+    description => "Create a new hardware mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Modify']],
+		    ['perm', '/hardware/{name}', ['Hardware.Configure']],
+		 ],
+    },
+    # todo parameters
+    parameters => PVE::HardwareMap::createSchema('usb'),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $node = extract_param($param, 'node');
+
+	PVE::HardwareMap::assert_device_valid('usb', $param);
+
+	PVE::HardwareMap::lock_config(sub {
+	    my $cfg = PVE::HardwareMap::config();
+
+	    # avoid autovification
+	    if (defined($cfg->{usb}->{$name}) && defined($cfg->{usb}->{$name}->{$node})) {
+		die "mapping '$name' for node '$node' already defined\n";
+	    }
+
+	    for my $type (keys %$cfg) {
+		next if $type eq 'usb';
+		next if $type eq 'digest';
+		die "'$name' already defined as type '$type'\n"
+		    if defined($cfg->{$type}->{$name});
+	    }
+
+	    $cfg->{usb}->{$name}->{$node} = $param;
+
+	    PVE::HardwareMap::write_config($cfg);
+
+	}, "create hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'update',
+    protected => 1,
+    proxyto => 'node',
+    path => '{name}',
+    method => 'PUT',
+    description => "Update a hardware mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Modify']],
+		    ['perm', '/hardware/{name}', ['Hardware.Configure']],
+		 ],
+    },
+    parameters => PVE::HardwareMap::updateSchema('usb'),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+	my $name = extract_param($param, 'name');
+	my $node = extract_param($param, 'node');
+	if ($delete) {
+	    $delete = [ PVE::Tools::split_list($delete) ];
+	}
+
+	PVE::HardwareMap::lock_config(sub {
+	    my $cfg = PVE::HardwareMap::config();
+
+	    PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest);
+
+	    die "no mapping '$name' on node '$node'\n" if !$cfg->{usb}->{$name} || !$cfg->{usb}->{$name}->{$node};
+	    my $data = $cfg->{usb}->{$name}->{$node};
+
+	    for my $k (keys %$param) {
+		$data->{$k} = $param->{$k};
+	    }
+
+	    if ($delete) {
+		my $options = PVE::HardwareMap::options('usb');
+		for my $k (@$delete) {
+		    my $d = $options->{$k} || die "no such option '$k'\n";
+		    die "unable to delete required option '$k'\n" if !$d->{optional};
+		    die "unable to delete fixed option '$k'\n" if $d->{fixed};
+		    die "cannot set and delete property '$k' at the same time!\n"
+			if defined($param->{$k});
+
+		    delete $data->{$k};
+		}
+	    }
+
+	    PVE::HardwareMap::assert_device_valid('usb', $data);
+
+	    PVE::HardwareMap::write_config($cfg);
+
+	}, "update hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete',
+    protected => 1,
+    proxyto => 'node',
+    path => '{name}',
+    method => 'DELETE',
+    description => "Remove Hardware Mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Modify']],
+		    ['perm', '/hardware/{name}', ['Hardware.Configure']],
+		 ],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = $param->{name};
+	my $node = $param->{node};
+
+	PVE::HardwareMap::lock_config(sub {
+	    my $cfg = PVE::HardwareMap::config();
+
+	    if ($cfg->{usb}->{$name}) {
+		delete $cfg->{usb}->{$name}->{$node};
+		if (keys $cfg->{usb}->{$name}->%* < 1) {
+		    delete $cfg->{usb}->{$name};
+		}
+	    }
+
+	    PVE::HardwareMap::write_config($cfg);
+
+	}, "delete hardware mapping failed");
+
+	return;
+    }});
+
+package PVE::API2::Hardware::Mapping::PCI;
+
+use strict;
+use warnings;
+
+use Storable qw(dclone);
+
+use PVE::Cluster qw(cfs_lock_file);
+use PVE::HardwareMap;
+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 => "PCI Hardware Mapping",
+    permissions => {
+	description => "Only lists entries where you have 'Hardware.Audit', 'Hardware.Use', 'Hardware.Configure' permissions on '/hardware/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => { name => { type => 'string'} },
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $node = $param->{node};
+
+	my $cfg = PVE::HardwareMap::config();
+
+	my $res = [];
+
+	my $privs = ['Hardware.Audit', 'Hardware.Use', 'Hardware.Configure'];
+
+	for my $id (keys $cfg->{pci}->%*) {
+
+	    next if !defined($cfg->{pci}->{$id}->{$node});
+	    next if !$rpcenv->check_hw_perm($authuser, $id, $privs, 1, 1);
+
+	    my $entry = dclone($cfg->{pci}->{$id}->{$node});
+	    $entry->{name} = $id;
+	    $entry->{node} = $node;
+	    $entry->{type} = 'pci';
+
+	    eval {
+		PVE::HardwareMap::assert_device_valid('pci', $entry);
+	    };
+	    if (my $err = $@) {
+		$entry->{valid} = 0;
+		$entry->{errmsg} = "$err";
+	    } else {
+		$entry->{valid} = 1;
+	    }
+
+	    push @$res, $entry;
+	}
+
+	return $res;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'get',
+    protected => 1,
+    proxyto => 'node',
+    path => '{name}',
+    method => 'GET',
+    description => "GET Hardware Mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Audit']],
+		    ['perm', '/hardware/{name}', ['Hardware.Audit']],
+		 ],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	    node => get_standard_option('pve-node'),
+	}
+    },
+    returns => { type => 'object' },
+    code => sub {
+	my ($param) = @_;
+
+	my $cfg = PVE::HardwareMap::config();
+	my $name = $param->{name};
+	my $node = $param->{node};
+
+	die "mapping '$param->{name}' not found on '$param->{node}'\n"
+	    if !defined($cfg->{pci}->{$name}) || !defined($cfg->{pci}->{$name}->{$node});
+
+	my $data = dclone($cfg->{pci}->{$name}->{$node});
+
+	eval {
+	    PVE::HardwareMap::assert_device_valid('pci', $data);
+	};
+	if (my $err = $@) {
+	    $data->{valid} = 0;
+	    $data->{errmsg} = "$err";
+	} else {
+	    $data->{valid} = 1;
+	}
+
+	$data->{digest} = $cfg->{digest};
+
+	return $data;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'create',
+    protected => 1,
+    proxyto => 'node',
+    path => '',
+    method => 'POST',
+    description => "Create a new hardware mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Modify']],
+		    ['perm', '/hardware/{name}', ['Hardware.Configure']],
+		 ],
+    },
+    # todo parameters
+    parameters => PVE::HardwareMap::createSchema('pci'),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $node = extract_param($param, 'node');
+
+	PVE::HardwareMap::assert_device_valid('pci', $param);
+
+	PVE::HardwareMap::lock_config(sub {
+	    my $cfg = PVE::HardwareMap::config();
+
+	    # avoid autovification
+	    if (defined($cfg->{pci}->{$name}) && defined($cfg->{pci}->{$name}->{$node})) {
+		die "mapping '$name' for node '$node' already defined\n";
+	    }
+
+	    for my $type (keys %$cfg) {
+		next if $type eq 'pci';
+		next if $type eq 'digest';
+		die "'$name' already defined as type '$type'\n"
+		    if defined($cfg->{$type}->{$name});
+	    }
+
+	    $cfg->{pci}->{$name}->{$node} = $param;
+
+	    PVE::HardwareMap::write_config($cfg);
+
+	}, "create hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'update',
+    protected => 1,
+    proxyto => 'node',
+    path => '{name}',
+    method => 'PUT',
+    description => "Update a hardware mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Modify']],
+		    ['perm', '/hardware/{name}', ['Hardware.Configure']],
+		 ],
+    },
+    parameters => PVE::HardwareMap::updateSchema('pci'),
+    returns => {
+	type => 'null',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+	my $name = extract_param($param, 'name');
+	my $node = extract_param($param, 'node');
+	if ($delete) {
+	    $delete = [ PVE::Tools::split_list($delete) ];
+	}
+
+	PVE::HardwareMap::lock_config(sub {
+	    my $cfg = PVE::HardwareMap::config();
+
+	    PVE::Tools::assert_if_modified($cfg->{digest}, $digest) if defined($digest);
+
+	    die "no mapping '$name' on node '$node'\n" if !$cfg->{pci}->{$name} || !$cfg->{pci}->{$name}->{$node};
+	    my $data = $cfg->{pci}->{$name}->{$node};
+
+	    for my $k (keys %$param) {
+		$data->{$k} = $param->{$k};
+	    }
+
+	    if ($delete) {
+		my $options = PVE::HardwareMap::options('pci');
+		for my $k (@$delete) {
+		    my $d = $options->{$k} || die "no such option '$k'\n";
+		    die "unable to delete required option '$k'\n" if !$d->{optional};
+		    die "unable to delete fixed option '$k'\n" if $d->{fixed};
+		    die "cannot set and delete property '$k' at the same time!\n"
+			if defined($param->{$k});
+
+		    delete $data->{$k};
+		}
+	    }
+
+	    PVE::HardwareMap::assert_device_valid('pci', $data);
+
+	    PVE::HardwareMap::write_config($cfg);
+
+	}, "update hardware mapping failed");
+
+	return;
+    },
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete',
+    protected => 1,
+    proxyto => 'node',
+    path => '{name}',
+    method => 'DELETE',
+    description => "Remove Hardware Mapping.",
+    permissions => {
+	check => [ 'and',
+		    ['perm', '/node/{node}', ['Sys.Modify']],
+		    ['perm', '/hardware/{name}', ['Hardware.Configure']],
+		 ],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = $param->{name};
+	my $node = $param->{node};
+
+	PVE::HardwareMap::lock_config(sub {
+	    my $cfg = PVE::HardwareMap::config();
+
+	    if ($cfg->{pci}->{$name}) {
+		delete $cfg->{pci}->{$name}->{$node};
+		if (keys $cfg->{pci}->{$name}->%* < 1) {
+		    delete $cfg->{pci}->{$name};
+		}
+	    }
+
+	    PVE::HardwareMap::write_config($cfg);
+
+	}, "delete hardware mapping failed");
+
+	return;
+    }});
+
+package PVE::API2::Hardware::Mapping;
+
+use strict;
+use warnings;
+
+use Storable qw(dclone);
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Hardware::Mapping::PCI",
+    path => 'pci',
+});
+
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Hardware::Mapping::USB",
+    path => 'usb',
+});
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => "Index of hardware types",
+    permissions => {
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => { type => { type => 'string'} },
+	},
+	links => [ { rel => 'child', href => "{type}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $res = [
+	    { type => 'pci' },
+	    { type => 'usb' },
+	    { type => 'all' },
+	];
+
+	return $res;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'get_all',
+    path => 'all',
+    protected => 1,
+    proxyto => 'node',
+    method => 'GET',
+    description => "Hardware Mapping",
+    permissions => {
+	description => "Only lists entries where you have 'Hardware.Audit', 'Hardware.Use', 'Hardware.Configure' permissions on '/hardware/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => { name => { type => 'string'} },
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $node = $param->{node};
+
+	my $cfg = PVE::HardwareMap::config();
+
+	my $res = [];
+
+	my $privs = ['Hardware.Audit', 'Hardware.Use', 'Hardware.Configure'];
+
+	for my $type (keys $cfg->%*) {
+	    next if $type eq 'digest';
+	    for my $id (keys $cfg->{$type}->%*) {
+
+		next if !defined($cfg->{$type}->{$id}->{$node});
+		next if !$rpcenv->check_hw_perm($authuser, $id, $privs, 1, 1);
+
+		my $entry = dclone($cfg->{$type}->{$id}->{$node});
+		$entry->{name} = $id;
+		$entry->{node} = $node;
+		$entry->{type} = $type;
+
+		eval {
+		    PVE::HardwareMap::assert_device_valid($type, $entry);
+		};
+		if (my $err = $@) {
+		    $entry->{valid} = 0;
+		    $entry->{errmsg} = "$err";
+		} else {
+		    $entry->{valid} = 1;
+		}
+
+		push @$res, $entry;
+	    }
+	}
+
+	return $res;
+    },
+});
+
+1;
-- 
2.30.2

[pve-devel] [PATCH manager 02/12] PVE/API2/Cluster: add Hardware mapping list api call

[Dominik Csapak]

this is a cluster wide api call that returns the list of mappings
in a manner that is easy to consume by the ui (as a tree)

it also automatically includes the validity of mappings on the node
where it is called.

for a consumer of this api call to get a complete picture, it is
necessary to do an api call for each node

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Cluster.pm          |   8 +++
 PVE/API2/Cluster/Hardware.pm | 117 +++++++++++++++++++++++++++++++++++
 PVE/API2/Cluster/Makefile    |   1 +
 3 files changed, 126 insertions(+)
 create mode 100644 PVE/API2/Cluster/Hardware.pm

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 525a95a1..8954d040 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -25,6 +25,7 @@ use PVE::API2::ACMEPlugin;
 use PVE::API2::Backup;
 use PVE::API2::Cluster::BackupInfo;
 use PVE::API2::Cluster::Ceph;
+use PVE::API2::Cluster::Hardware;
 use PVE::API2::Cluster::Jobs;
 use PVE::API2::Cluster::MetricServer;
 use PVE::API2::ClusterConfig;
@@ -89,6 +90,12 @@ __PACKAGE__->register_method ({
     subclass => "PVE::API2::Cluster::Jobs",
     path => 'jobs',
 });
+
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Cluster::Hardware",
+    path => 'hardware',
+});
+
 if ($have_sdn) {
     __PACKAGE__->register_method ({
        subclass => "PVE::API2::Network::SDN",
@@ -138,6 +145,7 @@ __PACKAGE__->register_method ({
 	    { name => 'firewall' },
 	    { name => 'ha' },
 	    { name => 'jobs' },
+	    { name => 'hardware' },
 	    { name => 'log' },
 	    { name => 'metrics' },
 	    { name => 'nextid' },
diff --git a/PVE/API2/Cluster/Hardware.pm b/PVE/API2/Cluster/Hardware.pm
new file mode 100644
index 00000000..19a9ba27
--- /dev/null
+++ b/PVE/API2/Cluster/Hardware.pm
@@ -0,0 +1,117 @@
+package PVE::API2::Cluster::Hardware;
+
+use strict;
+use warnings;
+
+use PVE::HardwareMap;
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => "Hardware index.",
+    permissions => { user => 'all' },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => {},
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $result = [
+	    { name => 'mapping' },
+	];
+
+	return $result;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'mapping_index',
+    path => 'mapping',
+    method => 'GET',
+    description => "List mapped hardware.",
+    permissions => {
+	description => "Only lists entries where you have 'Hardware.Audit', 'Hardware.Use', 'Hardware.Configure' permissions on '/hardware/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => {
+		type => 'string',
+		format => 'pve-node',
+		description => "Only show hardware mapped on this node.",
+		optional => 1,
+	    }
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $res = [];
+	my $cfg = PVE::HardwareMap::config();
+	my $nodename = PVE::INotify::nodename();
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $privs = ['Hardware.Audit', 'Hardware.Use', 'Hardware.Configure'];
+
+	for my $type (keys %$cfg) {
+	    next if $type eq 'digest';
+	    for my $id (keys $cfg->{$type}->%*) {
+		next if !$rpcenv->check_hw_perm($authuser, $id, $privs, 1, 1);
+		my $id_entry = {
+		    text => $id,
+		    ntype => 'entry',
+		    type => $type,
+		    children => [],
+		};
+		for my $node (keys $cfg->{$type}->{$id}->%*) {
+		    my $entry = {
+			text => $node,
+			node => $node,
+			entry => $id,
+			type => $type,
+			ntype => 'mapping',
+		    };
+		    for my $p (keys $cfg->{$type}->{$id}->{$node}->%*) {
+			$entry->{$p} = $cfg->{$type}->{$id}->{$node}->{$p};
+		    }
+		    if ($nodename eq $node) {
+			eval {
+			    PVE::HardwareMap::assert_device_valid($type, $entry);
+			};
+			if (my $err = $@) {
+			    $entry->{valid} = 0;
+			    $entry->{errmsg} = "$err";
+			} else {
+			    $entry->{valid} = 1;
+			}
+		    }
+		    push $id_entry->{children}->@*, $entry;
+		}
+		push @$res, $id_entry;
+	    }
+	}
+
+	return $res;
+    }});
+
+1;
diff --git a/PVE/API2/Cluster/Makefile b/PVE/API2/Cluster/Makefile
index 8d306507..2d9e2dde 100644
--- a/PVE/API2/Cluster/Makefile
+++ b/PVE/API2/Cluster/Makefile
@@ -5,6 +5,7 @@ include ../../../defines.mk
 PERLSOURCE= 			\
 	BackupInfo.pm		\
 	MetricServer.pm		\
+	Hardware.pm			\
 	Jobs.pm			\
 	Ceph.pm
 
-- 
2.30.2

[pve-devel] [PATCH manager 03/12] ui: form/USBSelector: make it more flexible with nodename

[Dominik Csapak]

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, 24 insertions(+), 8 deletions(-)

diff --git a/www/manager6/form/USBSelector.js b/www/manager6/form/USBSelector.js
index 0d511699..3a2f293e 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();
+    },
 
-	if (!nodename) {
-	    throw "no nodename specified";
+    initComponent: function() {
+	var me = this;
+
+	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,6 +113,8 @@ Ext.define('PVE.form.USBSelector', {
 
 	me.callParent();
 
+	me.setNodename(nodename);
+
 	store.load();
     },
 
-- 
2.30.2

[pve-devel] [PATCH manager 04/12] ui: form: add PCIMapSelector

[Dominik Csapak]

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 | 95 +++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)
 create mode 100644 www/manager6/form/PCIMapSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index d16770b1..20d508e7 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -48,6 +48,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..8f9b570c
--- /dev/null
+++ b/www/manager6/form/PCIMapSelector.js
@@ -0,0 +1,95 @@
+Ext.define('PVE.form.PCIMapSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    xtype: 'pvePCIMapSelector',
+
+    store: {
+	fields: ['name', 'path', 'vendor', 'device', 'iommugroup', 'mdev'],
+	filterOnLoad: true,
+	sorters: [
+	    {
+		property: 'name',
+		direction: 'ASC',
+	    },
+	],
+    },
+
+    autoSelect: false,
+    valueField: 'name',
+    displayField: 'name',
+
+    // can contain a load callback for the store
+    // useful to determine the state of the IOMMU
+    onLoadCallBack: undefined,
+
+    listConfig: {
+	width: 800,
+	columns: [
+	    {
+		header: gettext('Name'),
+		dataIndex: 'name',
+		flex: 1,
+	    },
+	    {
+		header: gettext('IOMMU Group'),
+		dataIndex: 'iommugroup',
+		renderer: v => v === undefined ? '-' : v,
+		width: 75,
+	    },
+	    {
+		header: gettext('Path'),
+		dataIndex: 'path',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Vendor'),
+		dataIndex: 'vendor',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Device'),
+		dataIndex: 'device',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Mediated Devices'),
+		dataIndex: 'mdev',
+		flex: 1,
+		renderer: function(val) {
+		    return Proxmox.Utils.format_boolean(!!val);
+		},
+	    },
+	],
+    },
+
+    setNodename: function(nodename) {
+	var me = this;
+
+	if (!nodename || me.nodename === nodename) {
+	    return;
+	}
+
+	me.nodename = nodename;
+
+	me.store.setProxy({
+	    type: 'proxmox',
+	    url: `/api2/json/nodes/${me.nodename}/hardware/mapping/pci`,
+	});
+
+	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

[pve-devel] [PATCH manager 05/12] ui: form: add USBMapSelector

[Dominik Csapak]

similar to PCIMapSelector

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

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 20d508e7..f5ae5364 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -67,6 +67,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..12495d01
--- /dev/null
+++ b/www/manager6/form/USBMapSelector.js
@@ -0,0 +1,73 @@
+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: 'name',
+    valueField: 'name',
+
+    listConfig: {
+	columns: [
+	    {
+		header: gettext('Name'),
+		dataIndex: 'name',
+		flex: 2,
+	    },
+	    {
+		header: gettext('Path'),
+		dataIndex: 'path',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Vendor'),
+		dataIndex: 'vendor',
+		flex: 1,
+	    },
+	    {
+		header: gettext('Device'),
+		dataIndex: 'device',
+		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/nodes/${me.nodename}/hardware/mapping/usb`,
+	});
+
+	me.store.load();
+    },
+
+    initComponent: function() {
+	var me = this;
+
+	var nodename = me.nodename;
+	me.nodename = undefined;
+
+        me.callParent();
+
+	me.setNodename(nodename);
+    },
+});
-- 
2.30.2

[pve-devel] [PATCH manager 06/12] ui: qemu/PCIEdit: rework panel to add a mapped configuration

[Dominik Csapak]

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 | 231 +++++++++++++++++++++++------------
 1 file changed, 155 insertions(+), 76 deletions(-)

diff --git a/www/manager6/qemu/PCIEdit.js b/www/manager6/qemu/PCIEdit.js
index 2f67aece..4718d508 100644
--- a/www/manager6/qemu/PCIEdit.js
+++ b/www/manager6/qemu/PCIEdit.js
@@ -3,71 +3,106 @@ 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'));
-	}
+	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) {
-	    me.down('field[name=romfile]').setVisible(true);
-	}
-    },
+	    if (values.hostmapped) {
+		values.host = values.hostmapped;
+		delete values.hostmapped;
+	    } else {
+		values.host.replace(/^0000:/, ''); // remove optional '0000' domain
 
-    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;
+		if (values.multifunction) {
+		    values.host = values.host.substring(0, values.host.indexOf('.')); // skip the '.X'
+		    delete values.multifunction;
 		}
 	    }
-	    // 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 (values.rombar) {
+		delete values.rombar;
+	    } else {
+		values.rombar = 0;
+	    }
 
-	if (values.rombar) {
-	    delete values.rombar;
-	} else {
-	    values.rombar = 0;
-	}
+	    if (!values.romfile) {
+		delete values.romfile;
+	    }
 
-	if (!values.romfile) {
-	    delete values.romfile;
-	}
+	    delete values.type;
 
-	let ret = {};
-	ret[me.confid] = PVE.Parser.printPropertyString(values, 'host');
-	return ret;
+	    let ret = {};
+	    ret[view.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,28 +113,77 @@ 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}',
+		},
+	    },
+	    {
+		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) {
@@ -129,27 +213,20 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 			    }
 			    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);
-			}
+			me.lookup('group_warning').setVisible(count > 0);
 		    },
 		},
 	    },
 	    {
 		xtype: 'proxmoxcheckbox',
 		fieldLabel: gettext('All Functions'),
+		reference: 'all_functions',
+		disabled: true,
+		labelAlign: 'right',
 		name: 'multifunction',
+		bind: {
+		    disabled: '{isMapped}',
+		},
 	    },
 	];
 
@@ -188,6 +265,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 		submitValue: true,
 		hidden: true,
 		fieldLabel: 'ROM-File',
+		reference: 'romfile',
 		name: 'romfile',
 	    },
 	    {
@@ -214,6 +292,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 	    {
 		xtype: 'proxmoxcheckbox',
 		fieldLabel: 'PCI-Express',
+		reference: 'pcie',
 		name: 'pcie',
 	    },
 	    {
-- 
2.30.2

[pve-devel] [PATCH manager 07/12] ui: qemu/USBEdit: add 'mapped' device case

[Dominik Csapak]

to be able to select 'mapped' usb devices

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

diff --git a/www/manager6/qemu/USBEdit.js b/www/manager6/qemu/USBEdit.js
index a2204584..82af78f9 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: {},
     },
@@ -31,6 +40,7 @@ Ext.define('PVE.qemu.USBInputPanel', {
 	    case 'spice':
 		val = 'spice';
 		break;
+	    case 'mapped':
 	    case 'hostdevice':
 	    case 'port':
 		val = 'host=' + values[type];
@@ -61,6 +71,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',
@@ -145,7 +172,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++) {
@@ -157,6 +184,10 @@ Ext.define('PVE.qemu.USBEdit', {
 			port = data[i];
 			port = port.replace('host=', '');
 			type = 'port';
+		    } else if (/^(host=)?[a-zA-Z0-9\-_]+$/.test(data[i])) {
+			mapped = data[i];
+			mapped = mapped.replace('host=', '');
+			type = 'mapped';
 		    }
 
 		    if (/^usb3=(1|on|true)$/.test(data[i])) {
@@ -168,6 +199,7 @@ Ext.define('PVE.qemu.USBEdit', {
 		    hostdevice: hostdevice,
 		    port: port,
 		    usb3: usb3,
+		    mapped,
 		};
 
 		ipanel.setValues(values);
-- 
2.30.2

[pve-devel] [PATCH manager 08/12] ui: add window/PCIEdit: edit window for pci mappings

[Dominik Csapak]

allows to add a single host mapping for pci entries

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Makefile            |   1 +
 www/manager6/form/PCISelector.js |  17 +-
 www/manager6/window/PCIEdit.js   | 323 +++++++++++++++++++++++++++++++
 3 files changed, 340 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/window/PCIEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index f5ae5364..dad2e6ca 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -115,6 +115,7 @@ JSSRC= 							\
 	window/ScheduleSimulator.js			\
 	window/Wizard.js				\
 	window/GuestDiskReassign.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/PCIEdit.js b/www/manager6/window/PCIEdit.js
new file mode 100644
index 00000000..475b91f1
--- /dev/null
+++ b/www/manager6/window/PCIEdit.js
@@ -0,0 +1,323 @@
+Ext.define('PVE.node.PCIEditWindow', {
+    extend: 'Proxmox.window.Edit',
+
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    title: gettext('Add PCI mapping'),
+
+    onlineHelp: 'qm_pci_passthrough',
+
+    method: 'POST',
+
+    cbindData: function(initialConfig) {
+	let me = this;
+	me.isCreate = !me.name || !me.nodename;
+	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 `/nodes/${data.node}/hardware/mapping/pci/${name}`;
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	onGetValues: function(values) {
+	    let me = this;
+
+	    if (values.iommugroup === -1) {
+		delete values.iommugroup;
+	    }
+
+	    if (values.multifunction) {
+		values.path = values.path.substring(0, values.path.indexOf('.')); // skip the '.X'
+		delete values.multifunction;
+	    }
+
+	    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),
+	    );
+	},
+
+	allFunctionsChange: function(_, value) {
+	    let me = this;
+	    if (value) {
+		let pcisel = me.lookup('pciselector');
+		let pcivalue = pcisel.getValue();
+		// replace the function by .0 so that we get the correct vendor/device
+		pcivalue = pcivalue.replace(/.$/, "0");
+		pcisel.setValue(pcivalue);
+	    }
+	},
+
+	pciChange: function(pcisel, value) {
+	    let me = this;
+	    if (!value) {
+		return;
+	    }
+	    let allField = me.lookup('all_functions');
+	    let all_functions = !!allField.getValue();
+
+	    // if we set the saved value without the function
+	    if (value.indexOf('.') === -1) {
+		all_functions = true;
+		allField.setValue(true);
+		allField.resetOriginalValue();
+
+		value += '.0';
+		pcisel.setValue(value);
+		pcisel.resetOriginalValue();
+	    } else if (all_functions) {
+		// replace the function by .0 so that we get the correct vendor/device
+		let newvalue = value.replace(/.$/, "0");
+		if (newvalue !== value) {
+		    pcisel.setValue(value);
+		}
+	    }
+
+	    let pciDev = pcisel.getStore().getById(value);
+	    if (!pciDev) {
+		return;
+	    }
+	    let iommu = pciDev.data.iommugroup;
+	    // 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;
+	    });
+
+	    me.lookup('group_warning').setVisible(count > 0);
+
+	    let fields = [
+		'vendor',
+		'device',
+		'subsystem-vendor',
+		'subsystem-device',
+		'mdev',
+		'iommugroup',
+	    ];
+
+	    fields.forEach((fieldName) => {
+		let field = me.lookup(fieldName);
+		let oldValue = field.getValue();
+		if (oldValue !== pciDev.data[fieldName]) {
+		    field.setValue(pciDev.data[fieldName]);
+		}
+	    });
+	},
+
+	mdevChange: function(mdevField, value) {
+	    let val = Proxmox.Utils.format_boolean(!!value);
+	    this.lookup('mdev-display').setValue(val);
+	},
+
+	nodeChange: function(_field, value) {
+	    this.lookup('pciselector').setNodename(value);
+	},
+
+	init: function(view) {
+	    let me = this;
+
+	    if (!view.nodename) {
+		//throw "no nodename given";
+	    }
+	},
+
+	control: {
+	    'field[name=multifunction]': {
+		change: 'allFunctionsChange',
+	    },
+	    'field[name=path]': {
+		change: 'pciChange',
+	    },
+	    'field[name=mdev]': {
+		change: 'mdevChange',
+	    },
+	    'pveNodeSelector': {
+		change: 'nodeChange',
+	    },
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+	    onGetValues: function(values) {
+		return this.up('window').getController().onGetValues(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: '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: 'name',
+		    allowBlank: false,
+		},
+		{
+		    xtype: 'pmxDisplayEditField',
+		    fieldLabel: gettext('Node'),
+		    labelWidth: 120,
+		    name: 'node',
+		    editConfig: {
+			xtype: 'pveNodeSelector',
+		    },
+		    cbind: {
+			editable: '{!nodename}',
+			value: '{nodename}',
+		    },
+		    submitValue: true,
+		    allowBlank: false,
+		},
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext('Vendor'),
+		    labelWidth: 120,
+		    submitValue: true,
+		    reference: 'vendor',
+		    name: 'vendor',
+		},
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext('Subsystem Vendor'),
+		    labelWidth: 120,
+		    submitValue: true,
+		    reference: 'subsystem-vendor',
+		    name: 'subsystem-vendor',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    reference: 'mdev',
+		    hidden: true,
+		    name: 'mdev',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext('Mediated Devices'),
+		    labelWidth: 120,
+		    value: Proxmox.Utils.noText,
+		    reference: 'mdev-display',
+		    name: 'mdev-display',
+		},
+	    ],
+
+	    column2: [
+		{
+		    xtype: 'pvePCISelector',
+		    fieldLabel: gettext('Device'),
+		    labelWidth: 120,
+		    reference: 'pciselector',
+		    name: 'path',
+		    cbind: {
+			nodename: '{nodename}',
+		    },
+		    allowBlank: false,
+		    onLoadCallBack: 'checkIommu',
+		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    fieldLabel: gettext('All Functions'),
+		    labelWidth: 120,
+		    reference: 'all_functions',
+		    name: 'multifunction',
+		},
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext('Device'),
+		    labelWidth: 120,
+		    submitValue: true,
+		    reference: 'device',
+		    name: 'device',
+		},
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext('Subsystem Device'),
+		    labelWidth: 120,
+		    submitValue: true,
+		    reference: 'subsystem-device',
+		    name: 'subsystem-device',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext('IOMMU group'),
+		    labelWidth: 120,
+		    submitValue: true,
+		    reference: 'iommugroup',
+		    name: 'iommugroup',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+
+	    columnB: [
+		{
+		    xtype: 'proxmoxtextfield',
+		    fieldLabel: gettext('Comment'),
+		    labelWidth: 120,
+		    submitValue: true,
+		    name: 'comment',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+	},
+    ],
+});
-- 
2.30.2

[pve-devel] [PATCH manager 09/12] ui: add window/USBEdit: edit window for usb mappings

[Dominik Csapak]

allows to add a single host mapping for usb entries

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

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index dad2e6ca..f6687ce5 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -116,6 +116,7 @@ JSSRC= 							\
 	window/Wizard.js				\
 	window/GuestDiskReassign.js				\
 	window/PCIEdit.js				\
+	window/USBEdit.js				\
 	ha/Fencing.js					\
 	ha/GroupEdit.js					\
 	ha/GroupSelector.js				\
diff --git a/www/manager6/window/USBEdit.js b/www/manager6/window/USBEdit.js
new file mode 100644
index 00000000..1fec541e
--- /dev/null
+++ b/www/manager6/window/USBEdit.js
@@ -0,0 +1,248 @@
+Ext.define('PVE.node.USBEditWindow', {
+    extend: 'Proxmox.window.Edit',
+
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    cbindData: function(initialConfig) {
+	let me = this;
+	me.isCreate = !me.name || !me.nodename;
+	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 `/nodes/${data.node}/hardware/mapping/usb/${name}`;
+    },
+
+    title: gettext('Add USB mapping'),
+
+    onlineHelp: 'qm_usb_passthrough',
+
+    method: 'POST',
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	onGetValues: function(values) {
+	    let me = this;
+
+	    var type = me.getView().down('radiofield').getGroupValue();
+
+	    let val = values[type];
+	    delete values[type];
+
+	    let usbsel = me.lookup(type);
+	    let usbDev = usbsel.getStore().findRecord('usbid', val, 0, false, true, true);
+	    if (!usbDev) {
+		return {};
+	    }
+
+	    if (type === 'path') {
+		values.path = val;
+	    } else if (!me.getView().isCreate) {
+		values.delete = 'path';
+	    }
+
+	    values.vendor = usbDev.data.vendid;
+	    values.device = usbDev.data.prodid;
+
+	    return values;
+	},
+
+	usbPathChange: function(usbsel, value) {
+	    let me = this;
+	    if (!value) {
+		return;
+	    }
+
+	    let usbDev = usbsel.getStore().findRecord('usbid', value, 0, false, true, true);
+	    if (!usbDev) {
+		return;
+	    }
+
+	    let usbData = {
+		vendor: usbDev.data.vendid,
+		device: usbDev.data.prodid,
+	    };
+
+	    ['vendor', 'device'].forEach((fieldName) => {
+		let field = me.lookup(fieldName);
+		let oldValue = field.getValue();
+		if (oldValue !== usbData[fieldName]) {
+		    field.setValue(usbData[fieldName]);
+		}
+	    });
+	},
+
+	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('hostdevice').setNodename(value);
+	    this.lookup('path').setNodename(value);
+	},
+
+
+	init: function(view) {
+	    let me = this;
+
+	    if (!view.nodename) {
+		//throw "no nodename given";
+	    }
+	},
+
+	control: {
+	    'field[name=path]': {
+		change: 'usbPathChange',
+	    },
+	    'radiofield': {
+		change: 'modeChange',
+	    },
+	    'pveNodeSelector': {
+		change: 'nodeChange',
+	    },
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+	    onGetValues: function(values) {
+		return this.up('window').getController().onGetValues(values);
+	    },
+
+
+	    column1: [
+		{
+		    xtype: 'proxmoxtextfield',
+		    hidden: true,
+		    reference: 'vendor',
+		    name: 'vendor',
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    hidden: true,
+		    reference: 'device',
+		    name: 'device',
+		},
+		{
+		    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}',
+		    },
+		    submitValue: true,
+		    allowBlank: false,
+		},
+	    ],
+
+	    column2: [
+		{
+		    xtype: 'fieldcontainer',
+		    defaultType: 'radiofield',
+		    layout: 'fit',
+		    items: [
+			{
+			    name: 'usb',
+			    inputValue: 'hostdevice',
+			    checked: true,
+			    boxLabel: gettext('Use USB Vendor/Device ID'),
+			    submitValue: false,
+			},
+			{
+			    xtype: 'pveUSBSelector',
+			    type: 'device',
+			    reference: 'hostdevice',
+			    name: 'hostdevice',
+			    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: 'comment',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+	me.callParent();
+
+	if (!me.name || !me.nodename) {
+	    return;
+	}
+	me.load({
+	    success: function(response) {
+		let data = response.result.data;
+		if (data.path) {
+		    data.usb = 'path';
+		} else {
+		    data.usb = 'hostdevice';
+		    data.hostdevice = `${data.vendor}:${data.device}`;
+		}
+		me.setValues(data);
+	    },
+	});
+    },
+});
-- 
2.30.2

[pve-devel] [PATCH manager 10/12] ui: add dc/HardwareView: a CRUD interface for hardware mapping

[Dominik Csapak]

it's possible to add/edit/remove mappings here, with a cluster
wide view on the mappings and validity.

to do that, we have to to an api call for each node, since
we don't have the pci status synced across them.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Makefile           |   1 +
 www/manager6/dc/Config.js       |  18 +-
 www/manager6/dc/HardwareView.js | 314 ++++++++++++++++++++++++++++++++
 3 files changed, 331 insertions(+), 2 deletions(-)
 create mode 100644 www/manager6/dc/HardwareView.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index f6687ce5..e0f92169 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -162,6 +162,7 @@ JSSRC= 							\
 	dc/UserEdit.js					\
 	dc/UserView.js					\
 	dc/MetricServerView.js				\
+	dc/HardwareView.js				\
 	lxc/CmdMenu.js					\
 	lxc/Config.js					\
 	lxc/CreateWizard.js				\
diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index 13ded12e..37148588 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -255,8 +255,22 @@ Ext.define('PVE.dc.Config', {
 		iconCls: 'fa fa-bar-chart',
 		itemId: 'metricservers',
 		onlineHelp: 'external_metric_server',
-	    },
-	    {
+	    });
+	}
+
+	if (caps.hardware['Hardware.Use'] ||
+	    caps.hardware['Hardware.Audit'] ||
+	    caps.hardware['Hardware.Configure']) {
+	    me.items.push({
+		xtype: 'pveDcHardwareView',
+		title: gettext('Hardware'),
+		iconCls: 'fa fa-desktop',
+		itemId: 'hardware',
+	    });
+	}
+
+	if (caps.dc['Sys.Audit']) {
+	    me.items.push({
 		xtype: 'pveDcSupport',
 		title: gettext('Support'),
 		itemId: 'support',
diff --git a/www/manager6/dc/HardwareView.js b/www/manager6/dc/HardwareView.js
new file mode 100644
index 00000000..f85a1088
--- /dev/null
+++ b/www/manager6/dc/HardwareView.js
@@ -0,0 +1,314 @@
+Ext.define('pve-hardware-tree', {
+    extend: 'Ext.data.Model',
+    fields: ['type', 'text', 'path', 'ntype',
+	{
+	    name: 'vendor',
+	    type: 'string',
+	},
+	{
+	    name: 'device',
+	    type: 'string',
+	},
+	{
+	    name: 'iconCls',
+	    calculate: function(data) {
+		if (data.ntype === 'entry') {
+		    if (data.type === 'usb') {
+			return 'fa fa-fw fa-usb';
+		    }
+		    if (data.type === 'pci') {
+			return 'pve-itype-icon-pci';
+		    }
+		    return 'fa fa-fw fa-folder-o';
+		}
+
+		return 'fa fa-fw fa-building';
+	    },
+	},
+	{
+	    name: 'leaf',
+	    calculate: function(data) {
+		return data.ntype && data.ntype !== 'entry';
+	    },
+	},
+    ],
+
+});
+
+Ext.define('PVE.dc.HardwareView', {
+    extend: 'Ext.tree.Panel',
+    alias: 'widget.pveDcHardwareView',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    rootVisible: false,
+
+    cbindData: function(initialConfig) {
+	let me = this;
+	const caps = Ext.state.Manager.get('GuiCap');
+	me.canConfigure = !!caps.nodes['Sys.Modify'] && !!caps.hardware['Hardware.Configure'];
+
+	return {};
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	addPCI: function() {
+	    let me = this;
+	    let nodename = Proxmox.NodeName;
+	    Ext.create('PVE.node.PCIEditWindow', {
+		url: `/nodes/${nodename}/hardware/mapping/pci`,
+		autoShow: true,
+		listeners: {
+		    destroy: () => me.load(),
+		},
+	    });
+	},
+
+	addUSB: function() {
+	    let me = this;
+	    let nodename = Proxmox.NodeName;
+	    Ext.create('PVE.node.USBEditWindow', {
+		url: `/nodes/${nodename}/hardware/mapping/usb`,
+		autoShow: true,
+		listeners: {
+		    destroy: () => me.load(),
+		},
+	    });
+	},
+
+	edit: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (!selection || !selection.length) {
+		return;
+	    }
+	    let rec = selection[0];
+	    if (!view.canConfigure) {
+		return;
+	    }
+
+	    let type = 'PVE.node.' + (rec.data.type === 'pci' ? 'PCIEditWindow' : 'USBEditWindow');
+
+	    Ext.create(type, {
+		url: `/nodes/${rec.data.node}/hardware/mapping/${rec.data.type}/${rec.data.entry}`,
+		autoShow: true,
+		autoLoad: rec.data.ntype !== 'entry',
+		nodename: rec.data.ntype !== 'entry' ? rec.data.node : undefined,
+		name: rec.data.entry ?? rec.data.text,
+		listeners: {
+		    destroy: () => me.load(),
+		},
+	    });
+	},
+
+	load: function() {
+	    let me = this;
+	    let view = me.getView();
+	    Proxmox.Utils.API2Request({
+		url: '/cluster/hardware/mapping',
+		method: 'GET',
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+		success: function({ result: { data } }) {
+		    view.setRootNode({
+			children: data,
+		    });
+		    let root = view.getRootNode();
+		    root.expand();
+		    root.childNodes.forEach(node => node.expand());
+		    me.loadRemainigNodes();
+		},
+	    });
+	},
+
+	loadRemainigNodes: function() {
+	    let me = this;
+	    let view = me.getView();
+	    PVE.data.ResourceStore.getNodes().forEach(({ node }) => {
+		if (node === Proxmox.NodeName) {
+		    return;
+		}
+		Proxmox.Utils.API2Request({
+		    url: `/nodes/${node}/hardware/mapping/all`,
+		    method: 'GET',
+		    failure: function(response) {
+			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 } }) {
+			let entries = {};
+			data.forEach((entry) => {
+			    entries[entry.name] = entry;
+			});
+			view.getRootNode()?.cascade(function(rec) {
+			    if (rec.data.node !== node) {
+				return;
+			    }
+
+			    let entry = entries[rec.data.entry];
+
+			    rec.set('valid', entry.valid);
+			    rec.set('errmsg', entry.errmsg);
+			    rec.commit();
+			});
+		    },
+		});
+	    });
+	},
+    },
+
+    store: {
+	sorters: 'text',
+	model: 'pve-hardware-tree',
+	data: {},
+    },
+
+
+    tbar: [
+	{
+	    text: gettext('Add new'),
+	    cbind: {
+		disabled: '{!canConfigure}',
+	    },
+	    menu: [
+		{
+		    text: gettext('PCI'),
+		    iconCls: 'pve-itype-icon-pci',
+		    handler: 'addPCI',
+		},
+		{
+		    text: gettext('USB'),
+		    iconCls: 'fa fa-fw fa-usb black',
+		    handler: 'addUSB',
+		},
+	    ],
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Add mapping'),
+	    disabled: true,
+	    parentXType: 'treepanel',
+	    enableFn: function(rec) {
+		return rec.data.ntype === 'entry' && this.up('treepanel').canConfigure;
+	    },
+	    cbind: {
+		disabled: '{!canConfigure}',
+	    },
+	    handler: 'edit',
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Edit'),
+	    disabled: true,
+	    parentXType: 'treepanel',
+	    enableFn: function(rec) {
+		return rec.data.ntype !== 'entry' && this.up('treepanel').canConfigure;
+	    },
+	    cbind: {
+		disabled: '{!canConfigure}',
+	    },
+	    handler: 'edit',
+	},
+	{
+	    xtype: 'proxmoxStdRemoveButton',
+	    parentXType: 'treepanel',
+	    getUrl: function(rec) {
+		let data = rec.data;
+		return `/api2/extjs/nodes/${data.node}/hardware/mapping/${data.type}/${data.entry}`;
+	    },
+	    confirmMsg: function(rec) {
+		let msg = gettext('Are you sure you want to remove entry {0} for {1}');
+		return Ext.String.format(msg, `'${rec.data.entry}'`, `'${rec.data.node}'`);
+	    },
+	    enableFn: function(rec) {
+		return rec.data.ntype !== 'entry' && this.up('treepanel').canConfigure;
+	    },
+	    callback: 'load',
+	    disabled: true,
+	    text: gettext('Remove'),
+	},
+    ],
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    text: gettext('Type/ID/Node'),
+	    dataIndex: 'text',
+	    renderer: function(value, _meta, record) {
+		if (record.data.ntype === 'entry') {
+		    let typeMap = {
+			usb: gettext('USB'),
+			pci: gettext('PCI'),
+		    };
+		    let type = typeMap[record.data.type] || Proxmox.Utils.unknownText;
+		    return `${value} (${type})`;
+		}
+		return value;
+	    },
+	    width: 200,
+	},
+	{
+	    text: gettext('Vendor'),
+	    dataIndex: 'vendor',
+	},
+	{
+	    text: gettext('Device'),
+	    dataIndex: 'device',
+	},
+	{
+	    text: gettext('Subsystem Vendor'),
+	    dataIndex: 'subsystem-vendor',
+	},
+	{
+	    text: gettext('Subsystem Device'),
+	    dataIndex: 'subsystem-device',
+	},
+	{
+	    text: gettext('IOMMU group'),
+	    dataIndex: 'iommugroup',
+	},
+	{
+	    text: gettext('Path'),
+	    dataIndex: 'path',
+	},
+	{
+	    header: gettext('Status'),
+	    dataIndex: 'valid',
+	    flex: 1,
+	    renderer: function(value, _metadata, record) {
+		if (record.data.ntype !== 'mapping') {
+		    return '';
+		}
+		let iconCls;
+		let status;
+		if (value === undefined) {
+		    iconCls = 'fa-spinner fa-spin';
+		    status = gettext('Loading...');
+		} 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}`;
+	    },
+	},
+	{
+	    header: gettext('Comment'),
+	    dataIndex: 'comment',
+	    flex: 1,
+	},
+    ],
+
+    listeners: {
+	activate: 'load',
+	itemdblclick: 'edit',
+    },
+});
-- 
2.30.2

[pve-devel] [PATCH manager 11/12] ui: window/Migrate: allow mapped devices

[Dominik Csapak]

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 | 37 +++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index 1c23abb3..5465702e 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -219,14 +219,26 @@ 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 missingResources = disallowed.unavailable_resources.join(', ');
+
+			migration.possible = false;
+			migration.preconditions.push({
+			    text: 'Mapped Resources (' + missingResources + ') not available on selected target. ',
+			    severity: 'error',
+			});
+		    }
 		}
 	    }
 
@@ -249,6 +261,17 @@ Ext.define('PVE.window.Migrate', {
 		}
 	    }
 
+	    if (migrateStats.mapped_resources && migrateStats.mapped_resources.length) {
+		if (vm.get('running')) {
+		    migration.possible = false;
+		    migration.preconditions.push({
+			text: Ext.String.format('Can\'t migrate running VM with mapped resources: {0}',
+			migrateStats.mapped_resources.join(', ')),
+			severity: 'error',
+		    });
+		}
+	    }
+
 	    if (migrateStats.local_disks.length) {
 		migrateStats.local_disks.forEach(function(disk) {
 		    if (disk.cdrom && disk.cdrom === 1) {
-- 
2.30.2

[pve-devel] [PATCH manager 12/12] ui: improve permission handling for hardware

[Dominik Csapak]

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..526cadbc 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': '/hardware' },
 	{ '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 3a2f293e..0b5f208f 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 6e9d03b4..283c0aad 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -253,8 +253,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.hardware['Hardware.Use'] ? 'PVE.qemu.USBEdit' : undefined,
+		never_delete: !caps.nodes['Sys.Console'] && !caps.hardware['Hardware.Use'],
 		header: gettext('USB Device') + ' (' + confid + ')',
 	    };
 	}
@@ -264,8 +264,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.hardware['Hardware.Use'],
+		editor: caps.nodes['Sys.Console'] || caps.hardware['Hardware.Use'] ? 'PVE.qemu.PCIEdit' : undefined,
 		header: gettext('PCI Device') + ' (' + confid + ')',
 	    };
 	}
@@ -566,12 +566,13 @@ 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.hardware['Hardware.Use'];
 	    const noVMConfigHWTypePerm = !caps.vms['VM.Config.HWType'];
 	    const noVMConfigNetPerm = !caps.vms['VM.Config.Network'];
 	    const noVMConfigDiskPerm = !caps.vms['VM.Config.Disk'];
 
-	    me.down('#addUsb').setDisabled(noSysConsolePerm || isAtLimit('usb'));
-	    me.down('#addPci').setDisabled(noSysConsolePerm || isAtLimit('hostpci'));
+	    me.down('#addUsb').setDisabled(noHWPerm || isAtLimit('usb'));
+	    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'));
@@ -680,14 +681,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.hardware['Hardware.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.hardware['Hardware.Use'],
 				handler: editorFactory('PCIEdit'),
 			    },
 			    {
-- 
2.30.2

Re: [pve-devel] [PATCH many] add cluster-wide hardware device mapping

[Dominik Csapak]

just a quick note, aaron noticed some small things immediately,

root@pam did not see the hardware panel:

access control needs this diff:

----8<----
diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
index c1b712d..b686c6b 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -191,7 +191,7 @@ sub compute_api_permission {
      };
      map { $res->{$_} = {} } keys %$priv_re_map;

-    my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
+    my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn', '/hardware'];

      my $checked_paths = {};
      foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
---->8----

and qemu-server does not build without root without this:

----8<----
diff --git a/test/MigrationTest/Shared.pm b/test/MigrationTest/Shared.pm
index 8ae6a6e..bcbd769 100644
--- a/test/MigrationTest/Shared.pm
+++ b/test/MigrationTest/Shared.pm
@@ -69,6 +69,16 @@ $cluster_module->mock(
      },
  );

+our $hardware_map_module = Test::MockModule->new("PVE::HardwareMap");
+$hardware_map_module->mock(
+    find_device_on_current_node => sub {
+       return {};
+    },
+    config => sub {
+       return {};
+    },
+);
+
  our $ha_config_module = Test::MockModule->new("PVE::HA::Config");
  $ha_config_module->mock(
      vm_is_ha_managed => sub {
---->8-----

i'll send a v2 when i get some more feedback
(i'll also include some tests then)

Re: [pve-devel] [PATCH many] add cluster-wide hardware device mapping

[Dominik Csapak]

On 7/21/22 14:29, Gilberto Ferreira via pve-devel wrote:
> 
> It's nice to hear about this patch.
> For now, as a workaround, when I need a usb device (like a stick for
> instance), I am using USB Over IP, like you can read in this article
> https://www.linux-magazine.com/Issues/2018/208/Tutorial-USB-IP#:~:text=It's%20called%20USB%2FIP%20(read,directly%20to%20the%20client%20machine
> .
> 
> Perhaps this patch will do the same job?
> 
> Gilberto


this patch alone, no i don't think so. it only enables offline migration
(because live migration with host hardware is not possible currently)

for usb we could in the future think about unplugging before and
plugging after live migration, but there are still missing pieces for that
aside from this series (usb hotplug is not finished yet completely)

so if you need live migration with your solution, i'd continue to use that

Re: [pve-devel] [PATCH access-control 2/2] PVE/RPCEnvironment: add helper for checking hw permissions

[Fabian Grünbichler]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> like check_vm_perm, etc.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/RPCEnvironment.pm | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
> index 7c37c6e..c1b712d 100644
> --- a/src/PVE/RPCEnvironment.pm
> +++ b/src/PVE/RPCEnvironment.pm
> @@ -356,6 +356,14 @@ sub check_vm_perm {
>      return $self->check_full($user, "/vms/$vmid", $privs, $any, $noerr);
>  };
>  
> +sub check_hw_perm {
> +    my ($self, $user, $id, $privs, $any, $noerr) = @_;
> +
> +    my $cfg = $self->{user_cfg};
> +
> +    return $self->check_full($user, "/hardware/$id", $privs, $any, $noerr);
> +}

is this really needed (here?)?

I mean, yes,

$rpcenv->check_hw_perm('foo@bar', "hardware_id", ['Hardware.Use'], 0, 0)

is a (tiny) bit shorter than

$rpcenv->check_full('foo@bar', "/hardware/hardware_id", ['Hardware.Use'], 0, 0)

but ;)

note that check_vm has a special job and is not just a wrapper for 
checking $ID against /$PREFIX/$ID, it is specifically for checking guest 
ACLs while honoring pool ACLs for the special case of "VM is currently 
being created and not formally part of the pool yet"..

similary, check_perm_modify serves the purpose of containing all the 
"modify $path" -> "actual privilege" mappings in a single place.

the rest of the check_foo subs are low-level building blocks/helpers.

> +
>  sub is_group_member {
>      my ($self, $group, $user) = @_;
>  
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH common 1/1] add PVE/HardwareMap

[Fabian Grünbichler]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> this adds functionality for the hardwaremap config (as json)
> the format of the config is like this:
> 
> {
>     usb => {
> 	name => {
> 	    nodename1 => { /* mapping object */ },
> 	    nodename2 => { /* mapping object */ }
> 	}
>     },
>     pci => {
> 	/* same as above */
>     },
>     digest => "<DIGEST-STRING>"
> }

kind of missing an argument for why
- this is a json file instead of a regular section config (the two 
  stored types share most of their structure?)
- this is a cluster-wide file containing a map per node instead of 
  being one config file per node?

from what I can piece together from the rest of the series ;)
- cannot be a cluster-wide section config since values differ 
  (potentially) on each node (so a simple `nodes` filter is not enough)
- we don't want ID foo to be type USB on one node and type PCI on 
  another
- we want to check all nodes for migration precondition checks

the first one is a given obviously. the type mismatch wouldn't actually 
cause any problems although it could be confusing possibly. the 
migration check could just check all (relevant) nodes.

not saying I'm opposed to the "one json file" approach per se, but it 
would be nice to weigh the pros and cons before deviating from our usual 
approach to config files.

> 
> it also adds some helpers for the api schema & asserting that the
> device mappings are valid (by checking the saved properties
> against the ones found on the current available devices)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/Makefile           |   1 +
>  src/PVE/HardwareMap.pm | 363 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 364 insertions(+)
>  create mode 100644 src/PVE/HardwareMap.pm
> 
> diff --git a/src/Makefile b/src/Makefile
> index 13de6c6..8527704 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -17,6 +17,7 @@ LIB_SOURCES = \
>  	Daemon.pm \
>  	Exception.pm \
>  	Format.pm \
> +	HardwareMap.pm \
>  	INotify.pm \
>  	JSONSchema.pm \
>  	LDAP.pm \
> diff --git a/src/PVE/HardwareMap.pm b/src/PVE/HardwareMap.pm
> new file mode 100644
> index 0000000..1b94abc
> --- /dev/null
> +++ b/src/PVE/HardwareMap.pm
> @@ -0,0 +1,363 @@
> +package PVE::HardwareMap;
> +
> +use strict;
> +use warnings;
> +
> +use Digest::SHA;
> +use JSON;
> +use Storable qw(dclone);
> +
> +use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
> +use PVE::INotify;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::SysFSTools;
> +
> +use base qw(Exporter);
> +
> +our @EXPORT_OK = qw(find_device_on_current_node);
> +
> +my $FILENAME = "nodes/hardware-map.conf";
> +cfs_register_file($FILENAME, \&read_hardware_map, \&write_hardware_map);
> +
> +# a mapping format per type
> +my $format = {
> +    usb => {
> +	vendor => {
> +	    description => "The vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	device => {
> +	    description => "The device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	'subsystem-vendor' => {
> +	    description => "The subsystem vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	'subsystem-device' => {
> +	    description => "The subsystem device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	path => {
> +	    description => "The path to the usb device.",
> +	    type => 'string',
> +	    optional => 1,
> +	    pattern => qr/^(\d+)\-(\d+(\.\d+)*)$/,
> +	},
> +	comment => {
> +	    description => "Description.",
> +	    type => 'string',
> +	    optional => 1,
> +	    maxLength => 4096,
> +	},
> +    },
> +    pci => {
> +	vendor => {
> +	    description => "The vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	device => {
> +	    description => "The device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	},
> +	'subsystem-vendor' => {
> +	    description => "The subsystem vendor ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	'subsystem-device' => {
> +	    description => "The subsystem device ID",
> +	    type => 'string',
> +	    pattern => qr/^(:?0x)?[0-9A-Fa-f]{4}$/,
> +	    optional => 1,
> +	},
> +	path => {
> +	    description => "The path to the device. If the function is omitted, the whole device is mapped. In that case use the attrubes of the first device.",
> +	    type => 'string',
> +	    pattern => "(?:[a-f0-9]{4,}:)?[a-f0-9]{2}:[a-f0-9]{2}(?:\.[a-f0-9])?",
> +	},
> +	mdev => {
> +	    description => "The Device supports mediated devices.",
> +	    type => 'boolean',
> +	    optional => 1,
> +	    default => 0,
> +	},
> +	iommugroup => {
> +	    type => 'integer',
> +	    description => "The IOMMU group in which the device is in.",
> +	    optional => 1,
> +	},
> +	comment => {
> +	    description => "Description.",
> +	    type => 'string',
> +	    optional => 1,
> +	    maxLength => 4096,
> +	},
> +    },
> +};
> +
> +my $name_format = {
> +    description => "The custom name for the device",
> +    type => 'string',
> +    format => 'pve-configid',
> +};
> +
> +sub find_device_on_current_node {
> +    my ($type, $id) = @_;
> +
> +    my $cfg = config();
> +    my $node = PVE::INotify::nodename();
> +
> +    return undef if !defined($cfg->{$type}->{$id}) || !defined($cfg->{$type}->{$id}->{$node});
> +    return $cfg->{$type}->{$id}->{$node};
> +}
> +
> +sub config {
> +    return cfs_read_file($FILENAME);
> +}
> +
> +sub lock_config {
> +    my ($code, $errmsg) = @_;
> +
> +    cfs_lock_file($FILENAME, undef, $code);
> +    if (my $err = $@) {
> +	$errmsg ? die "$errmsg: $err" : die $err;
> +    }
> +}
> +
> +sub write_config {
> +    my ($cfg) = @_;
> +
> +    cfs_write_file($FILENAME, $cfg);
> +}
> +
> +sub check_prop {
> +    my ($schema, $key, $value) = @_;
> +    my $errors = {};
> +    PVE::JSONSchema::check_prop($value, $schema, '', $errors);
> +    if (scalar(keys %$errors)) {
> +	die "$errors->{$key}\n" if $errors->{$key};
> +	die "$errors->{_root}\n" if $errors->{_root};
> +	die "unknown error\n";
> +    }
> +}
> +
> +sub check_config {
> +    my ($cfg) = @_;
> +
> +    for my $type (keys %$format) {
> +	my $type_cfg = $cfg->{$type};
> +	my $type_format = $format->{$type};
> +
> +	for my $name (keys %$type_cfg) {
> +	    check_prop($name_format, 'name', $name);
> +
> +	    for my $node (keys $type_cfg->{$name}->%*) {
> +		check_prop(get_standard_option('pve-node'), 'node', $node);
> +		my $entry = $type_cfg->{$name}->{$node};
> +
> +		# check required props
> +		for my $prop  (keys %$type_format) {
> +		    next if $type_format->{$prop}->{optional};
> +		    die "missing property '$prop' for $type entry '$name'\n"
> +			if !defined($entry->{$prop});
> +		}
> +
> +		for my $prop (keys %$entry) {
> +		    check_prop($type_format->{$prop}, $prop, $entry->{$prop});
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +sub read_hardware_map {
> +    my ($filename, $raw)  = @_;
> +
> +    my $digest = Digest::SHA::sha1_hex($raw);
> +
> +    if (!defined($raw) || $raw eq '') {
> +	return {
> +	    digest => $digest,
> +	};
> +    }
> +
> +    my $cfg = from_json($raw);
> +    check_config($cfg);
> +    $cfg->{digest} = $digest;
> +
> +    return $cfg;
> +}
> +
> +sub write_hardware_map {
> +    my ($filename, $cfg) = @_;
> +
> +    check_config($cfg);
> +
> +    return to_json($cfg);
> +}
> +
> +my $pci_valid = sub {
> +    my ($cfg) = @_;
> +
> +    my $path = $cfg->{path} // '';
> +
> +    if ($path !~ m/\.[a-f0-9]/i) {
> +	# whole device, add .0 (must exist)
> +	$path = "$path.0";
> +    }
> +
> +    my $info = PVE::SysFSTools::pci_device_info($path, 1);
> +    die "pci device '$path' not found\n" if !defined($info);
> +
> +    my $correct_props = {
> +	vendor => $info->{vendor},
> +	device => $info->{device},
> +	'subsystem-vendor' => $info->{'subsystem_vendor'},
> +	'subsystem-device' => $info->{'subsystem_device'},
> +	mdev => $info->{mdev},
> +	iommugroup => $info->{iommugroup},
> +    };
> +
> +    for my $prop (sort keys %$correct_props) {
> +	next if !defined($correct_props->{$prop}) && !defined($cfg->{$prop});
> +	die "no '$prop' for device '$path'\n"
> +	    if defined($correct_props->{$prop}) && !defined($cfg->{$prop});
> +	die "'$prop' configured but should not be\n"
> +	    if !defined($correct_props->{$prop}) && defined($cfg->{$prop});
> +
> +	my $correct_prop = $correct_props->{$prop};
> +	$correct_prop =~ s/^0x//;
> +	my $configured_prop = $cfg->{$prop};
> +	$configured_prop =~ s/^0x//;
> +
> +	die "'$prop' does not match for '$cfg->{name}' ($correct_prop != $configured_prop)\n"
> +	    if $correct_prop ne $configured_prop;
> +    }
> +
> +    return 1;
> +};
> +
> +my $usb_valid = sub {
> +    my ($cfg) = @_;
> +
> +    my $name = $cfg->{name};
> +    my $vendor = $cfg->{vendor};
> +    my $device = $cfg->{device};
> +
> +    my $usb_list = PVE::SysFSTools::scan_usb();
> +
> +    my $info;
> +    if (my $path = $cfg->{path}) {
> +	for my $dev (@$usb_list) {
> +	    next if !$dev->{usbpath} || !$dev->{busnum};
> +	    my $usbpath = "$dev->{busnum}-$dev->{usbpath}";
> +	    next if $usbpath ne $path;
> +	    $info = $dev;
> +	}
> +	die "usb device '$path' not found\n" if !defined($info);
> +
> +	die "'vendor' does not match for '$name'\n"
> +	    if $info->{vendid} ne $cfg->{vendor};
> +	die "'device' does not match for '$name'\n"
> +	    if $info->{prodid} ne $cfg->{device};
> +    } else {
> +	for my $dev (@$usb_list) {
> +	    next if $dev->{vendid} ne $vendor;
> +	    next if $dev->{prodid} ne $device;
> +	    $info = $dev;
> +	}
> +	die "usb device '$vendor:$device' not found\n" if !defined($info);
> +    }
> +
> +    return 1;
> +};
> +
> +sub assert_device_valid {
> +    my ($type, $cfg) = @_;
> +
> +    if ($type eq 'usb') {
> +	return $usb_valid->($cfg);
> +    } elsif ($type eq 'pci') {
> +	return $pci_valid->($cfg);
> +    }
> +
> +    die "invalid type $type\n";
> +}
> +
> +sub createSchema {
> +    my ($type) = @_;
> +
> +    my $schema = {};
> +
> +    $schema->{name} = $name_format;
> +    $schema->{node} = get_standard_option('pve-node');
> +
> +    for my $opt (sort keys $format->{$type}->%*) {
> +	$schema->{$opt} = $format->{$type}->{$opt};
> +    }
> +
> +    return {
> +	additionalProperties => 0,
> +	properties => $schema,
> +    };
> +
> +}
> +
> +sub updateSchema {
> +    my ($type) = @_;
> +
> +    my $schema = {};
> +
> +    $schema->{name} = $name_format;
> +    $schema->{node} = get_standard_option('pve-node');
> +
> +    my $deletable = [];
> +
> +    for my $opt (sort keys $format->{$type}->%*) {
> +	$schema->{$opt} = dclone($format->{$type}->{$opt});
> +	$schema->{$opt}->{optional} = 1;
> +	if ($format->{$type}->{$opt}->{optional}) {
> +	    push @$deletable, $opt;
> +	}
> +    }
> +
> +    my $deletable_pattern = join('|', @$deletable);
> +
> +    $schema->{delete} = {
> +	type => 'string', format => 'pve-configid-list',
> +	description => "A list of settings you want to delete.",
> +	maxLength => 4096,
> +	optional => 1,
> +    };
> +
> +    $schema->{digest} = get_standard_option('pve-config-digest');
> +
> +    return {
> +	additionalProperties => 0,
> +	properties => $schema,
> +    };
> +}
> +
> +sub options {
> +    my ($type) = @_;
> +
> +    my $opts = {};
> +
> +    for my $opt (sort keys $format->{$type}->%*) {
> +	$opts->{$opt}->{optional} = $format->{$type}->{$opt}->{optional};
> +    }
> +
> +    return $opts;
> +}
> +
> +1;
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH qemu-server 1/7] PVE/QemuServer: allow mapped usb devices in config

[Fabian Grünbichler]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/QemuServer.pm     |  2 ++
>  PVE/QemuServer/USB.pm | 21 ++++++++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 7d9cf22..a6ca80d 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -1069,6 +1069,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.
>  
> diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
> index 3c8da2c..279078a 100644
> --- a/PVE/QemuServer/USB.pm
> +++ b/PVE/QemuServer/USB.pm
> @@ -4,6 +4,7 @@ use strict;
>  use warnings;
>  use PVE::QemuServer::PCI qw(print_pci_addr);
>  use PVE::JSONSchema;
> +use PVE::HardwareMap;
>  use base 'Exporter';
>  
>  our @EXPORT_OK = qw(
> @@ -27,7 +28,25 @@ 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 $device = PVE::HardwareMap::find_device_on_current_node('usb', $value);
> +	return undef if !defined($device);
> +	eval {
> +	    PVE::HardwareMap::assert_device_valid('usb', $device);
> +	};
> +	if (my $err = $@) {
> +	    warn "USB Mapping invalid (hardware probably changed): $err\n";
> +	    return;
> +	}

nit: this would also trigger on deleting such an invalid mapping, which is a 
bit strange?

> +
> +	if ($device->{path}) {
> +	    $res = parse_usb_device($device->{path});
> +	} else {
> +	    $res->{vendorid} = $device->{vendor};
> +	    $res->{productid} = $device->{device};
> +	}
> +
> +	$res->{mapped} = 1;
>      }
>  
>      return $res;
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH qemu-server 2/7] PVE/QemuServer: allow mapped pci deviced in config

[Fabian Grünbichler]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> and get the correct pci device during parsing
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/QemuServer/PCI.pm | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 23fe508..1ff4bce 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -4,6 +4,7 @@ use warnings;
>  use strict;
>  
>  use PVE::JSONSchema;
> +use PVE::HardwareMap;
>  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 => {
> @@ -383,6 +386,19 @@ sub parse_hostpci {
>  
>      my $res = PVE::JSONSchema::parse_property_string($hostpci_fmt, $value);
>  
> +    if ($res->{host} !~ m/:/) {
> +	# we have no ordinary pci id, must be a mapping
> +	my $device = PVE::HardwareMap::find_device_on_current_node('pci', $res->{host});
> +	die "PCI device mapping not found for '$res->{host}'\n" if !defined($device);
> +	eval {
> +	    PVE::HardwareMap::assert_device_valid('pci', $device);
> +	};
> +	if (my $err = $@) {
> +	    die "PCI device mapping invalid (hardware probably changed): $err\n";

here we die, with USB we warn.. OTOH this gets called way less than the 
USB counter part, since we mostly just parse the property string without 
the extra checks for PCI.. might make sense to unify the approach taken.

> +	}
> +	$res->{host} = $device->{path};
> +    }
> +
>      my @idlist = split(/;/, $res->{host});
>      delete $res->{host};
>      foreach my $id (@idlist) {
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH qemu-server 3/7] PVE/API2/Qemu: add permission checks for mapped usb devices

[Fabian Grünbichler]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 39 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 99b426e..aa7ddea 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -26,6 +26,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;
> @@ -567,8 +568,12 @@ my $check_vm_create_usb_perm = sub {
>  
>      foreach my $opt (keys %{$param}) {
>  	next if $opt !~ m/^usb\d+$/;
> +	my $device = parse_usb_device($param->{$opt});
>  
> -	if ($param->{$opt} =~ m/spice/) {
> +	if ($device->{spice}) {
> +	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> +	} elsif ($device->{mapped}) {
> +	    $rpcenv->check_hw_perm($authuser, $device->{host}, ['Hardware.Use']);

maybe I am overlooking something, but where does $device->{host} come 
from?

parse_usb_device (for a mapped USB device) looks up device in the 
hardware map, asserts it's valid (for the local node), and then either 
returns

{
  vendorid => $map->{vendor},
  productid => $map->{device},
  mapped => 1,
}

or the result of parse_usb_device($map->{path}), with 'mapped' set.

since the lookup in the map doesn't set a 'host' member, wouldn't 
$device->{host} always be undef for mapped devices? maybe this was 
wrongly copied from the PCI code, where the hostpci property string has 
a 'host' property (that with this series, also possibly contains a 
mapping entry ID)? or is this supposed to parse the property string, and 
use the host property from there?

>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
>  	} else {
>  	    die "only root can set '$opt' config for real devices\n";
> @@ -1552,7 +1557,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::QemuServer::USB::parse_usb_device($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_hw_perm($authuser, $device->{host}, ['Hardware.Use']);

same question here..

>  			$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";
> @@ -1613,7 +1623,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::QemuServer::USB::parse_usb_device($conf->{$opt});
> +			$oldhost = parse_usb_device($olddevice->{host});

and here

> +		    }
> +		    if (defined($oldhost)) {
> +			if ($oldhost->{spice}) {
> +			    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +			} elsif ($oldhost->{mapped}) {
> +			    $rpcenv->check_hw_perm($authuser, $olddevice->{host}, ['Hardware.Use']);

and here

> +			    $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::QemuServer::USB::parse_usb_device($param->{$opt});
> +		    my $newhost = parse_usb_device($newdevice->{host});

and here

> +
> +		    if ($newhost->{spice}) {
> +			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    } elsif ($newhost->{mapped}) {
> +			$rpcenv->check_hw_perm($authuser, $newdevice->{host}, ['Hardware.Use']);

and here

>  			$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";
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH qemu-server 4/7] PVE/API2/Qemu: add permission checks for mapped pci devices

[Fabian Grünbichler]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 54 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index aa7ddea..a8029c2 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -26,6 +26,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;
> @@ -583,6 +584,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_hw_perm($authuser, $device->{host}, ['Hardware.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) = @_;
>  
> @@ -593,7 +614,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+$/;
>  
>  
>  	if ($cpuoptions->{$opt} || $opt =~ m/^numa\d+$/) {
> @@ -621,7 +642,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";
>  	}
> @@ -856,6 +877,7 @@ __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);
>  
>  	    &$check_cpu_model_access($rpcenv, $authuser, $param);
>  
> @@ -1569,6 +1591,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_hw_perm($authuser, $olddevice->{host}, ['Hardware.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);
>  		} else {
>  		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
> @@ -1651,6 +1683,24 @@ 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_hw_perm($authuser, $olddevice->{host}, ['Hardware.Use']);
> +			}
> +			$rpcenv->check_hw_perm($authuser, $newdevice->{host}, ['Hardware.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";
> +		    }

nit: this is a bit confusing to read, and might benefit from splitting the 
check for old and for new, like so

if (defined($old)) {
  parse old
  check old || check root
}
parse new
check new || check root
check VM.Config.HWType

> +		    PVE::QemuServer::PCI::parse_hostpci($param->{$opt});
>  		    $conf->{pending}->{$opt} = $param->{$opt};
>  		} else {
>  		    $conf->{pending}->{$opt} = $param->{$opt};
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH qemu-server 5/7] PVE/QemuServer: extend 'check_local_resources' for mapped resources

[Fabian Grünbichler]

On July 19, 2022 1:46 pm, Dominik Csapak wrote:
> by adding them to their own list, saving the nodes where
> they are not allowed, and return those on 'wantarray'

so this seems to be the main place where we benefit from a cluster-wide 
HW map - but it could just as well be a loop over the node specific 
config files, possibly after reducing the node list via means of 
storages..

and, for the call in migrate where we know the target node already we 
could pass it here to reduce the amount of work we have to do ;)

> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/QemuServer.pm | 36 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index a6ca80d..ea7e213 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2617,6 +2617,21 @@ sub check_local_resources {
>      my ($conf, $noerr) = @_;
>  
>      my @loc_res = ();
> +    my $mapped_res = [];
> +
> +    my $nodelist = PVE::Cluster::get_nodelist();
> +    my $hw_map = PVE::HardwareMap::config();
> +
> +    my $not_allowed_nodes = { map { $_ => [] } @$nodelist };
> +
> +    my $add_not_allowed_nodes = sub {
> +	my ($type, $key, $id) = @_;
> +	for my $node (@$nodelist) {
> +	    if (!defined($id) || !defined($hw_map->{$type}->{$id}->{$node})) {
> +		push @{$not_allowed_nodes->{$node}}, $key;
> +	    }
> +	}
> +    };
>  
>      push @loc_res, "hostusb" if $conf->{hostusb}; # old syntax
>      push @loc_res, "hostpci" if $conf->{hostpci}; # old syntax
> @@ -2624,7 +2639,24 @@ 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_not_allowed_nodes->('usb', $k, $entry->{host});
> +		push @$mapped_res, $k;
> +		next;
> +	    }
> +	}
> +	if ($k =~ m/^hostpci/) {
> +	    my $entry = parse_property_string('pve-qm-hostpci', $conf->{$k});
> +	    if ($entry->{host} !~ m/:/) {
> +		$add_not_allowed_nodes->('pci', $k, $entry->{host});
> +		push @$mapped_res, $k;
> +		next;
> +	    }
> +	}
>  	# 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+$/;
> @@ -2632,7 +2664,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, $not_allowed_nodes) : \@loc_res;
>  }
>  
>  # check if used storages are available on all nodes (use by migrate)
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH many] add cluster-wide hardware device mapping

[DERUMIER, Alexandre]

many thanks for this patch series.

I had a student at the previous training needing this feature.

I'll be usefull in the future to get mdev vgpu live migration (It's not 
yet working with qemu 6.2)


I'll try to test it next week when I'll back from holiday.



Le 19/07/22 à 13:46, Dominik Csapak a écrit :
> this series aims 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
>
> built-in are some additional safety checks in contrast to current
> passthrough, e.g. if pci addresses shift, with the mapping
> we can detect that and prevent a vm to boot with the wrong device
> (in most cases, there are some edge cases when one has multiple
> of the same device, e.g. the same gpu, that we cannot detect)
>
> a few pain points that are probably worth discussing/thinking about:
> (i did not really get feedback on my last RFC on this)
> * the config format
>      i changed to a json backed config, since it makes handling it much
>      easier (since we have a id -> nodenames -> mapping relation that
>      we cannot easily represent with a section config). some
>      (small) parts are written from scratch (update/createSchema for
>      instance) but we would have to do that anyway
>
>      if wanted i can make the section config work, but it makes the
>      handling quite a big uglier (for example, we have name the usb/pci
>      properties differently because the section config cannnot have
>      different formats for different sections)
>
> * getting the cluster wide info
>      the configuring of mappings is all done via node specific api paths,
>      but i created a cluster wide api path that returns the overall
>      structure for easy consumption from the gui. to get the remaining
>      data from the other nodes, i let the gui make an api call
>      for each node.
>
>      alternatively we could distribute the necessary info via pmxcfs,
>      but we'd have to broadcast basically the whole pci listing for all
>      nodes in a relatively short interval, only for it to be extremly
>      seldomly used (when looking at the cluster wide hardware
>      mappings...)
>
> * some minor things that can be improved are how the gui looks/behaves:
>      - 'add new' and 'add mapping' are probably to similar, but i did
>        not come up with really better alternatives
>      - i find the tree of entry -> node-mappings nice, but there may be
>        an even better representation?
>      - position in cluster menu is probably not optimal
>        (but where to put it?)
>
> changes from the rfc:
> * new cluster wide gui instead of node-local one (removed that, since
>    it's not necessary when we have a cluster-wide one)
> * uses json instead of a section config
> * api is quite different overall, i split the type into its own level
>    for configuring, similar to what we do in pbs
>    (e.g. /nodes/NODENAME/hardware/mapping/usb/)
> * fixed quite some bugs the rfc had
> * added patch for handling the gui with limited permissions better
> * added a 'comment' field for mappings
>
> dependencies are ofc:
>
>      manager depends on qemu-server,pve-access-control,pve-common
>      qemu-server depends on pve-access-control,pve-common
>      pve-common depends on pve-cluster
>
> pve-cluster:
>
> Dominik Csapak (1):
>    add nodes/hardware-map.conf
>
>   data/PVE/Cluster.pm | 1 +
>   data/src/status.c   | 1 +
>   2 files changed, 2 insertions(+)
>
> pve-access-control:
>
> Dominik Csapak (2):
>    PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
>    PVE/RPCEnvironment: add helper for checking hw permissions
>
>   src/PVE/AccessControl.pm  | 13 +++++++++++++
>   src/PVE/RPCEnvironment.pm |  9 +++++++++
>   2 files changed, 22 insertions(+)
>
> pve-common:
>
> Dominik Csapak (1):
>    add PVE/HardwareMap
>
>   src/Makefile           |   1 +
>   src/PVE/HardwareMap.pm | 363 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 364 insertions(+)
>   create mode 100644 src/PVE/HardwareMap.pm
>
> qemu-server:
>
> Dominik Csapak (7):
>    PVE/QemuServer: allow mapped usb devices in config
>    PVE/QemuServer: allow mapped pci deviced in config
>    PVE/API2/Qemu: add permission checks for mapped usb devices
>    PVE/API2/Qemu: add permission checks for mapped pci devices
>    PVE/QemuServer: extend 'check_local_resources' for mapped resources
>    PVE/API2/Qemu: migrate preconditions: use new check_local_resources
>      info
>    PVE/QemuMigrate: check for mapped resources on migration
>
>   PVE/API2/Qemu.pm      | 108 ++++++++++++++++++++++++++++++++++++++----
>   PVE/QemuMigrate.pm    |  13 ++++-
>   PVE/QemuServer.pm     |  38 ++++++++++++++-
>   PVE/QemuServer/PCI.pm |  20 +++++++-
>   PVE/QemuServer/USB.pm |  21 +++++++-
>   5 files changed, 185 insertions(+), 15 deletions(-)
>
> pve-manager:
>
> Dominik Csapak (12):
>    PVE/API2/Hardware: add Mapping.pm
>    PVE/API2/Cluster: add Hardware mapping list api call
>    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: add window/PCIEdit: edit window for pci mappings
>    ui: add window/USBEdit: edit window for usb mappings
>    ui: add dc/HardwareView: a CRUD interface for hardware mapping
>    ui: window/Migrate: allow mapped devices
>    ui: improve permission handling for hardware
>
>   PVE/API2/Cluster.pm                 |   8 +
>   PVE/API2/Cluster/Hardware.pm        | 117 +++++
>   PVE/API2/Cluster/Makefile           |   1 +
>   PVE/API2/Hardware.pm                |   6 +
>   PVE/API2/Hardware/Makefile          |   1 +
>   PVE/API2/Hardware/Mapping.pm        | 708 ++++++++++++++++++++++++++++
>   www/manager6/Makefile               |   5 +
>   www/manager6/data/PermPathStore.js  |   1 +
>   www/manager6/dc/Config.js           |  18 +-
>   www/manager6/dc/HardwareView.js     | 314 ++++++++++++
>   www/manager6/form/PCIMapSelector.js |  95 ++++
>   www/manager6/form/PCISelector.js    |  18 +-
>   www/manager6/form/USBMapSelector.js |  73 +++
>   www/manager6/form/USBSelector.js    |  33 +-
>   www/manager6/qemu/HardwareView.js   |  17 +-
>   www/manager6/qemu/PCIEdit.js        | 231 ++++++---
>   www/manager6/qemu/USBEdit.js        |  34 +-
>   www/manager6/window/Migrate.js      |  37 +-
>   www/manager6/window/PCIEdit.js      | 323 +++++++++++++
>   www/manager6/window/USBEdit.js      | 248 ++++++++++
>   20 files changed, 2185 insertions(+), 103 deletions(-)
>   create mode 100644 PVE/API2/Cluster/Hardware.pm
>   create mode 100644 PVE/API2/Hardware/Mapping.pm
>   create mode 100644 www/manager6/dc/HardwareView.js
>   create mode 100644 www/manager6/form/PCIMapSelector.js
>   create mode 100644 www/manager6/form/USBMapSelector.js
>   create mode 100644 www/manager6/window/PCIEdit.js
>   create mode 100644 www/manager6/window/USBEdit.js
>

Re: [pve-devel] [PATCH access-control 2/2] PVE/RPCEnvironment: add helper for checking hw permissions

[Dominik Csapak]

On 8/1/22 14:01, Fabian Grünbichler wrote:
> On July 19, 2022 1:46 pm, Dominik Csapak wrote:
>> like check_vm_perm, etc.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   src/PVE/RPCEnvironment.pm | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
>> index 7c37c6e..c1b712d 100644
>> --- a/src/PVE/RPCEnvironment.pm
>> +++ b/src/PVE/RPCEnvironment.pm
>> @@ -356,6 +356,14 @@ sub check_vm_perm {
>>       return $self->check_full($user, "/vms/$vmid", $privs, $any, $noerr);
>>   };
>>   
>> +sub check_hw_perm {
>> +    my ($self, $user, $id, $privs, $any, $noerr) = @_;
>> +
>> +    my $cfg = $self->{user_cfg};
>> +
>> +    return $self->check_full($user, "/hardware/$id", $privs, $any, $noerr);
>> +}
> 
> is this really needed (here?)?
> 
> I mean, yes,
> 
> $rpcenv->check_hw_perm('foo@bar', "hardware_id", ['Hardware.Use'], 0, 0)
> 
> is a (tiny) bit shorter than
> 
> $rpcenv->check_full('foo@bar', "/hardware/hardware_id", ['Hardware.Use'], 0, 0)
> 
> but ;)
> 
> note that check_vm has a special job and is not just a wrapper for
> checking $ID against /$PREFIX/$ID, it is specifically for checking guest
> ACLs while honoring pool ACLs for the special case of "VM is currently
> being created and not formally part of the pool yet"..
> 
> similary, check_perm_modify serves the purpose of containing all the
> "modify $path" -> "actual privilege" mappings in a single place.
> 
> the rest of the check_foo subs are low-level building blocks/helpers.
> 

you're right, the helper is not really necessary

>> +
>>   sub is_group_member {
>>       my ($self, $group, $user) = @_;
>>   
>> -- 
>> 2.30.2
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

Re: [pve-devel] [PATCH common 1/1] add PVE/HardwareMap

[Dominik Csapak]

On 8/1/22 14:58, Fabian Grünbichler wrote:
> On July 19, 2022 1:46 pm, Dominik Csapak wrote:
>> this adds functionality for the hardwaremap config (as json)
>> the format of the config is like this:
>>
>> {
>>      usb => {
>> 	name => {
>> 	    nodename1 => { /* mapping object */ },
>> 	    nodename2 => { /* mapping object */ }
>> 	}
>>      },
>>      pci => {
>> 	/* same as above */
>>      },
>>      digest => "<DIGEST-STRING>"
>> }
> 
> kind of missing an argument for why
> - this is a json file instead of a regular section config (the two
>    stored types share most of their structure?)

i gave the explanation in the cover letter only (sry)

basically, having some properties with different formats (e.g. path)
makes it really cumbersome (e.g.  i'd have to rename them 'usbpath' and 'pcipath'
everywhere only to merge them again in the ui somewhere...

also having a 'nodename' <-> 'properties' mapping is not really doable in a
section config. we could do an array, but that makes handling of duplicates
etc. also not really nice. a 'non section config' config makes this a lot easier,
and one other format we already use is json (e.g. tfa.json; also json handling
in generally is easy in perl)

> - this is a cluster-wide file containing a map per node instead of
>    being one config file per node?

the basic idea is to have the mappings together. the most use we get out of this
is the ui where we show the cluster-wide mapping config. there we would have
to load/parse all node configs...

i am also not opposed to make it per node, but with my notes above,
i'd rather not use a section config, and what are then the advantages of having
a config per node?

> 
> from what I can piece together from the rest of the series ;)
> - cannot be a cluster-wide section config since values differ
>    (potentially) on each node (so a simple `nodes` filter is not enough)

exactly (otherwise we wouldn't really need a mapping in the first place)

> - we don't want ID foo to be type USB on one node and type PCI on
>    another
> - we want to check all nodes for migration precondition checks
> 
> the first one is a given obviously. the type mismatch wouldn't actually
> cause any problems although it could be confusing possibly. the
> migration check could just check all (relevant) nodes.
> 
> not saying I'm opposed to the "one json file" approach per se, but it
> would be nice to weigh the pros and cons before deviating from our usual
> approach to config files.
> 

so to summarize:

single section config: not impossible, but hard/ugly in many ways
multiple section configs: easier, but still ugly in parts
single json: easy to use, but barely any code reuse with our existing configs
  (though the 'duplicate' code is not that much either)
multiple jsons: little code reuse, weird to use, don't really see the point of this
other config format: ???

code-wise i much prefer the 'single json' approach, but if you or anybody
else really don't want/like it, ofc i'd redo that. it's not really important
to me which way we go, but i'd like to decide at some point.

Re: [pve-devel] [PATCH qemu-server 3/7] PVE/API2/Qemu: add permission checks for mapped usb devices

[Dominik Csapak]

On 8/1/22 15:01, Fabian Grünbichler wrote:
> On July 19, 2022 1:46 pm, Dominik Csapak wrote:
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   PVE/API2/Qemu.pm | 39 ++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 36 insertions(+), 3 deletions(-)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 99b426e..aa7ddea 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -26,6 +26,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;
>> @@ -567,8 +568,12 @@ my $check_vm_create_usb_perm = sub {
>>   
>>       foreach my $opt (keys %{$param}) {
>>   	next if $opt !~ m/^usb\d+$/;
>> +	my $device = parse_usb_device($param->{$opt});
>>   
>> -	if ($param->{$opt} =~ m/spice/) {
>> +	if ($device->{spice}) {
>> +	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
>> +	} elsif ($device->{mapped}) {
>> +	    $rpcenv->check_hw_perm($authuser, $device->{host}, ['Hardware.Use']);
> 
> maybe I am overlooking something, but where does $device->{host} come
> from?
> 
> parse_usb_device (for a mapped USB device) looks up device in the
> hardware map, asserts it's valid (for the local node), and then either
> returns
> 
> {
>    vendorid => $map->{vendor},
>    productid => $map->{device},
>    mapped => 1,
> }
> 
> or the result of parse_usb_device($map->{path}), with 'mapped' set.
> 
> since the lookup in the map doesn't set a 'host' member, wouldn't
> $device->{host} always be undef for mapped devices? maybe this was
> wrongly copied from the PCI code, where the hostpci property string has
> a 'host' property (that with this series, also possibly contains a
> mapping entry ID)? or is this supposed to parse the property string, and
> use the host property from there?
> 

ok, either i did send from the wrong branch, or i redid that already since sending
the patches. my branch here locally already has all of the wrong 'parse_usb_device'
calls replaced with 'parse_property_string' (like with pci)

so in any case that is the correct approach here.
first parse the property string, then parse the usb device from the 'host' property

[pve-devel] [PATCH qemu-server 6/7] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info

[Dominik Csapak]

restrict the nodes also for mapped devices, and return them in their
own property

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 36b8c31..837185f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3624,6 +3624,10 @@ __PACKAGE__->register_method({
 
 	$res->{running} = PVE::QemuServer::check_running($vmid) ? 1:0;
 
+	my ($local_resources, $mapped_resources, $not_allowed_nodes) =
+	    PVE::QemuServer::check_local_resources($vmconf, 1);
+	delete $not_allowed_nodes->{$localnode};
+
 	# if vm is not running, return target nodes where local storage is available
 	# for offline migration
 	if (!$res->{running}) {
@@ -3632,7 +3636,12 @@ __PACKAGE__->register_method({
 	    delete $checked_nodes->{$localnode};
 
 	    foreach my $node (keys %$checked_nodes) {
-		if (!defined $checked_nodes->{$node}->{unavailable_storages}) {
+		if (scalar(@{$not_allowed_nodes->{$node}})) {
+		    $checked_nodes->{$node}->{unavailable_resources} = $not_allowed_nodes->{$node};
+		    next;
+		}
+
+		if (!defined($checked_nodes->{$node}->{unavailable_storages})) {
 		    push @{$res->{allowed_nodes}}, $node;
 		}
 
@@ -3640,13 +3649,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.20.1