* [pve-devel] [PATCH cluster v3 1/1] add nodes/hardware-map.conf
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-08 18:03 ` [pve-devel] applied: " Thomas Lamprecht
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 10/13] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info Dominik Csapak
` (36 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 10/13] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH cluster v3 1/1] add nodes/hardware-map.conf Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 11/13] PVE/QemuMigrate: check for mapped resources on migration Dominik Csapak
` (35 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 d6d393f..cc726bd 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4185,6 +4185,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}) {
@@ -4193,7 +4197,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;
}
@@ -4201,13 +4210,11 @@ __PACKAGE__->register_method({
$res->{not_allowed_nodes} = $checked_nodes;
}
-
my $local_disks = &$check_vm_disks_local($storecfg, $vmconf, $vmid);
$res->{local_disks} = [ values %$local_disks ];;
- my $local_resources = PVE::QemuServer::check_local_resources($vmconf, 1);
-
$res->{local_resources} = $local_resources;
+ $res->{mapped_resources} = $mapped_resources;
return $res;
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 11/13] PVE/QemuMigrate: check for mapped resources on migration
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH cluster v3 1/1] add nodes/hardware-map.conf Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 10/13] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 12/13] fix #3574: enable multi pci device mapping from config Dominik Csapak
` (34 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
they can only be migrated to nodes where there exists a mapping and if
the migration is done offline
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuMigrate.pm | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index d52dc8d..54530fd 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 ($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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 12/13] fix #3574: enable multi pci device mapping from config
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (2 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 11/13] PVE/QemuMigrate: check for mapped resources on migration Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 13/13] add tests for mapped pci devices Dominik Csapak
` (33 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
The hardware config now supports multiple devices as a semicolon
seperated list. With this, instead of only having one device in a pci mapping,
we now have a list of which we can choose from on vm start. This way one can
dynamically start vms with a pool of (identical) pci devices without
having to manually assign the proper ids.
For that we have to change the internal representation of a parsed
device, such that we have the seperately configured paths in the mapping
in different lists (because multifunction devices still are interpreted
as single devices)
For mdev devices we now can also have multiple devices, where we simply
try to create the appropriate type on each until we either have one
created, or bail out.
Since we now have to reserve the pci ids in print_hostpci_devices, we
have to add a 'reserve' parameter to config_to_command (and chain it
through to reserve_pci_usage) so that a 'qm showcmd' does not actually
reserve any pci id (this would break when using that on running vms).
Additionally this also prevents the migration tests from failing
(they use vm_commandline which in turn uses config_to_command)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer.pm | 43 ++++++++++++++++++++++++++++-------
PVE/QemuServer/PCI.pm | 53 +++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 18 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index d23cfc2..5833aba 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3507,8 +3507,9 @@ my sub should_disable_smm {
sub config_to_command {
my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
- $pbs_backing) = @_;
+ $pbs_backing, $reserve) = @_;
+ $reserve //= 1;
my $cmd = [];
my ($globalFlags, $machineFlags, $rtcFlags) = ([], [], []);
my $devices = [];
@@ -3724,7 +3725,7 @@ sub config_to_command {
# host pci device passthrough
my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) = PVE::QemuServer::PCI::print_hostpci_devices(
- $vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder);
+ $vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder, $reserve);
# usb devices
my $usb_dev_features = {};
@@ -5623,13 +5624,30 @@ sub vm_start_nolock {
my $uuid;
for my $id (sort keys %$pci_devices) {
my $d = $pci_devices->{$id}->{device};
- for my $dev ($d->{pciid}->@*) {
- my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
- # nvidia grid needs the uuid of the mdev as qemu parameter
- if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '10de') {
- $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $id);
+ # used pci devices for non-mdev
+ if (!$d->{mdev}) {
+ for my $dev ($pci_devices->{$id}->{used}->@*) {
+ PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id);
}
+ next;
+ }
+
+ # try each configured pci device for mdevs
+ my $devs = [map { $_->{id} } map { @$_ } $d->{ids}->@*]; # flatten ids
+
+ my $info;
+ for my $dev (@$devs) {
+ $info = eval { PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev, $id, $d->{mdev}) };
+ warn $@ if $@;
+ last if $info; # if successful, we're done
+ }
+
+ die "could not create mediated device\n" if !defined($info);
+
+ # nvidia grid needs the uuid of the mdev as qemu parameter
+ if (!defined($uuid) && $info->{vendor} eq '10de') {
+ $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $id);
}
}
push @$cmd, '-uuid', $uuid if defined($uuid);
@@ -5862,7 +5880,16 @@ sub vm_commandline {
my $defaults = load_defaults();
- my $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu);
+ my $cmd = config_to_command(
+ $storecfg,
+ $vmid,
+ $conf,
+ $defaults,
+ $forcemachine,
+ $forcecpu,
+ undef,
+ 0,
+ );
return PVE::Tools::cmd2string($cmd);
}
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 08244c1..1ad89ed 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -386,6 +386,7 @@ sub parse_hostpci {
my $res = PVE::JSONSchema::parse_property_string($hostpci_fmt, $value);
+ my $idlist = [];
my $mapping = 0;
if ($res->{host} !~ m/:/) {
# we have no ordinary pci id, must be a mapping
@@ -398,17 +399,29 @@ sub parse_hostpci {
if (my $err = $@) {
die "PCI device mapping invalid (hardware probably changed): $err\n";
}
- $res->{host} = $device->{path};
+ $idlist = [split(/;/, $device->{path})];
+ # if we have a list of mapped devices, we want to choose the first available one
+ $res->{choose} = 1 if scalar(@$idlist > 1);
+ } else {
+ $idlist = [split(/;/, $res->{host})];
}
- my @idlist = split(/;/, $res->{host});
delete $res->{host};
- foreach my $id (@idlist) {
+ my $ignore_mdev = !$res->{choose} && scalar(@$idlist) > 1;
+
+ $res->{ids} = [];
+ foreach my $id (@$idlist) {
my $devs = PVE::SysFSTools::lspci($id);
- die "cannot use mediated device with multifuntion device\n"
- if $mapping && $res->{mdev} && scalar(@$devs) > 1;
die "no PCI device found for '$id'\n" if !scalar(@$devs);
- push @{$res->{pciid}}, @$devs;
+ $ignore_mdev = 1 if scalar(@$devs) > 1;
+ push @{$res->{ids}}, $devs;
+ }
+ # ignore mdev for multiple devices, except when from mapping
+ if ($res->{mdev} && $ignore_mdev) {
+ # FIXME in 8.0 we should also disallow that for 'normal' passthrough
+ die "cannot use mediated device with multifunction device\n" if $mapping;
+ warn "ignoring mediated device with multifunction device\n";
+ delete $res->{mdev};
}
return $res;
}
@@ -437,11 +450,13 @@ my $print_pci_device = sub {
};
sub print_hostpci_devices {
- my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder) = @_;
+ my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder, $reserve) = @_;
+ $reserve //= 1;
my $kvm_off = 0;
my $gpu_passthrough = 0;
my $legacy_igd = 0;
+ my $used_pci_ids = {};
my $parsed_devices = {};
my $pciaddr;
@@ -473,7 +488,24 @@ sub print_hostpci_devices {
$pciaddr = print_pci_addr($pci_name, $bridges, $arch, $machine_type);
}
- my $pcidevices = $d->{pciid};
+ # choose devices
+ my $pcidevices = [];
+ if (!$d->{mdev}) {
+ for my $devs ($d->{ids}->@*) {
+ my $ids = [map { $_->{id} } @$devs];
+
+ if ($d->{choose}) {
+ next if grep { defined($used_pci_ids->{$_}) } @$ids; # already used
+ eval { reserve_pci_usage($ids, $vmid, 10, undef, $reserve) };
+ next if $@;
+ }
+
+ map { $used_pci_ids->{$_} = 1 } @$ids;
+ push @$pcidevices, @$devs;
+ last if $d->{choose};
+ }
+ die "could not find a free device\n" if scalar(@$pcidevices) < 1;
+ }
$parsed_devices->{$i}->{used} = $pcidevices;
my $multifunction = @$pcidevices > 1;
@@ -603,8 +635,9 @@ sub remove_pci_reservation {
}
sub reserve_pci_usage {
- my ($requested_ids, $vmid, $timeout, $pid) = @_;
+ my ($requested_ids, $vmid, $timeout, $pid, $reserve) = @_;
+ $reserve //= 1;
$requested_ids = [ $requested_ids ] if !ref($requested_ids);
return if !scalar(@$requested_ids); # do nothing for empty list
@@ -637,7 +670,7 @@ sub reserve_pci_usage {
$reservation_list->{$id}->{time} = $ctime + $timeout + 5;
}
}
- $write_pci_reservation_unlocked->($reservation_list);
+ $write_pci_reservation_unlocked->($reservation_list) if $reserve;
});
die $@ if $@;
}
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 13/13] add tests for mapped pci devices
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (3 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 12/13] fix #3574: enable multi pci device mapping from config Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths Dominik Csapak
` (32 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
test/cfg2cmd/q35-linux-hostpci-mapping.conf | 17 +++++
.../q35-linux-hostpci-mapping.conf.cmd | 36 +++++++++
test/run_config2command_tests.pl | 76 +++++++++++++++++++
3 files changed, 129 insertions(+)
create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf
create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
diff --git a/test/cfg2cmd/q35-linux-hostpci-mapping.conf b/test/cfg2cmd/q35-linux-hostpci-mapping.conf
new file mode 100644
index 0000000..2402cf2
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-mapping.conf
@@ -0,0 +1,17 @@
+# TEST: Config with q35, NUMA, hostpci mapping passthrough, EFI & Linux
+bios: ovmf
+bootdisk: scsi0
+cores: 1
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+hostpci0: someNic
+hostpci1: someGpu,mdev=some-model
+hostpci2: someNic
+machine: q35
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+numa: 1
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+sockets: 2
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
new file mode 100644
index 0000000..a5b3fe8
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
@@ -0,0 +1,36 @@
+/usr/bin/kvm \
+ -id 8006 \
+ -name 'vm8006,debug-threads=on' \
+ -no-shutdown \
+ -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+ -mon 'chardev=qmp,mode=control' \
+ -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+ -mon 'chardev=qmp-event,mode=control' \
+ -pidfile /var/run/qemu-server/8006.pid \
+ -daemonize \
+ -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
+ -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+ -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+ -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
+ -smp '2,sockets=2,cores=1,maxcpus=2' \
+ -nodefaults \
+ -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+ -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+ -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+ -m 512 \
+ -object 'memory-backend-ram,id=ram-node0,size=256M' \
+ -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \
+ -object 'memory-backend-ram,id=ram-node1,size=256M' \
+ -numa 'node,nodeid=1,cpus=1,memdev=ram-node1' \
+ -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+ -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+ -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+ -device 'vfio-pci,host=0000:07:10.0,id=hostpci0,bus=pci.0,addr=0x10' \
+ -device 'vfio-pci,sysfsdev=/sys/bus/mdev/devices/00000001-0000-0000-0000-000000008006,id=hostpci1,bus=pci.0,addr=0x11' \
+ -device 'vfio-pci,host=0000:07:10.4,id=hostpci2,bus=pci.0,addr=0x1b' \
+ -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+ -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
+ -machine 'type=q35+pve0'
diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index f097811..c12afac 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -86,8 +86,38 @@ my $pci_devs = [
"0000:f0:43.0",
"0000:f0:43.1",
"1234:f0:43.1",
+ "0000:01:00.4",
+ "0000:01:00.5",
+ "0000:01:00.6",
+ "0000:07:10.0",
+ "0000:07:10.1",
+ "0000:07:10.4",
];
+my $hardware_map_config = {
+ pci => {
+ someGpu => {
+ localhost => {
+ iommugroup => 1,
+ mdev => 1,
+ vendor => "0x10de",
+ device => "0x2231",
+ path => "0000:01:00.4;0000:01:00.5;0000:01:00.6",
+ }
+ },
+ someNic => {
+ localhost => {
+ iommugroup => 2,
+ mdev => 0,
+ vendor => "0x8086",
+ device => "0x1520",
+ path => "0000:07:10.0;0000:07:10.1;0000:07:10.4"
+ }
+ }
+ },
+ usb => {},
+};
+
my $current_test; # = {
# description => 'Test description', # if available
# qemu_version => '2.12',
@@ -259,6 +289,28 @@ $pve_common_sysfstools->mock(
} sort @$pci_devs
];
},
+ pci_device_info => sub {
+ my ($path, $noerr) = @_;
+
+ if ($path =~ m/^0000:01:00/) {
+ return {
+ mdev => 1,
+ iommugroup => 1,
+ mdev => 1,
+ vendor => "0x10de",
+ device => "0x2231",
+ };
+ } elsif ($path =~ m/^0000:07:10/) {
+ return {
+ iommugroup => 2,
+ mdev => 0,
+ vendor => "0x8086",
+ device => "0x1520",
+ };
+ } else {
+ return {};
+ }
+ },
);
my $qemu_monitor_module;
@@ -287,6 +339,30 @@ $qemu_monitor_module->mock(
);
$qemu_monitor_module->mock('qmp_cmd', \&qmp_cmd);
+my $hardware_map_module = Test::MockModule->new("PVE::HardwareMap");
+$hardware_map_module->mock(
+ config => sub {
+ return $hardware_map_config;
+ },
+);
+
+my $pci_module = Test::MockModule->new("PVE::QemuServer::PCI");
+$pci_module->mock(
+ reserve_pci_usage => sub {
+ my ($ids, $vmid, $timeout, $pid, $dryrun) = @_;
+
+ $ids = [$ids] if !ref($ids);
+
+ for my $id (@$ids) {
+ if ($id eq "0000:07:10.1") {
+ die "reserved";
+ }
+ }
+
+ return undef;
+ },
+);
+
sub diff($$) {
my ($a, $b) = @_;
return if $a eq $b;
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (4 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 13/13] add tests for mapped pci devices Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 12:05 ` Fabian Grünbichler
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 1/3] SysFSTools: make mdev cleanup independent of pciid Dominik Csapak
` (31 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 | 3 ++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index c32dcc3..9cbc376 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1080,6 +1080,17 @@ my $privgroups = {
'Pool.Audit',
],
},
+ Hardware => {
+ root => [
+ 'Hardware.Configure', # create/edit mappings
+ ],
+ admin => [
+ 'Hardware.Use',
+ ],
+ audit => [
+ 'Hardware.Audit',
+ ],
+ },
};
my $valid_privs = {};
@@ -1209,6 +1220,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..bcf911b 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -187,10 +187,11 @@ 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;
- 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}}) {
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
2022-09-20 12:50 ` [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths Dominik Csapak
@ 2022-11-09 12:05 ` Fabian Grünbichler
2022-11-09 12:39 ` Dominik Csapak
2022-11-09 12:52 ` Thomas Lamprecht
0 siblings, 2 replies; 54+ messages in thread
From: Fabian Grünbichler @ 2022-11-09 12:05 UTC (permalink / raw)
To: Proxmox VE development discussion
On September 20, 2022 2:50 pm, Dominik Csapak wrote:
> 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 | 3 ++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
> index c32dcc3..9cbc376 100644
> --- a/src/PVE/AccessControl.pm
> +++ b/src/PVE/AccessControl.pm
> @@ -1080,6 +1080,17 @@ my $privgroups = {
> 'Pool.Audit',
> ],
> },
> + Hardware => {
> + root => [
> + 'Hardware.Configure', # create/edit mappings
> + ],
> + admin => [
> + 'Hardware.Use',
> + ],
> + audit => [
> + 'Hardware.Audit',
> + ],
> + },
I guess the rationale here was that currently hardware is root only, so having
admin => Configure,
user => Use,
audit => Audit,
would mean the existing PVEAdmin roles would gain something that was previously
root only?
note that the current patch still means that for the "Administrator" role
anyway, since that gets *all* defined privileges.. (which might also be
something worthy of calling out somewhere?)
it still might make sense to put Hardware.Use into the user category for
consistency's sake? also not sure whether it would be worth it to re-think
"Configure" (a bit more explicit) vs. "Modify" (consistent with existing
schema)..
> };
>
> my $valid_privs = {};
> @@ -1209,6 +1220,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..bcf911b 100644
> --- a/src/PVE/RPCEnvironment.pm
> +++ b/src/PVE/RPCEnvironment.pm
> @@ -187,10 +187,11 @@ sub compute_api_permission {
> nodes => qr/Sys\.|Permissions\.Modify/,
> sdn => qr/SDN\.|Permissions\.Modify/,
> dc => qr/Sys\.Audit|SDN\./,
> + hardware => qr/Hardware\.|Permissiions\.Modify/,
typo "Permissiions"
> };
> 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}}) {
> --
> 2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
2022-11-09 12:05 ` Fabian Grünbichler
@ 2022-11-09 12:39 ` Dominik Csapak
2022-11-09 13:06 ` Fabian Grünbichler
2022-11-09 12:52 ` Thomas Lamprecht
1 sibling, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-11-09 12:39 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 11/9/22 13:05, Fabian Grünbichler wrote:
> On September 20, 2022 2:50 pm, Dominik Csapak wrote:
>> 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 | 3 ++-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
>> index c32dcc3..9cbc376 100644
>> --- a/src/PVE/AccessControl.pm
>> +++ b/src/PVE/AccessControl.pm
>> @@ -1080,6 +1080,17 @@ my $privgroups = {
>> 'Pool.Audit',
>> ],
>> },
>> + Hardware => {
>> + root => [
>> + 'Hardware.Configure', # create/edit mappings
>> + ],
>> + admin => [
>> + 'Hardware.Use',
>> + ],
>> + audit => [
>> + 'Hardware.Audit',
>> + ],
>> + },
>
> I guess the rationale here was that currently hardware is root only, so having
>
> admin => Configure,
> user => Use,
> audit => Audit,
>
> would mean the existing PVEAdmin roles would gain something that was previously
> root only?
>
> note that the current patch still means that for the "Administrator" role
> anyway, since that gets *all* defined privileges.. (which might also be
> something worthy of calling out somewhere?)
yes the idea was that existing roles don't get that privilege, but
i did not really find a way to add the privilige and not give it to the
administrator role
and yes putting it in the admin bucket would mean that pveadmin gets it too
>
> it still might make sense to put Hardware.Use into the user category for
> consistency's sake? also not sure whether it would be worth it to re-think
> "Configure" (a bit more explicit) vs. "Modify" (consistent with existing
> schema)..
Modify is fine by me, i didn't choose it because we don't actually
'modify' the hardware, but yes we modify the hardware configuration
putting the use in the user bracket has the side effect that
there would be a 'PVEHardwareAdmin' and 'PVEHardwareUser' role
with the same privileges, which i did find weird to have
this way we only get the PVEHardwareAdmin that can use/see the
devices
if there is a way to only have 'user' role without the admin one
please do tell ;)
>
>> };
>>
>> my $valid_privs = {};
>> @@ -1209,6 +1220,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..bcf911b 100644
>> --- a/src/PVE/RPCEnvironment.pm
>> +++ b/src/PVE/RPCEnvironment.pm
>> @@ -187,10 +187,11 @@ sub compute_api_permission {
>> nodes => qr/Sys\.|Permissions\.Modify/,
>> sdn => qr/SDN\.|Permissions\.Modify/,
>> dc => qr/Sys\.Audit|SDN\./,
>> + hardware => qr/Hardware\.|Permissiions\.Modify/,
>
> typo "Permissiions"
>
>> };
>> 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}}) {
>> --
>> 2.30.2
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
2022-11-09 12:39 ` Dominik Csapak
@ 2022-11-09 13:06 ` Fabian Grünbichler
2022-11-09 13:23 ` Dominik Csapak
0 siblings, 1 reply; 54+ messages in thread
From: Fabian Grünbichler @ 2022-11-09 13:06 UTC (permalink / raw)
To: Dominik Csapak, Proxmox VE development discussion
On November 9, 2022 1:39 pm, Dominik Csapak wrote:
> On 11/9/22 13:05, Fabian Grünbichler wrote:
>> On September 20, 2022 2:50 pm, Dominik Csapak wrote:
>>> 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 | 3 ++-
>>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
>>> index c32dcc3..9cbc376 100644
>>> --- a/src/PVE/AccessControl.pm
>>> +++ b/src/PVE/AccessControl.pm
>>> @@ -1080,6 +1080,17 @@ my $privgroups = {
>>> 'Pool.Audit',
>>> ],
>>> },
>>> + Hardware => {
>>> + root => [
>>> + 'Hardware.Configure', # create/edit mappings
>>> + ],
>>> + admin => [
>>> + 'Hardware.Use',
>>> + ],
>>> + audit => [
>>> + 'Hardware.Audit',
>>> + ],
>>> + },
>>
>> I guess the rationale here was that currently hardware is root only, so having
>>
>> admin => Configure,
>> user => Use,
>> audit => Audit,
>>
>> would mean the existing PVEAdmin roles would gain something that was previously
>> root only?
>>
>> note that the current patch still means that for the "Administrator" role
>> anyway, since that gets *all* defined privileges.. (which might also be
>> something worthy of calling out somewhere?)
>
> yes the idea was that existing roles don't get that privilege, but
> i did not really find a way to add the privilige and not give it to the
> administrator role
could only be done by manual filtering in create_roles (similar to how the
SuperUser series does it).
> and yes putting it in the admin bucket would mean that pveadmin gets it too
>
>>
>> it still might make sense to put Hardware.Use into the user category for
>> consistency's sake? also not sure whether it would be worth it to re-think
>> "Configure" (a bit more explicit) vs. "Modify" (consistent with existing
>> schema)..
>
> Modify is fine by me, i didn't choose it because we don't actually
> 'modify' the hardware, but yes we modify the hardware configuration
well, it does rather refer to the config entry/hardware map, not the hardware
itself :)
> putting the use in the user bracket has the side effect that
> there would be a 'PVEHardwareAdmin' and 'PVEHardwareUser' role
> with the same privileges, which i did find weird to have
>
> this way we only get the PVEHardwareAdmin that can use/see the
> devices
>
> if there is a way to only have 'user' role without the admin one
> please do tell ;)
no automatic way ;)
one way out would be to:
- give Hardware.Configure (/Modify) to the Admin role
- give Hardware.Use to the User role
- still require root in addition to Hardware.Configure (until point or major
release time, where the extra check is dropped and documented in the release notes)
my guess is that most people that currently hand out Administrator (as opposed
to "just" PVEAdmin) would actually want those users to also be able to configure
the hardware map anyway.. and no users would get the new role
"PVEHardwareAdmin" anyway by default, so handing that out is an explicit grant
of the associated privileges anyway.
also there is the similar can of worms like with SuperUser - any user that can
change permissions can give themselves or someone else Hardware.Configure (which
is possibly root-level access if misused?). if we add more "special"
privileges/roles like that, we might need to have some extension to the acl and
group membership code paths to handle that better.. but I haven't really thought
that through yet (at least more extensive docs would be a good idea I think).
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
2022-11-09 13:06 ` Fabian Grünbichler
@ 2022-11-09 13:23 ` Dominik Csapak
0 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-11-09 13:23 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox VE development discussion
On 11/9/22 14:06, Fabian Grünbichler wrote:
> On November 9, 2022 1:39 pm, Dominik Csapak wrote:
>> On 11/9/22 13:05, Fabian Grünbichler wrote:
>>> On September 20, 2022 2:50 pm, Dominik Csapak wrote:
>>>> 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 | 3 ++-
>>>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
>>>> index c32dcc3..9cbc376 100644
>>>> --- a/src/PVE/AccessControl.pm
>>>> +++ b/src/PVE/AccessControl.pm
>>>> @@ -1080,6 +1080,17 @@ my $privgroups = {
>>>> 'Pool.Audit',
>>>> ],
>>>> },
>>>> + Hardware => {
>>>> + root => [
>>>> + 'Hardware.Configure', # create/edit mappings
>>>> + ],
>>>> + admin => [
>>>> + 'Hardware.Use',
>>>> + ],
>>>> + audit => [
>>>> + 'Hardware.Audit',
>>>> + ],
>>>> + },
>>>
>>> I guess the rationale here was that currently hardware is root only, so having
>>>
>>> admin => Configure,
>>> user => Use,
>>> audit => Audit,
>>>
>>> would mean the existing PVEAdmin roles would gain something that was previously
>>> root only?
>>>
>>> note that the current patch still means that for the "Administrator" role
>>> anyway, since that gets *all* defined privileges.. (which might also be
>>> something worthy of calling out somewhere?)
>>
>> yes the idea was that existing roles don't get that privilege, but
>> i did not really find a way to add the privilige and not give it to the
>> administrator role
>
> could only be done by manual filtering in create_roles (similar to how the
> SuperUser series does it).
>
ah yes i see it now how we could do that
>> and yes putting it in the admin bucket would mean that pveadmin gets it too
>>
>>>
>>> it still might make sense to put Hardware.Use into the user category for
>>> consistency's sake? also not sure whether it would be worth it to re-think
>>> "Configure" (a bit more explicit) vs. "Modify" (consistent with existing
>>> schema)..
>>
>> Modify is fine by me, i didn't choose it because we don't actually
>> 'modify' the hardware, but yes we modify the hardware configuration
>
> well, it does rather refer to the config entry/hardware map, not the hardware
> itself :)
>
>> putting the use in the user bracket has the side effect that
>> there would be a 'PVEHardwareAdmin' and 'PVEHardwareUser' role
>> with the same privileges, which i did find weird to have
>>
>> this way we only get the PVEHardwareAdmin that can use/see the
>> devices
>>
>> if there is a way to only have 'user' role without the admin one
>> please do tell ;)
>
> no automatic way ;)
>
> one way out would be to:
> - give Hardware.Configure (/Modify) to the Admin role
> - give Hardware.Use to the User role
> - still require root in addition to Hardware.Configure (until point or major
> release time, where the extra check is dropped and documented in the release notes)
>
> my guess is that most people that currently hand out Administrator (as opposed
> to "just" PVEAdmin) would actually want those users to also be able to configure
> the hardware map anyway.. and no users would get the new role
> "PVEHardwareAdmin" anyway by default, so handing that out is an explicit grant
> of the associated privileges anyway.
>
> also there is the similar can of worms like with SuperUser - any user that can
> change permissions can give themselves or someone else Hardware.Configure (which
> is possibly root-level access if misused?). if we add more "special"
> privileges/roles like that, we might need to have some extension to the acl and
> group membership code paths to handle that better.. but I haven't really thought
> that through yet (at least more extensive docs would be a good idea I think).
mhmm didn't think about that until now. Independent of my patch, is it
really possible to give any privilege when having the privilege on
permissions? shouldn't it be limited to ones own privileges?
iow. why should i be able go give out privilege X when i don't have
that myself? (or am i misunderstanding something here)
if that's the case then there is really no way for now besides keeping
the configuration root only, since with 'modify' you can often do
pretty bad things (like pass through the sata/nvme controller of the
root disk, or taking away the network of the host and of course
make the host system crash by passing through some device that's
necessary for the host)
in that case we can omit the 'Cluster.HWMap.Modify' for now and only
introduce a 'Cluster.HWMap.Use' for users of these root configured
devices
when we decide how we can handle these permission issues, we can still
introduce a new 'Cluster.HWMap.Modify' (imho no point in adding
it now if it does nothing)
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
2022-11-09 12:05 ` Fabian Grünbichler
2022-11-09 12:39 ` Dominik Csapak
@ 2022-11-09 12:52 ` Thomas Lamprecht
1 sibling, 0 replies; 54+ messages in thread
From: Thomas Lamprecht @ 2022-11-09 12:52 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
Am 09/11/2022 um 13:05 schrieb Fabian Grünbichler:
> On September 20, 2022 2:50 pm, Dominik Csapak wrote:
>> 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 | 3 ++-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
>> index c32dcc3..9cbc376 100644
>> --- a/src/PVE/AccessControl.pm
>> +++ b/src/PVE/AccessControl.pm
>> @@ -1080,6 +1080,17 @@ my $privgroups = {
>> 'Pool.Audit',
>> ],
>> },
>> + Hardware => {
>> + root => [
>> + 'Hardware.Configure', # create/edit mappings
>> + ],
>> + admin => [
>> + 'Hardware.Use',
>> + ],
>> + audit => [
>> + 'Hardware.Audit',
>> + ],
>> + },
>
> I guess the rationale here was that currently hardware is root only, so having
>
> admin => Configure,
> user => Use,
> audit => Audit,
>
> would mean the existing PVEAdmin roles would gain something that was previously
> root only?
>
> note that the current patch still means that for the "Administrator" role
> anyway, since that gets *all* defined privileges.. (which might also be
> something worthy of calling out somewhere?)
>
> it still might make sense to put Hardware.Use into the user category for
> consistency's sake? also not sure whether it would be worth it to re-think
> "Configure" (a bit more explicit) vs. "Modify" (consistent with existing
> schema)..
I'd rather keep it at .Modify IIUC and it will one allow to modify mappings,
IMO its worth to keep this aligned, and actually we could go for
Sys.HW.Modify
Sys.HW.Use
For what is Hardware.Audit though? (the commit message really needs to contain
more info...) is it for just seeing a HW mapping? as the underlying device
details on a specific node are already handled by Sys.Audit.
Because if its for HW mappings only I feel like it may not be required initially
we should be able to cover all by Hardware.Modify for managing them and and
Hardware.Use, for being allowed to use a specific one, but just listing could
wait for some actual use case.
Also, maybe call it HWMap.Modify and possibly make it a sub-group of a (new)
Cluster. priv group and /cluster/hw-map acl path.
Cluster.HWMap.Use
Cluster.HWMap.Modify
and /cluster/hw-id/{id}
IMO a bit more clear about what this actually covers, as a general /hardware
one sounds a bit ominous IMO for our users, and we want to have Cluster.Modify
et al. anyway someday for making cluster non-root create/join/editable.
(yes this isn't all that important and a bit close to bike shedding, but we got
to keep this pretty much forever (or have a PITA upgrade) so it's IMO worth
to spent a bit more time picking colors of the shed here ;)
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pve-devel] [PATCH common v3 1/3] SysFSTools: make mdev cleanup independent of pciid
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (5 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 8:38 ` Thomas Lamprecht
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 2/3] add PVE/HardwareMap Dominik Csapak
` (30 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
mediated devices also appear under /sys/bus/mdev/devices with their
uuid, independent of the pci device, so we can use that instead
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PVE/SysFSTools.pm | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
index b4cd5cc..b95e934 100644
--- a/src/PVE/SysFSTools.pm
+++ b/src/PVE/SysFSTools.pm
@@ -367,11 +367,9 @@ sub pci_create_mdev_device {
}
sub pci_cleanup_mdev_device {
- my ($pciid, $uuid) = @_;
+ my ($uuid) = @_;
- $pciid = normalize_pci_id($pciid);
-
- my $basedir = "$pcisysfs/devices/$pciid/$uuid";
+ my $basedir = "/sys/bus/mdev/devices/$uuid";
if (! -e $basedir) {
return 1; # no cleanup necessary if it does not exist
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH common v3 1/3] SysFSTools: make mdev cleanup independent of pciid
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 1/3] SysFSTools: make mdev cleanup independent of pciid Dominik Csapak
@ 2022-11-09 8:38 ` Thomas Lamprecht
0 siblings, 0 replies; 54+ messages in thread
From: Thomas Lamprecht @ 2022-11-09 8:38 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 20/09/2022 um 14:50 schrieb Dominik Csapak:
> mediated devices also appear under /sys/bus/mdev/devices with their
> uuid, independent of the pci device, so we can use that instead
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> src/PVE/SysFSTools.pm | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
> index b4cd5cc..b95e934 100644
> --- a/src/PVE/SysFSTools.pm
> +++ b/src/PVE/SysFSTools.pm
> @@ -367,11 +367,9 @@ sub pci_create_mdev_device {
> }
>
> sub pci_cleanup_mdev_device {
> - my ($pciid, $uuid) = @_;
> + my ($uuid) = @_;
>
> - $pciid = normalize_pci_id($pciid);
> -
> - my $basedir = "$pcisysfs/devices/$pciid/$uuid";
> + my $basedir = "/sys/bus/mdev/devices/$uuid";
>
> if (! -e $basedir) {
> return 1; # no cleanup necessary if it does not exist
this is a breaking change for older qemu-server I'd like to avoid, meh why ain't such
relations not encoded in the actual patch diff-comment area? Re-reading the cover letter
each time for every 20 patches is rather a nuisance and really easy to overlook..
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pve-devel] [PATCH common v3 2/3] add PVE/HardwareMap
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (6 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 1/3] SysFSTools: make mdev cleanup independent of pciid Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 8:46 ` Thomas Lamprecht
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 3/3] HardwareMap: add support for multiple pci device paths per mapping Dominik Csapak
` (29 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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>"
}
a single mapping object contains some info about the device, e.g.
for pci the (sub)vendor, sub(device), the mdev capability, path, etc.
for pci multifunction devices (e.g. 01:02 instead of 01:02.0), we use
the values of the first function to identify it.
note that for multifunction devices, we require 'mdev' to be undef
regardless what the first function is capable of, since
we cannot use mediated devices with multifunction devices anyway.
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)
we use a single cluster wide json here, because section config is too
limited to properly represent the data we need and easily access the
config of all nodes when we need it (e.g. in api calls regarding
migration and configuration)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/Makefile | 1 +
src/PVE/HardwareMap.pm | 367 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 368 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..31841b4
--- /dev/null
+++ b/src/PVE/HardwareMap.pm
@@ -0,0 +1,367 @@
+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);
+
+my $PCI_RE = "[a-f0-9]{4,}:[a-f0-9]{2}:[a-f0-9]{2}(?:\.[a-f0-9])?";
+
+# 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 attributes of the first device.",
+ type => 'string',
+ pattern => "${PCI_RE}",
+ },
+ 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) = @_;
+
+ $raw //= '';
+ my $digest = Digest::SHA::sha1_hex($raw);
+
+ if ($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 $multifunction = 0;
+ if ($path !~ m/\.[a-f0-9]/i) {
+ # whole device, add .0 (must exist)
+ $path = "$path.0";
+ $multifunction = 1;
+ }
+
+ 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 => $multifunction ? undef : $info->{mdev}, # don't allow mdev for multifunction
+ 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
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH common v3 2/3] add PVE/HardwareMap
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 2/3] add PVE/HardwareMap Dominik Csapak
@ 2022-11-09 8:46 ` Thomas Lamprecht
0 siblings, 0 replies; 54+ messages in thread
From: Thomas Lamprecht @ 2022-11-09 8:46 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
high level comment: should this go into pve-guest-common instead?
Am 20/09/2022 um 14:50 schrieb 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>"
> }
>
> a single mapping object contains some info about the device, e.g.
> for pci the (sub)vendor, sub(device), the mdev capability, path, etc.
>
> for pci multifunction devices (e.g. 01:02 instead of 01:02.0), we use
> the values of the first function to identify it.
>
> note that for multifunction devices, we require 'mdev' to be undef
> regardless what the first function is capable of, since
> we cannot use mediated devices with multifunction devices anyway.
>
> 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)
>
> we use a single cluster wide json here, because section config is too
"json file"
If it's json it should end with .json and I misread where you put it, please
don't pollute /etc/pve/nodes/ with some files, rather just use /etc/pve directly,
while that directory is a bit growing it's at least fitting the common file
location semantics for cluster-wide shared files.
I unpulled the cluster patch for registering the file for now.
> limited to properly represent the data we need and easily access the
> config of all nodes when we need it (e.g. in api calls regarding
> migration and configuration)
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> src/Makefile | 1 +
> src/PVE/HardwareMap.pm | 367 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 368 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..31841b4
> --- /dev/null
> +++ b/src/PVE/HardwareMap.pm
> @@ -0,0 +1,367 @@
> +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);
are you really introducing a cyclic dependency from pve-common <-> pve-cluster, yeah no
NAK! tbh. it would think that long term Proxmox engineers know that this is a no-go if
just anyhow possible
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pve-devel] [PATCH common v3 3/3] HardwareMap: add support for multiple pci device paths per mapping
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (7 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 2/3] add PVE/HardwareMap Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 01/13] cleanup pci devices in more situations Dominik Csapak
` (28 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
With this, we can now tell qemu-server to choose the first avaiable
devices, which makes using vGPUs and SR-IOV capable devices much easier
to use, since the user does not have to hardcode the device, but can
give a list of identical ones, and qemu-server chooses dynamically.
note that we require the devices all to be the same vendor/device,
because we don't want to group unrelated devices, but we only check
the iommugroup for the first device, but there is a high chance
that this also changes when somethings off since e.g. SRIOV devices
are most often created at the same time, so when the any has a different
group, the first one will too
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PVE/HardwareMap.pm | 71 ++++++++++++++++++++++++------------------
1 file changed, 41 insertions(+), 30 deletions(-)
diff --git a/src/PVE/HardwareMap.pm b/src/PVE/HardwareMap.pm
index 31841b4..52c45e1 100644
--- a/src/PVE/HardwareMap.pm
+++ b/src/PVE/HardwareMap.pm
@@ -84,9 +84,11 @@ my $format = {
},
path => {
description => "The path to the device. If the function is omitted, the whole device is"
- ." mapped. In that case use the attributes of the first device.",
+ ." mapped. In that case use the attributes of the first device. You can give"
+ ." multiple paths as a semicolon seperated list, the first available will then"
+ ." be chosen on guest start.",
type => 'string',
- pattern => "${PCI_RE}",
+ pattern => "(?:${PCI_RE};)*${PCI_RE}",
},
mdev => {
description => "The Device supports mediated devices.",
@@ -213,39 +215,48 @@ sub write_hardware_map {
my $pci_valid = sub {
my ($cfg) = @_;
- my $multifunction = 0;
- if ($path !~ m/\.[a-f0-9]/i) {
- # whole device, add .0 (must exist)
- $path = "$path.0";
- $multifunction = 1;
- }
+ my @paths = split(';', $cfg->{path} // '');
- my $info = PVE::SysFSTools::pci_device_info($path, 1);
- die "pci device '$path' not found\n" if !defined($info);
+ my $idx = 0;
+ for my $path (@paths) {
- my $correct_props = {
- vendor => $info->{vendor},
- device => $info->{device},
- 'subsystem-vendor' => $info->{'subsystem_vendor'},
- 'subsystem-device' => $info->{'subsystem_device'},
- mdev => $multifunction ? undef : $info->{mdev}, # don't allow mdev for multifunction
- iommugroup => $info->{iommugroup},
- };
+ my $multifunction = 0;
+ if ($path !~ m/\.[a-f0-9]/i) {
+ # whole device, add .0 (must exist)
+ $path = "$path.0";
+ $multifunction = 1;
+ }
+
+ my $info = PVE::SysFSTools::pci_device_info($path, 1);
+ die "pci device '$path' not found\n" if !defined($info);
- 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_props = {
+ vendor => $info->{vendor},
+ device => $info->{device},
+ 'subsystem-vendor' => $info->{'subsystem_vendor'},
+ 'subsystem-device' => $info->{'subsystem_device'},
+ mdev => $multifunction ? undef : $info->{mdev}, # don't allow mdev for multifunction
+ iommugroup => $info->{iommugroup},
+ };
+
+ for my $prop (sort keys %$correct_props) {
+ next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on the first device
- my $correct_prop = $correct_props->{$prop};
- $correct_prop =~ s/^0x//;
- my $configured_prop = $cfg->{$prop};
- $configured_prop =~ s/^0x//;
+ 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});
- die "'$prop' does not match for '$cfg->{name}' ($correct_prop != $configured_prop)\n"
- if $correct_prop ne $configured_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;
+ }
+ $idx++;
}
return 1;
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 01/13] cleanup pci devices in more situations
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (8 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH common v3 3/3] HardwareMap: add support for multiple pci device paths per mapping Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 8:00 ` [pve-devel] applied: " Thomas Lamprecht
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 02/13] PCI: make mediated device path independent of pci id Dominik Csapak
` (27 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
if the preparing of pci devices or the start of the vm fails, we need
to cleanup the pci devices (reservations *and* mdevs), or else
it might happen that there are leftovers which must be manually removed.
to include also mdevs now, refactor the cleanup code from 'vm_stop_cleanup'
into it's own function, and call that instead of only 'remove_pci_reservation'
also simplifies the code, such that it now removes all pci ids reserved
for that vmid, since we cannot have multiple vms with the same vmid
anyway
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer.pm | 34 ++++++++++++++++++----------------
PVE/QemuServer/PCI.pm | 12 +++++++-----
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c706653..b3c3ce0 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5609,7 +5609,7 @@ sub vm_start_nolock {
push @$cmd, '-uuid', $uuid if defined($uuid);
};
if (my $err = $@) {
- eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
+ eval { cleanup_pci_devices($vmid, $conf) };
warn $@ if $@;
die $err;
}
@@ -5705,7 +5705,9 @@ sub vm_start_nolock {
if (my $err = $@) {
# deactivate volumes if start fails
eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); };
- eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
+ warn $@ if $@;
+ eval { cleanup_pci_devices($vmid, $conf) };
+ warn $@ if $@;
die "start failed: $err";
}
@@ -5870,6 +5872,19 @@ sub get_vm_volumes {
return $vollist;
}
+sub cleanup_pci_devices {
+ my ($vmid, $conf) = @_;
+
+ foreach my $key (keys %$conf) {
+ next if $key !~ m/^hostpci(\d+)$/;
+ my $hostpciindex = $1;
+ my $uuid = PVE::SysFSTools::generate_mdev_uuid($vmid, $hostpciindex);
+ my $d = parse_hostpci($conf->{$key});
+ PVE::SysFSTools::pci_cleanup_mdev_device($uuid) if $d->{mdev};
+ }
+ PVE::QemuServer::PCI::remove_pci_reservation($vmid);
+}
+
sub vm_stop_cleanup {
my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes) = @_;
@@ -5901,20 +5916,7 @@ sub vm_stop_cleanup {
unlink '/dev/shm/pve-shm-' . ($ivshmem->{name} // $vmid);
}
- my $ids = [];
- foreach my $key (keys %$conf) {
- next if $key !~ m/^hostpci(\d+)$/;
- my $hostpciindex = $1;
- my $d = parse_hostpci($conf->{$key});
- my $uuid = PVE::SysFSTools::generate_mdev_uuid($vmid, $hostpciindex);
-
- foreach my $pci (@{$d->{pciid}}) {
- my $pciid = $pci->{id};
- push @$ids, $pci->{id};
- PVE::SysFSTools::pci_cleanup_mdev_device($pciid, $uuid);
- }
- }
- PVE::QemuServer::PCI::remove_pci_reservation($ids);
+ cleanup_pci_devices($vmid, $conf);
vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
};
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 3d0e70e..788ab2a 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -552,15 +552,17 @@ my $write_pci_reservation_unlocked = sub {
PVE::Tools::file_set_contents($PCIID_RESERVATION_FILE, $data);
};
+# removes all pci reservations of the given vmid
sub remove_pci_reservation {
- my ($dropped_ids) = @_;
-
- $dropped_ids = [ $dropped_ids ] if !ref($dropped_ids);
- return if !scalar(@$dropped_ids); # do nothing for empty list
+ my ($vmid) = @_;
PVE::Tools::lock_file($PCIID_RESERVATION_LOCK, 2, sub {
my $reservation_list = $parse_pci_reservation_unlocked->();
- delete $reservation_list->@{$dropped_ids->@*};
+ for my $id (keys %$reservation_list) {
+ my $reservation = $reservation_list->{$id};
+ next if $reservation->{vmid} != $vmid;
+ delete $reservation_list->{$id};
+ }
$write_pci_reservation_unlocked->($reservation_list);
});
die $@ if $@;
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] applied: [PATCH qemu-server v3 01/13] cleanup pci devices in more situations
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 01/13] cleanup pci devices in more situations Dominik Csapak
@ 2022-11-09 8:00 ` Thomas Lamprecht
0 siblings, 0 replies; 54+ messages in thread
From: Thomas Lamprecht @ 2022-11-09 8:00 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 20/09/2022 um 14:50 schrieb Dominik Csapak:
> if the preparing of pci devices or the start of the vm fails, we need
> to cleanup the pci devices (reservations *and* mdevs), or else
> it might happen that there are leftovers which must be manually removed.
>
> to include also mdevs now, refactor the cleanup code from 'vm_stop_cleanup'
> into it's own function, and call that instead of only 'remove_pci_reservation'
>
> also simplifies the code, such that it now removes all pci ids reserved
> for that vmid, since we cannot have multiple vms with the same vmid
> anyway
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> PVE/QemuServer.pm | 34 ++++++++++++++++++----------------
> PVE/QemuServer/PCI.pm | 12 +++++++-----
> 2 files changed, 25 insertions(+), 21 deletions(-)
>
>
I find it almost a bit odd though that we bothered to use a set of $dropped_ids in
the first place
anyhow, applied thanks!
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pve-devel] [PATCH qemu-server v3 02/13] PCI: make mediated device path independent of pci id
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (9 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 01/13] cleanup pci devices in more situations Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 8:08 ` [pve-devel] applied: " Thomas Lamprecht
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 03/13] PCI: refactor print_pci_device Dominik Csapak
` (26 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
mdevs must have a host-unique uuid, and they appear in
/sys/bus/mdev/devices/<uuid>, so there is no need to reference the pciid
with it
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer/PCI.pm | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 788ab2a..1b82aca 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -459,9 +459,8 @@ sub print_hostpci_devices {
my $sysfspath;
if ($d->{mdev} && scalar(@$pcidevices) == 1) {
- my $pci_id = $pcidevices->[0]->{id};
my $uuid = generate_mdev_uuid($vmid, $i);
- $sysfspath = "/sys/bus/pci/devices/$pci_id/$uuid";
+ $sysfspath = "/sys/bus/mdev/devices/$uuid";
} elsif ($d->{mdev}) {
warn "ignoring mediated device '$id' with multifunction device\n";
}
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 03/13] PCI: refactor print_pci_device
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (10 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 02/13] PCI: make mediated device path independent of pci id Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 7:49 ` Thomas Lamprecht
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 04/13] PCI: reuse parsed info from print_hostpci_devices Dominik Csapak
` (25 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
into a private sub. This makes the 'print_hostpci_devices' function more
easier to read
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer/PCI.pm | 57 +++++++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 24 deletions(-)
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 1b82aca..7406246 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -393,6 +393,29 @@ sub parse_hostpci {
return $res;
}
+my $print_pci_device = sub {
+ my ($device, $id, $hostdevice, $pciaddr, $xvga, $bootindex, $function) = @_;
+
+ my $devicestr = "vfio-pci,$hostdevice";
+
+ my $mf_addr = defined($function) ? ".$function" : '';
+
+ $devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
+
+ if (!defined($function) || $function == 0) {
+ $devicestr .= ',rombar=0' if defined($device->{rombar}) && !$device->{rombar};
+ $devicestr .= "$xvga";
+ $devicestr .= ",multifunction=on" if defined($function);
+ $devicestr .= ",romfile=/usr/share/kvm/$device->{romfile}" if $device->{romfile};
+ $devicestr .= ",bootindex=$bootindex" if defined($bootindex);
+ for my $option (qw(vendor-id device-id sub-vendor-id sub-device-id)) {
+ $devicestr .= ",x-pci-$option=$device->{$option}" if $device->{$option};
+ }
+ }
+
+ return $devicestr;
+};
+
sub print_hostpci_devices {
my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder) = @_;
@@ -457,37 +480,23 @@ sub print_hostpci_devices {
$gpu_passthrough = 1;
}
- my $sysfspath;
- if ($d->{mdev} && scalar(@$pcidevices) == 1) {
+ my $bootindex = $bootorder->{$id};
+
+ if ($d->{mdev} && !$multifunction) {
my $uuid = generate_mdev_uuid($vmid, $i);
- $sysfspath = "/sys/bus/mdev/devices/$uuid";
+ my $sysfspath = "sysfsdev=/sys/bus/mdev/devices/$uuid";
+ my $devicestr = $print_pci_device->($d, $id, $sysfspath, $pciaddr, $xvga, $bootindex);
+ push @$devices, '-device', $devicestr;
+ next;
} elsif ($d->{mdev}) {
warn "ignoring mediated device '$id' with multifunction device\n";
}
my $j = 0;
foreach my $pcidevice (@$pcidevices) {
- my $devicestr = "vfio-pci";
-
- if ($sysfspath) {
- $devicestr .= ",sysfsdev=$sysfspath";
- } else {
- $devicestr .= ",host=$pcidevice->{id}";
- }
-
- my $mf_addr = $multifunction ? ".$j" : '';
- $devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
-
- if ($j == 0) {
- $devicestr .= ',rombar=0' if defined($d->{rombar}) && !$d->{rombar};
- $devicestr .= "$xvga";
- $devicestr .= ",multifunction=on" if $multifunction;
- $devicestr .= ",romfile=/usr/share/kvm/$d->{romfile}" if $d->{romfile};
- $devicestr .= ",bootindex=$bootorder->{$id}" if $bootorder->{$id};
- for my $option (qw(vendor-id device-id sub-vendor-id sub-device-id)) {
- $devicestr .= ",x-pci-$option=$d->{$option}" if $d->{$option};
- }
- }
+ my $host = "host=$pcidevice->{id}";
+ my $func = $multifunction ? $j : undef;
+ my $devicestr = $print_pci_device->($d, $id, $host, $pciaddr, $xvga, $bootindex, $func);
push @$devices, '-device', $devicestr;
$j++;
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH qemu-server v3 03/13] PCI: refactor print_pci_device
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 03/13] PCI: refactor print_pci_device Dominik Csapak
@ 2022-11-09 7:49 ` Thomas Lamprecht
0 siblings, 0 replies; 54+ messages in thread
From: Thomas Lamprecht @ 2022-11-09 7:49 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 20/09/2022 um 14:50 schrieb Dominik Csapak:
> into a private sub. This makes the 'print_hostpci_devices' function more
> easier to read
looks OK, some style nits inline
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> PVE/QemuServer/PCI.pm | 57 +++++++++++++++++++++++++------------------
> 1 file changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 1b82aca..7406246 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -393,6 +393,29 @@ sub parse_hostpci {
> return $res;
> }
>
> +my $print_pci_device = sub {
nit: could use a `my sub` here or?
> + my ($device, $id, $hostdevice, $pciaddr, $xvga, $bootindex, $function) = @_;
could use the opportunity to switch to a more consistent naming style, e.g., using snake_case
> +
> + my $devicestr = "vfio-pci,$hostdevice";
> +
> + my $mf_addr = defined($function) ? ".$function" : '';
s/mf/multi_function/
> +
> + $devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
> +
> + if (!defined($function) || $function == 0) {
> + $devicestr .= ',rombar=0' if defined($device->{rombar}) && !$device->{rombar};
> + $devicestr .= "$xvga";
> + $devicestr .= ",multifunction=on" if defined($function);
> + $devicestr .= ",romfile=/usr/share/kvm/$device->{romfile}" if $device->{romfile};
> + $devicestr .= ",bootindex=$bootindex" if defined($bootindex);
> + for my $option (qw(vendor-id device-id sub-vendor-id sub-device-id)) {
> + $devicestr .= ",x-pci-$option=$device->{$option}" if $device->{$option};
> + }
> + }
> +
> + return $devicestr;
> +};
> +
> sub print_hostpci_devices {
> my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder) = @_;
>
> @@ -457,37 +480,23 @@ sub print_hostpci_devices {
> $gpu_passthrough = 1;
> }
>
> - my $sysfspath;
> - if ($d->{mdev} && scalar(@$pcidevices) == 1) {
> + my $bootindex = $bootorder->{$id};
> +
> + if ($d->{mdev} && !$multifunction) {
> my $uuid = generate_mdev_uuid($vmid, $i);
> - $sysfspath = "/sys/bus/mdev/devices/$uuid";
> + my $sysfspath = "sysfsdev=/sys/bus/mdev/devices/$uuid";
> + my $devicestr = $print_pci_device->($d, $id, $sysfspath, $pciaddr, $xvga, $bootindex);
> + push @$devices, '-device', $devicestr;
maybe add a short comment or empty line here to make this `next` stand out a bit more, almost
overlooked it
> + next;
> } elsif ($d->{mdev}) {
> warn "ignoring mediated device '$id' with multifunction device\n";
> }
>
> my $j = 0;
> foreach my $pcidevice (@$pcidevices) {
> - my $devicestr = "vfio-pci";
> -
> - if ($sysfspath) {
> - $devicestr .= ",sysfsdev=$sysfspath";
> - } else {
> - $devicestr .= ",host=$pcidevice->{id}";
> - }
> -
> - my $mf_addr = $multifunction ? ".$j" : '';
> - $devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
> -
> - if ($j == 0) {
> - $devicestr .= ',rombar=0' if defined($d->{rombar}) && !$d->{rombar};
> - $devicestr .= "$xvga";
> - $devicestr .= ",multifunction=on" if $multifunction;
> - $devicestr .= ",romfile=/usr/share/kvm/$d->{romfile}" if $d->{romfile};
> - $devicestr .= ",bootindex=$bootorder->{$id}" if $bootorder->{$id};
> - for my $option (qw(vendor-id device-id sub-vendor-id sub-device-id)) {
> - $devicestr .= ",x-pci-$option=$d->{$option}" if $d->{$option};
> - }
> - }
> + my $host = "host=$pcidevice->{id}";
extra white space after =
> + my $func = $multifunction ? $j : undef;
> + my $devicestr = $print_pci_device->($d, $id, $host, $pciaddr, $xvga, $bootindex, $func);
>
> push @$devices, '-device', $devicestr;
> $j++;
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pve-devel] [PATCH qemu-server v3 04/13] PCI: reuse parsed info from print_hostpci_devices
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (11 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 03/13] PCI: refactor print_pci_device Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 8:23 ` Thomas Lamprecht
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 05/13] PVE/QemuServer: allow mapped usb devices in config Dominik Csapak
` (24 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
instead of parsing the config again when trying to reserver/prepare the
pci devices. also split the preparing into non-mdev devices and mdev
devices, this will come in handy later.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer.pm | 20 +++++---------------
PVE/QemuServer/PCI.pm | 8 +++++++-
2 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b3c3ce0..0a573de 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3687,7 +3687,7 @@ sub config_to_command {
my $bootorder = device_bootorder($conf);
# host pci device passthrough
- my ($kvm_off, $gpu_passthrough, $legacy_igd) = PVE::QemuServer::PCI::print_hostpci_devices(
+ my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) = PVE::QemuServer::PCI::print_hostpci_devices(
$vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder);
# usb devices
@@ -4103,7 +4103,7 @@ sub config_to_command {
push @$cmd, @$aa;
}
- return wantarray ? ($cmd, $vollist, $spice_port) : $cmd;
+ return wantarray ? ($cmd, $vollist, $spice_port, $pci_devices) : $cmd;
}
sub check_rng_source {
@@ -5493,7 +5493,7 @@ sub vm_start_nolock {
print "Resuming suspended VM\n";
}
- my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid,
+ my ($cmd, $vollist, $spice_port, $pci_devices) = config_to_command($storecfg, $vmid,
$conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'});
my $migration_ip;
@@ -5578,17 +5578,7 @@ sub vm_start_nolock {
my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $resume);
- my $pci_devices = {}; # host pci devices
- for (my $i = 0; $i < $PVE::QemuServer::PCI::MAX_HOSTPCI_DEVICES; $i++) {
- my $dev = $conf->{"hostpci$i"} or next;
- $pci_devices->{$i} = parse_hostpci($dev);
- }
-
- # do not reserve pciid for mediated devices, sysfs will error out for duplicate assignment
- my $real_pci_devices = [ grep { !(defined($_->{mdev}) && scalar($_->{pciid}->@*) == 1) } values $pci_devices->%* ];
-
- # map to a flat list of pci ids
- my $pci_id_list = [ map { $_->{id} } map { $_->{pciid}->@* } $real_pci_devices->@* ];
+ my $pci_id_list = [ map { $_->{id} } map { $_->{used}->@* } values %$pci_devices ];
# reserve all PCI IDs before actually doing anything with them
PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, $start_timeout);
@@ -5596,7 +5586,7 @@ sub vm_start_nolock {
eval {
my $uuid;
for my $id (sort keys %$pci_devices) {
- my $d = $pci_devices->{$id};
+ my $d = $pci_devices->{$id}->{device};
for my $dev ($d->{pciid}->@*) {
my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 7406246..b5284ef 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -422,12 +422,17 @@ sub print_hostpci_devices {
my $kvm_off = 0;
my $gpu_passthrough = 0;
my $legacy_igd = 0;
+ my $parsed_devices = {};
my $pciaddr;
for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++) {
my $id = "hostpci$i";
my $d = parse_hostpci($conf->{$id});
next if !$d;
+ $parsed_devices->{$i} = {
+ device => $d,
+ used => [],
+ };
if (my $pcie = $d->{pcie}) {
die "q35 machine model is not enabled" if !$q35;
@@ -449,6 +454,7 @@ sub print_hostpci_devices {
}
my $pcidevices = $d->{pciid};
+ $parsed_devices->{$i}->{used} = $pcidevices;
my $multifunction = @$pcidevices > 1;
if ($d->{'legacy-igd'}) {
@@ -503,7 +509,7 @@ sub print_hostpci_devices {
}
}
- return ($kvm_off, $gpu_passthrough, $legacy_igd);
+ return ($kvm_off, $gpu_passthrough, $legacy_igd, $parsed_devices);
}
sub prepare_pci_device {
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH qemu-server v3 04/13] PCI: reuse parsed info from print_hostpci_devices
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 04/13] PCI: reuse parsed info from print_hostpci_devices Dominik Csapak
@ 2022-11-09 8:23 ` Thomas Lamprecht
0 siblings, 0 replies; 54+ messages in thread
From: Thomas Lamprecht @ 2022-11-09 8:23 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 20/09/2022 um 14:50 schrieb Dominik Csapak:
> instead of parsing the config again when trying to reserver/prepare the
> pci devices. also split the preparing into non-mdev devices and mdev
> devices, this will come in handy later.
I'd rather factor out the parsing parts, see below.
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 7406246..b5284ef 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -422,12 +422,17 @@ sub print_hostpci_devices {
> my $kvm_off = 0;
> my $gpu_passthrough = 0;
> my $legacy_igd = 0;
> + my $parsed_devices = {};
>
> my $pciaddr;
> for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++) {
> my $id = "hostpci$i";
> my $d = parse_hostpci($conf->{$id});
> next if !$d;
> + $parsed_devices->{$i} = {
> + device => $d,
> + used => [],
> + };
note that iff we'd do that I'd want a comment on the sub describing the parsed_device
"struct" and its field; if it sounds like I'm really missing rust here you'd be spot
on..
>
> if (my $pcie = $d->{pcie}) {
> die "q35 machine model is not enabled" if !$q35;
> @@ -449,6 +454,7 @@ sub print_hostpci_devices {
> }
>
> my $pcidevices = $d->{pciid};
> + $parsed_devices->{$i}->{used} = $pcidevices;
so, this is already included in $parsed_devices->{$i} which contains the full $d
anyway, what gives?
How about splitting out the looping over all possible "hostpci$i" with the checks
and transformations done with the parsed $d into an parse_hostpci_devices returning
only the hash and pass then that to print_hostpci_devices (which then can probably
drop some parameters), that may be even better outcome in terms of readability and
maintainability than the "03/13 refactor print_pci_device", especially if you don't
plan to reuse that anyway (only skimmed remaining patches), so hopefully a cleaner
separation.
for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)
> my $multifunction = @$pcidevices > 1;
>
> if ($d->{'legacy-igd'}) {
> @@ -503,7 +509,7 @@ sub print_hostpci_devices {
> }
> }
>
> - return ($kvm_off, $gpu_passthrough, $legacy_igd);
> + return ($kvm_off, $gpu_passthrough, $legacy_igd, $parsed_devices);
> }
>
> sub prepare_pci_device {
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pve-devel] [PATCH qemu-server v3 05/13] PVE/QemuServer: allow mapped usb devices in config
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (12 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 04/13] PCI: reuse parsed info from print_hostpci_devices Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 06/13] PVE/QemuServer: allow mapped pci deviced " Dominik Csapak
` (23 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 0a573de..be38e10 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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 06/13] PVE/QemuServer: allow mapped pci deviced in config
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (13 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 05/13] PVE/QemuServer: allow mapped usb devices in config Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 07/13] PVE/API2/Qemu: add permission checks for mapped usb devices Dominik Csapak
` (22 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
and get the correct pci device during parsing
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer/PCI.pm | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index b5284ef..08244c1 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,10 +386,27 @@ sub parse_hostpci {
my $res = PVE::JSONSchema::parse_property_string($hostpci_fmt, $value);
+ my $mapping = 0;
+ if ($res->{host} !~ m/:/) {
+ # we have no ordinary pci id, must be a mapping
+ $mapping = 1;
+ 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) {
my $devs = PVE::SysFSTools::lspci($id);
+ die "cannot use mediated device with multifuntion device\n"
+ if $mapping && $res->{mdev} && scalar(@$devs) > 1;
die "no PCI device found for '$id'\n" if !scalar(@$devs);
push @{$res->{pciid}}, @$devs;
}
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 07/13] PVE/API2/Qemu: add permission checks for mapped usb devices
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (14 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 06/13] PVE/QemuServer: allow mapped pci deviced " Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 08/13] PVE/API2/Qemu: add permission checks for mapped pci devices Dominik Csapak
` (21 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/API2/Qemu.pm | 40 +++++++++++++++++++++++++++++++++++++---
PVE/QemuServer.pm | 2 ++
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 3ec31c2..7afd7a4 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;
@@ -586,8 +587,13 @@ my $check_vm_create_usb_perm = sub {
foreach my $opt (keys %{$param}) {
next if $opt !~ m/^usb\d+$/;
+ my $entry = PVE::JSONSchema::parse_property_string('pve-qm-usb', $param->{$opt});
+ my $device = parse_usb_device($entry->{host});
- if ($param->{$opt} =~ m/spice/) {
+ if ($device->{spice}) {
+ $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
+ } elsif ($device->{mapped}) {
+ $rpcenv->check_full($authuser, "/hardware/$entry->{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";
@@ -1571,7 +1577,12 @@ my $update_vm_api = sub {
PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
PVE::QemuConfig->write_config($vmid, $conf);
} elsif ($opt =~ m/^usb\d+$/) {
- if ($val =~ m/spice/) {
+ my $device = PVE::JSONSchema::parse_property_string('pve-qm-usb', $val);
+ my $host = parse_usb_device($device->{host});
+ if ($host->{spice}) {
+ $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+ } elsif ($host->{mapped}) {
+ $rpcenv->check_full($authuser, "/hardware/$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";
@@ -1632,7 +1643,30 @@ my $update_vm_api = sub {
}
$conf->{pending}->{$opt} = $param->{$opt};
} elsif ($opt =~ m/^usb\d+/) {
- if ((!defined($conf->{$opt}) || $conf->{$opt} =~ m/spice/) && $param->{$opt} =~ m/spice/) {
+ my $olddevice;
+ my $oldhost;
+ if (defined($conf->{$opt})) {
+ $olddevice = PVE::JSONSchema::parse_property_string('pve-qm-usb', $conf->{$opt});
+ $oldhost = parse_usb_device($olddevice->{host});
+ }
+ if (defined($oldhost)) {
+ if ($oldhost->{spice}) {
+ $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+ } elsif ($oldhost->{mapped}) {
+ $rpcenv->check_full($authuser, "/hardware/$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::JSONSchema::parse_property_string('pve-qm-usb', $param->{$opt});
+ my $newhost = parse_usb_device($newdevice->{host});
+
+ if ($newhost->{spice}) {
+ $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+ } elsif ($newhost->{mapped}) {
+ $rpcenv->check_full($authuser, "/hardware/$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";
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index be38e10..6a2ad8d 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1085,6 +1085,8 @@ EODESCR
},
};
+PVE::JSONSchema::register_format('pve-qm-usb', $usb_fmt);
+
my $usbdesc = {
optional => 1,
type => 'string', format => $usb_fmt,
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 08/13] PVE/API2/Qemu: add permission checks for mapped pci devices
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (15 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 07/13] PVE/API2/Qemu: add permission checks for mapped usb devices Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-11-09 12:14 ` Fabian Grünbichler
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 09/13] PVE/QemuServer: extend 'check_local_resources' for mapped resources Dominik Csapak
` (20 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 7afd7a4..d6d393f 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;
@@ -603,6 +604,26 @@ my $check_vm_create_usb_perm = sub {
return 1;
};
+my $check_vm_create_hostpci_perm = sub {
+ my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
+
+ return 1 if $authuser eq 'root@pam';
+
+ foreach my $opt (keys %{$param}) {
+ next if $opt !~ m/^hostpci\d+$/;
+
+ my $device = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $param->{$opt});
+ if ($device->{host} !~ m/:/) {
+ $rpcenv->check_full($authuser, "/hardware/$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) = @_;
@@ -613,7 +634,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+$/) {
@@ -641,7 +662,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";
}
@@ -876,6 +897,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);
@@ -1589,6 +1611,16 @@ my $update_vm_api = sub {
}
PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
PVE::QemuConfig->write_config($vmid, $conf);
+ } elsif ($opt =~ m/^hostpci\d+$/) {
+ my $olddevice = PVE::JSONSchema::parse_property_string('pve-qm-hostpci', $val);
+ if ($olddevice->{host} !~ m/:/) {
+ $rpcenv->check_full($authuser, "/hardware/$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);
@@ -1671,6 +1703,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_full($authuser, "/hardware/$olddevice->{host}", ['Hardware.Use']);
+ }
+ $rpcenv->check_full($authuser, "/hardware/$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
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH qemu-server v3 08/13] PVE/API2/Qemu: add permission checks for mapped pci devices
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 08/13] PVE/API2/Qemu: add permission checks for mapped pci devices Dominik Csapak
@ 2022-11-09 12:14 ` Fabian Grünbichler
2022-11-09 12:51 ` Dominik Csapak
0 siblings, 1 reply; 54+ messages in thread
From: Fabian Grünbichler @ 2022-11-09 12:14 UTC (permalink / raw)
To: Proxmox VE development discussion
On September 20, 2022 2:50 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 7afd7a4..d6d393f 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;
> @@ -603,6 +604,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/:/) {
while thinking about the priv patch I decided to check out the ACL handling as
well - sorry for not asking earlier about this aspect!
would it make sense to have a prefix for "ID of mapped hardware" instead of
claiming "everything that doesn't contain ':'" as namespace?
the same also applies to the USB ACL checks and the other checks in this patch..
> + $rpcenv->check_full($authuser, "/hardware/$device->{host}", ['Hardware.Use']);
this and similar sites would then also be more explicit:
my $hw_id = ..; # extract actual ID
$rpvenv->check_full($authuser, "/hardware/$hw_id", ['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;
> +};
> +
>
> [...]
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH qemu-server v3 08/13] PVE/API2/Qemu: add permission checks for mapped pci devices
2022-11-09 12:14 ` Fabian Grünbichler
@ 2022-11-09 12:51 ` Dominik Csapak
2022-11-09 13:28 ` Fabian Grünbichler
0 siblings, 1 reply; 54+ messages in thread
From: Dominik Csapak @ 2022-11-09 12:51 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 11/9/22 13:14, Fabian Grünbichler wrote:
> On September 20, 2022 2:50 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 7afd7a4..d6d393f 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;
>> @@ -603,6 +604,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/:/) {
>
> while thinking about the priv patch I decided to check out the ACL handling as
> well - sorry for not asking earlier about this aspect!
>
> would it make sense to have a prefix for "ID of mapped hardware" instead of
> claiming "everything that doesn't contain ':'" as namespace?
i mean we could, but a few downsides come to mind:
* if we prefix the ids in the hardware-map itself, the user always
sees that prefix, which is useless to them, or we have to remove it
everywhere when displaying, which is imho unnecessary work
* if we only prefix it in the vm config, we have to add/remove it
in the gui/cli (or api?) which also means users that edit
the config don't have the direct correlation with the hardware map
basically, more work for us all around
for usb devices we actually check the vendor/id/path regex first, then
for spice (which is the only exception to the namespace clash) and
all other values we interpret as mapped devices
(imho the small downside of not being able to name a mapping 'spice'
is worth the work we save by not introducing a prefix/namespace)
we could do the same for pci ids of course, i just used the shortcut
of having ':'
ofc if you insist on having some separated namespace by prefix (or
similar), i'll implement that, i'm not very attached to the current
implementation ;)
>
> the same also applies to the USB ACL checks and the other checks in this patch..
>
>> + $rpcenv->check_full($authuser, "/hardware/$device->{host}", ['Hardware.Use']);
>
> this and similar sites would then also be more explicit:
>
> my $hw_id = ..; # extract actual ID
> $rpvenv->check_full($authuser, "/hardware/$hw_id", ['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;
>> +};
>> +
>>
>> [...]
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH qemu-server v3 08/13] PVE/API2/Qemu: add permission checks for mapped pci devices
2022-11-09 12:51 ` Dominik Csapak
@ 2022-11-09 13:28 ` Fabian Grünbichler
0 siblings, 0 replies; 54+ messages in thread
From: Fabian Grünbichler @ 2022-11-09 13:28 UTC (permalink / raw)
To: Dominik Csapak, Proxmox VE development discussion
On November 9, 2022 1:51 pm, Dominik Csapak wrote:
>
> On 11/9/22 13:14, Fabian Grünbichler wrote:
>> On September 20, 2022 2:50 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 7afd7a4..d6d393f 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;
>>> @@ -603,6 +604,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/:/) {
>>
>> while thinking about the priv patch I decided to check out the ACL handling as
>> well - sorry for not asking earlier about this aspect!
>>
>> would it make sense to have a prefix for "ID of mapped hardware" instead of
>> claiming "everything that doesn't contain ':'" as namespace?
>
> i mean we could, but a few downsides come to mind:
> * if we prefix the ids in the hardware-map itself, the user always
> sees that prefix, which is useless to them, or we have to remove it
> everywhere when displaying, which is imho unnecessary work
> * if we only prefix it in the vm config, we have to add/remove it
> in the gui/cli (or api?) which also means users that edit
> the config don't have the direct correlation with the hardware map
I meant the latter ;) I think with something like "hw_map:NAME" it would be
pretty clear. and yeah, it would need to be added client/GUI side, not in the
API else it would be really confusing..
> basically, more work for us all around
>
> for usb devices we actually check the vendor/id/path regex first, then
> for spice (which is the only exception to the namespace clash) and
> all other values we interpret as mapped devices
> (imho the small downside of not being able to name a mapping 'spice'
> is worth the work we save by not introducing a prefix/namespace)
there is a second downside - we don't have a way to add another non-mapping
value that uses a valid (i.e., readable!) name. obviously doesn't matter if we
are sure no such values have to be added in the future (or if we don't care that
we need to encode them in way that makes them live in a different namespace than
the map entry names, which would in turn restrict future extensions of the name
schema there).
> we could do the same for pci ids of course, i just used the shortcut
> of having ':'
>
> ofc if you insist on having some separated namespace by prefix (or
> similar), i'll implement that, i'm not very attached to the current
> implementation ;)
no, I don't insist on it - just wanted to make sure it's a conscious decision
and not an oversight :) when in doubt, I'd rather err on the side of having a
simple, understandable prefix (preserving future extensibility).
what actually originally prompted me was that I find "/hardware/$device->{host}"
to be a really weird ACL path, as it's not immediately obvious that "host" here
is overloaded to either mean the original host-hardware-ID or the newly added
name of the map entry. having it prefixed with a helper to check for and extract
the name would make the code more readable IMHO.
>> the same also applies to the USB ACL checks and the other checks in this patch..
>>
>>> + $rpcenv->check_full($authuser, "/hardware/$device->{host}", ['Hardware.Use']);
>>
>> this and similar sites would then also be more explicit:
>>
>> my $hw_id = ..; # extract actual ID
>> $rpvenv->check_full($authuser, "/hardware/$hw_id", ['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;
>>> +};
>>> +
>>>
>>> [...]
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pve-devel] [PATCH qemu-server v3 09/13] PVE/QemuServer: extend 'check_local_resources' for mapped resources
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (16 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 08/13] PVE/API2/Qemu: add permission checks for mapped pci devices Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 10/13] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info Dominik Csapak
` (19 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
by adding them to their own list, saving the nodes where
they are not allowed, and return those on 'wantarray'
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer.pm | 36 ++++++++++++++++++++++++++++++++++--
test/MigrationTest/Shared.pm | 7 +++++++
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6a2ad8d..d23cfc2 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2619,6 +2619,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
@@ -2626,7 +2641,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+$/;
@@ -2634,7 +2666,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)
diff --git a/test/MigrationTest/Shared.pm b/test/MigrationTest/Shared.pm
index 8ae6a6e..0eb94f4 100644
--- a/test/MigrationTest/Shared.pm
+++ b/test/MigrationTest/Shared.pm
@@ -69,6 +69,13 @@ $cluster_module->mock(
},
);
+our $hardware_map_module = Test::MockModule->new("PVE::HardwareMap");
+$hardware_map_module->mock(
+ config => sub {
+ return {};
+ },
+);
+
our $ha_config_module = Test::MockModule->new("PVE::HA::Config");
$ha_config_module->mock(
vm_is_ha_managed => sub {
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 10/13] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (17 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 09/13] PVE/QemuServer: extend 'check_local_resources' for mapped resources Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 11/13] PVE/QemuMigrate: check for mapped resources on migration Dominik Csapak
` (18 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 d6d393f..cc726bd 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4185,6 +4185,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}) {
@@ -4193,7 +4197,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;
}
@@ -4201,13 +4210,11 @@ __PACKAGE__->register_method({
$res->{not_allowed_nodes} = $checked_nodes;
}
-
my $local_disks = &$check_vm_disks_local($storecfg, $vmconf, $vmid);
$res->{local_disks} = [ values %$local_disks ];;
- my $local_resources = PVE::QemuServer::check_local_resources($vmconf, 1);
-
$res->{local_resources} = $local_resources;
+ $res->{mapped_resources} = $mapped_resources;
return $res;
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 11/13] PVE/QemuMigrate: check for mapped resources on migration
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (18 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 10/13] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 12/13] fix #3574: enable multi pci device mapping from config Dominik Csapak
` (17 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
they can only be migrated to nodes where there exists a mapping and if
the migration is done offline
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuMigrate.pm | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index d52dc8d..54530fd 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 ($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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 12/13] fix #3574: enable multi pci device mapping from config
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (19 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 11/13] PVE/QemuMigrate: check for mapped resources on migration Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 13/13] add tests for mapped pci devices Dominik Csapak
` (16 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
The hardware config now supports multiple devices as a semicolon
seperated list. With this, instead of only having one device in a pci mapping,
we now have a list of which we can choose from on vm start. This way one can
dynamically start vms with a pool of (identical) pci devices without
having to manually assign the proper ids.
For that we have to change the internal representation of a parsed
device, such that we have the seperately configured paths in the mapping
in different lists (because multifunction devices still are interpreted
as single devices)
For mdev devices we now can also have multiple devices, where we simply
try to create the appropriate type on each until we either have one
created, or bail out.
Since we now have to reserve the pci ids in print_hostpci_devices, we
have to add a 'reserve' parameter to config_to_command (and chain it
through to reserve_pci_usage) so that a 'qm showcmd' does not actually
reserve any pci id (this would break when using that on running vms).
Additionally this also prevents the migration tests from failing
(they use vm_commandline which in turn uses config_to_command)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/QemuServer.pm | 43 ++++++++++++++++++++++++++++-------
PVE/QemuServer/PCI.pm | 53 +++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 18 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index d23cfc2..5833aba 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3507,8 +3507,9 @@ my sub should_disable_smm {
sub config_to_command {
my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
- $pbs_backing) = @_;
+ $pbs_backing, $reserve) = @_;
+ $reserve //= 1;
my $cmd = [];
my ($globalFlags, $machineFlags, $rtcFlags) = ([], [], []);
my $devices = [];
@@ -3724,7 +3725,7 @@ sub config_to_command {
# host pci device passthrough
my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) = PVE::QemuServer::PCI::print_hostpci_devices(
- $vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder);
+ $vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder, $reserve);
# usb devices
my $usb_dev_features = {};
@@ -5623,13 +5624,30 @@ sub vm_start_nolock {
my $uuid;
for my $id (sort keys %$pci_devices) {
my $d = $pci_devices->{$id}->{device};
- for my $dev ($d->{pciid}->@*) {
- my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id, $d->{mdev});
- # nvidia grid needs the uuid of the mdev as qemu parameter
- if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '10de') {
- $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $id);
+ # used pci devices for non-mdev
+ if (!$d->{mdev}) {
+ for my $dev ($pci_devices->{$id}->{used}->@*) {
+ PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $id);
}
+ next;
+ }
+
+ # try each configured pci device for mdevs
+ my $devs = [map { $_->{id} } map { @$_ } $d->{ids}->@*]; # flatten ids
+
+ my $info;
+ for my $dev (@$devs) {
+ $info = eval { PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev, $id, $d->{mdev}) };
+ warn $@ if $@;
+ last if $info; # if successful, we're done
+ }
+
+ die "could not create mediated device\n" if !defined($info);
+
+ # nvidia grid needs the uuid of the mdev as qemu parameter
+ if (!defined($uuid) && $info->{vendor} eq '10de') {
+ $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $id);
}
}
push @$cmd, '-uuid', $uuid if defined($uuid);
@@ -5862,7 +5880,16 @@ sub vm_commandline {
my $defaults = load_defaults();
- my $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu);
+ my $cmd = config_to_command(
+ $storecfg,
+ $vmid,
+ $conf,
+ $defaults,
+ $forcemachine,
+ $forcecpu,
+ undef,
+ 0,
+ );
return PVE::Tools::cmd2string($cmd);
}
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 08244c1..1ad89ed 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -386,6 +386,7 @@ sub parse_hostpci {
my $res = PVE::JSONSchema::parse_property_string($hostpci_fmt, $value);
+ my $idlist = [];
my $mapping = 0;
if ($res->{host} !~ m/:/) {
# we have no ordinary pci id, must be a mapping
@@ -398,17 +399,29 @@ sub parse_hostpci {
if (my $err = $@) {
die "PCI device mapping invalid (hardware probably changed): $err\n";
}
- $res->{host} = $device->{path};
+ $idlist = [split(/;/, $device->{path})];
+ # if we have a list of mapped devices, we want to choose the first available one
+ $res->{choose} = 1 if scalar(@$idlist > 1);
+ } else {
+ $idlist = [split(/;/, $res->{host})];
}
- my @idlist = split(/;/, $res->{host});
delete $res->{host};
- foreach my $id (@idlist) {
+ my $ignore_mdev = !$res->{choose} && scalar(@$idlist) > 1;
+
+ $res->{ids} = [];
+ foreach my $id (@$idlist) {
my $devs = PVE::SysFSTools::lspci($id);
- die "cannot use mediated device with multifuntion device\n"
- if $mapping && $res->{mdev} && scalar(@$devs) > 1;
die "no PCI device found for '$id'\n" if !scalar(@$devs);
- push @{$res->{pciid}}, @$devs;
+ $ignore_mdev = 1 if scalar(@$devs) > 1;
+ push @{$res->{ids}}, $devs;
+ }
+ # ignore mdev for multiple devices, except when from mapping
+ if ($res->{mdev} && $ignore_mdev) {
+ # FIXME in 8.0 we should also disallow that for 'normal' passthrough
+ die "cannot use mediated device with multifunction device\n" if $mapping;
+ warn "ignoring mediated device with multifunction device\n";
+ delete $res->{mdev};
}
return $res;
}
@@ -437,11 +450,13 @@ my $print_pci_device = sub {
};
sub print_hostpci_devices {
- my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder) = @_;
+ my ($vmid, $conf, $devices, $vga, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder, $reserve) = @_;
+ $reserve //= 1;
my $kvm_off = 0;
my $gpu_passthrough = 0;
my $legacy_igd = 0;
+ my $used_pci_ids = {};
my $parsed_devices = {};
my $pciaddr;
@@ -473,7 +488,24 @@ sub print_hostpci_devices {
$pciaddr = print_pci_addr($pci_name, $bridges, $arch, $machine_type);
}
- my $pcidevices = $d->{pciid};
+ # choose devices
+ my $pcidevices = [];
+ if (!$d->{mdev}) {
+ for my $devs ($d->{ids}->@*) {
+ my $ids = [map { $_->{id} } @$devs];
+
+ if ($d->{choose}) {
+ next if grep { defined($used_pci_ids->{$_}) } @$ids; # already used
+ eval { reserve_pci_usage($ids, $vmid, 10, undef, $reserve) };
+ next if $@;
+ }
+
+ map { $used_pci_ids->{$_} = 1 } @$ids;
+ push @$pcidevices, @$devs;
+ last if $d->{choose};
+ }
+ die "could not find a free device\n" if scalar(@$pcidevices) < 1;
+ }
$parsed_devices->{$i}->{used} = $pcidevices;
my $multifunction = @$pcidevices > 1;
@@ -603,8 +635,9 @@ sub remove_pci_reservation {
}
sub reserve_pci_usage {
- my ($requested_ids, $vmid, $timeout, $pid) = @_;
+ my ($requested_ids, $vmid, $timeout, $pid, $reserve) = @_;
+ $reserve //= 1;
$requested_ids = [ $requested_ids ] if !ref($requested_ids);
return if !scalar(@$requested_ids); # do nothing for empty list
@@ -637,7 +670,7 @@ sub reserve_pci_usage {
$reservation_list->{$id}->{time} = $ctime + $timeout + 5;
}
}
- $write_pci_reservation_unlocked->($reservation_list);
+ $write_pci_reservation_unlocked->($reservation_list) if $reserve;
});
die $@ if $@;
}
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH qemu-server v3 13/13] add tests for mapped pci devices
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (20 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 12/13] fix #3574: enable multi pci device mapping from config Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 01/13] PVE/API2/Hardware: add Mapping.pm Dominik Csapak
` (15 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
test/cfg2cmd/q35-linux-hostpci-mapping.conf | 17 +++++
.../q35-linux-hostpci-mapping.conf.cmd | 36 +++++++++
test/run_config2command_tests.pl | 76 +++++++++++++++++++
3 files changed, 129 insertions(+)
create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf
create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
diff --git a/test/cfg2cmd/q35-linux-hostpci-mapping.conf b/test/cfg2cmd/q35-linux-hostpci-mapping.conf
new file mode 100644
index 0000000..2402cf2
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-mapping.conf
@@ -0,0 +1,17 @@
+# TEST: Config with q35, NUMA, hostpci mapping passthrough, EFI & Linux
+bios: ovmf
+bootdisk: scsi0
+cores: 1
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+hostpci0: someNic
+hostpci1: someGpu,mdev=some-model
+hostpci2: someNic
+machine: q35
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+numa: 1
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+sockets: 2
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
new file mode 100644
index 0000000..a5b3fe8
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
@@ -0,0 +1,36 @@
+/usr/bin/kvm \
+ -id 8006 \
+ -name 'vm8006,debug-threads=on' \
+ -no-shutdown \
+ -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+ -mon 'chardev=qmp,mode=control' \
+ -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+ -mon 'chardev=qmp-event,mode=control' \
+ -pidfile /var/run/qemu-server/8006.pid \
+ -daemonize \
+ -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
+ -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+ -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+ -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
+ -smp '2,sockets=2,cores=1,maxcpus=2' \
+ -nodefaults \
+ -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+ -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+ -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+ -m 512 \
+ -object 'memory-backend-ram,id=ram-node0,size=256M' \
+ -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \
+ -object 'memory-backend-ram,id=ram-node1,size=256M' \
+ -numa 'node,nodeid=1,cpus=1,memdev=ram-node1' \
+ -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+ -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+ -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+ -device 'vfio-pci,host=0000:07:10.0,id=hostpci0,bus=pci.0,addr=0x10' \
+ -device 'vfio-pci,sysfsdev=/sys/bus/mdev/devices/00000001-0000-0000-0000-000000008006,id=hostpci1,bus=pci.0,addr=0x11' \
+ -device 'vfio-pci,host=0000:07:10.4,id=hostpci2,bus=pci.0,addr=0x1b' \
+ -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+ -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
+ -machine 'type=q35+pve0'
diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index f097811..c12afac 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -86,8 +86,38 @@ my $pci_devs = [
"0000:f0:43.0",
"0000:f0:43.1",
"1234:f0:43.1",
+ "0000:01:00.4",
+ "0000:01:00.5",
+ "0000:01:00.6",
+ "0000:07:10.0",
+ "0000:07:10.1",
+ "0000:07:10.4",
];
+my $hardware_map_config = {
+ pci => {
+ someGpu => {
+ localhost => {
+ iommugroup => 1,
+ mdev => 1,
+ vendor => "0x10de",
+ device => "0x2231",
+ path => "0000:01:00.4;0000:01:00.5;0000:01:00.6",
+ }
+ },
+ someNic => {
+ localhost => {
+ iommugroup => 2,
+ mdev => 0,
+ vendor => "0x8086",
+ device => "0x1520",
+ path => "0000:07:10.0;0000:07:10.1;0000:07:10.4"
+ }
+ }
+ },
+ usb => {},
+};
+
my $current_test; # = {
# description => 'Test description', # if available
# qemu_version => '2.12',
@@ -259,6 +289,28 @@ $pve_common_sysfstools->mock(
} sort @$pci_devs
];
},
+ pci_device_info => sub {
+ my ($path, $noerr) = @_;
+
+ if ($path =~ m/^0000:01:00/) {
+ return {
+ mdev => 1,
+ iommugroup => 1,
+ mdev => 1,
+ vendor => "0x10de",
+ device => "0x2231",
+ };
+ } elsif ($path =~ m/^0000:07:10/) {
+ return {
+ iommugroup => 2,
+ mdev => 0,
+ vendor => "0x8086",
+ device => "0x1520",
+ };
+ } else {
+ return {};
+ }
+ },
);
my $qemu_monitor_module;
@@ -287,6 +339,30 @@ $qemu_monitor_module->mock(
);
$qemu_monitor_module->mock('qmp_cmd', \&qmp_cmd);
+my $hardware_map_module = Test::MockModule->new("PVE::HardwareMap");
+$hardware_map_module->mock(
+ config => sub {
+ return $hardware_map_config;
+ },
+);
+
+my $pci_module = Test::MockModule->new("PVE::QemuServer::PCI");
+$pci_module->mock(
+ reserve_pci_usage => sub {
+ my ($ids, $vmid, $timeout, $pid, $dryrun) = @_;
+
+ $ids = [$ids] if !ref($ids);
+
+ for my $id (@$ids) {
+ if ($id eq "0000:07:10.1") {
+ die "reserved";
+ }
+ }
+
+ return undef;
+ },
+);
+
sub diff($$) {
my ($a, $b) = @_;
return if $a eq $b;
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 01/13] PVE/API2/Hardware: add Mapping.pm
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (21 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH qemu-server v3 13/13] add tests for mapped pci devices Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 02/13] PVE/API2/Cluster: add Hardware mapping list api call Dominik Csapak
` (14 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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..f07b3d00
--- /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_full($authuser, "/hardware/$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_full($authuser, "/hardware/$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_full($authuser, "/hardware/$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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 02/13] PVE/API2/Cluster: add Hardware mapping list api call
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (22 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 01/13] PVE/API2/Hardware: add Mapping.pm Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 03/13] ui: form/USBSelector: make it more flexible with nodename Dominik Csapak
` (13 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 d6b405e2..85c4965d 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..233d5efa
--- /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_full($authuser, "/hardware/$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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 03/13] ui: form/USBSelector: make it more flexible with nodename
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (23 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 02/13] PVE/API2/Cluster: add Hardware mapping list api call Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 04/13] ui: form: add PCIMapSelector Dominik Csapak
` (12 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
similar to the pciselector, make it accept a plain nodename,
or no node at all and provide a setNodename function
to keep backwards compatibility, also check pveSelNode for the nodename
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/form/USBSelector.js | 32 ++++++++++++++++++++++++--------
1 file changed, 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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 04/13] ui: form: add PCIMapSelector
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (24 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 03/13] ui: form/USBSelector: make it more flexible with nodename Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 05/13] ui: form: add USBMapSelector Dominik Csapak
` (11 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
akin to the PCISelector, but uses the api for mapped devices
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/form/PCIMapSelector.js | 102 ++++++++++++++++++++++++++++
2 files changed, 103 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..62b627bf
--- /dev/null
+++ b/www/manager6/form/PCIMapSelector.js
@@ -0,0 +1,102 @@
+Ext.define('pve-mapped-pci-model', {
+ extend: 'Ext.data.Model',
+
+ fields: ['name', 'path', 'vendor', 'device', 'iommugroup', 'mdev'],
+ idProperty: 'name',
+});
+
+Ext.define('PVE.form.PCIMapSelector', {
+ extend: 'Proxmox.form.ComboGrid',
+ xtype: 'pvePCIMapSelector',
+
+ store: {
+ model: 'pve-mapped-pci-model',
+ 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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 05/13] ui: form: add USBMapSelector
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (25 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 04/13] ui: form: add PCIMapSelector Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 06/13] ui: qemu/PCIEdit: rework panel to add a mapped configuration Dominik Csapak
` (10 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
similar to PCIMapSelector
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/form/USBMapSelector.js | 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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 06/13] ui: qemu/PCIEdit: rework panel to add a mapped configuration
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (26 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 05/13] ui: form: add USBMapSelector Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 07/13] ui: qemu/USBEdit: add 'mapped' device case Dominik Csapak
` (9 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
reworks the panel to use a controller, so that we can easily
add the selector for mapped pci devices
shows now a selection between 'raw' and 'mapped' devices, where
'raw' ones work like before, and 'mapped' ones take the values
form the hardware map config
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/qemu/PCIEdit.js | 314 +++++++++++++++++++++++------------
1 file changed, 210 insertions(+), 104 deletions(-)
diff --git a/www/manager6/qemu/PCIEdit.js b/www/manager6/qemu/PCIEdit.js
index 2f67aece..1593b7cd 100644
--- a/www/manager6/qemu/PCIEdit.js
+++ b/www/manager6/qemu/PCIEdit.js
@@ -3,71 +3,155 @@ Ext.define('PVE.qemu.PCIInputPanel', {
onlineHelp: 'qm_pci_passthrough_vm_config',
- setVMConfig: function(vmconfig) {
- var me = this;
- me.vmconfig = vmconfig;
+ controller: {
+ xclass: 'Ext.app.ViewController',
+
+ setVMConfig: function(vmconfig) {
+ let me = this;
+ let view = me.getView();
+ me.vmconfig = vmconfig;
- var hostpci = me.vmconfig[me.confid] || '';
+ let hostpci = me.vmconfig[view.confid] || '';
- var values = PVE.Parser.parsePropertyString(hostpci, 'host');
- if (values.host) {
- if (!values.host.match(/^[0-9a-f]{4}:/i)) { // add optional domain
- values.host = "0000:" + values.host;
+ let values = PVE.Parser.parsePropertyString(hostpci, 'host');
+ if (values.host) {
+ if (values.host.includes(':')) {
+ values.type = 'raw';
+ if (!values.host.match(/^[0-9a-f]{4}:/i)) { // add optional domain
+ values.host = "0000:" + values.host;
+ }
+ if (values.host.length < 11) { // 0000:00:00 format not 0000:00:00.0
+ values.host += ".0";
+ values.multifunction = true;
+ }
+ } else {
+ values.hostmapped = values.host;
+ delete values.host;
+ values.type = 'mapped';
+ }
}
- if (values.host.length < 11) { // 0000:00:00 format not 0000:00:00.0
- values.host += ".0";
- values.multifunction = true;
+
+ values['x-vga'] = PVE.Parser.parseBoolean(values['x-vga'], 0);
+ values.pcie = PVE.Parser.parseBoolean(values.pcie, 0);
+ values.rombar = PVE.Parser.parseBoolean(values.rombar, 1);
+
+ view.setValues(values);
+ if (!me.vmconfig.machine || me.vmconfig.machine.indexOf('q35') === -1) {
+ // machine is not set to some variant of q35, so we disable pcie
+ let pcie = me.lookup('pcie');
+ pcie.setDisabled(true);
+ pcie.setBoxLabel(gettext('Q35 only'));
}
- }
- values['x-vga'] = PVE.Parser.parseBoolean(values['x-vga'], 0);
- values.pcie = PVE.Parser.parseBoolean(values.pcie, 0);
- values.rombar = PVE.Parser.parseBoolean(values.rombar, 1);
+ if (values.romfile) {
+ me.lookup('romfile').setVisible(true);
+ }
+ },
- me.setValues(values);
- if (!me.vmconfig.machine || me.vmconfig.machine.indexOf('q35') === -1) {
- // machine is not set to some variant of q35, so we disable pcie
- var pcie = me.down('field[name=pcie]');
- pcie.setDisabled(true);
- pcie.setBoxLabel(gettext('Q35 only'));
- }
+ selectorEnable: function(selector) {
+ let me = this;
+ me.pciDevChange(selector, selector.getValue());
+ },
- if (values.romfile) {
- me.down('field[name=romfile]').setVisible(true);
- }
- },
+ pciDevChange: function(pcisel, value) {
+ let me = this;
+ let mdevfield = me.lookup('mdev');
+ if (!value) {
+ mdevfield.setDisabled(true);
+ return;
+ }
+ let pciDev = pcisel.getStore().getById(value);
- onGetValues: function(values) {
- let me = this;
- if (!me.confid) {
- for (let i = 0; i < PVE.Utils.hardware_counts.hostpci; i++) {
- if (!me.vmconfig['hostpci' + i.toString()]) {
- me.confid = 'hostpci' + i.toString();
- break;
+ mdevfield.setDisabled(!pciDev || !pciDev.data.mdev);
+ if (!pciDev) {
+ return;
+ }
+
+ let path = value;
+ if (pciDev.data.path) {
+ path = pciDev.data.path.split(';')[0];
+ if (path.indexOf('.') === -1) {
+ path += '.0';
}
}
- // FIXME: what if no confid was found??
- }
- values.host.replace(/^0000:/, ''); // remove optional '0000' domain
- if (values.multifunction) {
- values.host = values.host.substring(0, values.host.indexOf('.')); // skip the '.X'
- delete values.multifunction;
- }
+ if (pciDev.data.mdev) {
+ mdevfield.setPciID(path);
+ }
+ if (pcisel.reference === 'selector') {
+ let iommu = pciDev.data.iommugroup;
+ if (iommu === -1) {
+ return;
+ }
+ // try to find out if there are more devices in that iommu group
+ let id = path.substring(0, 5); // 00:00
+ let count = 0;
+ pcisel.getStore().each(({ data }) => {
+ if (data.iommugroup === iommu && data.id.substring(0, 5) !== id) {
+ count++;
+ return false;
+ }
+ return true;
+ });
+ me.lookup('group_warning').setVisible(count > 0);
+ }
+ },
- if (values.rombar) {
- delete values.rombar;
- } else {
- values.rombar = 0;
- }
+ onGetValues: function(values) {
+ let me = this;
+ let view = me.getView();
+ if (!view.confid) {
+ for (let i = 0; i < PVE.Utils.hardware_counts.hostpci; i++) {
+ if (!me.vmconfig['hostpci' + i.toString()]) {
+ view.confid = 'hostpci' + i.toString();
+ break;
+ }
+ }
+ // FIXME: what if no confid was found??
+ }
- if (!values.romfile) {
- delete values.romfile;
- }
+ if (values.hostmapped) {
+ values.host = values.hostmapped;
+ delete values.hostmapped;
+ } else {
+ values.host.replace(/^0000:/, ''); // remove optional '0000' domain
+
+ if (values.multifunction) {
+ values.host = values.host.substring(0, values.host.indexOf('.')); // skip the '.X'
+ delete values.multifunction;
+ }
+ }
+
+ if (values.rombar) {
+ delete values.rombar;
+ } else {
+ values.rombar = 0;
+ }
+
+ if (!values.romfile) {
+ delete values.romfile;
+ }
+
+ delete values.type;
+
+ let ret = {};
+ ret[view.confid] = PVE.Parser.printPropertyString(values, 'host');
+ return ret;
+ },
+ },
+
+ viewModel: {
+ data: {
+ isMapped: true,
+ },
+ },
- let ret = {};
- ret[me.confid] = PVE.Parser.printPropertyString(values, 'host');
- return ret;
+ setVMConfig: function(vmconfig) {
+ return this.getController().setVMConfig(vmconfig);
+ },
+
+ onGetValues: function(values) {
+ return this.getController().onGetValues(values);
},
initComponent: function() {
@@ -78,78 +162,97 @@ Ext.define('PVE.qemu.PCIInputPanel', {
throw "no node name specified";
}
+ me.columnT = [
+ {
+ xtype: 'displayfield',
+ reference: 'iommu_warning',
+ hidden: true,
+ columnWidth: 1,
+ padding: '0 0 10 0',
+ value: 'No IOMMU detected, please activate it.' +
+ 'See Documentation for further information.',
+ userCls: 'pmx-hint',
+ },
+ {
+ xtype: 'displayfield',
+ reference: 'group_warning',
+ hidden: true,
+ columnWidth: 1,
+ padding: '0 0 10 0',
+ itemId: 'iommuwarning',
+ value: 'The selected Device is not in a seperate IOMMU group, make sure this is intended.',
+ userCls: 'pmx-hint',
+ },
+ ];
+
me.column1 = [
+ {
+ xtype: 'radiofield',
+ name: 'type',
+ inputValue: 'mapped',
+ boxLabel: gettext('Mapped Device'),
+ bind: {
+ value: '{isMapped}',
+ },
+ },
+ {
+ xtype: 'pvePCIMapSelector',
+ fieldLabel: gettext('Device'),
+ reference: 'mapped_selector',
+ name: 'hostmapped',
+ labelAlign: 'right',
+ nodename: me.nodename,
+ allowBlank: false,
+ bind: {
+ disabled: '{!isMapped}',
+ },
+ listeners: {
+ change: 'pciDevChange',
+ enable: 'selectorEnable',
+ },
+ },
+ {
+ xtype: 'radiofield',
+ name: 'type',
+ inputValue: 'raw',
+ checked: true,
+ boxLabel: gettext('Raw Device'),
+ },
{
xtype: 'pvePCISelector',
fieldLabel: gettext('Device'),
name: 'host',
+ reference: 'selector',
nodename: me.nodename,
+ labelAlign: 'right',
allowBlank: false,
+ disabled: true,
+ bind: {
+ disabled: '{isMapped}',
+ },
onLoadCallBack: function(store, records, success) {
if (!success || !records.length) {
return;
}
- if (records.every((val) => val.data.iommugroup === -1)) { // no IOMMU groups
- let warning = Ext.create('Ext.form.field.Display', {
- columnWidth: 1,
- padding: '0 0 10 0',
- value: 'No IOMMU detected, please activate it.' +
- 'See Documentation for further information.',
- userCls: 'pmx-hint',
- });
- me.items.insert(0, warning);
- me.updateLayout(); // insert does not trigger that
- }
+ me.lookup('iommu_warning').setVisible(
+ records.every((val) => val.data.iommugroup === -1),
+ );
},
listeners: {
- change: function(pcisel, value) {
- if (!value) {
- return;
- }
- let pciDev = pcisel.getStore().getById(value);
- let mdevfield = me.down('field[name=mdev]');
- mdevfield.setDisabled(!pciDev || !pciDev.data.mdev);
- if (!pciDev) {
- return;
- }
- if (pciDev.data.mdev) {
- mdevfield.setPciID(value);
- }
- let iommu = pciDev.data.iommugroup;
- if (iommu === -1) {
- return;
- }
- // try to find out if there are more devices in that iommu group
- let id = pciDev.data.id.substring(0, 5); // 00:00
- let count = 0;
- pcisel.getStore().each(({ data }) => {
- if (data.iommugroup === iommu && data.id.substring(0, 5) !== id) {
- count++;
- return false;
- }
- return true;
- });
- let warning = me.down('#iommuwarning');
- if (count && !warning) {
- warning = Ext.create('Ext.form.field.Display', {
- columnWidth: 1,
- padding: '0 0 10 0',
- itemId: 'iommuwarning',
- value: 'The selected Device is not in a seperate IOMMU group, make sure this is intended.',
- userCls: 'pmx-hint',
- });
- me.items.insert(0, warning);
- me.updateLayout(); // insert does not trigger that
- } else if (!count && warning) {
- me.remove(warning);
- }
- },
+ change: 'pciDevChange',
+ enable: 'selectorEnable',
},
},
{
xtype: 'proxmoxcheckbox',
fieldLabel: gettext('All Functions'),
+ reference: 'all_functions',
+ disabled: true,
+ labelAlign: 'right',
name: 'multifunction',
+ bind: {
+ disabled: '{isMapped}',
+ },
},
];
@@ -157,6 +260,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
{
xtype: 'pveMDevSelector',
name: 'mdev',
+ reference: 'mdev',
disabled: true,
fieldLabel: gettext('MDev Type'),
nodename: me.nodename,
@@ -188,6 +292,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
submitValue: true,
hidden: true,
fieldLabel: 'ROM-File',
+ reference: 'romfile',
name: 'romfile',
},
{
@@ -214,6 +319,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
{
xtype: 'proxmoxcheckbox',
fieldLabel: 'PCI-Express',
+ reference: 'pcie',
name: 'pcie',
},
{
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 07/13] ui: qemu/USBEdit: add 'mapped' device case
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (27 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 06/13] ui: qemu/PCIEdit: rework panel to add a mapped configuration Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 08/13] ui: form: add MultiPCISelector Dominik Csapak
` (8 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
to be able to select 'mapped' usb devices
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/qemu/USBEdit.js | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/www/manager6/qemu/USBEdit.js b/www/manager6/qemu/USBEdit.js
index a2204584..1b017bc9 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,12 @@ Ext.define('PVE.qemu.USBEdit', {
port = data[i];
port = port.replace('host=', '');
type = 'port';
+ } else if (/^(host=)?[a-zA-Z0-9\-_]+$/.test(data[i])) {
+ if (data[i] !== 'spice') {
+ mapped = data[i];
+ mapped = mapped.replace('host=', '');
+ type = 'mapped';
+ }
}
if (/^usb3=(1|on|true)$/.test(data[i])) {
@@ -168,6 +201,7 @@ Ext.define('PVE.qemu.USBEdit', {
hostdevice: hostdevice,
port: port,
usb3: usb3,
+ mapped,
};
ipanel.setValues(values);
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 08/13] ui: form: add MultiPCISelector
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (28 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 07/13] ui: qemu/USBEdit: add 'mapped' device case Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 09/13] ui: add window/PCIEdit: edit window for pci mappings Dominik Csapak
` (7 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
this is a grid field for selecting multiple pci devices at once, like we
need for the mapped pci ui. There we want to be able to select multiple
devices such that one gets selected automatically
we can select a whole slot here, but that disables selecting the
individual functions of that device. we can also not select multiple
devices with different properties (e.g. vendor) so that the user
cannot select a set of pci devices which are not similar
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/css/ext6-pve.css | 4 +
www/manager6/Makefile | 1 +
www/manager6/form/MultiPCISelector.js | 289 ++++++++++++++++++++++++++
3 files changed, 294 insertions(+)
create mode 100644 www/manager6/form/MultiPCISelector.js
diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index dadb84a9..84205c11 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -651,3 +651,7 @@ table.osds td:first-of-type {
background-color: rgb(245, 245, 245);
color: #000;
}
+
+.x-grid-item .x-item-disabled {
+ opacity: 0.3;
+}
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index f5ae5364..c189af92 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -45,6 +45,7 @@ JSSRC= \
form/IPRefSelector.js \
form/MDevSelector.js \
form/MemoryField.js \
+ form/MultiPCISelector.js \
form/NetworkCardSelector.js \
form/NodeSelector.js \
form/PCISelector.js \
diff --git a/www/manager6/form/MultiPCISelector.js b/www/manager6/form/MultiPCISelector.js
new file mode 100644
index 00000000..9da3300a
--- /dev/null
+++ b/www/manager6/form/MultiPCISelector.js
@@ -0,0 +1,289 @@
+Ext.define('PVE.form.MultiPCISelector', {
+ extend: 'Ext.grid.Panel',
+ alias: 'widget.pveMultiPCISelector',
+
+ mixins: {
+ field: 'Ext.form.field.Field',
+ },
+
+ getValue: function() {
+ let me = this;
+
+ return (me.value ?? []).join(';');
+ },
+
+ setValue: function(value) {
+ let me = this;
+
+ value ??= [];
+
+ if (!Ext.isArray(value)) {
+ value = value.split(';').filter((v) => !!v);
+ me.value = value;
+ }
+
+ me.updateSelectedDevices(value);
+
+ return me.mixins.field.setValue.call(me, value);
+ },
+
+ getErrors: function() {
+ let me = this;
+
+
+ let error;
+ let value = me.getValue().split(';').filter((v) => !!v);
+ let store = me.getStore();
+
+ let fields = [
+ 'vendor',
+ 'device',
+ 'subsystem-vendor',
+ 'subsystem-device',
+ 'mdev',
+ ];
+
+ let props = {};
+
+ if (value.length < 1) {
+ error = gettext("Must choose at least one device");
+ }
+
+ value.forEach((id) => {
+ let rec = store.findRecord('id', id);
+ if (!rec) {
+ return;
+ }
+ for (const field of fields) {
+ props[field] ??= rec.data[field];
+
+ if (props[field] !== rec.data[field]) {
+ error = gettext('Cannot choose devices with different properties');
+ }
+ }
+ });
+
+ let el = me.getActionEl();
+ let errorCls = ['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid'];
+
+ if (error !== undefined) {
+ me.addCls(errorCls);
+ if (el) {
+ el.dom.setAttribute('data-errorqtip', error);
+ }
+
+ return [error];
+ }
+
+ me.removeCls(errorCls);
+ if (el) {
+ el.dom.setAttribute('data-errorqtip', "");
+ }
+
+ return [];
+ },
+
+ viewConfig: {
+ getRowClass: function(record) {
+ if (record.data.disabled === true) {
+ return 'x-item-disabled';
+ }
+ return '';
+ },
+ },
+
+ updateSelectedDevices: function(list = []) {
+ let me = this;
+
+ let recs = [];
+ let store = me.getStore();
+
+ store.each((rec) => {
+ if (list.indexOf(rec.data.id) !== -1) {
+ recs.push(rec);
+ }
+ });
+
+ me.suspendEvent('change');
+ me.setSelection(recs);
+ me.resumeEvent('change');
+ },
+
+ setNodename: function(nodename) {
+ var me = this;
+
+ if (!nodename || me.nodename === nodename) {
+ return;
+ }
+
+ me.nodename = nodename;
+
+ me.getStore().setProxy({
+ type: 'proxmox',
+ url: '/api2/json/nodes/' + me.nodename + '/hardware/pci?pci-class-blacklist=',
+ });
+
+ me.getStore().load({
+ callback: (recs, op, success) => me.addSlotRecords(recs, op, success),
+ });
+ },
+
+ // adds the virtual 'slot' records (e.g. '0000:01:00') to the store
+ addSlotRecords: function(records, _op, success) {
+ let me = this;
+ if (!success) {
+ return;
+ }
+
+ let slots = {};
+ records.forEach((rec) => {
+ let slotname = rec.data.id.slice(0, -2); // remove function
+ rec.set('slot', slotname);
+ if (slots[slotname] !== undefined) {
+ slots[slotname].count++;
+ return;
+ }
+
+ slots[slotname] = {
+ count: 1,
+ };
+
+ if (rec.data.id.endsWith('.0')) {
+ slots[slotname].device = rec.data;
+ }
+ });
+
+ let store = me.getStore();
+
+ for (const [slot, { count, device }] of Object.entries(slots)) {
+ if (count === 1) {
+ continue;
+ }
+ store.add(Ext.apply({}, {
+ id: slot,
+ mdev: undefined,
+ device_name: gettext('Pass through all functions as one device'),
+ }, device));
+ }
+
+ me.updateSelectedDevices(me.value);
+ },
+
+ selectionChange: function(_grid, selection) {
+ let me = this;
+
+ let ids = {};
+ selection
+ .filter(rec => rec.data.id.indexOf('.') === -1)
+ .forEach((rec) => { ids[rec.data.id] = true; });
+
+ let to_disable = [];
+
+ me.getStore().each(rec => {
+ let id = rec.data.id;
+ rec.set('disabled', false);
+ if (id.indexOf('.') === -1) {
+ return;
+ }
+ let slot = id.slice(0, -2); // remove function
+
+ if (ids[slot]) {
+ to_disable.push(rec);
+ rec.set('disabled', true);
+ }
+ });
+
+ me.suspendEvent('selectionchange');
+ me.getSelectionModel().deselect(to_disable);
+ me.resumeEvent('selectionchange');
+
+ me.value = me.getSelection().map((rec) => rec.data.id);
+ me.checkChange();
+ },
+
+ selModel: {
+ type: 'checkboxmodel',
+ mode: 'SIMPLE',
+ },
+
+ columns: [
+ {
+ header: 'ID',
+ dataIndex: 'id',
+ width: 150,
+ },
+ {
+ header: gettext('IOMMU Group'),
+ dataIndex: 'iommugroup',
+ renderer: (v, _md, rec) => {
+ console.log(rec.data);
+ return rec.data.slot === rec.data.id ? '' : v === -1 ? '-' : v;
+ },
+ width: 50,
+ },
+ {
+ header: gettext('Vendor'),
+ dataIndex: 'vendor_name',
+ flex: 3,
+ },
+ {
+ header: gettext('Device'),
+ dataIndex: 'device_name',
+ flex: 6,
+ },
+ {
+ header: gettext('Mediated Devices'),
+ dataIndex: 'mdev',
+ flex: 1,
+ renderer: function(val) {
+ return Proxmox.Utils.format_boolean(!!val);
+ },
+ },
+ ],
+
+ listeners: {
+ selectionchange: function() {
+ this.selectionChange(...arguments);
+ },
+ },
+
+ store: {
+ fields: [
+ 'id', 'vendor_name', 'device_name', 'vendor', 'device', 'iommugroup', 'mdev',
+ 'subsystem_vendor', 'subsystem_device', 'disabled',
+ {
+ name: 'subsystem-vendor',
+ calculate: function(data) {
+ return data.subsystem_vendor;
+ },
+ },
+ {
+ name: 'subsystem-device',
+ calculate: function(data) {
+ return data.subsystem_device;
+ },
+ },
+ ],
+ sorters: [
+ {
+ property: 'id',
+ direction: 'ASC',
+ },
+ ],
+ },
+
+ initComponent: function() {
+ let me = this;
+
+ var nodename = me.nodename;
+ me.nodename = undefined;
+
+ me.callParent();
+
+ Proxmox.Utils.monStoreErrors(me, me.getStore(), true);
+
+ me.setNodename(nodename);
+
+ me.initField();
+ },
+});
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 09/13] ui: add window/PCIEdit: edit window for pci mappings
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (29 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 08/13] ui: form: add MultiPCISelector Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 10/13] ui: add window/USBEdit: edit window for usb mappings Dominik Csapak
` (6 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 | 283 +++++++++++++++++++++++++++++++
3 files changed, 300 insertions(+), 1 deletion(-)
create mode 100644 www/manager6/window/PCIEdit.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index c189af92..4a6b02e5 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -116,6 +116,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..25ee87c2
--- /dev/null
+++ b/www/manager6/window/PCIEdit.js
@@ -0,0 +1,283 @@
+Ext.define('PVE.node.PCIEditWindow', {
+ extend: 'Proxmox.window.Edit',
+
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ width: 800,
+
+ 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;
+ }
+
+ 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),
+ );
+ },
+
+ pciChange: function(pcisel, values) {
+ let me = this;
+ if (!values) {
+ return;
+ }
+ values = values.split(';').filter(id => !!id);
+ me.lookup('multiple_warning').setVisible(values.length > 1);
+
+ let value = values[0];
+
+ let pciDev = pcisel.getStore().getById(value);
+ if (!pciDev) {
+ return;
+ }
+
+ 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: {
+ 'pveMultiPCISelector': {
+ 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: 'multiple_warning',
+ hidden: true,
+ columnWidth: 1,
+ padding: '0 0 10 0',
+ value: 'When multiple devices are selected, the first free one will be chosen' +
+ ' on guest start.',
+ userCls: 'pmx-hint',
+ },
+ {
+ xtype: 'displayfield',
+ reference: 'group_warning',
+ hidden: true,
+ columnWidth: 1,
+ padding: '0 0 10 0',
+ itemId: 'iommuwarning',
+ value: 'The selected Device is not in a seperate IOMMU group, make sure this is intended.',
+ userCls: 'pmx-hint',
+ },
+ ],
+
+ column1: [
+ {
+ xtype: 'pmxDisplayEditField',
+ fieldLabel: gettext('Name'),
+ labelWidth: 120,
+ cbind: {
+ editable: '{!name}',
+ value: '{name}',
+ submitValue: '{isCreate}',
+ },
+ name: 'name',
+ 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: 'pmxDisplayEditField',
+ fieldLabel: gettext('Node'),
+ labelWidth: 120,
+ name: 'node',
+ editConfig: {
+ xtype: 'pveNodeSelector',
+ },
+ cbind: {
+ editable: '{!nodename}',
+ value: '{nodename}',
+ },
+ submitValue: true,
+ allowBlank: false,
+ },
+ {
+ 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: 'pveMultiPCISelector',
+ fieldLabel: gettext('Device'),
+ labelWidth: 120,
+ height: 300,
+ reference: 'pciselector',
+ name: 'path',
+ cbind: {
+ nodename: '{nodename}',
+ },
+ allowBlank: false,
+ onLoadCallBack: 'checkIommu',
+ margin: '0 0 10 0',
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ fieldLabel: gettext('Comment'),
+ labelWidth: 120,
+ submitValue: true,
+ name: 'comment',
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ },
+ ],
+ },
+ ],
+});
--
2.30.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 10/13] ui: add window/USBEdit: edit window for usb mappings
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (30 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 09/13] ui: add window/PCIEdit: edit window for pci mappings Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 11/13] ui: add dc/HardwareView: a CRUD interface for hardware mapping Dominik Csapak
` (5 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 4a6b02e5..5507276e 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -117,6 +117,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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 11/13] ui: add dc/HardwareView: a CRUD interface for hardware mapping
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (31 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 10/13] ui: add window/USBEdit: edit window for usb mappings Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 12/13] ui: window/Migrate: allow mapped devices Dominik Csapak
` (4 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
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 | 324 ++++++++++++++++++++++++++++++++
3 files changed, 341 insertions(+), 2 deletions(-)
create mode 100644 www/manager6/dc/HardwareView.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 5507276e..869395e1 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -163,6 +163,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..7201d70f
--- /dev/null
+++ b/www/manager6/dc/HardwareView.js
@@ -0,0 +1,324 @@
+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(),
+ },
+ });
+ },
+
+ addHost: function() {
+ let me = this;
+ me.edit(false);
+ },
+
+ edit: function(includeNodename = true) {
+ let me = this;
+ let view = me.getView();
+ let selection = view.getSelection();
+ if (!selection || !selection.length) {
+ return;
+ }
+ let rec = selection[0];
+ if (!view.canConfigure) {
+ 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' && includeNodename ? 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'),
+ 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('New Host mapping'),
+ disabled: true,
+ parentXType: 'treepanel',
+ enableFn: function(_rec) {
+ return this.up('treepanel').canConfigure;
+ },
+ handler: 'addHost',
+ },
+ {
+ xtype: 'proxmoxButton',
+ text: gettext('Edit'),
+ disabled: true,
+ parentXType: 'treepanel',
+ enableFn: function(rec) {
+ return rec.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'),
+ flex: 1,
+ dataIndex: 'path',
+ renderer: function(value) {
+ value = value ?? '';
+ if (value.indexOf(';') !== -1) {
+ return value.split(';').join(', ');
+ }
+ return value;
+ },
+ },
+ {
+ 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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 12/13] ui: window/Migrate: allow mapped devices
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (32 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 11/13] ui: add dc/HardwareView: a CRUD interface for hardware mapping Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 13/13] ui: improve permission handling for hardware Dominik Csapak
` (3 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
if the migration is an offline migration and when the mapping on
the target node exists, otherwise not
this does not change the behaviour for 'raw' devices in the config
those can still be forced to be migrated, like before
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/window/Migrate.js | 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
^ permalink raw reply [flat|nested] 54+ messages in thread* [pve-devel] [PATCH manager v3 13/13] ui: improve permission handling for hardware
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (33 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 12/13] ui: window/Migrate: allow mapped devices Dominik Csapak
@ 2022-09-20 12:50 ` Dominik Csapak
2022-09-20 16:12 ` [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping DERUMIER, Alexandre
` (2 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Dominik Csapak @ 2022-09-20 12:50 UTC (permalink / raw)
To: pve-devel
qemu/HardwareView:
with the new Hardware privileges, we want to adapt a few places where
we now allow to show the add/edit window with those permissions.
form/{PCI,USB}Selector:
increase the minHeight property of the PCI/USBSelector, so that
the user can see the error message if he has not enough permissions.
data/PermPathStore:
add '/hardware' to the list of acl paths
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/data/PermPathStore.js | 1 +
www/manager6/form/PCISelector.js | 1 +
www/manager6/form/USBSelector.js | 1 +
www/manager6/qemu/HardwareView.js | 17 +++++++++--------
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/www/manager6/data/PermPathStore.js b/www/manager6/data/PermPathStore.js
index cf702c03..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
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (34 preceding siblings ...)
2022-09-20 12:50 ` [pve-devel] [PATCH manager v3 13/13] ui: improve permission handling for hardware Dominik Csapak
@ 2022-09-20 16:12 ` DERUMIER, Alexandre
2022-09-23 16:13 ` DERUMIER, Alexandre
2022-11-08 18:03 ` Thomas Lamprecht
37 siblings, 0 replies; 54+ messages in thread
From: DERUMIER, Alexandre @ 2022-09-20 16:12 UTC (permalink / raw)
To: pve-devel
Hi Dominik,
I will try to test it this week.
Le mardi 20 septembre 2022 à 14:50 +0200, 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
>
> this version is mostly the same as v2, aside from some bugfixes,
> rebase
> and preventing from having mdev: 1 set for multifunction devices.
> i would appreciate if somebody could take a look at this series
> again ;) (below is the old cover letter + changelog)
>
> 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)
>
> new in this version is the ability to specify multiple devices for
> each host mapping, such that we can select the first free one on
> starting the vm (this fixes #3574). That makes using vGPUs and SR-IOV
> much more useful as a user does not have to hardcode the pci ids
> anymore
>
> i left that feature seperated in a patch for pve-common(3/3) and in
> qemu-server ({12,13}/13) in the backend for easier review, but did
> not bother
> to do it for the gui (if we really don't want it, i can just send a
> different
> version for the ui)
>
> also pve-common 1/3 and qemu-server 1-4/13 are general cleanups that
> would even make sense without the remaining patches
> (qemu-server 1/13 depends on pve-common 1/3)
>
> changes from v2:
> * some bug fixes (e.g use of unitialized variable)
> * don't set mdev for multifunction devices
> -> this should fix alexandres issue, since it's not possible
> anymore
> to select a mediated device when having a multifunction device
> selected
>
> changes from v1:
> * dropped 'check_hw_perm' (just use 'check_full' now)
> * added some cleanups
> * renamed the buttons in the ui (hopefully better now)
> * added multi device mapping for each host
> this includes a new 'multi pci' selector for that window, which
> automatically adds entries for the whole slots which, when
> selected,
> disabled the selection of the individual functions
> * fixed some issues (e.g. missing entries in the 'caps' object, wrong
> usb config parsing, etc.)
>
> 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:
> pve-common (1) breaks current qemu-server
> pve-common (2,3) depends on pve-cluster
> qemu-server (1-4) depends on pve-common (1)
> qemu-server (5-11) depends on qemu-server(<5), pve-access-
> control,pve-common (2)
> qemu-server (12,13) depends on qemu-server(<12), pve-common (3)
> manager depends on qemu-server,pve-access-control,pve-common
>
> 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 (1):
> PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
>
> src/PVE/AccessControl.pm | 13 +++++++++++++
> src/PVE/RPCEnvironment.pm | 3 ++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> pve-common:
>
> Dominik Csapak (3):
> SysFSTools: make mdev cleanup independent of pciid
> add PVE/HardwareMap
> HardwareMap: add support for multiple pci device paths per mapping
>
> src/Makefile | 1 +
> src/PVE/HardwareMap.pm | 378
> +++++++++++++++++++++++++++++++++++++++++
> src/PVE/SysFSTools.pm | 6 +-
> 3 files changed, 381 insertions(+), 4 deletions(-)
> create mode 100644 src/PVE/HardwareMap.pm
>
> qemu-server:
>
> Dominik Csapak (13):
> cleanup pci devices in more situations
> PCI: make mediated device path independent of pci id
> PCI: refactor print_pci_device
> PCI: reuse parsed info from print_hostpci_devices
> 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
> fix #3574: enable multi pci device mapping from config
> add tests for mapped pci devices
>
> PVE/API2/Qemu.pm | 109 +++++++++++--
> PVE/QemuMigrate.pm | 13 +-
> PVE/QemuServer.pm | 137 +++++++++++-----
> PVE/QemuServer/PCI.pm | 149 +++++++++++++---
> --
> PVE/QemuServer/USB.pm | 21 ++-
> test/MigrationTest/Shared.pm | 7 +
> test/cfg2cmd/q35-linux-hostpci-mapping.conf | 17 ++
> .../q35-linux-hostpci-mapping.conf.cmd | 36 +++++
> test/run_config2command_tests.pl | 76 +++++++++
> 9 files changed, 473 insertions(+), 92 deletions(-)
> create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf
> create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
>
> pve-manager:
>
> Dominik Csapak (13):
> 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: form: add MultiPCISelector
> 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/css/ext6-pve.css | 4 +
> www/manager6/Makefile | 6 +
> www/manager6/data/PermPathStore.js | 1 +
> www/manager6/dc/Config.js | 18 +-
> www/manager6/dc/HardwareView.js | 324 ++++++++++++
> www/manager6/form/MultiPCISelector.js | 289 +++++++++++
> www/manager6/form/PCIMapSelector.js | 102 ++++
> 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 | 314 ++++++++----
> www/manager6/qemu/USBEdit.js | 36 +-
> www/manager6/window/Migrate.js | 37 +-
> www/manager6/window/PCIEdit.js | 283 ++++++++++
> www/manager6/window/USBEdit.js | 248 +++++++++
> 22 files changed, 2513 insertions(+), 131 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/MultiPCISelector.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
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (35 preceding siblings ...)
2022-09-20 16:12 ` [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping DERUMIER, Alexandre
@ 2022-09-23 16:13 ` DERUMIER, Alexandre
2022-11-08 18:03 ` Thomas Lamprecht
37 siblings, 0 replies; 54+ messages in thread
From: DERUMIER, Alexandre @ 2022-09-23 16:13 UTC (permalink / raw)
To: pve-devel
Hi Dominik,
I have finished my tests with pci passthrough && mdev,
I didn't have any problem this time, all is working fine for me !
Le 20/09/22 à 14:50, 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
>
> this version is mostly the same as v2, aside from some bugfixes, rebase
> and preventing from having mdev: 1 set for multifunction devices.
> i would appreciate if somebody could take a look at this series
> again ;) (below is the old cover letter + changelog)
>
> 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)
>
> new in this version is the ability to specify multiple devices for
> each host mapping, such that we can select the first free one on
> starting the vm (this fixes #3574). That makes using vGPUs and SR-IOV
> much more useful as a user does not have to hardcode the pci ids anymore
>
> i left that feature seperated in a patch for pve-common(3/3) and in
> qemu-server ({12,13}/13) in the backend for easier review, but did not bother
> to do it for the gui (if we really don't want it, i can just send a different
> version for the ui)
>
> also pve-common 1/3 and qemu-server 1-4/13 are general cleanups that
> would even make sense without the remaining patches
> (qemu-server 1/13 depends on pve-common 1/3)
>
> changes from v2:
> * some bug fixes (e.g use of unitialized variable)
> * don't set mdev for multifunction devices
> -> this should fix alexandres issue, since it's not possible anymore
> to select a mediated device when having a multifunction device
> selected
>
> changes from v1:
> * dropped 'check_hw_perm' (just use 'check_full' now)
> * added some cleanups
> * renamed the buttons in the ui (hopefully better now)
> * added multi device mapping for each host
> this includes a new 'multi pci' selector for that window, which
> automatically adds entries for the whole slots which, when selected,
> disabled the selection of the individual functions
> * fixed some issues (e.g. missing entries in the 'caps' object, wrong
> usb config parsing, etc.)
>
> 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:
> pve-common (1) breaks current qemu-server
> pve-common (2,3) depends on pve-cluster
> qemu-server (1-4) depends on pve-common (1)
> qemu-server (5-11) depends on qemu-server(<5), pve-access-control,pve-common (2)
> qemu-server (12,13) depends on qemu-server(<12), pve-common (3)
> manager depends on qemu-server,pve-access-control,pve-common
>
> 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 (1):
> PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
>
> src/PVE/AccessControl.pm | 13 +++++++++++++
> src/PVE/RPCEnvironment.pm | 3 ++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> pve-common:
>
> Dominik Csapak (3):
> SysFSTools: make mdev cleanup independent of pciid
> add PVE/HardwareMap
> HardwareMap: add support for multiple pci device paths per mapping
>
> src/Makefile | 1 +
> src/PVE/HardwareMap.pm | 378 +++++++++++++++++++++++++++++++++++++++++
> src/PVE/SysFSTools.pm | 6 +-
> 3 files changed, 381 insertions(+), 4 deletions(-)
> create mode 100644 src/PVE/HardwareMap.pm
>
> qemu-server:
>
> Dominik Csapak (13):
> cleanup pci devices in more situations
> PCI: make mediated device path independent of pci id
> PCI: refactor print_pci_device
> PCI: reuse parsed info from print_hostpci_devices
> 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
> fix #3574: enable multi pci device mapping from config
> add tests for mapped pci devices
>
> PVE/API2/Qemu.pm | 109 +++++++++++--
> PVE/QemuMigrate.pm | 13 +-
> PVE/QemuServer.pm | 137 +++++++++++-----
> PVE/QemuServer/PCI.pm | 149 +++++++++++++-----
> PVE/QemuServer/USB.pm | 21 ++-
> test/MigrationTest/Shared.pm | 7 +
> test/cfg2cmd/q35-linux-hostpci-mapping.conf | 17 ++
> .../q35-linux-hostpci-mapping.conf.cmd | 36 +++++
> test/run_config2command_tests.pl | 76 +++++++++
> 9 files changed, 473 insertions(+), 92 deletions(-)
> create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf
> create mode 100644 test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
>
> pve-manager:
>
> Dominik Csapak (13):
> 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: form: add MultiPCISelector
> 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/css/ext6-pve.css | 4 +
> www/manager6/Makefile | 6 +
> www/manager6/data/PermPathStore.js | 1 +
> www/manager6/dc/Config.js | 18 +-
> www/manager6/dc/HardwareView.js | 324 ++++++++++++
> www/manager6/form/MultiPCISelector.js | 289 +++++++++++
> www/manager6/form/PCIMapSelector.js | 102 ++++
> 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 | 314 ++++++++----
> www/manager6/qemu/USBEdit.js | 36 +-
> www/manager6/window/Migrate.js | 37 +-
> www/manager6/window/PCIEdit.js | 283 ++++++++++
> www/manager6/window/USBEdit.js | 248 +++++++++
> 22 files changed, 2513 insertions(+), 131 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/MultiPCISelector.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
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping
2022-09-20 12:50 [pve-devel] [PATCH many v3] add cluster-wide hardware device mapping Dominik Csapak
` (36 preceding siblings ...)
2022-09-23 16:13 ` DERUMIER, Alexandre
@ 2022-11-08 18:03 ` Thomas Lamprecht
37 siblings, 0 replies; 54+ messages in thread
From: Thomas Lamprecht @ 2022-11-08 18:03 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 20/09/2022 um 14:50 schrieb 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
>
> this version is mostly the same as v2, aside from some bugfixes, rebase
> and preventing from having mdev: 1 set for multifunction devices.
> i would appreciate if somebody could take a look at this series
> again ;) (below is the old cover letter + changelog)
>
docs patch is still missing, please send one along mentioning this in the
relevant places
@all: it'd be great in general if everybody could send one along from the initial
revision on every somewhat bigger series.
^ permalink raw reply [flat|nested] 54+ messages in thread