From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id A21951FF17A for ; Tue, 28 Oct 2025 10:50:29 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D2C0016F7F; Tue, 28 Oct 2025 10:51:01 +0100 (CET) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Tue, 28 Oct 2025 10:49:58 +0100 Message-ID: <20251028095006.29348-2-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251028095006.29348-1-f.ebner@proxmox.com> References: <20251028095006.29348-1-f.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761645016668 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.020 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 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/RFC storage 1/1] lvmplugin: improve error handling for saferemove 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" Ensure that volumes are not automatically removed after a failure to zero-out to give an admin the possibility to do so manually. The downside is that "del-.*" volumes might remain left-over, the upside is that data does not leak upon error. Also ensure that activation failure of a single volume does not error out early, but continue with removal of other volumes. Signed-off-by: Fiona Ebner --- src/PVE/Storage/LVMPlugin.pm | 59 ++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index 275e84a..5e9fd83 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -10,6 +10,7 @@ use IO::File; use PVE::Tools qw(run_command file_read_firstline trim); use PVE::Storage::Plugin; use PVE::JSONSchema qw(get_standard_option); +use PVE::RESTEnvironment qw(log_warn); use PVE::Storage::Common; @@ -297,8 +298,7 @@ my sub free_lvm_volumes { if ($bdev && $bdev =~ m|^/dev/(dm-\d+)|) { $sysdir = "/sys/block/$1"; } else { - warn "skip zero-out for volume '$lvmpath' - no device mapper link\n"; - return; + die "no device mapper link found\n"; } my $write_zeroes_max_bytes = @@ -345,43 +345,50 @@ my sub free_lvm_volumes { $stepsize = $write_zeroes_max_bytes; } - my $cmd = ['blkdiscard', $lvmpath, '-v', '--zeroout', '--step', "${stepsize}"]; - eval { run_command($cmd); }; - warn $@ if $@; + run_command(['blkdiscard', $lvmpath, '-v', '--zeroout', '--step', "${stepsize}"]); } }; # we need to zero out LVM data for security reasons # and to allow thin provisioning my $zero_out_worker = sub { + my $error_count = 0; for my $name (@$volnames) { my $lvmpath = "/dev/$vg/del-$name"; print "zero-out data on image $name ($lvmpath)\n"; - my $cmd_activate = ['/sbin/lvchange', '-aly', $lvmpath]; - run_command( - $cmd_activate, - errmsg => "can't activate LV '$lvmpath' to zero-out its data", - ); - $cmd_activate = ['/sbin/lvchange', '--refresh', $lvmpath]; - run_command( - $cmd_activate, - errmsg => "can't refresh LV '$lvmpath' to zero-out its data", - ); + eval { + my $cmd_activate = ['/sbin/lvchange', '-aly', $lvmpath]; + run_command( + $cmd_activate, + errmsg => "can't activate LV '$lvmpath' to zero-out its data", + ); + $cmd_activate = ['/sbin/lvchange', '--refresh', $lvmpath]; + run_command( + $cmd_activate, + errmsg => "can't refresh LV '$lvmpath' to zero-out its data", + ); - $secure_delete_cmd->($lvmpath); + $secure_delete_cmd->($lvmpath); - $class->cluster_lock_storage( - $storeid, - $scfg->{shared}, - undef, - sub { - my $cmd = ['/sbin/lvremove', '-f', "$vg/del-$name"]; - run_command($cmd, errmsg => "lvremove '$vg/del-$name' error"); - }, - ); - print "successfully removed volume $name ($vg/del-$name)\n"; + $class->cluster_lock_storage( + $storeid, + $scfg->{shared}, + undef, + sub { + my $cmd = ['/sbin/lvremove', '-f', "$vg/del-$name"]; + run_command($cmd, errmsg => "lvremove '$vg/del-$name' error"); + }, + ); + print "successfully removed volume $name ($vg/del-$name)\n"; + }; + if (my $err = $@) { + chomp($err); + log_warn("cannot zero-out '$lvmpath' - $err - please zero-out and remove manually"); + $error_count++; + } } + die "failed to zero-out $error_count volume(s) - check log\n" if $error_count > 0; }; if ($scfg->{saferemove}) { -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel