From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 62A261FF187 for ; Mon, 3 Nov 2025 17:23:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9BF0A2258F; Mon, 3 Nov 2025 17:24:08 +0100 (CET) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Mon, 3 Nov 2025 17:23:14 +0100 Message-ID: <20251103162330.112603-4-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251103162330.112603-1-f.ebner@proxmox.com> References: <20251103162330.112603-1-f.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762186997429 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.021 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH storage 3/4] lvm plugin: volume import: lock allocation and removal sections X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 --- Better viewed with "-w" or "-w --word-diff=color --word-diff-regex='\w+'". src/PVE/Storage/LVMPlugin.pm | 37 +++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index f5a2008..c5f71a2 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -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; -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel