[pve-devel] [RFC storage] lvm plugin: fix locking for rollback when using CLI

[Fiona Ebner]

For some reason still to be determined, doing a rollback via CLI on an
LVM storage with snapshot-as-volume-chain and saferemove would run
into locking issues (likely trying to re-acquire the lock it already
holds a second time). The same issue does not happen when the rollback
is done via UI.

Avoid doing fork_cleanup_worker() inside the locked section to avoid
this.

Fixes: 8eabcc7 ("lvm plugin: snapshot-as-volume-chain: use locking for snapshot operations")
Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 97f7bf4..164437a 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1117,23 +1117,17 @@ sub volume_rollback_is_possible {
 }
 
 my sub volume_snapshot_rollback_locked {
-    my ($class, $scfg, $storeid, $volname, $snap) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap, $cleanup_worker) = @_;
 
     my $format = ($class->parse_volname($volname))[6];
 
     die "can't rollback snapshot for '$format' volume\n" if $format ne 'qcow2';
 
-    my $cleanup_worker = eval { free_snap_image($class, $storeid, $scfg, $volname, 'current'); };
+    $cleanup_worker->$* = eval { free_snap_image($class, $storeid, $scfg, $volname, 'current'); };
     die "error deleting snapshot $snap $@\n" if $@;
 
     eval { alloc_snap_image($class, $storeid, $scfg, $volname, $snap) };
-    my $alloc_err = $@;
-
-    fork_cleanup_worker($cleanup_worker);
-
-    if ($alloc_err) {
-        die "can't allocate new volume $volname: $alloc_err\n";
-    }
+    die "can't allocate new volume $volname: $@\n" if $@;
 
     return undef;
 }
@@ -1141,14 +1135,22 @@ my sub volume_snapshot_rollback_locked {
 sub volume_snapshot_rollback {
     my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
-    return $class->cluster_lock_storage(
+    my $cleanup_worker;
+
+    $class->cluster_lock_storage(
         $storeid,
         $scfg->{shared},
         undef,
         sub {
-            return volume_snapshot_rollback_locked($class, $scfg, $storeid, $volname, $snap);
+            volume_snapshot_rollback_locked(
+                $class, $scfg, $storeid, $volname, $snap, \$cleanup_worker,
+            );
         },
     );
+
+    fork_cleanup_worker($cleanup_worker);
+
+    return;
 }
 
 sub volume_snapshot_delete {
-- 
2.47.3