[pve-devel] [PATCH qemu-server 09/11] Simplify removal of local volumes and get rid of self->{volumes}

[Fabian Ebner]

This changes the behavior to remove offline migrated volumes
only after the migration has finished successfully (mostly relevant
for mixed settings, e.g. online migration with unused/vmstate disks).

local_volumes contains both, the volumes previously in $self->{volumes}
and the volumes in $self->{online_local_volumes}, and hence is the place
to look for which volumes we need to remove. Of course, replicated
volumes still need to be skipped.

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

Who needs phase3 anyways ;)?

 PVE/QemuMigrate.pm | 45 ++++++++++++---------------------------------
 1 file changed, 12 insertions(+), 33 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 421b9d7..a6f42df 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -292,7 +292,6 @@ sub scan_local_volumes {
 
     # local volumes which have been copied
     # and their old_id => new_id pairs
-    $self->{volumes} = [];
     $self->{volume_map} = {};
     $self->{local_volumes} = {};
 
@@ -523,14 +522,10 @@ sub scan_local_volumes {
 	    my $ref = $local_volumes->{$volid}->{ref};
 	    if ($self->{running} && $ref eq 'config') {
 		push @{$self->{online_local_volumes}}, $volid;
-	    } elsif ($ref eq 'generated') {
-		die "can't live migrate VM with local cloudinit disk. use a shared storage instead\n" if $self->{running};
-		# skip all generated volumes but queue them for deletion in phase3_cleanup
-		push @{$self->{volumes}}, $volid;
-		next;
+	    } elsif ($self->{running} && $ref eq 'generated') {
+		die "can't live migrate VM with local cloudinit disk. use a shared storage instead\n";
 	    } else {
 		next if $self->{replicated_volumes}->{$volid};
-		push @{$self->{volumes}}, $volid;
 		$local_volumes->{$volid}->{migration_mode} = 'offline';
 	    }
 	}
@@ -1160,18 +1155,7 @@ sub phase2_cleanup {
 sub phase3 {
     my ($self, $vmid) = @_;
 
-    my $volids = $self->{volumes};
-    return if $self->{phase2errors};
-
-    # destroy local copies
-    foreach my $volid (@$volids) {
-	eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); };
-	if (my $err = $@) {
-	    $self->log('err', "removing local copy of '$volid' failed - $err");
-	    $self->{errors} = 1;
-	    last if $err =~ /^interrupted by signal$/;
-	}
-    }
+    return;
 }
 
 sub phase3_cleanup {
@@ -1303,22 +1287,17 @@ sub phase3_cleanup {
 	$self->{errors} = 1;
     }
 
-    if($self->{storage_migration}) {
-	# destroy local copies
-	my $volids = $self->{online_local_volumes};
-
-	foreach my $volid (@$volids) {
-	    # keep replicated volumes!
-	    next if $self->{replicated_volumes}->{$volid};
+    # destroy local copies
+    foreach my $volid (keys %{$self->{local_volumes}}) {
+	# keep replicated volumes!
+	next if $self->{replicated_volumes}->{$volid};
 
-	    eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); };
-	    if (my $err = $@) {
-		$self->log('err', "removing local copy of '$volid' failed - $err");
-		$self->{errors} = 1;
-		last if $err =~ /^interrupted by signal$/;
-	    }
+	eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); };
+	if (my $err = $@) {
+	    $self->log('err', "removing local copy of '$volid' failed - $err");
+	    $self->{errors} = 1;
+	    last if $err =~ /^interrupted by signal$/;
 	}
-
     }
 
     # clear migrate lock
-- 
2.20.1