[pve-devel] [PATCH v2 manager 2/2] pvesr: prepare local job: remove stale replicated volumes immediately

Fabian Ebner f.ebner at proxmox.com
Mon Jun 13 12:29:55 CEST 2022


Commit 0433b86df6dfdf1d64ee09322719a02a91690707 introduced a
regression where only stale replicated volumes with an older
timestamp would be cleaned up. This meant that after removing a volume
from the guest config, it would only be cleaned up the second time the
replication ran afterwards. And the volume could become completely
orphaned in case the relevant storage wasn't used by the job anymore.

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

Dependency bump for libpve-guest-common >= 4.0-3 needed for
is_replication_snapshot().

Changes from v1:
    * Adapt to changed behavior of prepare() so we still only catch
      volumes that had replication snapshots belonging to the job.

 PVE/CLI/pvesr.pm | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/PVE/CLI/pvesr.pm b/PVE/CLI/pvesr.pm
index a1be88af..95dad64e 100644
--- a/PVE/CLI/pvesr.pm
+++ b/PVE/CLI/pvesr.pm
@@ -137,8 +137,18 @@ __PACKAGE__->register_method ({
 	    push @$volids, map { $_->{volid} } @$images;
 	}
 	my ($local_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}) {
+	for my $volid ($volids->@*) {
+	    next if $wanted_volids->{$volid};
+
+	    my $stale = $cleaned_replicated_volumes->{$volid};
+	    # prepare() will not remove the last_sync snapshot, but if the volume was used by the
+	    # job and is not wanted anymore, it is stale too. And not removing it now might cause
+	    # it to be missed later, because the relevant storage might not get scanned anymore.
+	    $stale ||= grep {
+		PVE::Replication::is_replication_snapshot($_, $jobid)
+	    } keys %{$local_snapshots->{$volid} // {}};
+
+	    if ($stale) {
 		$logfunc->("$jobid: delete stale volume '$volid'");
 		PVE::Storage::vdisk_free($storecfg, $volid);
 		delete $local_snapshots->{$volid};
-- 
2.30.2






More information about the pve-devel mailing list