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] [RFC guest-common 3/3] fix 3111: replicate guest on rollback if there are replication jobs for it
Date: Mon, 12 Apr 2021 13:37:17 +0200	[thread overview]
Message-ID: <20210412113717.25356-3-f.ebner@proxmox.com> (raw)
In-Reply-To: <20210412113717.25356-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 once.

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

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 ;)

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

diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
index c4d1d6c..b169b8b 100644
--- a/PVE/AbstractConfig.pm
+++ b/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);
@@ -988,7 +990,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);
 	    }
 
@@ -1046,6 +1047,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/PVE/ReplicationConfig.pm b/PVE/ReplicationConfig.pm
index fd856a0..84a718f 100644
--- a/PVE/ReplicationConfig.pm
+++ b/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.20.1





  parent reply	other threads:[~2021-04-12 11:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-12 11:37 [pve-devel] [PATCH storage 1/3] volume export/import: allow uppercase letters Fabian Ebner
2021-04-12 11:37 ` [pve-devel] [PATCH guest-common 2/3] partially fix 3111: snapshot rollback: fix removing replication snapshots Fabian Ebner
2021-04-12 11:37 ` Fabian Ebner [this message]
2021-04-12 12:54 ` [pve-devel] applied: [PATCH storage 1/3] volume export/import: allow uppercase letters Thomas Lamprecht

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=20210412113717.25356-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