[pve-devel] [PATCH V3 pve-storage] lvm: use blkdiscard instead cstream to saferemove drive

[Fiona Ebner]

Am 20.10.25 um 4:33 PM schrieb Fiona Ebner:
> Am 20.10.25 um 9:56 AM schrieb DERUMIER, Alexandre:
>> Hi Fiona, I'm going to fix all your comment for this patch
>>
>>
>>> +                my $write_zeroes_max_bytes =
>>> +                   
>>> file_read_firstline("$sysdir/queue/write_zeroes_max_bytes") // 0;
>>
>>>> According to the docs: "The value 0 means that REQ_OP_WRITE_ZEROES
>>>> is not supported.", so it makes sense as a fallback too. But do we
>>>> need
>>>> to handle that somehow below, i.e. can --zeroout still be used if the
>>>> reported value is 0? Or do we need to fall back to the old way of
>>>> doing it?
>>
>> I don't have read the code of blkdiscard yet (and don't have found
>> device without write zeroes support to test), but according to redhat
>> doc
>> https://www.ovirt.org/develop/release-management/features/storage/wipe-volumes-using-blkdiscard.html
>>
>> "There are no restrictions when using blkdiscard -z, although
>> blkdiscard performs roughly the same as dd if the storage does not
>> support write same."
>>
> 
> At a glance, the implementation of blkdiscard doesn't seem to have a
> fallback and will just exit with error if the BLKZEROOUT ioctl() fails:
> https://github.com/util-linux/util-linux/blob/stable/v2.41/sys-utils/blkdiscard.c#L303

I do have a device which reports 0, but the ioctl() does not fail for it:

root at enia / # cat /sys/block/dm-0/queue/write_zeroes_max_bytes
0
root at enia / # blkdiscard -z /dev/dm-0
root at enia / # strace blkdiscard -z /dev/dm-0 2>&1 | grep BLKZEROOUT
ioctl(3, BLKZEROOUT, [0, 1073741824])   = 0

Now looking into the kernel there is:

 *  If %flags contains BLKDEV_ZERO_NOFALLBACK, the function will return
 *  -EOPNOTSUPP if no explicit hardware offload for zeroing is provided.
 */
int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
		sector_t nr_sects, gfp_t gfp_mask, struct bio **biop,
		unsigned flags)

and blk_ioctl_zeroout() calls blk_ioctl_zeroout() with flags
BLKDEV_ZERO_NOUNMAP | BLKDEV_ZERO_KILLABLE, so AFAICT, there is a
fallback and the fallback will be used :)