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 <[email protected]> --- 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 [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
