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

[Chao Yu <chao@kernel.org>]

On 2024/12/27 15:21, Zorro Lang wrote:
> On Thu, Dec 26, 2024 at 09:18:36PM +0800, Chao Yu wrote:
>> On 2024/12/24 17:58, Zorro Lang wrote:
>>> On Sat, Dec 21, 2024 at 04:23:45PM +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>
>>>> ---
>>>> v3:
>>>> - rebase to last for-next branch
>>>> - update according to Zorro's comments.
>>>>    tests/f2fs/008     | 57 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>    tests/f2fs/008.out |  2 ++
>>>>    2 files changed, 59 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..b85e321c
>>>> --- /dev/null
>>>> +++ b/tests/f2fs/008
>>>> @@ -0,0 +1,57 @@
>>>> +#! /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_$seq
>>>> +vgname=vg_$seq
>>>> +testfile=$SCRATCH_MNT/testfile
>>>> +
>>>> +_cleanup()
>>>> +{
>>>> +	_unmount $SCRATCH_MNT >>$seqres.full 2>&1
>>>> +	$LVM_PROG lvremove -ff /dev/mapper/$lvname-snapshot $vgname >>$seqres.full 2>&1
>>>> +	$LVM_PROG lvremove -ff /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
>>>> +	$LVM_PROG vgremove -ff $vgname >>$seqres.full 2>&1
>>>> +	$LVM_PROG pvremove -ff $SCRATCH_DEV >>$seqres.full 2>&1
>>>> +	_udev_wait --removed /dev/mapper/$vgname-$lvname
>>>> +	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
>>>
>>> The lvcreate might be blocked by:
>>>     "WARNING: f2fs signature detected on /dev/vg_008/lv_008 at offset 1024. Wipe it? [y/n]:"
>>
>> Oh, I didn't encounter this... is it due to I use low version lvm tool?
> 
> I tried to run this test twice, the second test hang there, then I found it.

I tested such case as well, but it didn't hang the testcase.

> 
>>
>> lvm version
>>    LVM version:     2.03.11(2) (2021-01-08)
>>    Library version: 1.02.175 (2021-01-08)
>>    Driver version:  4.48.0
>>
>>>
>>> So better to use "lvcreate -y" or "yes | ...".
> 
> `lvcreate -y` is good, but I'm not sure which lvm version starts to support
> this option. So the "yes | ..." might have better compatibility.

Alright...

Thanks,

> 
> Thanks,
> Zorro
> 
>>
>> Sure.
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>> Zorro
>>>
>>>> +_udev_wait /dev/mapper/$vgname-$lvname
>>>> +
>>>> +_mkfs_dev /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
>>>> +_mount -o discard /dev/mapper/$vgname-$lvname $SCRATCH_MNT >>$seqres.full 2>&1
>>>> +
>>>> +dd if=/dev/zero of=$testfile bs=1M count=20 >>$seqres.full 2>&1
>>>> +sync
>>>> +rm -f $testfile
>>>> +sync
>>>> +
>>>> +# create a snapshot on lvm device
>>>> +$LVM_PROG lvcreate -L 1024m -s -n $lvname-snapshot /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
>>>> +
>>>> +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
>>>>
>>>
>>
>