by only checking for replicatable volumes when a replication job is
defined, and passing only actually replicated volumes to the target node
via STDIN, and back via STDOUT.

otherwise this can pick up theoretically replicatable, but not actually
replicated volumes and treat them wrong.

Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
---

Notes:
    v1->v2:
    - communicate re-used disks back to source node for abort in case target 
node is too old
    
    the original bug effectively makes qemu-server 6.1-10 and 6.1-11 very 
broken qemu-server versions

 PVE/API2/Qemu.pm   |  5 ++++-
 PVE/QemuMigrate.pm | 22 ++++++++++++++++++----
 PVE/QemuServer.pm  |  7 +++----
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index ef8a7c3..8a7e98c 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2073,6 +2073,7 @@ __PACKAGE__->register_method({
        # read spice ticket from STDIN
        my $spice_ticket;
        my $nbd_protocol_version = 0;
+       my $replicated_volumes = {};
        if ($stateuri && ($stateuri eq 'tcp' || $stateuri eq 'unix') && 
$migratedfrom && ($rpcenv->{type} eq 'cli')) {
            while (defined(my $line = <STDIN>)) {
                chomp $line;
@@ -2080,6 +2081,8 @@ __PACKAGE__->register_method({
                    $spice_ticket = $1;
                } elsif ($line =~ m/^nbd_protocol_version: (\d+)$/) {
                    $nbd_protocol_version = $1;
+               } elsif ($line =~ m/^replicated_volume: (.*)$/) {
+                   $replicated_volumes->{$1} = 1;
                } else {
                    # fallback for old source node
                    $spice_ticket = $line;
@@ -2113,7 +2116,7 @@ __PACKAGE__->register_method({
 
                PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri, 
$skiplock, $migratedfrom, undef, $machine,
                                          $spice_ticket, $migration_network, 
$migration_type, $targetstorage, $timeout,
-                                         $nbd_protocol_version);
+                                         $nbd_protocol_version, 
$replicated_volumes);
                return;
            };
 
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 9cff64d..b14ee01 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -330,7 +330,9 @@ sub sync_disks {
            });
        }
 
-       my $replicatable_volumes = 
PVE::QemuConfig->get_replicatable_volumes($self->{storecfg}, $self->{vmid}, 
$conf, 0, 1);
+       my $rep_cfg = PVE::ReplicationConfig->new();
+       my $replication_jobcfg = $rep_cfg->find_local_replication_job($vmid, 
$self->{node});
+       my $replicatable_volumes = $replication_jobcfg ? 
PVE::QemuConfig->get_replicatable_volumes($self->{storecfg}, $self->{vmid}, 
$conf, 0, 1) : {};
 
        my $test_volid = sub {
            my ($volid, $attr) = @_;
@@ -449,8 +451,7 @@ sub sync_disks {
            }
        }
 
-       my $rep_cfg = PVE::ReplicationConfig->new();
-       if (my $jobcfg = $rep_cfg->find_local_replication_job($vmid, 
$self->{node})) {
+       if ($replication_jobcfg) {
            if ($self->{running}) {
 
                my $version = PVE::QemuServer::kvm_user_version();
@@ -484,7 +485,7 @@ sub sync_disks {
            my $start_time = time();
            my $logfunc = sub { $self->log('info', shift) };
            $self->{replicated_volumes} = PVE::Replication::run_replication(
-              'PVE::QemuConfig', $jobcfg, $start_time, $start_time, $logfunc);
+              'PVE::QemuConfig', $replication_jobcfg, $start_time, 
$start_time, $logfunc);
        }
 
        # sizes in config have to be accurate for remote node to correctly
@@ -657,6 +658,11 @@ sub phase2 {
     # TODO change to 'spice_ticket: <ticket>\n' in 7.0
     my $input = $spice_ticket ? "$spice_ticket\n" : "\n";
     $input .= "nbd_protocol_version: $nbd_protocol_version\n";
+    foreach my $volid (keys %{$self->{replicated_volumes}}) {
+       $input .= "replicated_volume: $volid\n";
+    }
+
+    my $target_replicated_volumes = {};
 
     # Note: We try to keep $spice_ticket secret (do not pass via command line 
parameter)
     # instead we pipe it through STDIN
@@ -701,6 +707,10 @@ sub phase2 {
            $self->{target_drive}->{$targetdrive}->{nbd_uri} = $nbd_uri;
            push @$tunnel_addr, "$nbd_unix_addr:$nbd_unix_addr";
            push @$sock_addr, $nbd_unix_addr;
+       } elsif ($line =~ m/^re-using replicated volume: (\S+) - (.*)$/) {
+           my $drive = $1;
+           my $volid = $2;
+           $target_replicated_volumes->{$volid} = $drive;
        } elsif ($line =~ m/^QEMU: (.*)$/) {
            $self->log('info', "[$self->{node}] $1\n");
        }
@@ -713,6 +723,10 @@ sub phase2 {
 
     die "unable to detect remote migration address\n" if !$raddr;
 
+    if (scalar(keys %$target_replicated_volumes) != scalar(keys 
%{$self->{replicated_volumes}})) {
+       die "number of replicated disks on source and target node do not match 
- target node too old?\n"
+    }
+
     $self->log('info', "start remote tunnel");
 
     if ($migration_type eq 'secure') {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a3e3269..9c9debf 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4706,7 +4706,7 @@ sub vmconfig_update_disk {
 sub vm_start {
     my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused,
        $forcemachine, $spice_ticket, $migration_network, $migration_type,
-       $targetstorage, $timeout, $nbd_protocol_version) = @_;
+       $targetstorage, $timeout, $nbd_protocol_version, $replicated_volumes) = 
@_;
 
     PVE::QemuConfig->lock_config($vmid, sub {
        my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
@@ -4755,16 +4755,15 @@ sub vm_start {
                $local_volumes->{$ds} = [$volid, $storeid, $volname];
            });
 
-           my $replicatable_volumes = 
PVE::QemuConfig->get_replicatable_volumes($storecfg, $vmid, $conf, 0, 1);
-
            my $format = undef;
 
            foreach my $opt (sort keys %$local_volumes) {
 
                my ($volid, $storeid, $volname) = @{$local_volumes->{$opt}};
-               if ($replicatable_volumes->{$volid}) {
+               if ($replicated_volumes->{$volid}) {
                    # re-use existing, replicated volume with bitmap on source 
side
                    $local_volumes->{$opt} = $conf->{${opt}};
+                   print "re-using replicated volume: $opt - $volid\n";
                    next;
                }
                my $drive = parse_drive($opt, $conf->{$opt});
-- 
2.20.1


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

Reply via email to