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

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

[Dominik Csapak]

and some useful cleanups

this series replaces both the initial pci live migration and the
fixup series[0][1]

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).
(though i tested the live-migration part manually here by using
"exec:cat > /tmp/test" for the migration target, and "exec: cat
/tmp/test" as the 'incoming' parameter for a new vm start, which worked ;) )

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)

the first 3 patches of guest-common, the first 6 patches of qemu-server
and the first 2 of pve-manager, only tangentially relate to the actual
title of this series and could be applied independently, since they're
mostly cleanups & code move

though qemu-server 3/10 and pve-manager 1/5 depend on the
pve-guest-common 2-3/5

also pve-manager 4/5 depends on all of qemu-server

0: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062226.html
1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062293.html


pve-guest-common:

Dominik Csapak (5):
  mapping: pci: fix missing description/default for mdev
  mapping: pci: rework properties check
  mapping: pci: check the mdev configuration on the device too
  mapping: pci: add 'live-migration-capable' flag to mappings
  mapping: remove find_on_current_node

 src/PVE/Mapping/PCI.pm | 69 ++++++++++++++++++++++++------------------
 src/PVE/Mapping/USB.pm |  9 ------
 2 files changed, 40 insertions(+), 38 deletions(-)

qemu-server:

Dominik Csapak (10):
  usb: mapping: move implementation of find_on_current_node here
  pci: mapping: move implementation of find_on_current_node here
  pci: mapping: check mdev config against hardware
  stop cleanup: remove unnecessary tpmstate cleanup
  vm_stop_cleanup: add noerr parameter
  migrate: call vm_stop_cleanup after stopping in phase3_cleanup
  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
  api: enable live migration for marked mapped pci devices
  api: include not mapped resources for running vms in migrate
    preconditions

 PVE/API2/Qemu.pm             | 48 ++++++++++++++++++++++--------------
 PVE/CLI/qm.pm                |  2 +-
 PVE/QemuMigrate.pm           | 28 ++++++++++++---------
 PVE/QemuServer.pm            | 38 ++++++++++++++--------------
 PVE/QemuServer/PCI.pm        | 14 +++++++++--
 PVE/QemuServer/USB.pm        |  5 +++-
 test/MigrationTest/Shared.pm |  3 +++
 7 files changed, 84 insertions(+), 54 deletions(-)

pve-manager:

Dominik Csapak (5):
  mapping: pci: include mdev in config checks
  bulk migrate: improve precondition checks
  bulk migrate: include checks for live-migratable local resources
  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                 | 27 ++++++++++++++--
 www/manager6/dc/PCIMapView.js     |  5 +++
 www/manager6/window/Migrate.js    | 51 ++++++++++++++++++++-----------
 www/manager6/window/PCIMapEdit.js | 12 ++++++++
 5 files changed, 75 insertions(+), 22 deletions(-)

pve-docs:

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

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

-- 
2.39.2

[pve-devel] [PATCH guest-common v2 1/5] mapping: pci: fix missing description/default for mdev

[Dominik Csapak]

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

diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
index 19ace98..725e106 100644
--- a/src/PVE/Mapping/PCI.pm
+++ b/src/PVE/Mapping/PCI.pm
@@ -100,8 +100,10 @@ my $defaultData = {
 	    maxLength => 4096,
 	},
 	mdev => {
+	    description => "Marks the device(s) as being capable of providing mediated devices.",
 	    type => 'boolean',
 	    optional => 1,
+	    default => 0,
 	},
 	map => {
 	    type => 'array',
-- 
2.39.2

[pve-devel] applied: Re: [PATCH guest-common v2 1/5] mapping: pci: fix missing description/default for mdev

[Thomas Lamprecht]

On 10/04/2024 13:03, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/Mapping/PCI.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
>

applied, thanks!

[pve-devel] [PATCH guest-common v2 2/5] mapping: pci: rework properties check

[Dominik Csapak]

refactors the actual checking out to its own sub, so we can reuse it
later

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/Mapping/PCI.pm | 43 +++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
index 725e106..fcf07c0 100644
--- a/src/PVE/Mapping/PCI.pm
+++ b/src/PVE/Mapping/PCI.pm
@@ -129,6 +129,26 @@ sub options {
     };
 }
 
