[pve-devel] [PATCH guest-common/qemu-server/manager/docs v7] implement experimental vgpu live migration

[pve-devel] [PATCH guest-common/qemu-server/manager/docs v7] implement experimental vgpu live migration

[Dominik Csapak]

== Summmary ==

This is implemented for mapped resources. This requires driver and
hardware support, but aside from nvidia vgpus there don't seem to be
many drivers (if any) that do support that.

qemu already supports that for vfio-pci devices, so nothing to be done
there besides actively enabling it.

Since we currently can't properly test it here and very much depends on
hardware/driver support, mark it as experimental everywhere
(docs/api/gui). (i tested it with a single server with multiple pve
"containers" that each got several virtual functions, so the migration
was actually to the same hardware but via our stack between two
different qemu processes)

i opted for marking them migratable at the mapping level, but we could
theoretically also put it in the hostpciX config instead. (though imho
it fits better in the cluster-wide resource mapping config)

also the naming/texts could probably be improved, but i think
'live-migration-capable' is very descriptive and i didn't want to use an
overly short name for it (which can be confusing, see the 'shared' flag
for storages)

== Dependencies ==

qemu-server depend on pve-guest-common and require a bumped pve-common

pve-manager depends on pve-guest-common and qemu-server

== Changelog ==

changes from v6:
* rebase on master
* include christophs and fioans reviewed-by where appropriate
* fixed some nits (see individual patches)
* don't add live migratable hostpci devices to 'local_resources' as blockers
* add a new patch to omit mapped hostpci devices to 'local_resources' as
  blockers (like it was intended in the first place)
  -> this make one of the bulk migration patches unnecessary so I dropped it
* don't change the check for bulk migration preconditions
  (was wrong for other local resources)

changes from v5:
* rebased on master
* new common patch that was missing last time
* dropped the move of find_on_current_node, since it only makes
  our lives harder and we don't gain much from it.
  -> this also fixed some bugs that were there in v5 due to
  the move.
* reordered some patches, so the dependencies are clearer
* added a patch that adds a state-migration summary for live
  migration (so we can see how much state was actually transferred)
* added missing colon in log output

changes from v4:
* rebased on master (some work due to the recent nvidia changes)
* incorporated thomas/alexanders feedback from v4

changes from v3:
* rebased on master
* split first guest-common patch into 3
* instead of merging keys, just write all expected keys in to expected_props
* made $cfg optional so it does not break callers that don't call it
* added patch to fix the cfg2cmd tests for mdev check
* added patch to show vfio state transferred for migration
* incorporated fionas feedback (mostly minor stuff)

changes from v2:
* rebased on master
* rework the rework of the properties check (pve-guest-common 1/4)
* properly check mdev in the gui (pve-manager 1/5)

pve-guest-common:

Dominik Csapak (2):
  mapping: pci: check the mdev configuration on the device too
  mapping: pci: add 'live-migration-capable' flag to mappings

 src/PVE/Mapping/PCI.pm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

qemu-server:

Dominik Csapak (9):
  tests: cfg2cmd: fix mdev tests
  pci: mapping: check mdev config against hardware
  pci: set 'enable-migration' to on for live-migration marked mapped
    devices
  check_local_resources: add more info per mapped device and return as
    hash
  check_local_resources: allow mapped devices for offline migration
  api: enable live migration for marked mapped pci devices
  api: include not mapped resources for running vms in migrate
    preconditions
  migrate: show vfio state transferred too
  migrate: add transfer summary

 PVE/API2/Qemu.pm                 | 59 +++++++++++++++++++-------------
 PVE/QemuMigrate.pm               | 46 +++++++++++++++++++++----
 PVE/QemuServer.pm                | 24 +++++++++----
 PVE/QemuServer/PCI.pm            | 10 +++++-
 test/run_config2command_tests.pl |  2 +-
 5 files changed, 101 insertions(+), 40 deletions(-)

pve-manager:

Dominik Csapak (4):
  mapping: pci: include mdev in config checks
  bulk migrate: improve precondition checks
  ui: adapt migration window to precondition api change
  fix #5175: ui: allow configuring and live migration of mapped pci
    resources

 PVE/API2/Cluster/Mapping/PCI.pm   |  2 +-
 PVE/API2/Nodes.pm                 | 12 ++++++++
 www/manager6/dc/PCIMapView.js     |  8 +++++
 www/manager6/window/Migrate.js    | 51 ++++++++++++++++++++-----------
 www/manager6/window/PCIMapEdit.js | 20 +++++++++---
 5 files changed, 70 insertions(+), 23 deletions(-)

pve-docs:

Dominik Csapak (2):
  qm: resource mapping: add description for `mdev` option
  qm: resource mapping: document `live-migration-capable` setting

 qm.adoc | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

-- 
2.39.5



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

[pve-devel] [PATCH guest-common v7 1/2] mapping: pci: check the mdev configuration on the device too

[Dominik Csapak]

but that lives int he 'global' part of the mapping config, not in a
specific mapping. To check that, add it to the $configured_props from
there.

