[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 at proxmox.com>
> Reviewed-by: Fiona Ebner <f.ebner at proxmox.com>
> Signed-off-by: Dominik Csapak <d.csapak at 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");
>  	}