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

lvm plugin: volume import: lock allocation and removal sections

Browse Source 
With a shared LVM storage, parallel imports, which might be done in
the context of remote migration, could lead to metadata corruption
with unlucky timing, because of missing locking. Add locking around
allocation and removal, which are the sections that modify LVM
metadata. Note that other plugins suffer from missing locking here as
well, but only regarding naming conflicts. Adding locking around the
full call to volume_import() would mean locking for much too long.
Other plugins could follow the approach here, or there could be a
reservation approach like proposed in [0].

[0]: https://lore.proxmox.com/pve-devel/20240403150712.262773-1-h.duerr@proxmox.com/

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Friedrich Weber <f.weber@proxmox.com>
Link: https://lore.proxmox.com/20251103162330.112603-4-f.ebner@proxmox.com
This commit is contained in:
Fiona Ebner
2025-11-03 17:23:14 +01:00
committed by Thomas Lamprecht
parent 0864fda2fd
commit 5988ac0250
1 changed files with 26 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1300,19 +1300,27 @@ sub volume_import {
die "cannot import format $format into a file of format $file_format\n"
if $file_format ne 'raw';
my $vg = $scfg->{vgname};
my $lvs = lvm_list_volumes($vg);
if ($lvs->{$vg}->{$volname}) {
die "volume $vg/$volname already exists\n" if !$allow_rename;
warn "volume $vg/$volname already exists - importing with a different name\n";
$name = undef;
}
my $allocname = $class->cluster_lock_storage(
$storeid,
$scfg->{shared},
undef,
sub {
my $vg = $scfg->{vgname};
my $lvs = lvm_list_volumes($vg);
if ($lvs->{$vg}->{$volname}) {
die "volume $vg/$volname already exists\n" if !$allow_rename;
warn "volume $vg/$volname already exists - importing with a different name\n";
$name = undef;
}
my ($size) = PVE::Storage::Plugin::read_common_header($fh);
$size = PVE::Storage::Common::align_size_up($size, 1024) / 1024;
my ($size) = PVE::Storage::Plugin::read_common_header($fh);
$size = PVE::Storage::Common::align_size_up($size, 1024) / 1024;
return $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
},
);
eval {
my $allocname = $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
my $oldname = $volname;
$volname = $allocname;
if (defined($name) && $allocname ne $oldname) {
@ -1324,7 +1332,14 @@ sub volume_import {
$class->volume_import_write($fh, $file);
};
if (my $err = $@) {
my $cleanup_worker = eval { $class->free_image($storeid, $scfg, $volname, 0) };
my $cleanup_worker = eval {
return $class->cluster_lock_storage(
$storeid,
$scfg->{shared},
undef,
sub { return $class->free_image($storeid, $scfg, $volname, 0); },
);
};
warn $@ if $@;
fork_cleanup_worker($cleanup_worker);
die $err;