[pve-devel] [PATCH v2 qemu-server 09/13] migration: cleanup_remotedisks: simplify and include more disks

[Fabian Ebner <f.ebner@proxmox.com>]

Namely, those migrated with storage_migrate by using the information from
volume_map. Call cleanup_remotedisks in phase1_cleanup as well, because that's
where we end if sync_offline_local_volumes fails, and some disks might already
have been transfered successfully. Note that the local disks are still here, so
this is fine.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * order changed, see previous patch
    * get rid of a fixme in the tests

 PVE/QemuMigrate.pm             | 22 ++++++----------------
 test/run_qemu_migrate_tests.pl |  5 +----
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 5d92028..94f3328 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -695,16 +695,11 @@ sub sync_offline_local_volumes {
 sub cleanup_remotedisks {
     my ($self) = @_;
 
-    foreach my $target_drive (keys %{$self->{target_drive}}) {
-	my $drivestr = $self->{target_drive}->{$target_drive}->{drivestr};
-	next if !defined($drivestr);
-
-	my $drive = PVE::QemuServer::parse_drive($target_drive, $drivestr);
-
+    foreach my $volid (values %{$self->{volume_map}}) {
 	# don't clean up replicated disks!
-	next if defined($self->{replicated_volumes}->{$drive->{file}});
+	next if defined($self->{replicated_volumes}->{$volid});
 
-	my ($storeid, $volname) = PVE::Storage::parse_volume_id($drive->{file});
+	my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid);
 
 	my $cmd = [@{$self->{rem_ssh}}, 'pvesm', 'free', "$storeid:$volname"];
 
@@ -759,20 +754,15 @@ sub phase1_cleanup {
 	$self->log('err', $err);
     }
 
-    my @volids = $self->filter_local_volumes('offline');
-    if (scalar(@volids)) {
-	foreach my $volid (@volids) {
-	    next if defined($self->{replicated_volumes}->{$volid});
-	    $self->log('err', "found stale volume copy '$volid' on node '$self->{node}'");
-	    # fixme: try to remove ?
-	}
+    eval { $self->cleanup_remotedisks() };
+    if (my $err = $@) {
+	$self->log('err', $err);
     }
 
     eval { $self->cleanup_bitmaps() };
     if (my $err =$@) {
 	$self->log('err', $err);
     }
-
 }
 
 sub phase2 {
diff --git a/test/run_qemu_migrate_tests.pl b/test/run_qemu_migrate_tests.pl
index 67b9d0e..4f7f021 100755
--- a/test/run_qemu_migrate_tests.pl
+++ b/test/run_qemu_migrate_tests.pl
@@ -1465,7 +1465,6 @@ my $tests = [
 	},
     },
     {
-	# FIXME also cleanup remote disks when failing this early
 	name => '149_storage_migrate_fail',
 	target => 'pve1',
 	vmid => 149,
@@ -1482,9 +1481,7 @@ my $tests = [
 	expect_die => "storage_migrate 'local-lvm:vm-149-disk-0' error",
 	expected => {
 	    source_volids => local_volids_for_vm(149),
-	    target_volids => {
-		'local-dir:149/vm-149-disk-0.qcow2' => 1,
-	    },
+	    target_volids => {},
 	    vm_config => $vm_configs->{149},
 	    vm_status => {
 		running => 0,
-- 
2.20.1