* [PATCH storage v8 1/5] lvm: saferemove: keep LVs where zero-out failed for manual zero-out
2026-07-07 14:32 [PATCH docs/manager/storage v8 0/5] fix #7339: lvmthick: add option to free storage for deleted VMs Lukas Sichert
@ 2026-07-07 14:32 ` Lukas Sichert
2026-07-07 14:32 ` [PATCH storage v8 2/5] lvm: saferemove: zero out volumes range by range Lukas Sichert
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Sichert @ 2026-07-07 14:32 UTC (permalink / raw)
To: pve-devel; +Cc: Lukas Sichert
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-<N>-del-*' LV name instead of immediately
removing them.
Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH storage v8 2/5] lvm: saferemove: zero out volumes range by range
2026-07-07 14:32 [PATCH docs/manager/storage v8 0/5] fix #7339: lvmthick: add option to free storage for deleted VMs Lukas Sichert
2026-07-07 14:32 ` [PATCH storage v8 1/5] lvm: saferemove: keep LVs where zero-out failed for manual zero-out Lukas Sichert
@ 2026-07-07 14:32 ` Lukas Sichert
2026-07-07 14:32 ` [PATCH storage v8 3/5] fix #7339: lvm: add discard action for removed volumes Lukas Sichert
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Sichert @ 2026-07-07 14:32 UTC (permalink / raw)
To: pve-devel; +Cc: Lukas Sichert
'saferemove' currently uses different full-volume zero-out paths:
`blkdiscard --zeroout` for devices with write-zeroes support and
`cstream` otherwise. This makes consistent progress reporting and
throttling difficult and prevents interleaving future discard cleanup
with zeroing. On thin-provisioned backing storage, zeroing the whole LV
first can force unnecessary allocation.
Move zeroing into an explicit range loop. Use BLKZEROOUT when supported,
cap to the device limit, and fall back to manually writing zeroes via
syswrite otherwise. Add progress reporting in the shared loop, and apply
a configured saferemove throughput limit there as well. Without an
explicit limit, keep BLKZEROOUT unthrottled and throttle only manual
writes to 10 MiB/s.
Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
---
src/PVE/Storage/LVMPlugin.pm | 146 +++++++++++++++++++++++++++--------
1 file changed, 112 insertions(+), 34 deletions(-)
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index a0feb5c..a3adeef 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -8,9 +8,12 @@ use File::Basename;
use IO::File;
use JSON;
use List::Util qw(max);
+use Fcntl qw(SEEK_SET);
use PVE::JSONSchema qw(get_standard_option);
use PVE::Tools qw(run_command file_read_firstline trim);
+use PVE::SafeSyslog;
+use PVE::Format qw(render_bytes render_duration);
use PVE::Storage::Common;
use PVE::Storage::Plugin;
@@ -19,6 +22,10 @@ use base qw(PVE::Storage::Plugin);
# lvm helper functions
+use constant {
+ BLKZEROOUT => 0x127f,
+};
+
my $ignore_no_medium_warnings = sub {
my $line = shift;
# ignore those, most of the time they're from (virtual) IPMI/iKVM devices
@@ -280,6 +287,13 @@ sub lvm_list_volumes {
return $lvs;
}
+my sub blockdev_ioctl_range {
+ my ($fh, $ioctl, $offset, $length) = @_;
+
+ my $range = pack('QQ', $offset, $length);
+ ioctl($fh, $ioctl, $range) or die "$!\n";
+}
+
my sub free_lvm_volumes_locked {
my ($class, $scfg, $storeid, $volnames) = @_;
@@ -305,48 +319,112 @@ my sub free_lvm_volumes_locked {
file_read_firstline("$sysdir/queue/write_zeroes_max_bytes") // 0;
($write_zeroes_max_bytes) = $write_zeroes_max_bytes =~ m/^(\d+)$/; #untaint
+ my $size = file_read_firstline("$sysdir/size") or die "size of $sysdir cannot be read";
+ ($size) = $size =~ m/^(\d+)$/; # untaint
+ $size *= 512; # sysfs size is in 512-byte sectors
+
+ my $zeroout_variant = 'blkzeroout';
+ my $throughput = -1;
+ if ($scfg->{saferemove_throughput}) {
+ # use abs as legacy cstream accepted negative values
+ $throughput = abs($scfg->{saferemove_throughput});
+ my $throughput_in_mibs = render_bytes($throughput);
+ print "using saferemove throughput limit: $throughput_in_mibs/s\n";
+ }
+
+ # If the storage does not support write_zeroes fall back to writing zeroes manually using
+ # syswrite. Otherwise if the storage supports write_zeroes but stepsize is too big,
+ # reduce the stepsize to the maximum supported by the storage.
+ my $zeroes;
if ($write_zeroes_max_bytes == 0) {
- # If the storage does not support 'write zeroes', we fallback to cstream.
- # wipe throughput up to 10MB/s by default; may be overwritten with saferemove_throughput
- my $throughput = '-10485760';
- if ($scfg->{saferemove_throughput}) {
- $throughput = $scfg->{saferemove_throughput};
+ print
+ "WRITE_ZEROES operation not supported, falling back to syswrite to zero-out '$lvmpath'\n";
+ $zeroout_variant = 'syswrite';
+ $stepsize = 1024 * 1024; # 1 MiB
+ print "reduce stepsize to 1 MiB for syswrite\n";
+ $zeroes = "\0" x $stepsize;
+ # limit throughput to 10MiB/s for syswrite, if throughput was not set
+ if ($throughput <= 0) {
+ # FIXME: increase to 100 MiB/s with 10.0
+ $throughput = 10485760;
+ print "using default syswrite-saferemove throughput limit: 10 MiB/s\n";
}
+ } elsif ($stepsize > $write_zeroes_max_bytes) {
+ print "reduce stepsize to the maximum supported by the storage:"
+ . " $write_zeroes_max_bytes bytes\n";
+ $stepsize = $write_zeroes_max_bytes;
+ }
+ open(my $fh, '+<', $lvmpath) or die "can't open '$lvmpath' - $!\n";
- my $cmd = [
- '/usr/bin/cstream',
- '-i',
- '/dev/zero',
- '-o',
- $lvmpath,
- '-T',
- '10',
- '-v',
- '1',
- '-b',
- '1048576',
- '-t',
- "$throughput",
- ];
- 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.
- if ($write_zeroes_max_bytes > 0 && $stepsize > $write_zeroes_max_bytes) {
- print "reduce stepsize to the maximum supported by the storage:"
- . " $write_zeroes_max_bytes bytes\n";
+ # eval block, so filehandle is closed even if something fails below
+ eval {
+ my $start = time();
+ my $written_total = 0;
+ my $lastprint = -1;
+ my $written;
+
+ for (my $offset = 0; $offset < $size; $offset += $written) {
- $stepsize = $write_zeroes_max_bytes;
+ if ($offset + $stepsize > $size) {
+ $stepsize = $size - $offset;
+ }
+
+ if ($zeroout_variant eq 'blkzeroout') {
+ eval { blockdev_ioctl_range($fh, BLKZEROOUT, $offset, $stepsize); };
+ if ($@) {
+ die "blkzeroout for $stepsize bytes at offset $offset failed: $@";
+ }
+ $written = $stepsize;
+ } elsif ($zeroout_variant eq 'syswrite') {
+ # if the $offset is 0, sysseek can return 0, therefore use // to only
+ # throw an error, if it returns undef
+ sysseek($fh, $offset, SEEK_SET) // die "sysseek failed: $!\n";
+
+ # use or as we also want to die if no progress was made, i.e. if $written is 0
+ $written = syswrite($fh, $zeroes, $stepsize)
+ or die "syswrite failed: $!\n";
+
+ if ($written != $stepsize) {
+ warn "short syswrite: wrote $written of $stepsize bytes\n";
+ }
+
+ }
+ $written_total += $written;
+
+ my $curr_time = time();
+ if (($curr_time - $lastprint) >= 3) {
+ my $percent_finished = 100 * $written_total / $size;
+ my $curr_seconds = $curr_time - $start;
+
+ printf(
+ "zeroed out %s of %s (%.2f%%) using %s in %s seconds\n",
+ render_bytes($written_total),
+ render_bytes($size),
+ $percent_finished,
+ $zeroout_variant,
+ render_duration($curr_seconds),
+ );
+ $lastprint = $curr_time;
+ }
+
+ if ($throughput > 0) {
+ my $expected_elapsed = $written_total / $throughput;
+ my $actual_elapsed = $curr_time - $start;
+ my $delay = $expected_elapsed - $actual_elapsed;
+ if ($delay > 0) {
+ sleep($delay);
+ }
+ }
}
- my $cmd = ['blkdiscard', $lvmpath, '-v', '--zeroout', '--step', "${stepsize}"];
- run_command($cmd);
+ };
+ # close filehandle before throwing an error
+ my $err = $@;
+ close($fh);
+ if ($err) {
+ die "$err";
}
};
-
# we need to zero out LVM data for security reasons
# and to allow thin provisioning
my $zero_out_worker = sub {
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH storage v8 3/5] fix #7339: lvm: add discard action for removed volumes
2026-07-07 14:32 [PATCH docs/manager/storage v8 0/5] fix #7339: lvmthick: add option to free storage for deleted VMs Lukas Sichert
2026-07-07 14:32 ` [PATCH storage v8 1/5] lvm: saferemove: keep LVs where zero-out failed for manual zero-out Lukas Sichert
2026-07-07 14:32 ` [PATCH storage v8 2/5] lvm: saferemove: zero out volumes range by range Lukas Sichert
@ 2026-07-07 14:32 ` Lukas Sichert
2026-07-07 14:32 ` [PATCH manager v8 4/5] fix #7339: lvmthick: ui: add UI option to free storage Lukas Sichert
2026-07-07 14:32 ` [PATCH docs v8 5/5] fix #7339: lvm: document discard option Lukas Sichert
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Sichert @ 2026-07-07 14:32 UTC (permalink / raw)
To: pve-devel; +Cc: Lukas Sichert
On LVM storages backed by thin-provisioned SAN LUNs, removing an LV does
not release the allocated space on the backing storage. Using LVM's
`issue_discards` can avoid that, but makes `lvremove` issue the discards
while holding the cluster-wide storage lock, which can hit the lock
timeout for large volumes.
Add an `on-volume-remove` property with an initial `discard` action.
Reject the option if discard is not supported by the backing devices.
The cleanup worker issues the discard for the renamed LV before the
final remove, so the long-running operation happens outside the storage
lock. When combined with `saferemove`, the worker zeroes and discards
the LV range by range, avoiding allocation of the whole LV with zeroes
on thin-provisioned backing storage.
Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
Link: bugzilla.proxmox.com/show_bug.cgi?id=7339
---
src/PVE/Storage/LVMPlugin.pm | 186 +++++++++++++++++++++++++++++++++--
1 file changed, 177 insertions(+), 9 deletions(-)
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index a3adeef..8653331 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -12,8 +12,10 @@ use Fcntl qw(SEEK_SET);
use PVE::JSONSchema qw(get_standard_option);
use PVE::Tools qw(run_command file_read_firstline trim);
+use PVE::RESTEnvironment qw(log_warn);
use PVE::SafeSyslog;
use PVE::Format qw(render_bytes render_duration);
+use PVE::Exception qw(raise_param_exc);
use PVE::Storage::Common;
use PVE::Storage::Plugin;
@@ -23,6 +25,7 @@ use base qw(PVE::Storage::Plugin);
# lvm helper functions
use constant {
+ BLKDISCARD => 0x1277,
BLKZEROOUT => 0x127f,
};
@@ -299,6 +302,12 @@ my sub free_lvm_volumes_locked {
my $vg = $scfg->{vgname};
+ my $on_remove_opts = {};
+ if ($scfg->{'on-volume-remove'}) {
+ $on_remove_opts =
+ PVE::JSONSchema::parse_property_string('on-volume-remove', $scfg->{'on-volume-remove'});
+ }
+
my $secure_delete_cmd = sub {
my ($lvmpath) = @_;
@@ -362,6 +371,8 @@ my sub free_lvm_volumes_locked {
my $written_total = 0;
my $lastprint = -1;
my $written;
+ my $discard_attempts = 0;
+ my $discard_failures = 0;
for (my $offset = 0; $offset < $size; $offset += $written) {
@@ -389,6 +400,18 @@ my sub free_lvm_volumes_locked {
}
}
+ if ($on_remove_opts->{discard}) {
+ $discard_attempts++;
+
+ eval { blockdev_ioctl_range($fh, BLKDISCARD, $offset, $written); };
+ if ($@) {
+ if ($discard_failures == 0) {
+ log_warn(
+ "blkdiscard for $written bytes at offset $offset failed: $@");
+ }
+ $discard_failures += 1;
+ }
+ }
$written_total += $written;
my $curr_time = time();
@@ -416,7 +439,9 @@ my sub free_lvm_volumes_locked {
}
}
}
-
+ if ($discard_failures != 0) {
+ die "$discard_failures out of $discard_attempts blkdiscards failed\n";
+ }
};
# close filehandle before throwing an error
my $err = $@;
@@ -426,24 +451,44 @@ my sub free_lvm_volumes_locked {
}
};
# we need to zero out LVM data for security reasons
- # and to allow thin provisioning
- my $zero_out_worker = sub {
+ # and discard images to free storage space to allow
+ # thin provisioning
+ my $cleanup_worker = sub {
+
for my $name (@$volnames) {
my $lvmpath = "/dev/$vg/del-$name";
- print "zero-out data on image $name ($lvmpath)\n";
+
+ my $discard_action;
+ if ($scfg->{saferemove} && $on_remove_opts->{discard}) {
+ $discard_action = 'zero-out and discard (TRIM)';
+ } elsif ($scfg->{saferemove}) {
+ $discard_action = 'zero-out';
+ } elsif ($on_remove_opts->{discard}) {
+ $discard_action = 'discard (TRIM)';
+ }
+ print "$discard_action 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",
+ errmsg => "can't activate LV '$lvmpath' to $discard_action its data",
);
$cmd_activate = ['/sbin/lvchange', '--refresh', $lvmpath];
run_command(
$cmd_activate,
- errmsg => "can't refresh LV '$lvmpath' to zero-out its data",
+ errmsg => "can't refresh LV '$lvmpath' to $discard_action its data",
);
+ syslog('info', "starting to $discard_action $name ($lvmpath)");
- eval { $secure_delete_cmd->($lvmpath); };
+ eval {
+ if ($scfg->{saferemove}) {
+ $secure_delete_cmd->($lvmpath);
+ }
+
+ if ($on_remove_opts->{discard} && !$scfg->{saferemove}) {
+ run_command(['/sbin/blkdiscard', $lvmpath]);
+ }
+ };
if (my $cleanup_err = $@) {
my $failed_name;
$class->cluster_lock_storage(
@@ -491,13 +536,13 @@ my sub free_lvm_volumes_locked {
}
};
- if ($scfg->{saferemove}) {
+ if ($scfg->{saferemove} || $on_remove_opts->{discard}) {
for my $name (@$volnames) {
# avoid long running task, so we only rename here
my $cmd = ['/sbin/lvrename', $vg, $name, "del-$name"];
run_command($cmd, errmsg => "lvrename '$vg/$name' error");
}
- return $zero_out_worker;
+ return $cleanup_worker;
} else {
for my $name (@$volnames) {
my $cmd = ['/sbin/lvremove', '-f', "$vg/$name"];
@@ -520,6 +565,36 @@ sub plugindata {
};
}
+my $on_volume_remove_format = {
+ discard => {
+ description => "Issue discard (TRIM) requests for LVs before removing them.",
+ type => 'boolean',
+ optional => 1,
+ verbose_description => "If enabled, blkdiscard is issued for the LV before removing it."
+ . " This sends discard (TRIM) requests for the LV's block range, allowing"
+ . " thin-provisioned storage to reclaim previously allocated physical"
+ . " space, provided the storage supports discard.",
+ },
+};
+
+sub verify_on_volume_remove {
+ my ($value, $noerr) = @_;
+
+ return undef if !defined($value);
+
+ if (!keys %$value) {
+ return undef if $noerr;
+ die "at least one on-volume-remove option must be specified if the property is set\n";
+ }
+ return $value;
+}
+
+PVE::JSONSchema::register_format(
+ 'on-volume-remove',
+ $on_volume_remove_format,
+ \&verify_on_volume_remove,
+);
+
sub properties {
return {
vgname => {
@@ -536,6 +611,13 @@ sub properties {
description => "Zero-out data when removing LVs.",
type => 'boolean',
},
+ 'on-volume-remove' => {
+ description => "Optional actions when removing LVs.",
+ type => 'string',
+ format => 'on-volume-remove',
+ verbose_description => "Configure actions performed before removing an LV."
+ . " Use 'discard=1' to issue discard (TRIM) requests before removal.",
+ },
'saferemove-stepsize' => {
description => "Wipe step size in MiB."
. " It will be capped to the maximum supported by the storage.",
@@ -561,6 +643,7 @@ sub options {
shared => { optional => 1 },
disable => { optional => 1 },
saferemove => { optional => 1 },
+ 'on-volume-remove' => { optional => 1 },
'saferemove-stepsize' => { optional => 1 },
saferemove_throughput => { optional => 1 },
content => { optional => 1 },
@@ -583,6 +666,88 @@ sub get_formats {
return { default => 'raw', valid => { 'raw' => 1 } };
}
+my sub get_discard_max {
+ my ($dev_path) = @_;
+
+ my $output = '';
+ # use lsblk as it resolves discard support in setups with
+ # nested partitions or lv/vgs
+ my $cmd = [
+ '/bin/lsblk',
+ '--json',
+ '--bytes',
+ '--discard',
+ '--nodeps',
+ '--output',
+ 'PATH,DISC-MAX',
+ $dev_path,
+ ];
+
+ eval {
+ run_command($cmd, outfunc => sub { $output .= "$_[0]\n"; });
+ };
+ if ($@) {
+ raise_param_exc({
+ 'on-volume-remove' => "discard on remove is enabled, but lsblk could not "
+ . "query discard support for the backing device '$dev_path'",
+ });
+ }
+
+ my $parsed = eval { decode_json($output) };
+ if ($@ || !$parsed->{blockdevices} || !$parsed->{blockdevices}->[0]) {
+ raise_param_exc({
+ 'on-volume-remove' => "discard on remove is enabled, but lsblk could not "
+ . "parse discard support for the backing device '$dev_path'",
+ });
+
+ }
+
+ return $parsed->{blockdevices}->[0]->{'disc-max'} // 0;
+}
+
+sub assert_discard_supported {
+ my ($vgname, $on_remove) = @_;
+
+ return if !defined($on_remove);
+
+ my $on_remove_opts = PVE::JSONSchema::parse_property_string('on-volume-remove', $on_remove);
+ if ($on_remove_opts->{discard}) {
+
+ my $vgs = lvm_vgs(1);
+ my $vg = $vgs->{$vgname};
+ die "no such volume group '$vgname'\n" if !$vg;
+
+ my $pvs = $vg->{pvs};
+ die "volume group '$vgname' has no physical volumes\n"
+ if !defined($pvs) || scalar($pvs->@*) == 0;
+
+ # check if all the block devices configured to the volume group support discard
+ for my $pv ($vg->{pvs}->@*) {
+
+ my $dev_path = abs_path($pv->{name}) // $pv->{name};
+
+ if ($dev_path !~ m!^(/dev/[A-Za-z0-9_+./=-]+)$!) {
+ raise_param_exc({
+ 'on-volume-remove' => "discard on remove is enabled, but discard support "
+ . "cannot be resolved for the backing device '$dev_path'",
+ });
+ } else {
+ $dev_path = $1; # untaint
+ }
+
+ my $discard_max_bytes = get_discard_max($dev_path);
+
+ # use discard_max_bytes as indicator if discard is supported
+ if (!$discard_max_bytes) {
+ raise_param_exc({
+ 'on-volume-remove' => "discard on remove is enabled, but discard is not "
+ . "supported by the backing device '$dev_path'",
+ });
+ }
+ }
+ }
+}
+
sub on_add_hook {
my ($class, $storeid, $scfg, %param) = @_;
@@ -604,6 +769,8 @@ sub on_add_hook {
lvm_create_volume_group($path, $scfg->{vgname}, $scfg->{shared});
}
+ assert_discard_supported($scfg->{vgname}, $scfg->{'on-volume-remove'});
+
return;
}
@@ -624,6 +791,7 @@ sub on_update_hook_full {
die "$storeid - cannot disable 'snapshot-as-volume-chain' while a qcow2 image exists\n"
if grep { $_->{format} eq 'qcow2' } $images->@*;
}
+ assert_discard_supported($scfg->{vgname}, $update->{'on-volume-remove'});
}
sub parse_volname {
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH manager v8 4/5] fix #7339: lvmthick: ui: add UI option to free storage
2026-07-07 14:32 [PATCH docs/manager/storage v8 0/5] fix #7339: lvmthick: add option to free storage for deleted VMs Lukas Sichert
` (2 preceding siblings ...)
2026-07-07 14:32 ` [PATCH storage v8 3/5] fix #7339: lvm: add discard action for removed volumes Lukas Sichert
@ 2026-07-07 14:32 ` Lukas Sichert
2026-07-07 14:32 ` [PATCH docs v8 5/5] fix #7339: lvm: document discard option Lukas Sichert
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Sichert @ 2026-07-07 14:32 UTC (permalink / raw)
To: pve-devel; +Cc: Lukas Sichert
Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
---
www/manager6/Utils.js | 2 +-
www/manager6/storage/LVMEdit.js | 45 +++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 040b5ae0..3088e6a7 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2184,7 +2184,7 @@ Ext.define('PVE.Utils', {
hastart: ['HA', gettext('Start')],
hastop: ['HA', gettext('Stop')],
imgcopy: ['', gettext('Copy data')],
- imgdel: ['', gettext('Erase data')],
+ imgdel: ['', gettext('Destroy image')],
lvmcreate: [gettext('LVM Storage'), gettext('Create')],
lvmremove: ['Volume Group', gettext('Remove')],
lvmthincreate: [gettext('LVM-Thin Storage'), gettext('Create')],
diff --git a/www/manager6/storage/LVMEdit.js b/www/manager6/storage/LVMEdit.js
index 148f0601..30ae275b 100644
--- a/www/manager6/storage/LVMEdit.js
+++ b/www/manager6/storage/LVMEdit.js
@@ -148,6 +148,39 @@ Ext.define('PVE.storage.LVMInputPanel', {
onlineHelp: 'storage_lvm',
+ onGetValues: function (values) {
+ let me = this;
+
+ let onRemove = {};
+ if (values['on-remove-discard']) {
+ onRemove.discard = 1;
+ }
+ delete values['on-remove-discard'];
+
+ let onRemoveString = PVE.Parser.printPropertyString(onRemove);
+ if (onRemoveString !== '') {
+ values['on-volume-remove'] = onRemoveString;
+ } else if (!me.isCreate) {
+ if (!values.delete) {
+ values.delete = [];
+ }
+ values.delete.push('on-volume-remove');
+ }
+
+ return me.callParent([values]);
+ },
+
+ setValues: function (values) {
+ if (values['on-volume-remove']) {
+ let onRemove = PVE.Parser.parsePropertyString(values['on-volume-remove']);
+ values['on-remove-discard'] = onRemove.discard;
+ }
+
+ delete values['on-volume-remove'];
+
+ return this.callParent([values]);
+ },
+
column1: [
{
xtype: 'pveBaseStorageSelector',
@@ -241,5 +274,17 @@ Ext.define('PVE.storage.LVMInputPanel', {
uncheckedValue: 0,
fieldLabel: gettext('Wipe Removed Volumes'),
},
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'on-remove-discard',
+ uncheckedValue: 0,
+ fieldLabel: gettext('Discard Removed Volumes'),
+ autoEl: {
+ tag: 'div',
+ 'data-qtip': gettext(
+ 'Enable to issue discard (TRIM) requests for logical volumes before removing them.',
+ ),
+ },
+ },
],
});
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH docs v8 5/5] fix #7339: lvm: document discard option
2026-07-07 14:32 [PATCH docs/manager/storage v8 0/5] fix #7339: lvmthick: add option to free storage for deleted VMs Lukas Sichert
` (3 preceding siblings ...)
2026-07-07 14:32 ` [PATCH manager v8 4/5] fix #7339: lvmthick: ui: add UI option to free storage Lukas Sichert
@ 2026-07-07 14:32 ` Lukas Sichert
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Sichert @ 2026-07-07 14:32 UTC (permalink / raw)
To: pve-devel; +Cc: Lukas Sichert
Document the new `on-volume-remove` property for LVM storage and its
initial `discard` action.
Also update the `saferemove` description to match the range-based
zero-out worker and avoid referring to the old command-specific
implementation details.
Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
Link: bugzilla.proxmox.com/show_bug.cgi?id=7339
---
pve-storage-lvm.adoc | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/pve-storage-lvm.adoc b/pve-storage-lvm.adoc
index ba78663..3ddb26c 100644
--- a/pve-storage-lvm.adoc
+++ b/pve-storage-lvm.adoc
@@ -44,18 +44,39 @@ accessed by other LVs created later (which happen to be assigned the same
physical extents). This is a costly operation, but may be required as a security
measure in certain environments.
+
-Storage devices that support the "write zeroes" operation will use `blkdiscard`
-to zero blocks. Otherwise, a fallback to `cstream` is performed.
+Storage devices that support the "write zeroes" operation use it to zero blocks.
+Otherwise, zeroes are written manually. The volume is processed range by range,
+according to `saferemove-stepsize`.
+
+`on-volume-remove`::
+
+Configure additional actions to run before an LV is removed.
++
+Set `discard=1` to issue discard (TRIM) requests for the LV's blocks before the
+LV is removed, so thin-provisioned backing storage, such as a SAN LUN, can
+reclaim space the LV occupied. This is called "Discard Removed Volumes"
+in the web UI. Discard is rejected if any backing device in the
+LV's volume group is detected as not supporting it.
++
+If wiping or discarding fails, the renamed `del-*` LV is kept and renamed to
+`failed-<N>-del-*`, so one can inspect it, retry cleanup manually, or remove
+it explicitly.
++
+If `saferemove` and `discard` are both enabled, the LV is processed range by
+range: one range is zeroed out and then discarded before continuing with the
+next range. This avoids allocating the whole LV with zeroes on thin-provisioned
+backing storage before the space can be reclaimed again.
`saferemove-stepsize`::
-Wipe step size in MiB (`blkdiscard -p` parameter value), capped to the maximum
-step size supported by the underlying storage. Up to 32 MiB (maximum) by
-default.
+Wipe step size in MiB, capped to the maximum step size supported by the
+underlying storage. Up to 32 MiB (maximum) by default.
`saferemove_throughput`::
-Wipe throughput (`cstream -t` parameter value), up to 10 MiB/s by default.
+Limits wipe throughput. If the backing storage supports the "write zeroes"
+operation, throughput is unlimited by default. Otherwise, manually written
+zeroes are limited to 10 MiB/s by default.
`snapshot-as-volume-chain`::
@@ -150,4 +171,3 @@ See Also
endif::wiki[]
-
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread