From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id DCBE36CF97 for ; Thu, 12 Aug 2021 13:01:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9BFA42218D for ; Thu, 12 Aug 2021 13:01:34 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id A982E21FF7 for ; Thu, 12 Aug 2021 13:01:24 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 931B243318 for ; Thu, 12 Aug 2021 13:01:18 +0200 (CEST) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Thu, 12 Aug 2021 13:01:07 +0200 Message-Id: <20210812110111.73883-9-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210812110111.73883-1-f.ebner@proxmox.com> References: <20210812110111.73883-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.407 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH v3 guest-common 3/7] partially fix #3111: further improve removing replication snapshots X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Aug 2021 11:01:34 -0000 by using the new $blocker parameter. No longer remove all replication snapshots from affected volumes unconditionally, but check first if all blocking snapshots are replication snapshots. If they are, remove them and proceed with rollback. If they are not, die without removing any. For backwards compatibility, it's still necessary to remove all replication snapshots if $blockers is not available. Signed-off-by: Fabian Ebner --- src/PVE/AbstractConfig.pm | 47 +++++++++++++++++++++++++++++++++++---- src/PVE/Replication.pm | 6 +++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm index 0d1c7ca..a5a15bf 100644 --- a/src/PVE/AbstractConfig.pm +++ b/src/PVE/AbstractConfig.pm @@ -945,7 +945,7 @@ sub snapshot_delete { # Remove replication snapshots to make a rollback possible. my $rollback_remove_replication_snapshots = sub { - my ($class, $vmid, $snap) = @_; + my ($class, $vmid, $snap, $snapname) = @_; my $storecfg = PVE::Storage::config(); @@ -954,17 +954,56 @@ my $rollback_remove_replication_snapshots = sub { my $volumes = $class->get_replicatable_volumes($storecfg, $vmid, $snap, 1); - # filter by what we actually iterate over below (excludes vmstate!) + # For these, all replication snapshots need to be removed for backwards compatibility. my $volids = []; + + # For these, we know more and can remove only the required replication snapshots. + my $blocking_snapshots = {}; + + # filter by what we actually iterate over below (excludes vmstate!) $class->foreach_volume($snap, sub { my ($vs, $volume) = @_; my $volid_key = $class->volid_key(); my $volid = $volume->{$volid_key}; - push @{$volids}, $volid if $volumes->{$volid}; + return if !$volumes->{$volid}; + + my $blockers = []; + eval { $class->__snapshot_rollback_vol_possible($volume, $snapname, $blockers); }; + if (my $err = $@) { + # FIXME die instead, once $blockers is required by the storage plugin API + # and the guest plugins are required to be new enough to support it too. + # Currently, it's not possible to distinguish between blockers being empty + # because the plugin is old versus because there is a different error. + if (scalar($blockers->@*) == 0) { + push @{$volids}, $volid; + return; + } + + for my $blocker ($blockers->@*) { + die $err if !PVE::Replication::is_replication_snapshot($blocker); + } + + $blocking_snapshots->{$volid} = $blockers; + } }); + my $removed_repl_snapshot; + for my $volid (sort keys $blocking_snapshots->%*) { + my $blockers = $blocking_snapshots->{$volid}; + + for my $blocker ($blockers->@*) { + warn "WARN: removing replication snapshot '$volid\@$blocker'\n"; + $removed_repl_snapshot = 1; + eval { PVE::Storage::volume_snapshot_delete($storecfg, $volid, $blocker); }; + die $@ if $@; + } + } + warn "WARN: you shouldn't remove '$snapname' before running the next replication!\n" + if $removed_repl_snapshot; + + # Need to keep using a hammer for backwards compatibility... # remove all local replication snapshots (jobid => undef) my $logfunc = sub { my $line = shift; chomp $line; print "$line\n"; }; PVE::Replication::prepare($storecfg, $volids, undef, 1, undef, $logfunc); @@ -997,7 +1036,7 @@ sub snapshot_rollback { $snap = $get_snapshot_config->($conf); if ($prepare) { - $rollback_remove_replication_snapshots->($class, $vmid, $snap); + $rollback_remove_replication_snapshots->($class, $vmid, $snap, $snapname); $class->foreach_volume($snap, sub { my ($vs, $volume) = @_; diff --git a/src/PVE/Replication.pm b/src/PVE/Replication.pm index ae0f145..3da79be 100644 --- a/src/PVE/Replication.pm +++ b/src/PVE/Replication.pm @@ -435,4 +435,10 @@ sub run_replication { return $volumes; } +sub is_replication_snapshot { + my ($snapshot_name) = @_; + + return $snapshot_name =~ m/^__replicate_/ ? 1 : 0; +} + 1; -- 2.30.2