all lists on 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 v3 storage 2/3] zfspool: add blockers parameter to volume_snapshot_is_possible
Date: Thu, 12 Aug 2021 13:01:01 +0200	[thread overview]
Message-ID: <20210812110111.73883-3-f.ebner@proxmox.com> (raw)
In-Reply-To: <20210812110111.73883-1-f.ebner@proxmox.com>

useful for rollback, so that only the required replication snapshots
can be removed, and it's possible to abort early without deleting any
replication snapshots if there are other non-replication snasphots
blocking rollback.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Storage.pm               |  4 ++--
 PVE/Storage/BTRFSPlugin.pm   |  2 +-
 PVE/Storage/Plugin.pm        |  5 ++++-
 PVE/Storage/ZFSPoolPlugin.pm | 23 +++++++++++++++++------
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index c04b5a2..3b3ce93 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -269,13 +269,13 @@ sub volume_resize {
 }
 
 sub volume_rollback_is_possible {
-    my ($cfg, $volid, $snap) = @_;
+    my ($cfg, $volid, $snap, $blockers) = @_;
 
     my ($storeid, $volname) = parse_volume_id($volid, 1);
     if ($storeid) {
         my $scfg = storage_config($cfg, $storeid);
         my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
-        return $plugin->volume_rollback_is_possible($scfg, $storeid, $volname, $snap);
+        return $plugin->volume_rollback_is_possible($scfg, $storeid, $volname, $snap, $blockers);
     } elsif ($volid =~ m|^(/.+)$| && -e $volid) {
         die "snapshot rollback file/device '$volid' is not possible\n";
     } else {
diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
index 411cab9..60fb027 100644
--- a/PVE/Storage/BTRFSPlugin.pm
+++ b/PVE/Storage/BTRFSPlugin.pm
@@ -513,7 +513,7 @@ sub volume_snapshot {
 }
 
 sub volume_rollback_is_possible {
-    my ($class, $scfg, $storeid, $volname, $snap) = @_; 
+    my ($class, $scfg, $storeid, $volname, $snap, $blockers) = @_;
 
     return 1; 
 }
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index b1865cb..c265e53 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -900,8 +900,11 @@ sub volume_snapshot {
     return undef;
 }
 
+# Asserts that a rollback to $snap on $volname is possible.
+# If certain snapshots are preventing the rollback and $blockers is an array
+# reference, the snapshot names can be pushed onto $blockers prior to dying.
 sub volume_rollback_is_possible {
-    my ($class, $scfg, $storeid, $volname, $snap) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap, $blockers) = @_;
 
     return 1;
 }
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index c2a0385..8d79f38 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -480,18 +480,29 @@ sub volume_snapshot_rollback {
 }
 
 sub volume_rollback_is_possible {
-    my ($class, $scfg, $storeid, $volname, $snap) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap, $blockers) = @_;
 
     # can't use '-S creation', because zfs list won't reverse the order when the
     # creation time is the same second, breaking at least our tests.
     my $snapshots = $class->zfs_get_sorted_snapshot_list($scfg, $volname, ['-s', 'creation']);
-    my $recentsnap = $snapshots->[-1];
 
-    die "can't rollback, no snapshots exist at all\n"
-	if !defined($recentsnap);
+    my $found;
+    $blockers //= []; # not guaranteed to be set by caller
+    for my $snapshot ($snapshots->@*) {
+	if ($snapshot eq $snap) {
+	    $found = 1;
+	} elsif ($found) {
+	    push $blockers->@*, $snapshot;
+	}
+    }
+
+    my $volid = "${storeid}:${volname}";
+
+    die "can't rollback, snapshot '$snap' does not exist on '$volid'\n"
+	if !$found;
 
-    die "can't rollback, '$snap' is not most recent snapshot\n"
-	if $snap ne $recentsnap;
+    die "can't rollback, '$snap' is not most recent snapshot on '$volid'\n"
+	if scalar($blockers->@*) > 0;
 
     return 1;
 }
-- 
2.30.2





  parent reply	other threads:[~2021-08-12 11:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12 11:00 [pve-devel] [PATCH-SERIES v3] fix #3111: fix interaction of snapshot operations with replication Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 storage 1/3] zfspool: add zfs_get_sorted_snapshot_list helper Fabian Ebner
2021-08-12 11:01 ` Fabian Ebner [this message]
2021-08-13  6:54   ` [pve-devel] [PATCH v3 storage 2/3] zfspool: add blockers parameter to volume_snapshot_is_possible Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 storage 3/3] test: zfspool: extend some rollback is possible tests with new blockers parameter Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 container 1/1] config: rollback is possible: add " Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 qemu-server " Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 guest-common 1/7] partially fix #3111: snapshot rollback: improve removing replication snapshots Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 guest-common 2/7] config: rollback: factor out helper for " Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 guest-common 3/7] partially fix #3111: further improve " Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 guest-common 4/7] replication: remove unused variable and style fixes Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 guest-common 5/7] replication: pass guest config to find_common_replication_snapshot Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [PATCH v3 guest-common 6/7] partially fix #3111: replication: be less picky when selecting incremental base Fabian Ebner
2021-08-12 11:01 ` [pve-devel] [RFC v3 guest-common 7/7] fix #3111: config: snapshot delete: check if replication still needs it Fabian Ebner
2021-10-06  6:59 ` [pve-devel] [PATCH-SERIES v3] fix #3111: fix interaction of snapshot operations with replication Fabian Ebner
2021-11-09 16:49 ` [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=20210812110111.73883-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal