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 v2 guest-common 2/2] fix 3111: replicate guest on rollback if there are replication jobs for it
Date: Wed,  9 Jun 2021 11:18:58 +0200	[thread overview]
Message-ID: <20210609091858.27219-2-f.ebner@proxmox.com> (raw)
In-Reply-To: <20210609091858.27219-1-f.ebner@proxmox.com>

so that there will be a valid replication snapshot again.

Otherwise, replication will be broken after a rollback if the last
(non-replication) snapshot is removed before replication can run again.

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

No changes from v1 (except rebase).

Not a huge fan of this, but the alternatives I could come up with don't seem
much better IMHO:

1. Invalidate/remove replicated volumes after a rollback altogether and require
a full sync on the next replication job afterwards.

2. Another one is to disallow removing the last non-replication snapshot if:
    * there is a replication job configured
    * no replication snapshot for that job currently exists
      (which likely means it was removed by a previous rollback operation, but
      can also happen for a new job that didn't run yet).

3. Hope not very many people immediately delete their snapshots after rollback.

Pick a favorite or suggest your own ;)

 src/PVE/AbstractConfig.pm    | 19 +++++++++++++++++--
 src/PVE/ReplicationConfig.pm | 14 ++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index 6542ae4..6cc0537 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -951,6 +951,9 @@ sub snapshot_rollback {
 
     my $storecfg = PVE::Storage::config();
 
+    my $repl_conf = PVE::ReplicationConfig->new();
+    my $logfunc = sub { my $line = shift; chomp $line; print "$line\n"; };
+
     my $data = {};
 
     my $get_snapshot_config = sub {
@@ -972,7 +975,6 @@ sub snapshot_rollback {
 	$snap = $get_snapshot_config->($conf);
 
 	if ($prepare) {
-	    my $repl_conf = PVE::ReplicationConfig->new();
 	    if ($repl_conf->check_for_existing_jobs($vmid, 1)) {
 		# remove replication snapshots on volumes affected by rollback *only*!
 		my $volumes = $class->get_replicatable_volumes($storecfg, $vmid, $snap, 1);
@@ -989,7 +991,6 @@ sub snapshot_rollback {
 		});
 
 		# 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);
 	    }
 
@@ -1047,6 +1048,20 @@ sub snapshot_rollback {
 
     $prepare = 0;
     $class->lock_config($vmid, $updatefn);
+
+    my $replication_jobs = $repl_conf->list_guests_replication_jobs($vmid);
+    for my $job (@{$replication_jobs}) {
+	my $target = $job->{target};
+	$logfunc->("replicating rolled back guest to node '$target'");
+
+	my $start_time = time();
+	eval {
+	    PVE::Replication::run_replication($class, $job, $start_time, $start_time, $logfunc);
+	};
+	if (my $err = $@) {
+	    warn "unable to replicate rolled back guest to node '$target' - $err";
+	}
+    }
 }
 
 # bash completion helper
diff --git a/src/PVE/ReplicationConfig.pm b/src/PVE/ReplicationConfig.pm
index fd856a0..84a718f 100644
--- a/src/PVE/ReplicationConfig.pm
+++ b/src/PVE/ReplicationConfig.pm
@@ -228,6 +228,20 @@ sub find_local_replication_job {
     return undef;
 }
 
+sub list_guests_replication_jobs {
+    my ($cfg, $vmid) = @_;
+
+    my $jobs = [];
+
+    for my $job (values %{$cfg->{ids}}) {
+	next if $job->{type} ne 'local' || $job->{guest} != $vmid;
+
+	push @{$jobs}, $job;
+    }
+
+    return $jobs;
+}
+
 # makes old_target the new source for all local jobs of this guest
 # makes new_target the target for the single local job with target old_target
 sub switch_replication_job_target_nolock {
-- 
2.30.2





  reply	other threads:[~2021-06-09  9:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Fabian Ebner [this message]
2021-06-22  7:41   ` [pve-devel] [PATCH v2 guest-common 2/2] fix 3111: replicate guest on rollback if there are replication jobs for it Fabian Grünbichler
2021-08-12  9:12     ` Fabian Ebner

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=20210609091858.27219-2-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