all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v2 guest-common 1/2] partially fix 3111: snapshot rollback: improve removing replication snapshots
@ 2021-06-09  9:18 Fabian Ebner
  2021-06-09  9:18 ` [pve-devel] [PATCH v2 guest-common 2/2] fix 3111: replicate guest on rollback if there are replication jobs for it Fabian Ebner
  0 siblings, 1 reply; 4+ messages in thread
From: Fabian Ebner @ 2021-06-09  9:18 UTC (permalink / raw)
  To: pve-devel

Get the replicatable volumes from the snapshot config rather than the current
config. And filter those volumes further to those that will actually be rolled
back.

Previously, a volume that only had replication snapshots (e.g. because it was
added after the snapshot was taken, or the vmstate volume) would lose them.
Then, on the next replication run, such a volume would lead to an error, because
replication tried to do a full sync, but the target volume still exists.

Should be enough for most real-world scenarios, but not a complete fix:
It is still possible to run into the problem by removing the last
(non-replication) snapshots after a rollback before replication can run once.

The list of volumes is not required to be sorted for prepare(), but it is sorted
by how foreach_volume() iterates now, so not random.

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

Changes from v1:
    * dropped already applied patch
    * rebased on top of the new filename (since a src/ prefix was added)
    * add another comment mentioning why the additional filtering is necessary

 src/PVE/AbstractConfig.pm | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index 3348d8a..6542ae4 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -974,13 +974,23 @@ sub snapshot_rollback {
 	if ($prepare) {
 	    my $repl_conf = PVE::ReplicationConfig->new();
 	    if ($repl_conf->check_for_existing_jobs($vmid, 1)) {
-		# remove all replication snapshots
-		my $volumes = $class->get_replicatable_volumes($storecfg, $vmid, $conf, 1);
-		my $sorted_volids = [ sort keys %$volumes ];
+		# remove replication snapshots on volumes affected by rollback *only*!
+		my $volumes = $class->get_replicatable_volumes($storecfg, $vmid, $snap, 1);
+
+		# filter by what we actually iterate over below (excludes vmstate!)
+		my $volids = [];
+		$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};
+		});
 
 		# remove all local replication snapshots (jobid => undef)
 		my $logfunc = sub { my $line = shift; chomp $line; print "$line\n"; };
-		PVE::Replication::prepare($storecfg, $sorted_volids, undef, 1, undef, $logfunc);
+		PVE::Replication::prepare($storecfg, $volids, undef, 1, undef, $logfunc);
 	    }
 
 	    $class->foreach_volume($snap, sub {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-12  9:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09  9:18 [pve-devel] [PATCH v2 guest-common 1/2] partially fix 3111: snapshot rollback: improve removing replication snapshots Fabian Ebner
2021-06-09  9:18 ` [pve-devel] [PATCH v2 guest-common 2/2] fix 3111: replicate guest on rollback if there are replication jobs for it Fabian Ebner
2021-06-22  7:41   ` Fabian Grünbichler
2021-08-12  9:12     ` Fabian Ebner

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