[pve-devel] [PATCH qemu-server] fix #3861: migrate: fix live migration when cloud-init changes storage

[pve-devel] [PATCH qemu-server] fix #3861: migrate: fix live migration when cloud-init changes storage

[Fabian Ebner]

Generalizes fd95d780 ("migrate: send updated TPM state volid to target
node") to also handle other offline migrated disks appearing in the
VM config, which currently should only be cloud-init.

Breaks migration new -> old under similar (edge-case-)conditions as
fd95d780 did.

Keep sending the 'tpmstate0' STDIN parameter to avoid breaking new ->
old in the scenario fd95d780 fixed.

Keep parsing the vm_start 'tpmstate0' STDIN parameter to avoid
breaking old -> new, and to be able to keep sending it.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm   | 10 ++++++----
 PVE/QemuMigrate.pm | 19 ++++++++++++++-----
 PVE/QemuServer.pm  | 15 +++++++++------
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 54955ee2..1d09cab3 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2542,7 +2542,7 @@ __PACKAGE__->register_method({
 	my $spice_ticket;
 	my $nbd_protocol_version = 0;
 	my $replicated_volumes = {};
-	my $tpmstate_vol;
+	my $offline_volumes = {};
 	if ($stateuri && ($stateuri eq 'tcp' || $stateuri eq 'unix') && $migratedfrom && ($rpcenv->{type} eq 'cli')) {
 	    while (defined(my $line = <STDIN>)) {
 		chomp $line;
@@ -2552,8 +2552,10 @@ __PACKAGE__->register_method({
 		    $nbd_protocol_version = $1;
 		} elsif ($line =~ m/^replicated_volume: (.*)$/) {
 		    $replicated_volumes->{$1} = 1;
-		} elsif ($line =~ m/^tpmstate0: (.*)$/) {
-		    $tpmstate_vol = $1;
+		} elsif ($line =~ m/^tpmstate0: (.*)$/) { # Deprecated, use offline_volume instead
+		    $offline_volumes->{tpmstate0} = $1;
+		} elsif ($line =~ m/^offline_volume: ([^:]+): (.*)$/) {
+		    $offline_volumes->{$1} = $2;
 		} elsif (!$spice_ticket) {
 		    # fallback for old source node
 		    $spice_ticket = $line;
@@ -2595,7 +2597,7 @@ __PACKAGE__->register_method({
 		    storagemap => $storagemap,
 		    nbd_proto_version => $nbd_protocol_version,
 		    replicated_volumes => $replicated_volumes,
-		    tpmstate_vol => $tpmstate_vol,
+		    offline_volumes => $offline_volumes,
 		};
 
 		my $params = {
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index dfe92325..d52dc8db 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -710,11 +710,20 @@ sub phase2 {
     my $nbd_protocol_version = 1;
     my $input = "nbd_protocol_version: $nbd_protocol_version\n";
 
-    if ($conf->{tpmstate0}) {
-	my $tpmdrive = PVE::QemuServer::parse_drive('tpmstate0', $conf->{tpmstate0});
-	my $tpmvol = $tpmdrive->{file};
-	$input .= "tpmstate0: $self->{volume_map}->{$tpmvol}"
-	    if $self->{volume_map}->{$tpmvol} && $tpmvol ne $self->{volume_map}->{$tpmvol};
+    my @offline_local_volumes = $self->filter_local_volumes('offline');
+    for my $volid (@offline_local_volumes) {
+	my $drivename = $local_volumes->{$volid}->{drivename};
+	next if !$drivename || !$conf->{$drivename};
+
+	my $new_volid = $self->{volume_map}->{$volid};
+	next if !$new_volid || $volid eq $new_volid;
+
+	# FIXME PVE 8.x only use offline_volume variant once all targets can handle it
+	if ($drivename eq 'tpmstate0') {
+	    $input .= "$drivename: $new_volid\n"
+	} else {
+	    $input .= "offline_volume: $drivename: $new_volid\n"
+	}
     }
 
     $input .= "spice_ticket: $spice_ticket\n" if $spice_ticket;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 7f983904..629b64fe 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5432,7 +5432,8 @@ sub vm_start {
 #   type => secure/insecure - tunnel over encrypted connection or plain-text
 #   nbd_proto_version => int, 0 for TCP, 1 for UNIX
 #   replicated_volumes => which volids should be re-used with bitmaps for nbd migration
-#   tpmstate_vol => new volid of tpmstate0, not yet contained in config
+#   offline_volumes => new volids of offline migrated disks like tpmstate and cloudinit, not yet
+#       contained in config
 sub vm_start_nolock {
     my ($storecfg, $vmid, $conf, $params, $migrate_opts) = @_;
 
@@ -5457,11 +5458,13 @@ sub vm_start_nolock {
     # this way we can reuse the old ISO with the correct config
     PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid) if !$migratedfrom;
 
-    # override TPM state vol if migrated, conf is out of date still
-    if (my $tpmvol = $migrate_opts->{tpmstate_vol}) {
-        my $parsed = parse_drive("tpmstate0", $conf->{tpmstate0});
-        $parsed->{file} = $tpmvol;
-        $conf->{tpmstate0} = print_drive($parsed);
+    # override offline migrated volumes, conf is out of date still
+    if (my $offline_volumes = $migrate_opts->{offline_volumes}) {
+	for my $key (sort keys $offline_volumes->%*) {
+	    my $parsed = parse_drive($key, $conf->{$key});
+	    $parsed->{file} = $offline_volumes->{$key};
+	    $conf->{$key} = print_drive($parsed);
+	}
     }
 
     my $defaults = load_defaults();
-- 
2.30.2

[pve-devel] applied: [PATCH qemu-server] fix #3861: migrate: fix live migration when cloud-init changes storage

[Thomas Lamprecht]

On 28.04.22 13:37, Fabian Ebner wrote:
> Generalizes fd95d780 ("migrate: send updated TPM state volid to target
> node") to also handle other offline migrated disks appearing in the
> VM config, which currently should only be cloud-init.
> 
> Breaks migration new -> old under similar (edge-case-)conditions as
> fd95d780 did.
> 
> Keep sending the 'tpmstate0' STDIN parameter to avoid breaking new ->
> old in the scenario fd95d780 fixed.
> 
> Keep parsing the vm_start 'tpmstate0' STDIN parameter to avoid
> breaking old -> new, and to be able to keep sending it.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  PVE/API2/Qemu.pm   | 10 ++++++----
>  PVE/QemuMigrate.pm | 19 ++++++++++++++-----
>  PVE/QemuServer.pm  | 15 +++++++++------
>  3 files changed, 29 insertions(+), 15 deletions(-)
> 
>

applied, thanks!