[pve-devel] [RFC manager 2/2] pvesr: prepare_local_job: use PVE::replication::prepare

Fabian Grünbichler f.gruenbichler at proxmox.com
Thu Jun 8 10:17:45 CEST 2017


to reduce code duplication. this slightly changes behaviour
compared to the previous version:

only disks with the correct prefix are cleaned up, not all
disks with __replication* snapshots.

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
IMHO, the behaviour change is okay - if there are disks with a different prefix,
they either were left intentionally (and thus should not be deleted) or a "full
remove" job is still pending, which will clean them up.

OTOH, a prefix parameter could be introduced to prepare - but then we should
probably restructure it to have mandatory and optional parameters, as the list
is already quite long and unwieldy..

 PVE/CLI/pvesr.pm | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/PVE/CLI/pvesr.pm b/PVE/CLI/pvesr.pm
index 4c76b416..f0170b01 100644
--- a/PVE/CLI/pvesr.pm
+++ b/PVE/CLI/pvesr.pm
@@ -129,30 +129,16 @@ __PACKAGE__->register_method ({
 	my $snapname = PVE::ReplicationState::replication_snapshot_name($jobid, $last_sync);
 
 	# find replication snapshots
-	my $last_snapshots = {};
 	foreach my $storeid (@$storage_list) {
 	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
 	    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
 	    my $volids = $plugin->list_images($storeid, $scfg, $vmid, undef, $cache);
-	    foreach my $volid (@$volids) {
-		my ($storeid, $volname) = parse_volume_id($volid);
-		my $list = $plugin->volume_snapshot_list($scfg, $storeid, $volname); # fixme: pass $cache
-		my $found_replication_snapshots = 0;
-		foreach my $snap (@$list) {
-		    if ($snap eq $snapname || (defined($parent_snapname) && ($snap eq $parent_snapname))) {
-			$last_snapshots->{$volid}->{$snap} = 1 if $wanted_volids->{$volid};
-		    } elsif ($snap =~ m/^__replication_/) {
-			$found_replication_snapshots = 1;
-			if ($wanted_volids->{$volid}) {
-			    $logfunc->("$jobid: delete stale replication snapshot '$snap' on $volid");
-			    PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snap);
-			}
-		    }
-		}
-		# remove stale volumes
-		if ($found_replication_snapshots && !$wanted_volids->{$volid}) {
+	    my ($last_snapshots, $cleaned_replicated_volumes) = PVE::Replication::prepare($storecfg, $volids, $jobid, $last_sync, $parent_snapname, $logfunc);
+	    foreach my $volid (keys %$cleaned_replicated_volumes) {
+		if (!$wanted_volids->{$volid}) {
 		    $logfunc->("$jobid: delete stale volume '$volid'");
 		    PVE::Storage::vdisk_free($storecfg, $volid);
+		    delete $last_snapshots->{$volid};
 		}
 	    }
 	}
-- 
2.11.0





More information about the pve-devel mailing list