this requires all call sites to be adapted otherwise the check will
always fail for devices that are capable of mediated devices

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/Mapping/PCI.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
index aa56496..cdd73d9 100644
--- a/src/PVE/Mapping/PCI.pm
+++ b/src/PVE/Mapping/PCI.pm
@@ -131,7 +131,7 @@ sub options {
 
 # checks if the given config is valid for the current node
 sub assert_valid {
-    my ($name, $mapping) = @_;
+    my ($name, $mapping, $cluster_mapping_cfg) = @_;
 
     my @paths = split(';', $mapping->{path} // '');
 
@@ -161,6 +161,12 @@ sub assert_valid {
 
 	my $configured_props = { $mapping->%{qw(id iommugroup subsystem-id)} };
 
+	# check mdev from globabl mapping config, if that is given
+	if (defined($cluster_mapping_cfg)) {
+	    $expected_props->{mdev} = $info->{mdev} ? 1 : 0;
+	    $configured_props->{mdev} = $cluster_mapping_cfg->{mdev} ? 1 : 0;
+	}
+
 	for my $prop (sort keys $expected_props->%*) {
 	    next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on the first device
 
-- 
2.39.5



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

[pve-devel] [PATCH guest-common v7 2/2] mapping: pci: add 'live-migration-capable' flag to mappings

[Dominik Csapak]

so that we can decide in qemu-server to allow live-migration.
The driver and QEMU must be capable of that, and it's the
admin's responsibility to know and configure that

Mark the option as experimental in the description.

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/Mapping/PCI.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
index cdd73d9..3e93429 100644
--- a/src/PVE/Mapping/PCI.pm
+++ b/src/PVE/Mapping/PCI.pm
@@ -105,6 +105,13 @@ my $defaultData = {
 	    optional => 1,
 	    default => 0,
 	},
+	'live-migration-capable' => {
+	    description => "Marks the device(s) as being able to be live-migrated (Experimental)."
+		." This needs hardware and driver support to work.",
+	    type => 'boolean',
+	    optional => 1,
+	    default => 0,
+	},
 	map => {
 	    type => 'array',
 	    description => 'A list of maps for the cluster nodes.',
@@ -125,6 +132,7 @@ sub options {
     return {
 	description => { optional => 1 },
 	mdev => { optional => 1 },
+	'live-migration-capable' => { optional => 1 },
 	map => {},
     };
 }
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 1/9] tests: cfg2cmd: fix mdev tests

[Dominik Csapak]

this will fail with the new checks for mdev when we don't have the
correct config.

namely a device that has mediated devices, should have 'mdev' set in the
mapping config

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 test/run_config2command_tests.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 7e3d10e6..e2e762ba 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -108,6 +108,7 @@ my $pci_map_config = {
     ids => {
 	someGpu => {
 	    type => 'pci',
+	    mdev => 1,
 	    map => [
 		'node=localhost,path=0000:01:00.4,id=10de:2231,iommugroup=1',
 		'node=localhost,path=0000:01:00.5,id=10de:2231,iommugroup=1',
@@ -330,7 +331,6 @@ $pve_common_sysfstools->mock(
 	} elsif ($path =~ m/^0000:07:10/) {
 	    return {
 		iommugroup => 2,
-		mdev => 0,
 		vendor => "0x8086",
 		device => "0x1520",
 	    };
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 2/9] pci: mapping: check mdev config against hardware

[Dominik Csapak]

by giving the mapping config to assert_valid, not only the specific mapping

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer/PCI.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index d758ae9d..a0d99692 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -432,8 +432,10 @@ sub parse_hostpci {
 	my $devices = PVE::Mapping::PCI::find_on_current_node($mapping);
 	die "PCI device mapping not found for '$mapping'\n" if !$devices || !scalar($devices->@*);
 
+	my $config = PVE::Mapping::PCI::config();
+
 	for my $device ($devices->@*) {
-	    eval { PVE::Mapping::PCI::assert_valid($mapping, $device) };
+	    eval { PVE::Mapping::PCI::assert_valid($mapping, $device, $config->{ids}->{$mapping}) };
 	    die "PCI device mapping invalid (hardware probably changed): $@\n" if $@;
 	    push $alternatives->@*, [split(/;/, $device->{path})];
 	}
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 3/9] pci: set 'enable-migration' to on for live-migration marked mapped devices

[Dominik Csapak]

the default is 'auto', but for those which are marked as capable for
live migration, we want to explicitly enable that, so we get an early
error on start if the driver does not support that.

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* renamed $cfg to $mapping_cfg
* added missing semicolon

 PVE/QemuServer/PCI.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index a0d99692..afb60ad2 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -433,9 +433,11 @@ sub parse_hostpci {
 	die "PCI device mapping not found for '$mapping'\n" if !$devices || !scalar($devices->@*);
 
 	my $config = PVE::Mapping::PCI::config();
+	my $mapping_cfg = $config->{ids}->{$mapping};
+	$res->{'live-migration-capable'} = 1 if $mapping_cfg->{'live-migration-capable'};
 
 	for my $device ($devices->@*) {
-	    eval { PVE::Mapping::PCI::assert_valid($mapping, $device, $config->{ids}->{$mapping}) };
+	    eval { PVE::Mapping::PCI::assert_valid($mapping, $device, $mapping_cfg) };
 	    die "PCI device mapping invalid (hardware probably changed): $@\n" if $@;
 	    push $alternatives->@*, [split(/;/, $device->{path})];
 	}
@@ -692,6 +694,10 @@ sub print_hostpci_devices {
 		$devicestr .= ",host=$pcidevice->{id}";
 	    }
 
+	    if ($d->{'live-migration-capable'}) {
+		$devicestr .= ",enable-migration=on";
+	    }
+
 	    my $mf_addr = $multifunction ? ".$j" : '';
 	    $devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
 
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 4/9] check_local_resources: add more info per mapped device and return as hash

[Dominik Csapak]

such as the mapping name and if it's marked for live-migration (pci only)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm   |  2 +-
 PVE/QemuMigrate.pm |  7 ++++---
 PVE/QemuServer.pm  | 21 ++++++++++++++-------
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index dc8915a7..f6157a3e 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4736,7 +4736,7 @@ __PACKAGE__->register_method({
 	$res->{local_disks} = [ values %$local_disks ];;
 
 	$res->{local_resources} = $local_resources;
-	$res->{'mapped-resources'} = $mapped_resources;
+	$res->{'mapped-resources'} = [ sort keys $mapped_resources->%* ];
 
 	return $res;
 
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index c2e36334..2153ac42 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -229,7 +229,7 @@ sub prepare {
     my ($loc_res, $mapped_res, $missing_mappings_by_node) = PVE::QemuServer::check_local_resources($conf, $running, 1);
     my $blocking_resources = [];
     for my $res ($loc_res->@*) {
-	if (!grep($res, $mapped_res->@*)) {
+	if (!defined($mapped_res->{$res})) {
 	    push $blocking_resources->@*, $res;
 	}
     }
@@ -241,10 +241,11 @@ sub prepare {
 	}
     }
 
-    if (scalar($mapped_res->@*)) {
+    if (scalar(keys $mapped_res->%*)) {
 	my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
 	if ($running) {
-	    die "can't migrate running VM which uses mapped devices: " . join(", ", $mapped_res->@*) . "\n";
+	    my $mapped_text = join(", ", sort keys $mapped_res->%*);
+	    die "can't migrate running VM which uses mapped devices: $mapped_text\n";
 	} elsif (scalar($missing_mappings->@*)) {
 	    die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
 	} else {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ffd5d56b..ce3c36d8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2472,7 +2472,7 @@ sub check_local_resources {
     my ($conf, $state, $noerr) = @_;
 
     my @loc_res = ();
-    my $mapped_res = [];
+    my $mapped_res = {};
 
     my @non_migratable_resources = check_non_migratable_resources($conf, $state, $noerr);
     push(@loc_res, @non_migratable_resources);
@@ -2507,16 +2507,23 @@ sub check_local_resources {
 	if ($k =~ m/^usb/) {
 	    my $entry = parse_property_string('pve-qm-usb', $conf->{$k});
 	    next if $entry->{host} && $entry->{host} =~ m/^spice$/i;
-	    if ($entry->{mapping}) {
-		$add_missing_mapping->('usb', $k, $entry->{mapping});
-		push @$mapped_res, $k;
+	    if (my $name = $entry->{mapping}) {
+		$add_missing_mapping->('usb', $k, $name);
+		$mapped_res->{$k} = { name => $name };
 	    }
 	}
 	if ($k =~ m/^hostpci/) {
 	    my $entry = parse_property_string('pve-qm-hostpci', $conf->{$k});
-	    if ($entry->{mapping}) {
-		$add_missing_mapping->('pci', $k, $entry->{mapping});
-		push @$mapped_res, $k;
+	    if (my $name = $entry->{mapping}) {
+		$add_missing_mapping->('pci', $k, $name);
+		my $mapped_device = { name => $name };
+		$mapped_res->{$k} = $mapped_device;
+
+		if ($pci_map->{ids}->{$name}->{'live-migration-capable'}) {
+		    $mapped_device->{'live-migration'} = 1;
+		    # don't add mapped device with live migration as blocker
+		    next;
+		}
 	    }
 	}
 	# sockets are safe: they will recreated be on the target side post-migrate
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 5/9] check_local_resources: allow mapped devices for offline migration

[Dominik Csapak]

by not marking them as 'local_resources'.

Change the description for the 'migrate' precondition api call, to
clarify what 'local_resources' was meant to be (a list of devices that
are blockers for migration).

It could be argued that this is a breaking change, since some api client
might depend on this list to block or allow migration. The worst case
that should happen though is that migrations are allowed that wouldn't
before, but only for devices that are mapped anyway, and that fails if
the target node does not have a mapping, and should succeed otherwise.

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

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index f6157a3e..060bca8b 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4663,7 +4663,7 @@ __PACKAGE__->register_method({
 		    type => 'string',
 		    description => "A local resource",
 		},
-		description => "List local resources e.g. pci, usb"
+		description => "List local resources (e.g. pci, usb) that block migration."
 	    },
 	    'mapped-resources' => {
 		type => 'array',
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ce3c36d8..97662704 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2524,6 +2524,9 @@ sub check_local_resources {
 		    # don't add mapped device with live migration as blocker
 		    next;
 		}
+
+		# don't add mapped devices as blocker for offline migration
+		next if !$state;
 	    }
 	}
 	# sockets are safe: they will recreated be on the target side post-migrate
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 6/9] api: enable live migration for marked mapped pci devices

[Dominik Csapak]

They have to be marked as 'live-migration-capable' in the mapping
config, and the driver and qemu must support it.

For the gui checks, we now return the whole object of the mapped
resources, which includes info like the name and if it's marked as
live-migration capable. (while deprecating the old 'mapped-resource'
return value, since that returns strictly less information)

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm   |  8 +++++++-
 PVE/QemuMigrate.pm | 17 ++++++++++++-----
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 060bca8b..15bf7548 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4665,13 +4665,18 @@ __PACKAGE__->register_method({
 		},
 		description => "List local resources (e.g. pci, usb) that block migration."
 	    },
+	    # FIXME: remove with 9.0
 	    'mapped-resources' => {
 		type => 'array',
 		items => {
 		    type => 'string',
 		    description => "A mapped resource",
 		},
-		description => "List of mapped resources e.g. pci, usb"
+		description => "List of mapped resources e.g. pci, usb. Deprecated, use 'mapped-resource-info' instead."
+	    },
+	    'mapped-resource-info' => {
+		type => 'object',
+		description => "Object of mapped resources with additional information such if they're live migratable.",
 	    },
 	},
     },
@@ -4737,6 +4742,7 @@ __PACKAGE__->register_method({
 
 	$res->{local_resources} = $local_resources;
 	$res->{'mapped-resources'} = [ sort keys $mapped_resources->%* ];
+	$res->{'mapped-resource-info'} = $mapped_resources;
 
 	return $res;
 
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 2153ac42..6909fc82 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -243,11 +243,18 @@ sub prepare {
 
     if (scalar(keys $mapped_res->%*)) {
 	my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
-	if ($running) {
-	    my $mapped_text = join(", ", sort keys $mapped_res->%*);
-	    die "can't migrate running VM which uses mapped devices: $mapped_text\n";
-	} elsif (scalar($missing_mappings->@*)) {
-	    die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
+	my $missing_live_mappings = [];
+	for my $key (sort keys $mapped_res->%*) {
+	    my $res = $mapped_res->{$key};
+	    my $name = "$key:$res->{name}";
+	    push $missing_live_mappings->@*, $name if !$res->{'live-migration'};
+	}
+	if (scalar($missing_mappings->@*)) {
+	    my $missing = join(", ", $missing_mappings->@*);
+	    die "can't migrate to '$self->{node}': missing mapped devices $missing\n";
+	} elsif ($running && scalar($missing_live_mappings->@*)) {
+	    my $missing = join(", ", $missing_live_mappings->@*);
+	    die "can't live migrate running VM which uses following mapped devices: $missing\n";
 	} else {
 	    $self->log('info', "migrating VM which uses mapped local devices");
 	}
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 7/9] api: include not mapped resources for running vms in migrate preconditions

[Dominik Csapak]

so that we can show a proper warning in the migrate dialog and check it
in the bulk migrate precondition check

the unavailable_storages and should be the same as before, but
we now always return (not_)allowed_nodes too.

to make the code a bit easier to read, reorganize how we construct
the (not_)allowed_nodes properties.

also add a note that we want to redesign the return values here, to make
* the api call simpler
* return better structured values

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* changed 'List x' to 'List of x'
* removed outdated comment
* included a hunk from the next patch that was accidentally committed
  there

 PVE/API2/Qemu.pm | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 15bf7548..d2345bc1 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4601,6 +4601,8 @@ __PACKAGE__->register_method({
 	},
     },
     returns => {
+	# TODO 9.x: rework the api call to return more sensible structures
+	# e.g. a simple list of nodes with their blockers and/or notices to show
 	type => "object",
 	properties => {
 	    running => {
@@ -4614,7 +4616,7 @@ __PACKAGE__->register_method({
 		    description => "An allowed node",
 		},
 		optional => 1,
-		description => "List nodes allowed for offline migration, only passed if VM is offline"
+		description => "List of nodes allowed for migration.",
 	    },
 	    not_allowed_nodes => {
 		type => 'object',
@@ -4630,7 +4632,7 @@ __PACKAGE__->register_method({
 			},
 		    },
 		},
-		description => "List not allowed nodes with additional information, only passed if VM is offline"
+		description => "List of not allowed nodes with additional information.",
 	    },
 	    local_disks => {
 		type => 'array',
@@ -4708,37 +4710,40 @@ __PACKAGE__->register_method({
 
 	my ($local_resources, $mapped_resources, $missing_mappings_by_node) =
 	    PVE::QemuServer::check_local_resources($vmconf, $res->{running}, 1);
-	delete $missing_mappings_by_node->{$localnode};
 
 	my $vga = PVE::QemuServer::parse_vga($vmconf->{vga});
 	if ($res->{running} && $vga->{'clipboard'} && $vga->{'clipboard'} eq 'vnc') {
 	    push $local_resources->@*, "clipboard=vnc";
 	}
 
-	# if vm is not running, return target nodes where local storage/mapped devices are available
-	# for offline migration
-	if (!$res->{running}) {
-	    $res->{allowed_nodes} = [];
-	    my $checked_nodes = PVE::QemuServer::check_local_storage_availability($vmconf, $storecfg);
-	    delete $checked_nodes->{$localnode};
-
-	    foreach my $node (keys %$checked_nodes) {
-		my $missing_mappings = $missing_mappings_by_node->{$node};
-		if (scalar($missing_mappings->@*)) {
-		    $checked_nodes->{$node}->{'unavailable-resources'} = $missing_mappings;
-		    next;
-		}
+	$res->{allowed_nodes} = [];
+	$res->{not_allowed_nodes} = {};
 
-		if (!defined($checked_nodes->{$node}->{unavailable_storages})) {
-		    push @{$res->{allowed_nodes}}, $node;
-		}
+	my $storage_nodehash = PVE::QemuServer::check_local_storage_availability($vmconf, $storecfg);
+
+	my $nodelist = PVE::Cluster::get_nodelist();
+	for my $node ($nodelist->@*) {
+	    next if $node eq $localnode;
+
+	    # extract missing storage info
+	    if (my $storage_info = $storage_nodehash->{$node}) {
+		$res->{not_allowed_nodes}->{$node} = $storage_info;
+	    }
+
+	    # extract missing mappings info
+	    my $missing_mappings = $missing_mappings_by_node->{$node};
+	    if (scalar($missing_mappings->@*)) {
+		$res->{not_allowed_nodes}->{$node}->{'unavailable-resources'} = $missing_mappings;
+	    }
 
+	    # if nothing came up, add it to the allowed nodes
+	    if (scalar($res->{not_allowed_nodes}->{$node}->%*) == 0) {
+		push $res->{allowed_nodes}->@*, $node;
 	    }
-	    $res->{not_allowed_nodes} = $checked_nodes;
 	}
 
 	my $local_disks = &$check_vm_disks_local($storecfg, $vmconf, $vmid);
-	$res->{local_disks} = [ values %$local_disks ];;
+	$res->{local_disks} = [ values %$local_disks ];
 
 	$res->{local_resources} = $local_resources;
 	$res->{'mapped-resources'} = [ sort keys $mapped_resources->%* ];
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 8/9] migrate: show vfio state transferred too

[Dominik Csapak]

Show the transferred VFIO state (when there is one), but since there is
no total here, so we can't show that, just what was transferred up until
now.

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* removed unrelated hunk (added in the previous patch)
 PVE/QemuMigrate.pm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 6909fc82..d6f9132b 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1241,6 +1241,7 @@ sub phase2 {
     $self->log('info', "migrate uri => $migrate_uri failed: $merr") if $merr;
 
     my $last_mem_transferred = 0;
+    my $last_vfio_transferred = 0;
     my $usleep = 1000000;
     my $i = 0;
     my $err_count = 0;
@@ -1300,8 +1301,11 @@ sub phase2 {
 	    last;
 	}
 
-	if ($memstat->{transferred} ne $last_mem_transferred) {
+	if ($memstat->{transferred} ne $last_mem_transferred ||
+	    $stat->{vfio}->{transferred} ne $last_vfio_transferred
+	) {
 	    my $trans = $memstat->{transferred} || 0;
+	    my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
 	    my $rem = $memstat->{remaining} || 0;
 	    my $total = $memstat->{total} || 0;
 	    my $speed = ($memstat->{'pages-per-second'} // 0) * ($memstat->{'page-size'} // 0);
@@ -1319,6 +1323,11 @@ sub phase2 {
 
 	    my $progress = "transferred $transferred_h of $total_h VM-state, ${speed_h}/s";
 
+	    if ($vfio_transferred > 0) {
+		my $vfio_h = render_bytes($vfio_transferred, 1);
+		$progress .= " (+ $vfio_h VFIO-state)";
+	    }
+
 	    if ($dirty_rate > $speed) {
 		my $dirty_rate_h = render_bytes($dirty_rate, 1);
 		$progress .= ", VM dirties lots of memory: $dirty_rate_h/s";
@@ -1360,6 +1369,7 @@ sub phase2 {
 	}
 
 	$last_mem_transferred = $memstat->{transferred};
+	$last_vfio_transferred = $stat->{vfio}->{transferred};
     }
 
     if ($self->{storage_migration}) {
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 9/9] migrate: add transfer summary

[Dominik Csapak]

showing a final transfer log line helps with identifying what was
actually transferred. E.g. it could happen that the VFIO state was only
transferred in the last iteration. In such a case we would not see that
information at all.

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuMigrate.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index d6f9132b..babd81a1 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1288,6 +1288,20 @@ sub phase2 {
 		my $downtime = $stat->{downtime} || 0;
 		$self->log('info', "average migration speed: $avg_speed/s - downtime $downtime ms");
 	    }
+	    my $trans = $memstat->{transferred} || 0;
+	    my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
+
+	    if ($trans > 0 || $vfio_transferred > 0) {
+		my $transferred_h = render_bytes($trans, 1);
+		my $summary = "transferred $transferred_h VM-state";
+
+		if ($vfio_transferred > 0) {
+		    my $vfio_h = render_bytes($vfio_transferred, 1);
+		    $summary .= " (+ $vfio_h VFIO-state)";
+		}
+
+		$self->log('info', "migration $status, $summary");
+	    }
 	}
 
 	if ($status eq 'failed' || $status eq 'cancelled') {
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 1/4] mapping: pci: include mdev in config checks

[Dominik Csapak]

by also providing the global config in assert_valid, and by also
adding the mdev config in the 'toCheck' object in the gui

For the gui, we extract the mdev property from the global entry, and add
it to the individual mapping entries, that way we can reuse the checking
logic of the other properties.

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* added short comment about mdev checking

 PVE/API2/Cluster/Mapping/PCI.pm | 2 +-
 www/manager6/dc/PCIMapView.js   | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Cluster/Mapping/PCI.pm b/PVE/API2/Cluster/Mapping/PCI.pm
index 64462d25..f85623bb 100644
--- a/PVE/API2/Cluster/Mapping/PCI.pm
+++ b/PVE/API2/Cluster/Mapping/PCI.pm
@@ -115,7 +115,7 @@ __PACKAGE__->register_method ({
 			};
 		    }
 		    for my $mapping ($mappings->@*) {
-			eval { PVE::Mapping::PCI::assert_valid($id, $mapping) };
+			eval { PVE::Mapping::PCI::assert_valid($id, $mapping, $entry) };
 			if (my $err = $@) {
 			    push $entry->{checks}->@*, {
 				severity => 'error',
diff --git a/www/manager6/dc/PCIMapView.js b/www/manager6/dc/PCIMapView.js
index 80fe3c0f..2242f1a5 100644
--- a/www/manager6/dc/PCIMapView.js
+++ b/www/manager6/dc/PCIMapView.js
@@ -20,10 +20,17 @@ Ext.define('PVE.dc.PCIMapView', {
 	data.forEach((entry) => {
 	    ids[entry.id] = entry;
 	});
+	// extract the mdev property from the global entry and insert to the individiual entries,
+	// so we can reuse the normal checking logic
+	let mdev;
 	me.getRootNode()?.cascade(function(rec) {
+	    if (rec.data.type === 'entry') {
+		mdev = rec.data.mdev ?? 0;
+	    }
 	    if (rec.data.node !== node || rec.data.type !== 'map') {
 		return;
 	    }
+	    rec.data.mdev = mdev;
 
 	    let id = rec.data.path;
 	    if (!id.match(/\.\d$/)) {
@@ -44,6 +51,7 @@ Ext.define('PVE.dc.PCIMapView', {
 	    let toCheck = {
 		id: deviceId,
 		'subsystem-id': subId,
+		mdev: device.mdev ?? 0,
 		iommugroup: device.iommugroup !== -1 ? device.iommugroup : undefined,
 	    };
 
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 2/4] bulk migrate: improve precondition checks

[Dominik Csapak]

this now takes into account the 'not_allowed_nodes' hash we get from the
api call. With that, we can now limit the 'local_resources' check for
online vms only, as for offline guests, the 'unavailable-resources' hash
already includes mapped devices that don't exist on the target node.

This now also includes unavailable storages on target nodes.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* dropped the change to check for 'online' (since it was wrong)
 PVE/API2/Nodes.pm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 9cdf19db..61d41304 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -2336,6 +2336,18 @@ my $create_migrate_worker = sub {
 	    $invalidConditions .= join(', ', @{$preconditions->{local_resources}});
 	}
 
+	if (my $not_allowed_nodes = $preconditions->{not_allowed_nodes}) {
+	    if (my $unavail_storages = $not_allowed_nodes->{$target}->{unavailable_storages}) {
+		$invalidConditions .= "\n  Has unavailable storages: ";
+		$invalidConditions .= join(', ', $unavail_storages->@*);
+	    }
+
+	    if (my $unavail_resources = $not_allowed_nodes->{$target}->{'unavailable-resources'}) {
+		$invalidConditions .= "\n  Has unavailable resources: ";
+		$invalidConditions .= join(', ', $unavail_resources->@*);
+	    }
+	}
+
 	if ($invalidConditions && $invalidConditions ne '') {
 	    print STDERR "skip VM $vmid - precondition check failed:";
 	    die "$invalidConditions\n";
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 3/4] ui: adapt migration window to precondition api change

[Dominik Csapak]

we now return the 'allowed_nodes'/'not_allowed_nodes' also if the vm is
running, when it has mapped resources. So do that checks independently
so that the user has instant feedback where those resources exist.

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/Migrate.js | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index 78d03921..604b63e7 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -104,7 +104,7 @@ Ext.define('PVE.window.Migrate', {
 	onTargetChange: function(nodeSelector) {
 	    // Always display the storages of the currently selected migration target
 	    this.lookup('pveDiskStorageSelector').setNodename(nodeSelector.value);
-	    this.checkMigratePreconditions();
+	    this.checkMigratePreconditions(true);
 	},
 
 	startMigration: function() {
@@ -214,12 +214,12 @@ Ext.define('PVE.window.Migrate', {
 		migration.possible = true;
 	    }
 	    migration.preconditions = [];
+	    let target = me.lookup('pveNodeSelector').value;
+	    let disallowed = migrateStats.not_allowed_nodes?.[target] ?? {};
 
-	    if (migrateStats.allowed_nodes) {
+	    if (migrateStats.allowed_nodes && !vm.get('running')) {
 		migration.allowedNodes = migrateStats.allowed_nodes;
-		let target = me.lookup('pveNodeSelector').value;
 		if (target.length && !migrateStats.allowed_nodes.includes(target)) {
-		    let disallowed = migrateStats.not_allowed_nodes[target] ?? {};
 		    if (disallowed.unavailable_storages !== undefined) {
 			let missingStorages = disallowed.unavailable_storages.join(', ');
 
@@ -230,17 +230,17 @@ Ext.define('PVE.window.Migrate', {
 			    severity: 'error',
 			});
 		    }
+		}
+	    }
 
-		    if (disallowed['unavailable-resources'] !== undefined) {
-			let unavailableResources = disallowed['unavailable-resources'].join(', ');
+	    if (disallowed['unavailable-resources'] !== undefined) {
+		let unavailableResources = disallowed['unavailable-resources'].join(', ');
 
-			migration.possible = false;
-			migration.preconditions.push({
-			    text: 'Mapped Resources (' + unavailableResources + ') not available on selected target. ',
-			    severity: 'error',
-			});
-		    }
-		}
+		migration.possible = false;
+		migration.preconditions.push({
+		    text: 'Mapped Resources (' + unavailableResources + ') not available on selected target. ',
+		    severity: 'error',
+		});
 	    }
 
 	    let blockingResources = [];
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 4/4] fix #5175: ui: allow configuring and live migration of mapped pci resources

[Dominik Csapak]

if the hardware/driver is capable, the admin can now mark a pci device
as 'live-migration-capable', which then tries enabling live migration
for such devices.

mark it as experimental when configuring and in the migrate window

also change 'hideComment' to 'globalEdit' and reverse the logic
(so hideComment => !globalEdit) to better reflect what it does now.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* dropped the check for 'mappedResources' since it was always true
* renamed 'hideComment' to 'globalEdit' (and reversed it)
  -> no change in logic

 www/manager6/window/Migrate.js    | 25 ++++++++++++++++++++-----
 www/manager6/window/PCIMapEdit.js | 20 ++++++++++++++++----
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index 604b63e7..dba5556b 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -244,10 +244,10 @@ Ext.define('PVE.window.Migrate', {
 	    }
 
 	    let blockingResources = [];
-	    let mappedResources = migrateStats['mapped-resources'] ?? [];
+	    let mappedResources = migrateStats['mapped-resource-info'] ?? {};
 
 	    for (const res of migrateStats.local_resources) {
-		if (mappedResources.indexOf(res) === -1) {
+		if (!mappedResources[res]) {
 		    blockingResources.push(res);
 		}
 	    }
@@ -271,14 +271,29 @@ Ext.define('PVE.window.Migrate', {
 		}
 	    }
 
-	    if (mappedResources && mappedResources.length) {
-		if (vm.get('running')) {
+	    if (vm.get('running')) {
+		let allowed = [];
+		let notAllowed = [];
+		for (const [key, resource] of Object.entries(mappedResources)) {
+		    if (resource['live-migration']) {
+			allowed.push(key);
+		    } else {
+			notAllowed.push(key);
+		    }
+		}
+		if (notAllowed.length > 0) {
 		    migration.possible = false;
 		    migration.preconditions.push({
 			text: Ext.String.format('Can\'t migrate running VM with mapped resources: {0}',
-			mappedResources.join(', ')),
+			notAllowed.join(', ')),
 			severity: 'error',
 		    });
+		} else if (allowed.length > 0) {
+		    migration.preconditions.push({
+			text: Ext.String.format('Live-migrating running VM with mapped resources (Experimental): {0}',
+			allowed.join(', ')),
+			severity: 'warning',
+		    });
 		}
 	    }
 
diff --git a/www/manager6/window/PCIMapEdit.js b/www/manager6/window/PCIMapEdit.js
index faf58255..8b7ff826 100644
--- a/www/manager6/window/PCIMapEdit.js
+++ b/www/manager6/window/PCIMapEdit.js
@@ -16,7 +16,7 @@ Ext.define('PVE.window.PCIMapEditWindow', {
 	me.isCreate = (!me.name || !me.nodename) && !me.entryOnly;
 	me.method = me.name ? 'PUT' : 'POST';
 	me.hideMapping = !!me.entryOnly;
-	me.hideComment = me.name && !me.entryOnly;
+	me.globalEdit = !me.name || me.entryOnly;
 	me.hideNodeSelector = me.nodename || me.entryOnly;
 	me.hideNode = !me.nodename || !me.hideNodeSelector;
 	return {
@@ -241,7 +241,19 @@ Ext.define('PVE.window.PCIMapEditWindow', {
 		    name: 'mdev',
 		    cbind: {
 			deleteEmpty: '{!isCreate}',
-			disabled: '{hideComment}',
+			disabled: '{!globalEdit}',
+		    },
+		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    fieldLabel: gettext('Live Migration Capable'),
+		    labelWidth: 200,
+		    boxLabel: `<i class="fa fa-exclamation-triangle warning"></i> ${gettext('Experimental')}`,
+		    reference: 'live-migration-capable',
+		    name: 'live-migration-capable',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+			disabled: '{!globalEdit}',
 		    },
 		},
 	    ],
@@ -271,8 +283,8 @@ Ext.define('PVE.window.PCIMapEditWindow', {
 		    name: 'description',
 		    cbind: {
 			deleteEmpty: '{!isCreate}',
-			disabled: '{hideComment}',
-			hidden: '{hideComment}',
+			disabled: '{!globalEdit}',
+			hidden: '{!globalEdit}',
 		    },
 		},
 	    ],
-- 
2.39.5



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

[pve-devel] [PATCH docs v7 1/2] qm: resource mapping: add description for `mdev` option

[Dominik Csapak]

in a new section about additional options

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 qm.adoc | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 4bb8f2c..6f337fe 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -1944,6 +1944,18 @@ To create mappings `Mapping.Modify` on `/mapping/<type>/<name>` is necessary
 To use these mappings, `Mapping.Use` on `/mapping/<type>/<name>` is necessary
 (in addition to the normal guest privileges to edit the configuration).
 
+.Additional Options
+
+There are additional options when defining a cluster wide resource mapping.
+Currently there are the following options:
+
+* `mdev` (PCI): This marks the PCI device as being capable of providing
+  `mediated devices`. When this is enabled, you can select a type when
+  configuring it on the guest. If multiple PCI devices are selected for the
+  mapping, the mediated device will be created on the first one where there are
+  any available instances of the selected type.
+
+
 Managing Virtual Machines with `qm`
 ------------------------------------
 
-- 
2.39.5



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

[pve-devel] [PATCH docs v7 2/2] qm: resource mapping: document `live-migration-capable` setting

[Dominik Csapak]

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 qm.adoc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 6f337fe..0d18d7e 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -1955,6 +1955,12 @@ Currently there are the following options:
   mapping, the mediated device will be created on the first one where there are
   any available instances of the selected type.
 
+* `live-migration-capable` (PCI): This marks the PCI device as being capable of
+  being live migrated between nodes. This requires driver and hardware support.
+  Only NVIDIA GPUs with recent kernel are known to support this. Note that live
+  migrating passed through devices is an experimental feature and may not work
+  or cause issues.
+
 
 Managing Virtual Machines with `qm`
 ------------------------------------
-- 
2.39.5



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

Re: [pve-devel] [PATCH guest-common/qemu-server/manager/docs v7] implement experimental vgpu live migration

[Dominik Csapak]

ping, I think additional testing/reviewing would be good here. @Christoph do you have the time?


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

Re: [pve-devel] [PATCH guest-common/qemu-server/manager/docs v7] implement experimental vgpu live migration

[Christoph Heiss]

Built all packages on their latest master w/ the patches, respectively,
applied and tested it using the same setup as Dominik (as we don't have
anything other currently). Plus latest pve-common master as
prerequisite.

Updated all nodes to the latest packages beforehand and also re-created
all resource-mappings from scratch, to ensure to hidden dependencies and
properly test all the pve-manager changes to the GUI.

Live-migration with a mapped vGPU worked as it should (although had to
manually trigger it via an API call, to bypass the broken uptime/online
check in the GUI due to the PVE-in-CT setup).
Also tried out all the different combinations of non-/existing mappings,
offline/online migration etc. that they work (or not) as expected.

Finally had a look over the new/changed patches w/o my R-b, nothing
jumped at me there.

Please consider the series thus:

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>


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

Re: [pve-devel] [PATCH guest-common v7 1/2] mapping: pci: check the mdev configuration on the device too

[Thomas Lamprecht]

Am 11.03.25 um 14:20 schrieb Dominik Csapak:
> but that lives int he 'global' part of the mapping config, not in a
> specific mapping. To check that, add it to the $configured_props from
> there.
> 
> this requires all call sites to be adapted otherwise the check will
> always fail for devices that are capable of mediated devices

But that's not true, or? As the check only happens if the $cluster_mapping_cfg
param is passed, which call-sites need to do first?

>  # checks if the given config is valid for the current node
>  sub assert_valid {
> -    my ($name, $mapping) = @_;
> +    my ($name, $mapping, $cluster_mapping_cfg) = @_;

                            ^-  new param here

>  
>      my @paths = split(';', $mapping->{path} // '');
>  
> @@ -161,6 +161,12 @@ sub assert_valid {
>  
>  	my $configured_props = { $mapping->%{qw(id iommugroup subsystem-id)} };
>  
> +	# check mdev from globabl mapping config, if that is given
> +	if (defined($cluster_mapping_cfg)) {

guarded witch check for defindness here

> +	    $expected_props->{mdev} = $info->{mdev} ? 1 : 0;
> +	    $configured_props->{mdev} = $cluster_mapping_cfg->{mdev} ? 1 : 0;
> +	}
> +
>  	for my $prop (sort keys $expected_props->%*) {
>  	    next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on the first device
>  



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

Re: [pve-devel] [PATCH guest-common v7 1/2] mapping: pci: check the mdev configuration on the device too

[Dominik Csapak]

On 4/3/25 11:40, Thomas Lamprecht wrote:
> Am 11.03.25 um 14:20 schrieb Dominik Csapak:
>> but that lives int he 'global' part of the mapping config, not in a
>> specific mapping. To check that, add it to the $configured_props from
>> there.
>>
>> this requires all call sites to be adapted otherwise the check will
>> always fail for devices that are capable of mediated devices
> 
> But that's not true, or? As the check only happens if the $cluster_mapping_cfg
> param is passed, which call-sites need to do first?
> 

true, I think the commit message is outdated. I faintly remember changing
the semantic at some point but probably forgot to update the commit message.

>>   # checks if the given config is valid for the current node
>>   sub assert_valid {
>> -    my ($name, $mapping) = @_;
>> +    my ($name, $mapping, $cluster_mapping_cfg) = @_;
> 
>                              ^-  new param here
> 
>>   
>>       my @paths = split(';', $mapping->{path} // '');
>>   
>> @@ -161,6 +161,12 @@ sub assert_valid {
>>   
>>   	my $configured_props = { $mapping->%{qw(id iommugroup subsystem-id)} };
>>   
>> +	# check mdev from globabl mapping config, if that is given
>> +	if (defined($cluster_mapping_cfg)) {
> 
> guarded witch check for defindness here
> 
>> +	    $expected_props->{mdev} = $info->{mdev} ? 1 : 0;
>> +	    $configured_props->{mdev} = $cluster_mapping_cfg->{mdev} ? 1 : 0;
>> +	}
>> +
>>   	for my $prop (sort keys $expected_props->%*) {
>>   	    next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on the first device
>>   
> 



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

Re: [pve-devel] [PATCH guest-common v7 1/2] mapping: pci: check the mdev configuration on the device too

[Thomas Lamprecht]

Am 03.04.25 um 11:43 schrieb Dominik Csapak:
> On 4/3/25 11:40, Thomas Lamprecht wrote:
>> Am 11.03.25 um 14:20 schrieb Dominik Csapak:
>>> but that lives int he 'global' part of the mapping config, not in a
>>> specific mapping. To check that, add it to the $configured_props from
>>> there.
>>>
>>> this requires all call sites to be adapted otherwise the check will
>>> always fail for devices that are capable of mediated devices
>> But that's not true, or? As the check only happens if the $cluster_mapping_cfg
>> param is passed, which call-sites need to do first?
>>
> true, I think the commit message is outdated. I faintly remember changing
> the semantic at some point but probably forgot to update the commit message.

ack, thx for confirmaiton, then I adapt that on applying – got just
"scared" that I have to do a whole break/depends update over multiple
repos ^^


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

Re: [pve-devel] [PATCH qemu-server v7 6/9] api: enable live migration for marked mapped pci devices

[Thomas Lamprecht]

Am 11.03.25 um 14:20 schrieb Dominik Csapak:
> They have to be marked as 'live-migration-capable' in the mapping
> config, and the driver and qemu must support it.
> 
> For the gui checks, we now return the whole object of the mapped
> resources, which includes info like the name and if it's marked as
> live-migration capable. (while deprecating the old 'mapped-resource'
> return value, since that returns strictly less information)
> 
> Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/API2/Qemu.pm   |  8 +++++++-
>  PVE/QemuMigrate.pm | 17 ++++++++++++-----
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 060bca8b..15bf7548 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -4665,13 +4665,18 @@ __PACKAGE__->register_method({
>  		},
>  		description => "List local resources (e.g. pci, usb) that block migration."
>  	    },
> +	    # FIXME: remove with 9.0
>  	    'mapped-resources' => {
>  		type => 'array',
>  		items => {
>  		    type => 'string',
>  		    description => "A mapped resource",
>  		},
> -		description => "List of mapped resources e.g. pci, usb"
> +		description => "List of mapped resources e.g. pci, usb. Deprecated, use 'mapped-resource-info' instead."
> +	    },
> +	    'mapped-resource-info' => {
> +		type => 'object',
> +		description => "Object of mapped resources with additional information such if they're live migratable.",

Where is the schema/format for this? New stuff really should be documented
here, I mean, have you not sent enough schema updates to unlock basic features
in the rust based PVE api crate used in PDM? ;-)

>  	    },
>  	},
>      },
> @@ -4737,6 +4742,7 @@ __PACKAGE__->register_method({
>  
>  	$res->{local_resources} = $local_resources;
>  	$res->{'mapped-resources'} = [ sort keys $mapped_resources->%* ];
> +	$res->{'mapped-resource-info'} = $mapped_resources;

would be IMO nicer to add returning that info upfront and then combine this
with patch with 4/9 to have two patches that each do one thing, and not a
split/mix.

But I probably will just apply this as is with trying to bend the commit
message from implementation details to the effect of the patch, will not
be ideal, but I want to finish this finally..

No offense to anybody, but IMO does not pain the best picture considering
this has two R-b tags..

>  
>  	return $res;
>  
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 2153ac42..6909fc82 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -243,11 +243,18 @@ sub prepare {
>  
>      if (scalar(keys $mapped_res->%*)) {
>  	my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
> -	if ($running) {
> -	    my $mapped_text = join(", ", sort keys $mapped_res->%*);
> -	    die "can't migrate running VM which uses mapped devices: $mapped_text\n";
> -	} elsif (scalar($missing_mappings->@*)) {
> -	    die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
> +	my $missing_live_mappings = [];
> +	for my $key (sort keys $mapped_res->%*) {
> +	    my $res = $mapped_res->{$key};
> +	    my $name = "$key:$res->{name}";
> +	    push $missing_live_mappings->@*, $name if !$res->{'live-migration'};
> +	}
> +	if (scalar($missing_mappings->@*)) {
> +	    my $missing = join(", ", $missing_mappings->@*);
> +	    die "can't migrate to '$self->{node}': missing mapped devices $missing\n";
> +	} elsif ($running && scalar($missing_live_mappings->@*)) {
> +	    my $missing = join(", ", $missing_live_mappings->@*);
> +	    die "can't live migrate running VM which uses following mapped devices: $missing\n";
>  	} else {
>  	    $self->log('info', "migrating VM which uses mapped local devices");
>  	}



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

Re: [pve-devel] [PATCH qemu-server v7 6/9] api: enable live migration for marked mapped pci devices

[Dominik Csapak]

On 4/3/25 15:00, Thomas Lamprecht wrote:
> Am 11.03.25 um 14:20 schrieb Dominik Csapak:
>> They have to be marked as 'live-migration-capable' in the mapping
>> config, and the driver and qemu must support it.
>>
>> For the gui checks, we now return the whole object of the mapped
>> resources, which includes info like the name and if it's marked as
>> live-migration capable. (while deprecating the old 'mapped-resource'
>> return value, since that returns strictly less information)
>>
>> Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
>> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   PVE/API2/Qemu.pm   |  8 +++++++-
>>   PVE/QemuMigrate.pm | 17 ++++++++++++-----
>>   2 files changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 060bca8b..15bf7548 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -4665,13 +4665,18 @@ __PACKAGE__->register_method({
>>   		},
>>   		description => "List local resources (e.g. pci, usb) that block migration."
>>   	    },
>> +	    # FIXME: remove with 9.0
>>   	    'mapped-resources' => {
>>   		type => 'array',
>>   		items => {
>>   		    type => 'string',
>>   		    description => "A mapped resource",
>>   		},
>> -		description => "List of mapped resources e.g. pci, usb"
>> +		description => "List of mapped resources e.g. pci, usb. Deprecated, use 'mapped-resource-info' instead."
>> +	    },
>> +	    'mapped-resource-info' => {
>> +		type => 'object',
>> +		description => "Object of mapped resources with additional information such if they're live migratable.",
> 
> Where is the schema/format for this? New stuff really should be documented
> here, I mean, have you not sent enough schema updates to unlock basic features
> in the rust based PVE api crate used in PDM? ;-)

Yes, you're right. I'll send a follow up for that if that's fine with you.

> 
>>   	    },
>>   	},
>>       },
>> @@ -4737,6 +4742,7 @@ __PACKAGE__->register_method({
>>   
>>   	$res->{local_resources} = $local_resources;
>>   	$res->{'mapped-resources'} = [ sort keys $mapped_resources->%* ];
>> +	$res->{'mapped-resource-info'} = $mapped_resources;
> 
> would be IMO nicer to add returning that info upfront and then combine this
> with patch with 4/9 to have two patches that each do one thing, and not a
> split/mix.
> 
> But I probably will just apply this as is with trying to bend the commit
> message from implementation details to the effect of the patch, will not
> be ideal, but I want to finish this finally..

If you want I can try to rework this part of the series, since it does make
sense like you described, but I'm not sure I could finish that today.

> 
> No offense to anybody, but IMO does not pain the best picture considering
> this has two R-b tags..
> 
>>   
>>   	return $res;
>>   
>> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
>> index 2153ac42..6909fc82 100644
>> --- a/PVE/QemuMigrate.pm
>> +++ b/PVE/QemuMigrate.pm
>> @@ -243,11 +243,18 @@ sub prepare {
>>   
>>       if (scalar(keys $mapped_res->%*)) {
>>   	my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
>> -	if ($running) {
>> -	    my $mapped_text = join(", ", sort keys $mapped_res->%*);
>> -	    die "can't migrate running VM which uses mapped devices: $mapped_text\n";
>> -	} elsif (scalar($missing_mappings->@*)) {
>> -	    die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
>> +	my $missing_live_mappings = [];
>> +	for my $key (sort keys $mapped_res->%*) {
>> +	    my $res = $mapped_res->{$key};
>> +	    my $name = "$key:$res->{name}";
>> +	    push $missing_live_mappings->@*, $name if !$res->{'live-migration'};
>> +	}
>> +	if (scalar($missing_mappings->@*)) {
>> +	    my $missing = join(", ", $missing_mappings->@*);
>> +	    die "can't migrate to '$self->{node}': missing mapped devices $missing\n";
>> +	} elsif ($running && scalar($missing_live_mappings->@*)) {
>> +	    my $missing = join(", ", $missing_live_mappings->@*);
>> +	    die "can't live migrate running VM which uses following mapped devices: $missing\n";
>>   	} else {
>>   	    $self->log('info', "migrating VM which uses mapped local devices");
>>   	}
> 



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

[pve-devel] [PATCH follow-up qemu-server] api: migrate preconditions: add schema description for 'mapped-resource-info'

[Dominik Csapak]

Since it's an object that has the config-properties as keys (e.g.
hostpci0), we have to add all possible values (hostpciX/usbX) to it as
optional, like we do for e.g. config GET api calls.

The actual info per property is not much currently, just the name of
the mapping and for PCI devices if they're marked as live-migratable.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
this is a follow-up for:
[PATCH qemu-server v7 6/9] api: enable live migration for marked mapped pci devices
https://lore.proxmox.com/pve-devel/20250311132055.2826686-9-d.csapak@proxmox.com/

could also be squashed in on applying if wanted

 PVE/API2/Qemu.pm | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 459c7e29..a2e200c7 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4580,6 +4580,46 @@ my $check_vm_disks_local = sub {
     return $local_disks;
 };
 
+sub mapped_resource_info_properties {
+    my $properties= {};
+
+    for (my $i = 0; $i < $PVE::QemuServer::PCI::MAX_HOSTPCI_DEVICES; $i++) {
+	$properties->{"hostpci$i"} = {
+	    type => 'object',
+	    description => "Information about hostpci$i",
+	    optional => 1,
+	    properties => {
+		name => {
+		    type => 'string',
+		    description => 'The name of the mapping.',
+		},
+		'live-migration' => {
+		    type => 'boolean',
+		    optional => 1,
+		    default => 0,
+		    description => "True if the mapping is marked as live-migration capable.",
+		},
+	    },
+	};
+    }
+
+    for (my $i = 0; $i < $PVE::QemuServer::USB::MAX_USB_DEVICES; $i++) {
+	$properties->{"usb$i"} = {
+	    type => 'object',
+	    description => "Information about usb$i",
+	    optional => 1,
+	    properties => {
+		name => {
+		    type => 'string',
+		    description => 'The name of the mapping.',
+		},
+	    },
+	};
+    }
+
+    return $properties;
+}
+
 __PACKAGE__->register_method({
     name => 'migrate_vm_precondition',
     path => '{vmid}/migrate',
@@ -4681,6 +4721,7 @@ __PACKAGE__->register_method({
 	    'mapped-resource-info' => {
 		type => 'object',
 		description => "Object of mapped resources with additional information such if they're live migratable.",
+		properties => mapped_resource_info_properties(),
 	    },
 	},
     },
-- 
2.39.5



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

[pve-devel] applied-series: [PATCH guest-common/qemu-server/manager/docs v7] implement experimental vgpu live migration

[Thomas Lamprecht]

Am 11.03.25 um 14:20 schrieb Dominik Csapak:
> pve-guest-common:
> 
> Dominik Csapak (2):
>   mapping: pci: check the mdev configuration on the device too
>   mapping: pci: add 'live-migration-capable' flag to mappings
> 
>  src/PVE/Mapping/PCI.pm | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> qemu-server:
> 
> Dominik Csapak (9):
>   tests: cfg2cmd: fix mdev tests
>   pci: mapping: check mdev config against hardware
>   pci: set 'enable-migration' to on for live-migration marked mapped
>     devices
>   check_local_resources: add more info per mapped device and return as
>     hash
>   check_local_resources: allow mapped devices for offline migration
>   api: enable live migration for marked mapped pci devices
>   api: include not mapped resources for running vms in migrate
>     preconditions
>   migrate: show vfio state transferred too
>   migrate: add transfer summary
> 
>  PVE/API2/Qemu.pm                 | 59 +++++++++++++++++++-------------
>  PVE/QemuMigrate.pm               | 46 +++++++++++++++++++++----
>  PVE/QemuServer.pm                | 24 +++++++++----
>  PVE/QemuServer/PCI.pm            | 10 +++++-
>  test/run_config2command_tests.pl |  2 +-
>  5 files changed, 101 insertions(+), 40 deletions(-)
> 
> pve-manager:
> 
> Dominik Csapak (4):
>   mapping: pci: include mdev in config checks
>   bulk migrate: improve precondition checks
>   ui: adapt migration window to precondition api change
>   fix #5175: ui: allow configuring and live migration of mapped pci
>     resources
> 
>  PVE/API2/Cluster/Mapping/PCI.pm   |  2 +-
>  PVE/API2/Nodes.pm                 | 12 ++++++++
>  www/manager6/dc/PCIMapView.js     |  8 +++++
>  www/manager6/window/Migrate.js    | 51 ++++++++++++++++++++-----------
>  www/manager6/window/PCIMapEdit.js | 20 +++++++++---
>  5 files changed, 70 insertions(+), 23 deletions(-)
> 
> pve-docs:
> 
> Dominik Csapak (2):
>   qm: resource mapping: add description for `mdev` option
>   qm: resource mapping: document `live-migration-capable` setting
> 
>  qm.adoc | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 


applied series, with some clean-ups/re-ordering (mostly commits and their
messages), thanks!

Towards the end I might have overlooked picking up a few T-b trailers from
Christoph, sorry about that, they nonetheless played a vital role when
applying this, so please be not discouraged by that.


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