[pve-devel] [PATCH qemu-server 3/3] migrate: cleanup disk/bitmaps if 'qm start' failed

[Fabian Grünbichler]

since bitmaps are set early on, and 'qm start' potentially has allocated
the disks but still failed. we can only clean up what we know about
anyway, so the disk part is still only best effort.

also use replicated_volumes instead of bitmap existence to check for
replicated volumes, since 'qm start' on an old node that does not
understand replicated volumes might have allocated a new volume that we
DO want to clean up, and not skip.

also cleanup disks after stopping target VM, otherwise we might end up
in a situation where the target VM is still running and using the disks,
thus blocking the disk cleanup.

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

Notes:
    v1->v2:
    - also cleanup remote disks that might have been allocated
    - use new tracked replicated_volumes in cleanup instead of bitmap existence

 PVE/QemuMigrate.pm | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 9ded16c..8cb9403 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -536,10 +536,11 @@ sub cleanup_remotedisks {
     my ($self) = @_;
 
     foreach my $target_drive (keys %{$self->{target_drive}}) {
+	my $drive = PVE::QemuServer::parse_drive($target_drive, $self->{target_drive}->{$target_drive}->{drivestr});
+
 	# don't clean up replicated disks!
-	next if defined($self->{target_drive}->{$target_drive}->{bitmap});
+	next if defined($self->{replicated_volumes}->{$drive->{file}});
 
-	my $drive = PVE::QemuServer::parse_drive($target_drive, $self->{target_drive}->{$target_drive}->{drivestr});
 	my ($storeid, $volname) = PVE::Storage::parse_volume_id($drive->{file});
 
 	my $cmd = [@{$self->{rem_ssh}}, 'pvesm', 'free', "$storeid:$volname"];
@@ -1012,20 +1013,15 @@ sub phase2_cleanup {
 
     # cleanup ressources on target host
     if ($self->{storage_migration}) {
-
 	eval { PVE::QemuServer::qemu_blockjobs_cancel($vmid, $self->{storage_migration_jobs}) };
 	if (my $err = $@) {
 	    $self->log('err', $err);
 	}
+    }
 
-	eval { PVE::QemuMigrate::cleanup_remotedisks($self) };
-	if (my $err = $@) {
-	    $self->log('err', $err);
-	}
-	eval { $self->cleanup_bitmaps() };
-	if (my $err =$@) {
-	    $self->log('err', $err);
-	}
+    eval { $self->cleanup_bitmaps() };
+    if (my $err =$@) {
+	$self->log('err', $err);
     }
 
     my $nodename = PVE::INotify::nodename();
@@ -1037,6 +1033,13 @@ sub phase2_cleanup {
         $self->{errors} = 1;
     }
 
+    # cleanup after stopping, otherwise disks might be in-use by target VM!
+    eval { PVE::QemuMigrate::cleanup_remotedisks($self) };
+    if (my $err = $@) {
+	$self->log('err', $err);
+    }
+
+
     if ($self->{tunnel}) {
 	eval { finish_tunnel($self, $self->{tunnel});  };
 	if (my $err = $@) {
-- 
2.20.1