From 9b3f5a5c993004598bb8a002d0cb145d75500c09 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
Date: Thu, 26 Mar 2020 15:42:59 +0100
Subject: [PATCH] migrate: cleanup disk/bitmaps if 'qm start' failed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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@proxmox.com>
---
 PVE/QemuMigrate.pm | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 9ded16cf..8cb94030 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}}) {
-	# don't clean up replicated disks!
-	next if defined($self->{target_drive}->{$target_drive}->{bitmap});
-
 	my $drive = PVE::QemuServer::parse_drive($target_drive, $self->{target_drive}->{$target_drive}->{drivestr});
+
+	# don't clean up replicated disks!
+	next if defined($self->{replicated_volumes}->{$drive->{file}});
+
 	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 = $@) {