One not-patch-related observation inline.

On 27.04.20 10:24, Fabian Grünbichler wrote:
to protect checks against concurrent modifications

Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
---

Notes:
     bested viewed with --patience -w

  PVE/AbstractConfig.pm | 45 +++++++++++++++++++++----------------------
  1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
index 15368de..70311df 100644
--- a/PVE/AbstractConfig.pm
+++ b/PVE/AbstractConfig.pm
@@ -919,9 +919,10 @@ sub snapshot_rollback {
my $storecfg = PVE::Storage::config(); - my $conf = $class->load_config($vmid);
+    my $data = {};
my $get_snapshot_config = sub {
+       my ($conf) = @_;
die "you can't rollback if vm is a template\n" if $class->is_template($conf); @@ -932,32 +933,30 @@ sub snapshot_rollback {
        return $res;
      };
- 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 all local replication snapshots (jobid => undef)
-       my $logfunc = sub { my $line = shift; chomp $line; print "$line\n"; };
-       PVE::Replication::prepare($storecfg, $sorted_volids, undef, 0, undef, 
$logfunc);
-    }
-
-    my $snap = &$get_snapshot_config();
-
-    $class->foreach_volume($snap, sub {
-       my ($vs, $volume) = @_;
-
-       $class->__snapshot_rollback_vol_possible($volume, $snapname);
-    });
-
-    my $data = {};
+    my $snap;
my $updatefn = sub {
+       my $conf = $class->load_config($vmid);
+       $snap = $get_snapshot_config->($conf);
- $conf = $class->load_config($vmid);
+       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 all local replication snapshots (jobid => undef)
+               my $logfunc = sub { my $line = shift; chomp $line; print 
"$line\n"; };
+               PVE::Replication::prepare($storecfg, $sorted_volids, undef, 0, 
undef, $logfunc);

This has nothing to do with the locking/this patch, but here we'd need to call prepare with last_sync=1 instead of last_sync=0, no? This is because of commit a1dfeff3a8502544123245ea61ad62cbe97803b7 which made last_sync=0 a special case and changed the behavior. I can send a patch once this is applied.

+           }
+
+           $class->foreach_volume($snap, sub {
+              my ($vs, $volume) = @_;
- $snap = &$get_snapshot_config();
+              $class->__snapshot_rollback_vol_possible($volume, $snapname);
+           });
+        }
die "unable to rollback to incomplete snapshot (snapstate = $snap->{snapstate})\n"
            if $snap->{snapstate};


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to