Re: [PATCH] f2fs/008: test snapshot creation/deletion on lvm device

[Chao Yu <chao@kernel.org>]

On 2024/12/19 21:16, Zorro Lang wrote:
> On Mon, Dec 02, 2024 at 07:43:07PM +0800, Chao Yu wrote:
>> This patch introduce a regression testcase to check whether
>> f2fs can handle discard correctly once underlying lvm device
>> changes to not support discard after user creates snapshot
>> on it.
>>
>> Related bug was fixed by commit bc8aeb04fd80 ("f2fs: fix to
>> drop all discards after creating snapshot on lvm device")
>>
>> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   tests/f2fs/008     | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/f2fs/008.out |  2 ++
>>   2 files changed, 55 insertions(+)
>>   create mode 100755 tests/f2fs/008
>>   create mode 100644 tests/f2fs/008.out
>>
>> diff --git a/tests/f2fs/008 b/tests/f2fs/008
>> new file mode 100755
>> index 00000000..a9a3f263
>> --- /dev/null
>> +++ b/tests/f2fs/008
>> @@ -0,0 +1,53 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 Oppo.  All Rights Reserved.
>> +#
>> +# FS QA Test No. f2fs/008
>> +#
>> +# This is a regression test to check whether f2fs can handle
>> +# discard correctly once underlying lvm device changes to not
>> +# support discard after user creates snapshot on it.
>> +#
>> +
>> +. ./common/preamble
>> +_begin_fstest auto quick rw
>> +
>> +_fixed_by_kernel_commit bc8aeb04fd80 \
>> +        "f2fs: fix to drop all discards after creating snapshot on lvm device"
>> +
>> +_require_scratch_nolvm
>> +_require_block_device $SCRATCH_DEV
>> +_require_command "$LVM_PROG" lvm
>> +
>> +lvname=lv_$sed
>> +vgname=vg_$Sseg
> 
> What's "$sed" and "$Sseg", I didn't find the definition of them.

Zorro,

Sorry, these are typos caused by copying codes, I have noticed this and
fixed it in v2.

https://lore.kernel.org/fstests/20241203124657.4997-1-chao@kernel.org

> 
>> +testfile=$SCRATCH_MNT/testfile
>> +
>> +_cleanup()
>> +{
> 
>    _unmount $SCRATCH_MNT >>$seqres.full 2>&1
> 
> for this test might be interrupted.
> 
>> +	$LVM_PROG lvremove -f /dev/mapper/$lvname-snapshot $vgname >>$seqres.full 2>&1
>> +	$LVM_PROG lvremove -f /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
>> +	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
>> +	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
> 
> "-ff" might be better for cleanup.
> 
> According to 5f7a66b6dc4c ("fstests: use udevadm wait in preference to settle"), Dave
> recommend doing below command after above lines:

Thank you for this information. :)

> 
>    _udev_wait --removed /dev/mapper/$vgname-$lvname
> 
> Then, don't forget the default cleanup steps:
> 
>    cd /
>    rm -f $tmp.*
> 
>> +}
>> +
>> +$LVM_PROG pvcreate -f $SCRATCH_DEV >>$seqres.full 2>&1
>> +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
>> +$LVM_PROG lvcreate -L 1024m -n $lvname $vgname >>$seqres.full 2>&1
> 
> Also according to 5f7a66b6dc4c ("fstests: use udevadm wait in preference to settle"),
> Dave recommend using a:
>    _udev_wait /dev/mapper/$vgname-$lvname
> 
>> +
>> +$MKFS_F2FS_PROG /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
> 
> "-f" to force overwrite ? Or use _mkfs_dev? But _mkfs_dev doesn't has a f2f2
> specific line, maybe you'd like to write one.

Sure, maybe adapting f2fs in a separated patch, anyway, let me check this.

> 
>> +mount -o discard /dev/mapper/$vgname-$lvname $SCRATCH_MNT >>$seqres.full 2>&1
> 
> _mount
> 
>> +
>> +dd if=/dev/zero of=$testfile bs=1M count=20 >>$seqres.full 2>&1
>> +sync
>> +rm $testfile
> 
> "-f" ?
> 
>> +sync
>> +
>> +# create a snapshot on lvm device
>> +$LVM_PROG lvcreate -L 1024m -s -n $lvname-snapshot /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
>> +umount $SCRATCH_MNT
> 
> Move this line to _cleanup, to always try to do it before removing lvm.

Will update in v3 according to all your comments.

Thanks,

> 
>> +
>> +echo "Silence is golden"
>> +
>> +status=0
>> +exit
>> diff --git a/tests/f2fs/008.out b/tests/f2fs/008.out
>> new file mode 100644
>> index 00000000..dd68d5a4
>> --- /dev/null
>> +++ b/tests/f2fs/008.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 008
>> +Silence is golden
>> -- 
>> 2.40.1
>>
>