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

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

[Chao Yu]

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
+testfile=$SCRATCH_MNT/testfile
+
+_cleanup()
+{
+	$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
+}
+
+$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
+
+$MKFS_F2FS_PROG /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 $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
+umount $SCRATCH_MNT
+
+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

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

[Zorro Lang]

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.

> +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:

  _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.

> +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.

> +
> +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
> 

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

[Chao Yu]

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
>>
>