+my sub check_properties {
+    my ($correct, $configured, $path, $name) = @_;
+    for my $prop (sort keys %$correct) {
+	next if !defined($correct->{$prop}) && !defined($configured->{$prop});
+
+	die "no '$prop' for device '$path'\n"
+	    if defined($correct->{$prop}) && !defined($configured->{$prop});
+	die "'$prop' configured but should not be\n"
+	    if !defined($correct->{$prop}) && defined($configured->{$prop});
+
+	my $correct_prop = $correct->{$prop};
+	$correct_prop =~ s/0x//g;
+	my $configured_prop = $configured->{$prop};
+	$configured_prop =~ s/0x//g;
+
+	die "'$prop' does not match for '$name' ($correct_prop != $configured_prop)\n"
+	    if $correct_prop ne $configured_prop;
+    }
+}
+
 # checks if the given config is valid for the current node
 sub assert_valid {
     my ($name, $cfg) = @_;
@@ -150,30 +170,19 @@ sub assert_valid {
 
 	my $correct_props = {
 	    id => "$info->{vendor}:$info->{device}",
-	    iommugroup => $info->{iommugroup},
 	};
 
+	# check iommu only on the first device
+	if ($idx == 0) {
+	    $correct_props->{iommugroup} = $info->{iommugroup};
+	}
+
 	if (defined($info->{'subsystem_vendor'}) && defined($info->{'subsystem_device'})) {
 	    $correct_props->{'subsystem-id'} = "$info->{'subsystem_vendor'}:$info->{'subsystem_device'}";
 	}
 
-	for my $prop (sort keys %$correct_props) {
-	    next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on the first device
-
-	    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});
+	check_properties($correct_props, $cfg, $path, $name);
 
-	    my $correct_prop = $correct_props->{$prop};
-	    $correct_prop =~ s/0x//g;
-	    my $configured_prop = $cfg->{$prop};
-	    $configured_prop =~ s/0x//g;
-
-	    die "'$prop' does not match for '$name' ($correct_prop != $configured_prop)\n"
-		if $correct_prop ne $configured_prop;
-	}
 	$idx++;
     }
 
-- 
2.39.2

Re: [pve-devel] [PATCH guest-common v2 2/5] mapping: pci: rework properties check

[Thomas Lamprecht]

