yesBad/pve-storage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lvm plugin: fix locking for rollback when using CLI

Browse Source 
Doing a rollback via CLI on an LVM storage with 'saferemove' and
'snapshot-as-volume-chain' would run into a locking issue, because
the forked zero-out worker would try to acquire the lock while the
main CLI task is still inside the locked section for
volume_snapshot_rollback_locked(). The same issue does not happen when
the rollback is done via UI. The reason for this can be found in the
note regarding fork_worker():

> we simulate running in foreground if ($self->{type} eq 'cli')

So the worker will be awaited synchronously in CLI context, resulting
in the deadlock, while via API/UI, the main task would move on and
release the lock allowing the zero-out worker to acquire it.

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

Fixes: 8eabcc7 ("lvm plugin: snapshot-as-volume-chain: use locking for snapshot operations")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Link: https://lore.proxmox.com/20251120101742.24843-1-f.ebner@proxmox.com
This commit is contained in:
Fiona Ebner
2025-11-20 11:17:30 +01:00
committed by Thomas Lamprecht
parent bb958344ec
commit 5001f03269
1 changed files with 26 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
src/PVE/Storage/LVMPlugin.pm
View File

@ -1117,23 +1117,17 @@ sub volume_rollback_is_possible {
} }
my sub volume_snapshot_rollback_locked { 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]; my $format = ($class->parse_volname($volname))[6];
die "can't rollback snapshot for '$format' volume\n" if $format ne 'qcow2'; 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 $@; die "error deleting snapshot $snap $@\n" if $@;
eval { alloc_snap_image($class, $storeid, $scfg, $volname, $snap) }; eval { alloc_snap_image($class, $storeid, $scfg, $volname, $snap) };
my $alloc_err = $@; die "can't allocate new volume $volname: $@\n" if $@;
fork_cleanup_worker($cleanup_worker);
if ($alloc_err) {
die "can't allocate new volume $volname: $alloc_err\n";
}
return undef; return undef;
} }
@ -1141,14 +1135,29 @@ my sub volume_snapshot_rollback_locked {
sub volume_snapshot_rollback { sub volume_snapshot_rollback {
my ($class, $scfg, $storeid, $volname, $snap) = @_; my ($class, $scfg, $storeid, $volname, $snap) = @_;
return $class->cluster_lock_storage( my $cleanup_worker;
$storeid,
$scfg->{shared}, eval {
undef, $class->cluster_lock_storage(
sub { $storeid,
return volume_snapshot_rollback_locked($class, $scfg, $storeid, $volname, $snap); $scfg->{shared},
}, undef,
); sub {
volume_snapshot_rollback_locked(
$class, $scfg, $storeid, $volname, $snap, \$cleanup_worker,
);
},
);
};
my $err = $@;
# Spawn outside of the locked section, because with 'saferemove', the cleanup worker also needs
# to obtain the lock, and in CLI context, it will be awaited synchronously, see fork_worker().
fork_cleanup_worker($cleanup_worker);
die $err if $err;
return;
} }
sub volume_snapshot_delete { sub volume_snapshot_delete {