From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 09A881FF165 for ; Thu, 14 Aug 2025 11:36:10 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 86167D550; Thu, 14 Aug 2025 11:37:48 +0200 (CEST) Date: Thu, 14 Aug 2025 11:37:44 +0200 From: Wolfgang Bumiller To: Proxmox VE development discussion Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1755164231861 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.075 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH pve-storage] lvm: use blkdiscard instead cstream to saferemove drive X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On Thu, Aug 14, 2025 at 11:24:48AM +0200, Alexandre Derumier via pve-devel wrote: > From: Alexandre Derumier > To: pve-devel@lists.proxmox.com > Subject: [PATCH pve-storage] lvm: use blkdiscard instead cstream to > saferemove drive > Date: Thu, 14 Aug 2025 11:24:48 +0200 > Message-ID: <20250814092448.919114-1-alexandre.derumier@groupe-cyllene.com> > X-Mailer: git-send-email 2.47.2 > > Current cstream implementation is pretty slow as hell, even without throttling. > > use blkdiscard --zeroout instead, which is lot of magnetude faster Makes sense, given that it probably uses dedicated zero-out `ioctls()`. > > Another benefit is that blkdiscard is skipping already zeroed block, so for empty > temp images like snapshot, is pretty fast. > > blkdiscard don't have throttling like cstream, but we can tune the step size > of zeroes pushed to the storage. > I'm using 32MB stepsize by default , like ovirt, where it seem to be the best > balance between speed and load. > https://github.com/oVirt/vdsm/commit/79f1d79058aad863ca4b6672d4a5ce2be8e48986 > > but it can be reduce with "saferemove_stepsize" option. > > Also adding an option "saferemove_discard", to use discard instead zeroing. > > test with a 100G volume (empty): > > time /usr/bin/cstream -i /dev/zero -o /dev/test/vm-100-disk-0.qcow2 -T 10 -v 1 -b 1048576 > > 13561233408 B 12.6 GB 10.00 s 1356062979 B/s 1.26 GB/s > 26021462016 B 24.2 GB 20.00 s 1301029969 B/s 1.21 GB/s > 38585499648 B 35.9 GB 30.00 s 1286135343 B/s 1.20 GB/s > 50998542336 B 47.5 GB 40.00 s 1274925312 B/s 1.19 GB/s > 63702765568 B 59.3 GB 50.00 s 1274009877 B/s 1.19 GB/s > 76721885184 B 71.5 GB 60.00 s 1278640698 B/s 1.19 GB/s > 89126539264 B 83.0 GB 70.00 s 1273178488 B/s 1.19 GB/s > 101666459648 B 94.7 GB 80.00 s 1270779024 B/s 1.18 GB/s > 107390959616 B 100.0 GB 84.39 s 1272531142 B/s 1.19 GB/s > write: No space left on device > > real 1m24.394s > user 0m0.171s > sys 1m24.052s > > time blkdiscard --zeroout /dev/test/vm-100-disk-0.qcow2 -v > /dev/test/vm-100-disk-0.qcow2: Zero-filled 107390959616 bytes from the offset 0 > > real 0m3.641s > user 0m0.001s > sys 0m3.433s > > test with a 100G volume with random data: > > time blkdiscard --zeroout /dev/test/vm-100-disk-0.qcow2 -v > > /dev/test/vm-112-disk-1: Zero-filled 4764729344 bytes from the offset 0 > /dev/test/vm-112-disk-1: Zero-filled 4664066048 bytes from the offset 4764729344 > /dev/test/vm-112-disk-1: Zero-filled 4831838208 bytes from the offset 9428795392 > /dev/test/vm-112-disk-1: Zero-filled 4831838208 bytes from the offset 14260633600 > /dev/test/vm-112-disk-1: Zero-filled 4831838208 bytes from the offset 19092471808 > /dev/test/vm-112-disk-1: Zero-filled 4865392640 bytes from the offset 23924310016 > /dev/test/vm-112-disk-1: Zero-filled 4596957184 bytes from the offset 28789702656 > /dev/test/vm-112-disk-1: Zero-filled 4731174912 bytes from the offset 33386659840 > /dev/test/vm-112-disk-1: Zero-filled 4294967296 bytes from the offset 38117834752 > /dev/test/vm-112-disk-1: Zero-filled 4664066048 bytes from the offset 42412802048 > /dev/test/vm-112-disk-1: Zero-filled 4697620480 bytes from the offset 47076868096 > /dev/test/vm-112-disk-1: Zero-filled 4664066048 bytes from the offset 51774488576 > /dev/test/vm-112-disk-1: Zero-filled 4261412864 bytes from the offset 56438554624 > /dev/test/vm-112-disk-1: Zero-filled 4362076160 bytes from the offset 60699967488 > /dev/test/vm-112-disk-1: Zero-filled 4127195136 bytes from the offset 65062043648 > /dev/test/vm-112-disk-1: Zero-filled 4328521728 bytes from the offset 69189238784 > /dev/test/vm-112-disk-1: Zero-filled 4731174912 bytes from the offset 73517760512 > /dev/test/vm-112-disk-1: Zero-filled 4026531840 bytes from the offset 78248935424 > /dev/test/vm-112-disk-1: Zero-filled 4194304000 bytes from the offset 82275467264 > /dev/test/vm-112-disk-1: Zero-filled 4664066048 bytes from the offset 86469771264 > /dev/test/vm-112-disk-1: Zero-filled 4395630592 bytes from the offset 91133837312 > /dev/test/vm-112-disk-1: Zero-filled 3623878656 bytes from the offset 95529467904 > /dev/test/vm-112-disk-1: Zero-filled 4462739456 bytes from the offset 99153346560 > /dev/test/vm-112-disk-1: Zero-filled 3758096384 bytes from the offset 103616086016 > > real 0m23.969s > user 0m0.030s > sys 0m0.144s > > Signed-off-by: Alexandre Derumier > --- > src/PVE/Storage/LVMPlugin.pm | 43 ++++++++++++------------------------ > 1 file changed, 14 insertions(+), 29 deletions(-) > > diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm > index 0416c9e..5ea9d8b 100644 > --- a/src/PVE/Storage/LVMPlugin.pm > +++ b/src/PVE/Storage/LVMPlugin.pm > @@ -287,35 +287,15 @@ my sub free_lvm_volumes { > # we need to zero out LVM data for security reasons > # and to allow thin provisioning > my $zero_out_worker = sub { > - # 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}; > - } > for my $name (@$volnames) { > print "zero-out data on image $name (/dev/$vg/del-$name)\n"; > - > + my $stepsize = $scfg->{saferemove_stepsize} // 32; > my $cmd = [ > - '/usr/bin/cstream', > - '-i', > - '/dev/zero', > - '-o', > - "/dev/$vg/del-$name", > - '-T', > - '10', > - '-v', > - '1', > - '-b', > - '1048576', > - '-t', > - "$throughput", > + '/usr/sbin/blkdiscard', "/dev/$vg/del-$name", '-v', '--step', "${stepsize}M", > ]; > - eval { > - run_command( > - $cmd, > - errmsg => "zero out finished (note: 'No space left on device' is ok here)", > - ); > - }; > + push @$cmd, '--zeroout' if !$scfg->{saferemove_discard}; > + > + eval { run_command($cmd); }; > warn $@ if $@; > > $class->cluster_lock_storage( > @@ -376,9 +356,13 @@ sub properties { > description => "Zero-out data when removing LVs.", > type => 'boolean', > }, > - saferemove_throughput => { > - description => "Wipe throughput (cstream -t parameter value).", > - type => 'string', We do still need to keep the old option around to not break existing configs. We should document its deprecation and add a warning if it is set. > + saferemove_stepsize => { > + description => "Wipe step size (default 32MB).", > + enum => [qw(1 2 4 8 16 32)], > + }, > + saferemove_discard => { > + description => "Wipe with discard instead zeroing.", > + type => 'boolean', Not sure we need this (not sure when this is actually useful), but it's cheap enough to have around. Should add a `default => 0` for documentation purposes, though. > }, > tagged_only => { > description => "Only use logical volumes tagged with 'pve-vm-ID'.", > @@ -394,7 +378,8 @@ sub options { > shared => { optional => 1 }, > disable => { optional => 1 }, > saferemove => { optional => 1 }, > - saferemove_throughput => { optional => 1 }, > + saferemove_discard => { optional => 1 }, > + saferemove_stepsize => { optional => 1 }, > content => { optional => 1 }, > base => { fixed => 1, optional => 1 }, > tagged_only => { optional => 1 }, > -- > 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel