* [pve-devel] [PATCH/RFC storage 1/1] lvmplugin: improve error handling for saferemove
2025-10-28 9:49 [pve-devel] [PATCH/RFC storage 0/1] lvmplugin: improve error handling for saferemove Fiona Ebner
@ 2025-10-28 9:49 ` Fiona Ebner
0 siblings, 0 replies; 2+ messages in thread
From: Fiona Ebner @ 2025-10-28 9:49 UTC (permalink / raw)
To: 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 <f.ebner@proxmox.com>
---
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
^ permalink raw reply [flat|nested] 2+ messages in thread