public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fabian Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v2 manager 2/2] pvesr: prepare local job: remove stale replicated volumes immediately
Date: Mon, 13 Jun 2022 12:29:55 +0200	[thread overview]
Message-ID: <20220613102959.36556-3-f.ebner@proxmox.com> (raw)
In-Reply-To: <20220613102959.36556-1-f.ebner@proxmox.com>

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@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





  parent reply	other threads:[~2022-06-13 10:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-13 10:29 [pve-devel] [PATCH-SERIES v2 manager/guest-common] replication: improve removal of stale snapshots/volumes Fabian Ebner
2022-06-13 10:29 ` [pve-devel] [PATCH v2 manager 1/2] pvesr: rename last_snapshots to local_snapshots Fabian Ebner
2022-06-13 10:29 ` Fabian Ebner [this message]
2022-06-13 10:29 ` [pve-devel] [PATCH v2 guest-common 1/4] replication: prepare: adapt/expand function comment Fabian Ebner
2022-06-13 10:29 ` [pve-devel] [PATCH v2 guest-common 2/4] replication: rename last_snapshots to local_snapshots Fabian Ebner
2022-06-13 10:29 ` [pve-devel] [PATCH v2 guest-common 3/4] replication: also consider storages from replication state upon removal Fabian Ebner
2022-06-13 10:29 ` [pve-devel] [RFC v2 guest-common 4/4] replication: prepare: safeguard against removal if expected snapshot is missing Fabian Ebner
2022-07-27 11:22 ` [pve-devel] [PATCH-SERIES v2 manager/guest-common] replication: improve removal of stale snapshots/volumes Fiona Ebner
2022-08-02  9:08   ` [pve-devel] applied-series: " Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220613102959.36556-3-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal