[pve-devel] [PATCH qemu-server 3/4] check_local_resources: add more info per mapped device
Fiona Ebner
f.ebner at proxmox.com
Fri Mar 22 14:37:27 CET 2024
Am 18.03.24 um 12:18 schrieb Dominik Csapak:
> such as the mapping name and if it's marked for live-migration (pci only)
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> PVE/API2/Qemu.pm | 2 +-
> PVE/QemuMigrate.pm | 5 +++--
> PVE/QemuServer.pm | 10 ++++++----
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 497987ff..4ecaeb91 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'} = [ map { "$_->{key}" } $mapped_resources->@* ];
Or should it become a hash? Then you could use 'keys' instead of map and
a 'key' property.
>
> return $res;
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 8d9b35ae..6fe8157d 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -232,7 +232,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->@*)) {
Seems like this is currently broken. I.e. it should be $res eq $_
> + if (!grep { $_->{key} eq $res } $mapped_res->@*) {
> push $blocking_resources->@*, $res;
> }
> }
> @@ -246,8 +246,9 @@ sub prepare {
>
> if (scalar($mapped_res->@*)) {
> my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
> + my $mapped_text = join(", ", map { $_->{key} } $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";
Style nit: no need for the concatenation anymore, can just use the
variable inside the string.
> } 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 6e2c8052..ef3aee20 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2600,14 +2600,16 @@ sub check_local_resources {
> next if $entry->{host} && $entry->{host} =~ m/^spice$/i;
> if ($entry->{mapping}) {
> $add_missing_mapping->('usb', $k, $entry->{mapping});
> - push @$mapped_res, $k;
> + push @$mapped_res, { key => $k, device => $entry->{mapping} };
Should we name the second key 'name'/'mapping'/'id' instead of 'device'?
> }
> }
> 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 = { key => $k, device => $name };
> + $mapped_device->{'live-migration'} = 1 if $pci_map->{ids}->{$name}->{'live-migration-capable'};
Style nit: line too long
> + push @$mapped_res, $mapped_device;
> }
> }
> # sockets are safe: they will recreated be on the target side post-migrate
More information about the pve-devel
mailing list