On 10/04/2024 13:03, Dominik Csapak wrote:
> refactors the actual checking out to its own sub, so we can reuse it
> later
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/Mapping/PCI.pm | 43 +++++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
> index 725e106..fcf07c0 100644
> --- a/src/PVE/Mapping/PCI.pm
> +++ b/src/PVE/Mapping/PCI.pm
> @@ -129,6 +129,26 @@ sub options {
>      };
>  }
>  
> +my sub check_properties {

s/check/assert/ and ideally some words that better describe what is
actually asserted here.

> +    my ($correct, $configured, $path, $name) = @_;

maybe s/correct/expected/ would be slightly better in conveying that the
passed $configured one do not only need to be all in the first hash, but
that all keys of the first hash

> +    for my $prop (sort keys %$correct) {
> +	next if !defined($correct->{$prop}) && !defined($configured->{$prop});
> +
> +	die "no '$prop' for device '$path'\n"

pre-existing, but maybe this would be slightly better worded like:

"missing expected property '$prop' for device '$path'\n" 

(no hard feelings though)

> +	    if defined($correct->{$prop}) && !defined($configured->{$prop});
> +	die "'$prop' configured but should not be\n"

also pre-existing, but I would adapt the error message to something like:

"unknown property '$prop' configured for device '$path'\n"

(slightly hard feelings here ;-))

(btw. would it make sense to also add $name?)


> +	    if !defined($correct->{$prop}) && defined($configured->{$prop});

can above check even trigger if we just go through the expected ($correct)
set of properties? Or are existing, but undef, entries in $correct the
forbidden ones, and other extra properties in $configured do not matter?

(I dind't check the full picture, so excuse me if this would be obvious,
but them IMO some comments would be warranted)

> +
> +	my $correct_prop = $correct->{$prop};
> +	$correct_prop =~ s/0x//g;
> +	my $configured_prop = $configured->{$prop};
> +	$configured_prop =~ s/0x//g;
> +
> +	die "'$prop' does not match for '$name' ($correct_prop != $configured_prop)\n"
> +	    if $correct_prop ne $configured_prop;
> +    }
> +}
> +
>  # checks if the given config is valid for the current node
>  sub assert_valid {
>      my ($name, $cfg) = @_;
> @@ -150,30 +170,19 @@ sub assert_valid {
>  
>  	my $correct_props = {
>  	    id => "$info->{vendor}:$info->{device}",
> -	    iommugroup => $info->{iommugroup},
>  	};
>  
> +	# check iommu only on the first device
> +	if ($idx == 0) {
> +	    $correct_props->{iommugroup} = $info->{iommugroup};
> +	}

is this really the same than what got removed in the loop?

As if the next ID 

> +
>  	if (defined($info->{'subsystem_vendor'}) && defined($info->{'subsystem_device'})) {
>  	    $correct_props->{'subsystem-id'} = "$info->{'subsystem_vendor'}:$info->{'subsystem_device'}";
>  	}
>  
> -	for my $prop (sort keys %$correct_props) {
> -	    next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on the first device
> -
> -	    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});
> +	check_properties($correct_props, $cfg, $path, $name);
>  
> -	    my $correct_prop = $correct_props->{$prop};
> -	    $correct_prop =~ s/0x//g;
> -	    my $configured_prop = $cfg->{$prop};
> -	    $configured_prop =~ s/0x//g;
> -
> -	    die "'$prop' does not match for '$name' ($correct_prop != $configured_prop)\n"
> -		if $correct_prop ne $configured_prop;
> -	}
>  	$idx++;
>      }
>  

[pve-devel] [PATCH guest-common v2 3/5] 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 a new (optional) parameter to assert_valid
that includes said global config.

by making that check optional, we don't break older users of that
function.

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

diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
index fcf07c0..d8a5962 100644
--- a/src/PVE/Mapping/PCI.pm
+++ b/src/PVE/Mapping/PCI.pm
@@ -151,9 +151,9 @@ my sub check_properties {
 
 # checks if the given config is valid for the current node
 sub assert_valid {
-    my ($name, $cfg) = @_;
+    my ($name, $mapping, $cfg) = @_;
 
-    my @paths = split(';', $cfg->{path} // '');
+    my @paths = split(';', $mapping->{path} // '');
 
     my $idx = 0;
     for my $path (@paths) {
@@ -181,7 +181,9 @@ sub assert_valid {
 	    $correct_props->{'subsystem-id'} = "$info->{'subsystem_vendor'}:$info->{'subsystem_device'}";
 	}
 
-	check_properties($correct_props, $cfg, $path, $name);
+	check_properties($correct_props, $mapping, $path, $name);
+	# check mdev from globabl mapping config
+	check_properties({ mdev => $info->{mdev}, }, $cfg, $path, $name) if defined($cfg);
 
 	$idx++;
     }
-- 
2.39.2

[pve-devel] [PATCH guest-common v2 4/5] 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.

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 d8a5962..c2ebcf6 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.2

[pve-devel] [PATCH guest-common v2 5/5] mapping: remove find_on_current_node

[Dominik Csapak]

they only have one user each (where we can inline the implementation).
It's easy enough to recreate should we need to.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/Mapping/PCI.pm | 10 ----------
 src/PVE/Mapping/USB.pm |  9 ---------
 2 files changed, 19 deletions(-)

diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
index c2ebcf6..72b6812 100644
--- a/src/PVE/Mapping/PCI.pm
+++ b/src/PVE/Mapping/PCI.pm
@@ -218,16 +218,6 @@ sub write_pci_config {
     cfs_write_file($FILENAME, $cfg);
 }
 
-sub find_on_current_node {
-    my ($id) = @_;
-
-    my $cfg = PVE::Mapping::PCI::config();
-    my $node = PVE::INotify::nodename();
-
-    # ignore errors
-    return get_node_mapping($cfg, $id, $node);
-}
-
 sub get_node_mapping {
     my ($cfg, $id, $nodename) = @_;
 
diff --git a/src/PVE/Mapping/USB.pm b/src/PVE/Mapping/USB.pm
index 34bc3cb..6dd15c4 100644
--- a/src/PVE/Mapping/USB.pm
+++ b/src/PVE/Mapping/USB.pm
@@ -155,15 +155,6 @@ sub write_usb_config {
     cfs_write_file($FILENAME, $cfg);
 }
 
-sub find_on_current_node {
-    my ($id) = @_;
-
-    my $cfg = config();
-    my $node = PVE::INotify::nodename();
-
-    return get_node_mapping($cfg, $id, $node);
-}
-
 sub get_node_mapping {
     my ($cfg, $id, $nodename) = @_;
 
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 01/10] usb: mapping: move implementation of find_on_current_node here

[Dominik Csapak]

this was the only user, and it's easy enough

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

diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index 49957444..ecd0361d 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -5,6 +5,7 @@ use warnings;
 use PVE::QemuServer::PCI qw(print_pci_addr);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Helpers qw(min_version windows_version);
+use PVE::INotify;
 use PVE::JSONSchema;
 use PVE::Mapping::USB;
 use base 'Exporter';
@@ -91,7 +92,9 @@ sub parse_usb_device {
 	    $res->{spice} = 1;
 	}
     } elsif (defined($mapping)) {
-	my $devices = PVE::Mapping::USB::find_on_current_node($mapping);
+	my $config = PVE::Mapping::USB::config();
+	my $node = PVE::INotify::nodename();
+	my $devices = PVE::Mapping::USB::get_node_mapping($config, $mapping, $node);
 	die "USB device mapping not found for '$mapping'\n" if !$devices || !scalar($devices->@*);
 	die "More than one USB mapping per host not supported\n" if scalar($devices->@*) > 1;
 	eval {
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 02/10] pci: mapping: move implementation of find_on_current_node here

[Dominik Csapak]

this was the only user, and it's easy enough

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

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 1673041b..7ff9cad7 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -3,6 +3,7 @@ package PVE::QemuServer::PCI;
 use warnings;
 use strict;
 
+use PVE::INotify;
 use PVE::JSONSchema;
 use PVE::Mapping::PCI;
 use PVE::SysFSTools;
@@ -429,7 +430,9 @@ sub parse_hostpci {
 
     if ($mapping) {
 	# we have no ordinary pci id, must be a mapping
-	my $devices = PVE::Mapping::PCI::find_on_current_node($mapping);
+	my $config = PVE::Mapping::PCI::config();
+	my $node = PVE::INotify::nodename();
+	my $devices = PVE::Mapping::PCI::get_node_mapping($config, $mapping, $node);
 	die "PCI device mapping not found for '$mapping'\n" if !$devices || !scalar($devices->@*);
 
 	for my $device ($devices->@*) {
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 03/10] pci: mapping: check mdev config against hardware

[Dominik Csapak]

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

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

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 7ff9cad7..6ba43ee8 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -436,7 +436,7 @@ sub parse_hostpci {
 	die "PCI device mapping not found for '$mapping'\n" if !$devices || !scalar($devices->@*);
 
 	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.2

[pve-devel] [PATCH qemu-server v2 04/10] stop cleanup: remove unnecessary tpmstate cleanup

[Dominik Csapak]

tpmstate0 is already included in `get_vm_volumes`, and our only storage
plugin that has unmap_volume implemented is the RBDPlugin, where we call
unmap in `deactivate_volume`. So it's already ummapped by the
`deactivate_volumes` calls above.

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

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6e2c8052..8a71e8aa 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6165,14 +6165,6 @@ sub vm_stop_cleanup {
 	if (!$keepActive) {
 	    my $vollist = get_vm_volumes($conf);
 	    PVE::Storage::deactivate_volumes($storecfg, $vollist);
-
-	    if (my $tpmdrive = $conf->{tpmstate0}) {
-		my $tpm = parse_drive("tpmstate0", $tpmdrive);
-		my ($storeid, $volname) = PVE::Storage::parse_volume_id($tpm->{file}, 1);
-		if ($storeid) {
-		    PVE::Storage::unmap_volume($storecfg, $tpm->{file});
-		}
-	    }
 	}
 
 	foreach my $ext (qw(mon qmp pid vnc qga)) {
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 05/10] vm_stop_cleanup: add noerr parameter

[Dominik Csapak]

and set it on all current users

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/CLI/qm.pm     |  2 +-
 PVE/QemuServer.pm | 13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index b105830f..fbc590f5 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -933,7 +933,7 @@ __PACKAGE__->register_method({
 
 	    if (!$clean || $guest) {
 		# vm was shutdown from inside the guest or crashed, doing api cleanup
-		PVE::QemuServer::vm_stop_cleanup($storecfg, $vmid, $conf, 0, 0);
+		PVE::QemuServer::vm_stop_cleanup($storecfg, $vmid, $conf, 0, 0, 1);
 	    }
 	    PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-stop');
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8a71e8aa..771bef82 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6158,7 +6158,7 @@ sub cleanup_pci_devices {
 }
 
 sub vm_stop_cleanup {
-    my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes) = @_;
+    my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes, $noerr) = @_;
 
     eval {
 
@@ -6184,7 +6184,10 @@ sub vm_stop_cleanup {
 
 	vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
     };
-    warn $@ if $@; # avoid errors - just warn
+    if (my $err = $@) {
+	die $err if !$noerr;
+	warn $err;
+    }
 }
 
 # call only in locked context
@@ -6235,7 +6238,7 @@ sub _do_vm_stop {
 		die "VM quit/powerdown failed - got timeout\n";
 	    }
 	} else {
-	    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
+	    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1, 1) if $conf;
 	    return;
 	}
     } else {
@@ -6266,7 +6269,7 @@ sub _do_vm_stop {
 	sleep 1;
     }
 
-    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
+    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1, 1) if $conf;
 }
 
 # Note: use $nocheck to skip tests if VM configuration file exists.
@@ -6281,7 +6284,7 @@ sub vm_stop {
 	my $pid = check_running($vmid, $nocheck, $migratedfrom);
 	kill 15, $pid if $pid;
 	my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
-	vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 0);
+	vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 0, 1);
 	return;
     }
 
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 06/10] migrate: call vm_stop_cleanup after stopping in phase3_cleanup

[Dominik Csapak]

we currently only call deactivate_volumes, but we actually want to call
the whole vm_stop_cleanup, since that is not invoked by the vm_stop
above (we cannot parse the config anymore) and might do other cleanups
we also want to do (like mdev cleanup).

For this to work properly we have to clone the original config at the
beginning, since we might modify the volids.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuMigrate.pm           | 12 ++++++------
 test/MigrationTest/Shared.pm |  3 +++
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 8d9b35ae..381022f5 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -5,6 +5,7 @@ use warnings;
 
 use IO::File;
 use IPC::Open2;
+use Storable qw(dclone);
 use Time::HiRes qw( usleep );
 
 use PVE::Cluster;
@@ -1455,7 +1456,8 @@ sub phase3_cleanup {
 
     my $tunnel = $self->{tunnel};
 
-    my $sourcevollist = PVE::QemuServer::get_vm_volumes($conf);
+    # we'll need an unmodified copy of the config later for the cleanup
+    my $oldconf = dclone($conf);
 
     if ($self->{volume_map} && !$self->{opts}->{remote}) {
 	my $target_drives = $self->{target_drive};
@@ -1586,12 +1588,10 @@ sub phase3_cleanup {
 	$self->{errors} = 1;
     }
 
-    # always deactivate volumes - avoid lvm LVs to be active on several nodes
-    eval {
-	PVE::Storage::deactivate_volumes($self->{storecfg}, $sourcevollist);
-    };
+    # stop with nocheck does not do a cleanup, so do it here with the original config
+    eval { PVE::QemuServer::vm_stop_cleanup($self->{storecfg}, $vmid, $oldconf) };
     if (my $err = $@) {
-	$self->log('err', $err);
+	$self->log('err', "cleanup for vm failed - $err");
 	$self->{errors} = 1;
     }
 
diff --git a/test/MigrationTest/Shared.pm b/test/MigrationTest/Shared.pm
index aa7203d1..2347e60a 100644
--- a/test/MigrationTest/Shared.pm
+++ b/test/MigrationTest/Shared.pm
@@ -130,6 +130,9 @@ $qemu_server_module->mock(
     clear_reboot_request => sub {
 	return 1;
     },
+    vm_stop_cleanup => sub {
+	return 1;
+    },
     get_efivars_size => sub {
 	 return 128 * 1024;
     },
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 07/10] 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.

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

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 6ba43ee8..df2ea3eb 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -435,8 +435,11 @@ sub parse_hostpci {
 	my $devices = PVE::Mapping::PCI::get_node_mapping($config, $mapping, $node);
 	die "PCI device mapping not found for '$mapping'\n" if !$devices || !scalar($devices->@*);
 
+	my $cfg = $config->{ids}->{$mapping};
+	$res->{'live-migration-capable'} = 1 if $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, $cfg) };
 	    die "PCI device mapping invalid (hardware probably changed): $@\n" if $@;
 	    push $alternatives->@*, [split(/;/, $device->{path})];
 	}
@@ -635,6 +638,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.2

[pve-devel] [PATCH qemu-server v2 08/10] 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  | 17 ++++++++++-------
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 497987ff..26ede5d1 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4516,7 +4516,7 @@ __PACKAGE__->register_method({
 	$res->{local_disks} = [ values %$local_disks ];;
 
 	$res->{local_resources} = $local_resources;
-	$res->{'mapped-resources'} = $mapped_resources;
+	$res->{'mapped-resources'} = [ keys $mapped_resources->%* ];
 
 	return $res;
 
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 381022f5..a46eb2a3 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -233,7 +233,7 @@ sub prepare {
     my ($loc_res, $mapped_res, $missing_mappings_by_node) = PVE::QemuServer::check_local_resources($conf, 1);
     my $blocking_resources = [];
     for my $res ($loc_res->@*) {
-	if (!grep($res, $mapped_res->@*)) {
+	if (!defined($mapped_res->{$res})) {
 	    push $blocking_resources->@*, $res;
 	}
     }
@@ -245,10 +245,11 @@ sub prepare {
 	}
     }
 
-    if (scalar($mapped_res->@*)) {
+    if (scalar(keys $mapped_res->%*)) {
 	my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
+	my $mapped_text = join(", ", keys $mapped_res->%*);
 	if ($running) {
-	    die "can't migrate running VM which uses mapped devices: " . join(", ", $mapped_res->@*) . "\n";
+	    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 771bef82..91ad61a5 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2566,7 +2566,7 @@ sub check_local_resources {
     my ($conf, $noerr) = @_;
 
     my @loc_res = ();
-    my $mapped_res = [];
+    my $mapped_res = {};
 
     my $nodelist = PVE::Cluster::get_nodelist();
     my $pci_map = PVE::Mapping::PCI::config();
@@ -2598,16 +2598,19 @@ 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_device->{'live-migration'} = 1
+		    if $pci_map->{ids}->{$name}->{'live-migration-capable'};
+		$mapped_res->{$k} = $mapped_device;
 	    }
 	}
 	# sockets are safe: they will recreated be on the target side post-migrate
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 09/10] 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)

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

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 26ede5d1..286355bd 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4451,7 +4451,11 @@ __PACKAGE__->register_method({
 	    },
 	    'mapped-resources' => {
 		type => 'array',
-		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.",
 	    },
 	},
     },
@@ -4517,6 +4521,7 @@ __PACKAGE__->register_method({
 
 	$res->{local_resources} = $local_resources;
 	$res->{'mapped-resources'} = [ keys $mapped_resources->%* ];
+	$res->{'mapped-resource-info'} = $mapped_resources;
 
 	return $res;
 
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index a46eb2a3..89caefc7 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -247,11 +247,16 @@ sub prepare {
 
     if (scalar(keys $mapped_res->%*)) {
 	my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
-	my $mapped_text = join(", ", keys $mapped_res->%*);
-	if ($running) {
-	    die "can't migrate running VM which uses mapped devices: $mapped_text\n";
-	} elsif (scalar($missing_mappings->@*)) {
+	my $missing_live_mappings = [];
+	for my $key (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->@*)) {
 	    die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
+	} elsif ($running && scalar($missing_live_mappings->@*)) {
+	    die "can't live migrate running VM which uses following mapped devices: " . join(", ", $missing_live_mappings->@*) . "\n";
 	} else {
 	    $self->log('info', "migrating VM which uses mapped local devices");
 	}
-- 
2.39.2

[pve-devel] [PATCH qemu-server v2 10/10] 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 allowed_nodes too.

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

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

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 286355bd..5372ba75 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4428,18 +4428,20 @@ __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 => { type => 'boolean' },
 	    allowed_nodes => {
 		type => 'array',
 		optional => 1,
-		description => "List nodes allowed for offline migration, only passed if VM is offline"
+		description => "List nodes allowed for offline migration.",
 	    },
 	    not_allowed_nodes => {
 		type => 'object',
 		optional => 1,
-		description => "List not allowed nodes with additional informations, only passed if VM is offline"
+		description => "List not allowed nodes with additional information.",
 	    },
 	    local_disks => {
 		type => 'array',
@@ -4496,28 +4498,31 @@ __PACKAGE__->register_method({
 
 	# if vm is not running, return target nodes where local storage/mapped devices are available
 	# for offline migration
+	$res->{allowed_nodes} = [];
+	$res->{not_allowed_nodes} = {};
 	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->{not_allowed_nodes} = PVE::QemuServer::check_local_storage_availability($vmconf, $storecfg);
+	    delete $res->{not_allowed_nodes}->{$localnode};
+	}
 
-		if (!defined($checked_nodes->{$node}->{unavailable_storages})) {
-		    push @{$res->{allowed_nodes}}, $node;
-		}
+	my $merged = { $res->{not_allowed_nodes}->%*, $missing_mappings_by_node->%* };
 
+	for my $node (keys $merged->%*) {
+	    my $missing_mappings = $missing_mappings_by_node->{$node};
+	    if (scalar($missing_mappings->@*)) {
+		$res->{not_allowed_nodes}->{$node}->{'unavailable-resources'} = $missing_mappings;
+		next;
+	    }
+
+	    if (!$res->{running}) {
+		if (!defined($res->{not_allowed_nodes}->{$node}->{unavailable_storages})) {
+		    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'} = [ keys $mapped_resources->%* ];
-- 
2.39.2

[pve-devel] [PATCH manager v2 1/5] 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

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Cluster/Mapping/PCI.pm | 2 +-
 www/manager6/dc/PCIMapView.js   | 5 +++++
 2 files changed, 6 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..777e0247 100644
--- a/www/manager6/dc/PCIMapView.js
+++ b/www/manager6/dc/PCIMapView.js
@@ -20,7 +20,11 @@ Ext.define('PVE.dc.PCIMapView', {
 	data.forEach((entry) => {
 	    ids[entry.id] = entry;
 	});
+	let mdev;
 	me.getRootNode()?.cascade(function(rec) {
+	    if (rec.data.type === 'entry') {
+		mdev = rec.data.mdev;
+	    }
 	    if (rec.data.node !== node || rec.data.type !== 'map') {
 		return;
 	    }
@@ -44,6 +48,7 @@ Ext.define('PVE.dc.PCIMapView', {
 	    let toCheck = {
 		id: deviceId,
 		'subsystem-id': subId,
+		mdev,
 		iommugroup: device.iommugroup !== -1 ? device.iommugroup : undefined,
 	    };
 
-- 
2.39.2

[pve-devel] [PATCH manager v2 2/5] 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>
---
 PVE/API2/Nodes.pm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 3bc17534..535facdf 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -2254,11 +2254,23 @@ my $create_migrate_worker = sub {
 	    $invalidConditions .= join(', ', map { $_->{volid} } @{$preconditions->{local_disks}});
 	}
 
-	if (@{$preconditions->{local_resources}}) {
+	if ($online && scalar($preconditions->{local_resources}->@*)) {
 	    $invalidConditions .= "\n  Has local resources: ";
 	    $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.2

[pve-devel] [PATCH manager v2 3/5] bulk migrate: include checks for live-migratable local resources

[Dominik Csapak]

those should be able to migrate even for online vms. If the mapping does
not exist on the target node, that will be caught further down anyway.

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

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 535facdf..56c6a4c4 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -2254,9 +2254,18 @@ my $create_migrate_worker = sub {
 	    $invalidConditions .= join(', ', map { $_->{volid} } @{$preconditions->{local_disks}});
 	}
 
+	# for a live migration all local_resources must be marked as live-migratable
 	if ($online && scalar($preconditions->{local_resources}->@*)) {
-	    $invalidConditions .= "\n  Has local resources: ";
-	    $invalidConditions .= join(', ', @{$preconditions->{local_resources}});
+	    my $resource_not_live = [];
+	    for my $resource ($preconditions->{local_resources}->@*) {
+		next if $preconditions->{'mapped-resource-info'}->{$resource}->{'live-migration'};
+		push $resource_not_live->@*, $resource;
+	    }
+
+	    if (scalar($resource_not_live->@*)) {
+		$invalidConditions .= "\n  Has local resources not marked as live migratable: ";
+		$invalidConditions .= join(', ', $resource_not_live->@*);
+	    }
 	}
 
 	if (my $not_allowed_nodes = $preconditions->{not_allowed_nodes}) {
-- 
2.39.2

[pve-devel] [PATCH manager v2 4/5] 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.

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 5473821b..c553a1ee 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 seleceted 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.2

[pve-devel] [PATCH manager v2 5/5] 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

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/Migrate.js    | 25 ++++++++++++++++++++-----
 www/manager6/window/PCIMapEdit.js | 12 ++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index c553a1ee..272ab022 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 (mappedResources && 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 d43f04eb..731269a0 100644
--- a/www/manager6/window/PCIMapEdit.js
+++ b/www/manager6/window/PCIMapEdit.js
@@ -242,6 +242,18 @@ Ext.define('PVE.window.PCIMapEditWindow', {
 			disabled: '{hideComment}',
 		    },
 		},
+		{
+		    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: '{hideComment}',
+		    },
+		},
 	    ],
 
 	    columnB: [
-- 
2.39.2

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

[Dominik Csapak]

in a new section about additional options

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 qm.adoc | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 1170dd1..c146ce9 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -1734,6 +1734,19 @@ 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 create on the first one where
+  there are any available instances of the selected type.
+
+
 Managing Virtual Machines with `qm`
 ------------------------------------
 
-- 
2.39.2

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

[Dominik Csapak]

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 c146ce9..c77cb7b 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -1746,6 +1746,12 @@ Currently there are the following options:
   the mapping, the mediated device will be create on the first one where
   there are any available instances of the selected type.
 
+* `live-migration-capable` (PCI): This marks the 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.2