[pve-devel] [PATCH qemu-server] migrate: fix replication false-positives

Fabian Grünbichler f.gruenbichler at proxmox.com
Thu Mar 26 10:10:01 CET 2020


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

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

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---

Notes:
    this effectively makes qemu-server 6.1-10 and 6.1-11 very broken qemu-server versions

 PVE/API2/Qemu.pm   |  5 ++++-
 PVE/QemuMigrate.pm | 12 ++++++++----
 PVE/QemuServer.pm  |  6 ++----
 3 files changed, 14 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..ef5f6fd 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,9 @@ 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";
+    }
 
     # Note: We try to keep $spice_ticket secret (do not pass via command line parameter)
     # instead we pipe it through STDIN
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a3e3269..f077915 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,14 +4755,12 @@ 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}};
 		    next;
-- 
2.20.1





More information about the pve-devel mailing list