From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 6513B1FF146 for ; Tue, 07 Jul 2026 16:33:03 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 8CEDA2144C; Tue, 07 Jul 2026 16:33:02 +0200 (CEST) From: Lukas Sichert To: pve-devel@lists.proxmox.com Subject: [PATCH storage v8 1/5] lvm: saferemove: keep LVs where zero-out failed for manual zero-out Date: Tue, 7 Jul 2026 16:32:44 +0200 Message-ID: <20260707143252.101757-2-l.sichert@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260707143252.101757-1-l.sichert@proxmox.com> References: <20260707143252.101757-1-l.sichert@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1783434771505 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.000 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: Y4USXMC6AK7C3MN2UKYOO3EZMAVHKEKG X-Message-ID-Hash: Y4USXMC6AK7C3MN2UKYOO3EZMAVHKEKG X-MailFrom: l.sichert@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Lukas Sichert X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Currently even if 'zeroout' fails, the LV is removed and can't be zeroed out manually later. Let zeroing errors propagate from the secure delete command, and rename failed removals to a 'failed--del-*' LV name instead of immediately removing them. Signed-off-by: Lukas Sichert --- src/PVE/Storage/LVMPlugin.pm | 70 +++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index a313ecc..a0feb5c 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -7,6 +7,7 @@ use Cwd qw(abs_path); use File::Basename; use IO::File; use JSON; +use List::Util qw(max); use PVE::JSONSchema qw(get_standard_option); use PVE::Tools qw(run_command file_read_firstline trim); @@ -327,13 +328,10 @@ my sub free_lvm_volumes_locked { '-t', "$throughput", ]; - eval { - run_command( - $cmd, - errmsg => "zero out finished (note: 'No space left on device' is ok here)", - ); - }; - warn $@ if $@; + run_command( + $cmd, + errmsg => "zero out finished (note: 'No space left on device' is ok here)", + ); } else { # If the storage supports write_zeroes but stepsize is too big, reduce the stepsize to # the maximum supported by the storage. @@ -345,8 +343,7 @@ my sub free_lvm_volumes_locked { } my $cmd = ['blkdiscard', $lvmpath, '-v', '--zeroout', '--step', "${stepsize}"]; - eval { run_command($cmd); }; - warn $@ if $@; + run_command($cmd); } }; @@ -368,18 +365,51 @@ my sub free_lvm_volumes_locked { errmsg => "can't refresh LV '$lvmpath' to zero-out its data", ); - $secure_delete_cmd->($lvmpath); + eval { $secure_delete_cmd->($lvmpath); }; + if (my $cleanup_err = $@) { + my $failed_name; + $class->cluster_lock_storage( + $storeid, + $scfg->{shared}, + undef, + sub { + my $lvs = lvm_list_volumes(); + my $existing = $lvs->{$vg} // {}; + + my $prefix = 'failed-'; + my $suffix = "-del-$name"; + + my $last_fail = max( + 0, + map { + /^\Q$prefix\E(\d+)\Q$suffix\E$/ ? $1 : () + } keys %$existing, + ); + + $failed_name = 'failed-' . ($last_fail + 1) . $suffix; + + my $cmd = ['/sbin/lvrename', $vg, "del-$name", $failed_name]; + run_command( + $cmd, + errmsg => "lvrename '$vg/del-$name' to '$vg/$failed_name' error", + ); + }, + ); - $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"; + die "cleanup failed for lv $name: $cleanup_err\n"; + + } else { + $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"; + } } }; -- 2.47.3