On 3/22/24 14:37, Fiona Ebner wrote:
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.csa...@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.

yes that makes sense. Since we change the internal structure, we have
to adapt all call sites anyway...



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 $_

yep, seems i have to improve my tests running with that
(maybe i can whip up some automated ones to send along)


+       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'?

yeah name is probably better here...


            }
        }
        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



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

Reply via email to