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 CB1C41FF0E1 for ; Mon, 13 Jul 2026 18:00:32 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2FB3C21494; Mon, 13 Jul 2026 18:00:25 +0200 (CEST) From: Lukas Sichert To: pve-devel@lists.proxmox.com Subject: [PATCH storage v9 2/5] lvm: saferemove: zero out volumes range by range Date: Mon, 13 Jul 2026 18:00:03 +0200 Message-ID: <20260713160008.121125-3-l.sichert@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260713160008.121125-1-l.sichert@proxmox.com> References: <20260713160008.121125-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: 1783958404594 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.286 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) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust 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: OTIWWEXE6RPFBTLA63EJQ3DPVF4TQBKN X-Message-ID-Hash: OTIWWEXE6RPFBTLA63EJQ3DPVF4TQBKN 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: '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 syswrites to 10 MiB/s. Redefine saferemove_throughput as integer instead of string to catch configuration errors with the schema already. Signed-off-by: Lukas Sichert --- src/PVE/Storage/LVMPlugin.pm | 162 +++++++++++++++++++++++++---------- 1 file changed, 118 insertions(+), 44 deletions(-) diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index 478a90b..d8b5c8c 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -4,11 +4,13 @@ use strict; use warnings; use Cwd qw(abs_path); +use Fcntl qw(SEEK_SET); use File::Basename; use IO::File; use JSON; use List::Util qw(max); +use PVE::Format qw(render_bytes render_duration); use PVE::JSONSchema qw(get_standard_option); use PVE::RESTEnvironment qw(log_warn); use PVE::SafeSyslog; @@ -21,6 +23,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 @@ -320,6 +326,13 @@ my sub cleanup_failed_lvm_volume { return $failed_name; } +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) = @_; @@ -345,56 +358,117 @@ 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 from $sysdir cannot be read\n"; + ($size) = $size =~ m/^(\d+)$/; # untaint + $size *= 512; # sysfs size is in 512-byte sectors + + my $zeroout_variant = 'blkzeroout'; + my $throughput = undef; + if ($scfg->{saferemove_throughput}) { + # use abs as legacy cstream accepted negative values + $throughput = abs($scfg->{saferemove_throughput}); + my $rendered_throughput = render_bytes($throughput); + print "using saferemove throughput limit: $rendered_throughput/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 (!defined($throughput)) { + # FIXME: MAJOR VERSION: increase to 100 MiB/s + $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", - ]; - # FIXME: handle cstream's expected ENOSPC failure explicitly and let other - # errors propagate. For now, preserve the old behavior where cstream can - # fail successfully with ENOSPC after writing until the device is full. - eval { - run_command( - $cmd, - errmsg => "zero out finished (note: 'No space left on device' is ok here)", - ); - }; - warn $@ if $@; - } 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; - $stepsize = $write_zeroes_max_bytes; - } + for (my $offset = 0; $offset < $size; $offset += $written) { + + if ($offset + $stepsize > $size) { + $stepsize = $size - $offset; + } - my $cmd = ['blkdiscard', $lvmpath, '-v', '--zeroout', '--step', "${stepsize}"]; - run_command($cmd); + 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"; + + while ($written < $stepsize) { + my $remaining = $stepsize - $written; + my $retried_write = syswrite($fh, $zeroes, $remaining) + or die "syswrite failed: $!\n"; + $written += $retried_write; + } + + } + $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 (defined($throughput)) { + my $expected_elapsed = $written_total / $throughput; + my $actual_elapsed = $curr_time - $start; + my $delay = $expected_elapsed - $actual_elapsed; + if ($delay > 0) { + sleep($delay); + } + } + } + }; + # 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 + # we need to zero out LVM data for security reasons and to allow thin provisioning my $zero_out_worker = sub { for my $name (@$volnames) { my $lvmpath = "/dev/$vg/del-$name"; @@ -494,8 +568,8 @@ sub properties { type => 'integer', }, saferemove_throughput => { - description => "Wipe throughput (cstream -t parameter value).", - type => 'string', + description => "Wipe throughput in bytes.", + type => 'integer', }, tagged_only => { description => "Only list logical volumes tagged with 'pve-vm-ID'.", -- 2.47.3