[PATCH v2 0/2] fstests: new test cases for generic group

[PATCH v2 0/2] fstests: new test cases for generic group

[Anand Jain]

v2: Fixes as per review comments on the individual patches.

Patch 1/2 relocates a tempfsid (clone device) test case from btrfs group
  to the generic group.
Patch 2/2 validates a recently discovered and resolved bug in Btrfs;
  however, the test case can be made generic.


Anand Jain (2):
  shared: move btrfs clone device testcase to the shared group
  generic: test mount fails on physical device with configured dm volume

 common/rc             | 14 +++++++
 tests/btrfs/312       | 78 -------------------------------------
 tests/btrfs/312.out   | 19 ---------
 tests/generic/741     | 60 +++++++++++++++++++++++++++++
 tests/generic/741.out |  3 ++
 tests/shared/001      | 89 +++++++++++++++++++++++++++++++++++++++++++
 tests/shared/001.out  |  4 ++
 7 files changed, 170 insertions(+), 97 deletions(-)
 delete mode 100755 tests/btrfs/312
 delete mode 100644 tests/btrfs/312.out
 create mode 100755 tests/generic/741
 create mode 100644 tests/generic/741.out
 create mode 100755 tests/shared/001
 create mode 100644 tests/shared/001.out

-- 
2.39.3

[PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[Anand Jain]

Given that ext4 also allows mounting of a cloned filesystem, the btrfs
test case btrfs/312, which assesses the functionality of cloned filesystem
support, can be refactored to be under the shared group.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2:
Move to shared testcase instead of generic.
Add _require_block_device $TEST_DEV.
Add _require_duplicated_fsid.

 common/rc            | 14 +++++++
 tests/btrfs/312      | 78 --------------------------------------
 tests/btrfs/312.out  | 19 ----------
 tests/shared/001     | 89 ++++++++++++++++++++++++++++++++++++++++++++
 tests/shared/001.out |  4 ++
 5 files changed, 107 insertions(+), 97 deletions(-)
 delete mode 100755 tests/btrfs/312
 delete mode 100644 tests/btrfs/312.out
 create mode 100755 tests/shared/001
 create mode 100644 tests/shared/001.out

diff --git a/common/rc b/common/rc
index 36cad89cfc5d..2638dfb8e9b3 100644
--- a/common/rc
+++ b/common/rc
@@ -5408,6 +5408,20 @@ _random_file() {
 	echo "$basedir/$(ls -U $basedir | shuf -n 1)"
 }
 
+_require_duplicate_fsid()
+{
+	case "$FSTYP" in
+	"btrfs")
+		_require_btrfs_fs_feature temp_fsid
+		;;
+	"ext4")
+		;;
+	*)
+		_notrun "$FSTYP cannot support mounting with duplicate fsid"
+		;;
+	esac
+}
+
 init_rc
 
 ################################################################################
diff --git a/tests/btrfs/312 b/tests/btrfs/312
deleted file mode 100755
index eedcf11a2308..000000000000
--- a/tests/btrfs/312
+++ /dev/null
@@ -1,78 +0,0 @@
-#! /bin/bash
-# SPDX-License-Identifier: GPL-2.0
-# Copyright (c) 2024 Oracle.  All Rights Reserved.
-#
-# FS QA Test 312
-#
-# On a clone a device check to see if tempfsid is activated.
-#
-. ./common/preamble
-_begin_fstest auto quick clone tempfsid
-
-_cleanup()
-{
-	cd /
-	$UMOUNT_PROG $mnt1 > /dev/null 2>&1
-	rm -r -f $tmp.*
-	rm -r -f $mnt1
-}
-
-. ./common/filter.btrfs
-. ./common/reflink
-
-_supported_fs btrfs
-_require_scratch_dev_pool 2
-_scratch_dev_pool_get 2
-_require_btrfs_fs_feature temp_fsid
-
-mnt1=$TEST_DIR/$seq/mnt1
-mkdir -p $mnt1
-
-create_cloned_devices()
-{
-	local dev1=$1
-	local dev2=$2
-
-	echo -n Creating cloned device...
-	_mkfs_dev -fq -b $((1024 * 1024 * 300)) $dev1
-
-	_mount $dev1 $SCRATCH_MNT
-
-	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
-								_filter_xfs_io
-	$UMOUNT_PROG $SCRATCH_MNT
-	# device dump of $dev1 to $dev2
-	dd if=$dev1 of=$dev2 bs=300M count=1 conv=fsync status=none || \
-							_fail "dd failed: $?"
-	echo done
-}
-
-mount_cloned_device()
-{
-	echo ---- $FUNCNAME ----
-	create_cloned_devices ${SCRATCH_DEV_NAME[0]} ${SCRATCH_DEV_NAME[1]}
-
-	echo Mounting original device
-	_mount ${SCRATCH_DEV_NAME[0]} $SCRATCH_MNT
-	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
-								_filter_xfs_io
-	check_fsid ${SCRATCH_DEV_NAME[0]}
-
-	echo Mounting cloned device
-	_mount ${SCRATCH_DEV_NAME[1]} $mnt1 || \
-				_fail "mount failed, tempfsid didn't work"
-
-	echo cp reflink must fail
-	_cp_reflink $SCRATCH_MNT/foo $mnt1/bar 2>&1 | \
-						_filter_testdir_and_scratch
-
-	check_fsid ${SCRATCH_DEV_NAME[1]}
-}
-
-mount_cloned_device
-
-_scratch_dev_pool_put
-
-# success, all done
-status=0
-exit
diff --git a/tests/btrfs/312.out b/tests/btrfs/312.out
deleted file mode 100644
index b7de6ce3cc6e..000000000000
--- a/tests/btrfs/312.out
+++ /dev/null
@@ -1,19 +0,0 @@
-QA output created by 312
----- mount_cloned_device ----
-Creating cloned device...wrote 9000/9000 bytes at offset 0
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-done
-Mounting original device
-wrote 9000/9000 bytes at offset 0
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-On disk fsid:		FSID
-Metadata uuid:		FSID
-Temp fsid:		FSID
-Tempfsid status:	0
-Mounting cloned device
-cp reflink must fail
-cp: failed to clone 'TEST_DIR/312/mnt1/bar' from 'SCRATCH_MNT/foo': Invalid cross-device link
-On disk fsid:		FSID
-Metadata uuid:		FSID
-Temp fsid:		TEMPFSID
-Tempfsid status:	1
diff --git a/tests/shared/001 b/tests/shared/001
new file mode 100755
index 000000000000..3f2b85a41099
--- /dev/null
+++ b/tests/shared/001
@@ -0,0 +1,89 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle. All Rights Reserved.
+#
+# FS QA Test 001
+#
+# Set up a filesystem, create a clone, mount both, and verify if the cp reflink
+# operation between these two mounts fails.
+#
+. ./common/preamble
+_begin_fstest auto quick clone volume tempfsid
+
+_cleanup()
+{
+	cd /
+	rm -r -f $tmp.*
+
+	$UMOUNT_PROG $mnt2 &> /dev/null
+	rm -r -f $mnt2
+	_destroy_loop_device $loop_dev2 &> /dev/null
+	rm -r -f $loop_file2
+
+	$UMOUNT_PROG $mnt1 &> /dev/null
+	rm -r -f $mnt1
+	_destroy_loop_device $loop_dev1 &> /dev/null
+	rm -r -f $loop_file1
+}
+
+. ./common/filter
+. ./common/reflink
+
+# Modify as appropriate.
+_supported_fs btrfs ext4
+_require_duplicate_fsid
+_require_cp_reflink
+_require_test
+_require_block_device $TEST_DEV
+_require_loop
+
+[[ $FSTYP == "btrfs" ]] && _require_btrfs_fs_feature temp_fsid
+
+clone_filesystem()
+{
+	local dev1=$1
+	local dev2=$2
+
+	_mkfs_dev $dev1
+
+	_mount $dev1 $mnt1
+	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $mnt1/foo >> $seqres.full
+	$UMOUNT_PROG $mnt1
+
+	# device dump of $dev1 to $dev2
+	dd if=$dev1 of=$dev2 conv=fsync status=none || _fail "dd failed: $?"
+}
+
+mnt1=$TEST_DIR/$seq/mnt1
+rm -r -f $mnt1
+mkdir -p $mnt1
+
+mnt2=$TEST_DIR/$seq/mnt2
+rm -r -f $mnt2
+mkdir -p $mnt2
+
+loop_file1="$TEST_DIR/$seq/image1"
+rm -r -f $loop_file1
+truncate -s 300m "$loop_file1"
+loop_dev1=$(_create_loop_device "$loop_file1")
+
+loop_file2="$TEST_DIR/$seq/image2"
+rm -r -f $loop_file2
+truncate -s 300m "$loop_file2"
+loop_dev2=$(_create_loop_device "$loop_file2")
+
+clone_filesystem ${loop_dev1} ${loop_dev2}
+
+# Mounting original device
+_mount $loop_dev1 $mnt1
+$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $mnt1/foo | _filter_xfs_io
+
+# Mounting cloned device
+_mount $loop_dev2 $mnt2 || _fail "mount of cloned device failed"
+
+# cp reflink across two different filesystems must fail
+_cp_reflink $mnt1/foo $mnt2/bar 2>&1 | _filter_test_dir
+
+# success, all done
+status=0
+exit
diff --git a/tests/shared/001.out b/tests/shared/001.out
new file mode 100644
index 000000000000..56b697ca3972
--- /dev/null
+++ b/tests/shared/001.out
@@ -0,0 +1,4 @@
+QA output created by 001
+wrote 9000/9000 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+cp: failed to clone 'TEST_DIR/001/mnt2/bar' from 'TEST_DIR/001/mnt1/foo': Invalid cross-device link
-- 
2.39.3

[PATCH v2 2/2] generic: test mount fails on physical device with configured dm volume

[Anand Jain]

When a dm Flakey device is configured, (or similar dm where both physical
and dm devices are accessible) we have access to both the physical device
and the dm flakey device, ensure that the physical device mount fails.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
Remove redirect for rm
Add _require_test
Add _filter_error_mount
Change log - make it more sound generic with a dm device
Add quick group

 tests/generic/741     | 60 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/741.out |  3 +++
 2 files changed, 63 insertions(+)
 create mode 100755 tests/generic/741
 create mode 100644 tests/generic/741.out

diff --git a/tests/generic/741 b/tests/generic/741
new file mode 100755
index 000000000000..f8f9a7be7619
--- /dev/null
+++ b/tests/generic/741
@@ -0,0 +1,60 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle.  All Rights Reserved.
+#
+# FS QA Test 741
+#
+# Attempt to mount both the DM physical device and the DM flakey device.
+# Verify the returned error message.
+#
+. ./common/preamble
+_begin_fstest auto quick volume tempfsid
+
+# Override the default cleanup function.
+_cleanup()
+{
+	umount $extra_mnt &> /dev/null
+	rm -rf $extra_mnt
+	_unmount_flakey
+	_cleanup_flakey
+	cd /
+	rm -r -f $tmp.*
+}
+
+# Import common functions.
+. ./common/filter
+. ./common/dmflakey
+
+# real QA test starts here
+_supported_fs generic
+_require_test
+_require_scratch
+_require_dm_target flakey
+
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit XXXXXXXXXXXX \
+			"btrfs: return accurate error code on open failure"
+
+_scratch_mkfs >> $seqres.full
+_init_flakey
+_mount_flakey
+
+extra_mnt=$TEST_DIR/extra_mnt
+rm -rf $extra_mnt
+mkdir -p $extra_mnt
+
+# Mount must fail because the physical device has a dm created on it.
+# Filters alter the return code of the mount.
+_mount $SCRATCH_DEV $extra_mnt 2>&1 | \
+			_filter_testdir_and_scratch | _filter_error_mount
+
+# Try again with flakey unmounted, must fail.
+_unmount_flakey
+_mount $SCRATCH_DEV $extra_mnt 2>&1 | \
+			_filter_testdir_and_scratch | _filter_error_mount
+
+# Removing dm should make mount successful.
+_cleanup_flakey
+_scratch_mount
+
+status=0
+exit
diff --git a/tests/generic/741.out b/tests/generic/741.out
new file mode 100644
index 000000000000..b694f5fad6b8
--- /dev/null
+++ b/tests/generic/741.out
@@ -0,0 +1,3 @@
+QA output created by 741
+mount: TEST_DIR/extra_mnt: SCRATCH_DEV already mounted or mount point busy
+mount: TEST_DIR/extra_mnt: SCRATCH_DEV already mounted or mount point busy
-- 
2.39.3

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[David Sterba]

On Sat, Mar 16, 2024 at 10:32:33PM +0530, Anand Jain wrote:
> Given that ext4 also allows mounting of a cloned filesystem, the btrfs
> test case btrfs/312, which assesses the functionality of cloned filesystem
> support, can be refactored to be under the shared group.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2:
> Move to shared testcase instead of generic.

What's the purpose of shared/ ? We have tests that make sense for a
subset of supported filesystems in generic/, with proper _required and
other the checks it works fine.

I see that v1 did the move to generic/ but then the 'shared' got
suggested, which is IMHO the wrong direction. I remember some distant
past discussions about shared/ and what to put there. Right now there
are 3 remaining tests which I think is a good opportunity to make it 0.

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[Christoph Hellwig]

On Mon, Mar 18, 2024 at 11:02:19PM +0100, David Sterba wrote:
> What's the purpose of shared/ ? We have tests that make sense for a
> subset of supported filesystems in generic/, with proper _required and
> other the checks it works fine.

Yes.  I'd rather get rid of shared and the few tets in there as it just
creats confusion.

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[Zorro Lang]

On Mon, Mar 18, 2024 at 11:02:19PM +0100, David Sterba wrote:
> On Sat, Mar 16, 2024 at 10:32:33PM +0530, Anand Jain wrote:
> > Given that ext4 also allows mounting of a cloned filesystem, the btrfs
> > test case btrfs/312, which assesses the functionality of cloned filesystem
> > support, can be refactored to be under the shared group.
> > 
> > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > ---
> > v2:
> > Move to shared testcase instead of generic.
> 
> What's the purpose of shared/ ? We have tests that make sense for a
> subset of supported filesystems in generic/, with proper _required and
> other the checks it works fine.
> 
> I see that v1 did the move to generic/ but then the 'shared' got
> suggested, which is IMHO the wrong direction. I remember some distant
> past discussions about shared/ and what to put there. Right now there
> are 3 remaining tests which I think is a good opportunity to make it 0.

I didn't suggest to make it a shared case directly, I asked if there's a
_require_xxxx helper to make this case notrun on "not proper" fs, not
just use "btrfs ext4" to be whitelist :

https://lore.kernel.org/fstests/20240312044629.hpaqdkl24nxaa3dv@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/

In my personal opinion, the "shared" directory is a place to store the cases
which are nearly to be generic, but not ready. It's a place to remind us
there're still some cases use something likes "supported btrfs ext4" as the
hard condition of _notrun, rather than a flexible _require_xxx helper. These
cases in shared better to be moved to generic, if we can improve it in one day.

It more likes a "TODO" list of generic. If we just write it in generic/
directory, I'm afraid we'll leave it in hundreds of generic cases then forget it.

What do you think?

Thanks,
Zorro

> 

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[Anand Jain]

On 3/19/24 09:46, Zorro Lang wrote:
> On Mon, Mar 18, 2024 at 11:02:19PM +0100, David Sterba wrote:
>> On Sat, Mar 16, 2024 at 10:32:33PM +0530, Anand Jain wrote:
>>> Given that ext4 also allows mounting of a cloned filesystem, the btrfs
>>> test case btrfs/312, which assesses the functionality of cloned filesystem
>>> support, can be refactored to be under the shared group.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>> v2:
>>> Move to shared testcase instead of generic.
>>
>> What's the purpose of shared/ ? We have tests that make sense for a
>> subset of supported filesystems in generic/, with proper _required and
>> other the checks it works fine.
>>
>> I see that v1 did the move to generic/ but then the 'shared' got
>> suggested, which is IMHO the wrong direction. I remember some distant
>> past discussions about shared/ and what to put there. Right now there
>> are 3 remaining tests which I think is a good opportunity to make it 0.
> 
> I didn't suggest to make it a shared case directly,

> I asked if there's a
> _require_xxxx helper to make this case notrun on "not proper" fs,




> not just use "btrfs ext4" to be whitelist :
> 
> https://lore.kernel.org/fstests/20240312044629.hpaqdkl24nxaa3dv@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/
> 
> In my personal opinion, the "shared" directory is a place to store the cases
> which are nearly to be generic, but not ready. It's a place to remind us
> there're still some cases use something likes "supported btrfs ext4" as the
> hard condition of _notrun, rather than a flexible _require_xxx helper. These
> cases in shared better to be moved to generic, if we can improve it in one day.
> 
> It more likes a "TODO" list of generic. If we just write it in generic/
> directory, I'm afraid we'll leave it in hundreds of generic cases then forget it.
> 

> What do you think?


Based on my understanding, here is the original approach:

  fstests/generic: Includes tests applicable to all file systems.

  tests/shared: Consists of tests supported by two or more file systems.

  However, currently, the test cases are not properly organized.

  Moreover, fstests/generic is nearing 999 test cases.

  Segregating tests between shared and generic can optimize group size.

  But, I am fine if we give away 'shared' to `generic` instead.


Thanks, Anand

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[David Sterba]

On Tue, Mar 19, 2024 at 12:16:33PM +0800, Zorro Lang wrote:
> On Mon, Mar 18, 2024 at 11:02:19PM +0100, David Sterba wrote:
> > On Sat, Mar 16, 2024 at 10:32:33PM +0530, Anand Jain wrote:
> > > Given that ext4 also allows mounting of a cloned filesystem, the btrfs
> > > test case btrfs/312, which assesses the functionality of cloned filesystem
> > > support, can be refactored to be under the shared group.
> > > 
> > > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > > ---
> > > v2:
> > > Move to shared testcase instead of generic.
> > 
> > What's the purpose of shared/ ? We have tests that make sense for a
> > subset of supported filesystems in generic/, with proper _required and
> > other the checks it works fine.
> > 
> > I see that v1 did the move to generic/ but then the 'shared' got
> > suggested, which is IMHO the wrong direction. I remember some distant
> > past discussions about shared/ and what to put there. Right now there
> > are 3 remaining tests which I think is a good opportunity to make it 0.
> 
> I didn't suggest to make it a shared case directly, I asked if there's a
> _require_xxxx helper to make this case notrun on "not proper" fs, not
> just use "btrfs ext4" to be whitelist :
> 
> https://lore.kernel.org/fstests/20240312044629.hpaqdkl24nxaa3dv@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/
> 
> In my personal opinion, the "shared" directory is a place to store the cases
> which are nearly to be generic, but not ready. It's a place to remind us
> there're still some cases use something likes "supported btrfs ext4" as the
> hard condition of _notrun, rather than a flexible _require_xxx helper. These
> cases in shared better to be moved to generic, if we can improve it in one day.

Well, I disagree with that, we don't need to track the nearly-generic
state in a different way than we have right now with the _supported_fs
and ^exceptions.

> It more likes a "TODO" list of generic. If we just write it in generic/
> directory, I'm afraid we'll leave it in hundreds of generic cases then forget it.

A TODO for who? If I take the current state of shared/ as example, the
test 298, there's "_supported_fs ext4 xfs btrfs", this can live in
generic as it covers the current filesystems. If this does not apply to
NFS then it's IMHO fine to explicitly list the supported filesystems
rather than do long list of exceptions.

Support for btrfs to that test was added in 2019 in commit
0680ff2ea5313b3. If the state hasn't changed and is still in TODO then I
don't think this works (although back then the todo-status of the
shared/ was not defined as such).

> What do you think?

What I suggest:

- move everything from shared/ to generic
- document (as guidelines) what to do if there's a generic test that
  applies only to subset of filesystems, what helpers to use and
  possibly comment in the test cases why this canont be fully generic so
  that a new filesystems to add can be evaluated as needed
  - good example is shared/002, one could decide if eg. ext4 is missing
    or not
  - bad example is shared/032, there's xfs and btrfs but from the test
    it looks like it's relevant for any local filesystem

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[Christoph Hellwig]

On Tue, Mar 19, 2024 at 12:16:33PM +0800, Zorro Lang wrote:
> I didn't suggest to make it a shared case directly, I asked if there's a
> _require_xxxx helper to make this case notrun on "not proper" fs, not
> just use "btrfs ext4" to be whitelist :
> 
> https://lore.kernel.org/fstests/20240312044629.hpaqdkl24nxaa3dv@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/
> 
> In my personal opinion, the "shared" directory is a place to store the cases
> which are nearly to be generic, but not ready. It's a place to remind us
> there're still some cases use something likes "supported btrfs ext4" as the
> hard condition of _notrun, rather than a flexible _require_xxx helper. These
> cases in shared better to be moved to generic, if we can improve it in one day.
> 
> It more likes a "TODO" list of generic. If we just write it in generic/
> directory, I'm afraid we'll leave it in hundreds of generic cases then forget it.
> 
> What do you think?

I like we're you're going, but I'd like to take it a step further:

I think we should just kill _supported_fs entirely.

tests/$FSTYPE is run for $FSTYP only, period.
tests/generic/ is run for all file systems, and run/notrun deciѕions
should be based on feature checks.  Where they can't happen without
fs-sepcific infrastructure we need a _require/_have check that
switches on $FSTYP like we already have in many places.  shared
should be folded into generic.

And a list of all hte places where we have to or should plug fs
knowledge in would be really nice as well..

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[David Sterba]

On Tue, Mar 19, 2024 at 01:43:03PM -0700, Christoph Hellwig wrote:
> On Tue, Mar 19, 2024 at 12:16:33PM +0800, Zorro Lang wrote:
> > I didn't suggest to make it a shared case directly, I asked if there's a
> > _require_xxxx helper to make this case notrun on "not proper" fs, not
> > just use "btrfs ext4" to be whitelist :
> > 
> > https://lore.kernel.org/fstests/20240312044629.hpaqdkl24nxaa3dv@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/
> > 
> > In my personal opinion, the "shared" directory is a place to store the cases
> > which are nearly to be generic, but not ready. It's a place to remind us
> > there're still some cases use something likes "supported btrfs ext4" as the
> > hard condition of _notrun, rather than a flexible _require_xxx helper. These
> > cases in shared better to be moved to generic, if we can improve it in one day.
> > 
> > It more likes a "TODO" list of generic. If we just write it in generic/
> > directory, I'm afraid we'll leave it in hundreds of generic cases then forget it.
> > 
> > What do you think?
> 
> I like we're you're going, but I'd like to take it a step further:
> 
> I think we should just kill _supported_fs entirely.
> 
> tests/$FSTYPE is run for $FSTYP only, period.
> tests/generic/ is run for all file systems, and run/notrun deciѕions
> should be based on feature checks.  Where they can't happen without
> fs-sepcific infrastructure we need a _require/_have check that
> switches on $FSTYP like we already have in many places.

This could work, the number of exceptions is short:

tests/generic:
    547 _supported_fs generic
      4 _supported_fs ^nfs
      2 _supported_fs ^overlay
      1 _supported_fs ^xfs
      1 _supported_fs ^btrfs ^nfs
      1 _supported_fs ^btrfs

The example from generic/187:

# btrfs can't fragment free space. This test is unreliable on NFS, as it
# depends on the exported filesystem.
_supported_fs ^btrfs ^nfs

There are different reasons for the exclusion but at least for btrfs
could be transformed to e.g.

_require_can_fragment_free_space

Not sure about the NFS part, it can be either excluding remote
filesystems or mandating a local one.

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[Christoph Hellwig]

On Wed, Mar 20, 2024 at 05:08:02PM +0100, David Sterba wrote:
> This could work, the number of exceptions is short:
> 
> tests/generic:
>     547 _supported_fs generic
>       4 _supported_fs ^nfs
>       2 _supported_fs ^overlay
>       1 _supported_fs ^xfs
>       1 _supported_fs ^btrfs ^nfs
>       1 _supported_fs ^btrfs
> 
> The example from generic/187:
> 
> # btrfs can't fragment free space. This test is unreliable on NFS, as it
> # depends on the exported filesystem.
> _supported_fs ^btrfs ^nfs
> 
> There are different reasons for the exclusion but at least for btrfs
> could be transformed to e.g.
> 
> _require_can_fragment_free_space
> 
> Not sure about the NFS part, it can be either excluding remote
> filesystems or mandating a local one.

I think the same applies for NFS.  The underlying fs on the other
side might or might not allow fragmenting free space, but it doesn't
matter to NFS testing.

(then again reading through the actual test I have no idea what it has
to do with fragmenting freespace vs just stressing the test, and
I hope not work / unreliable just means it takes forever as no
one should be corrupting files with this workload).

Re: [PATCH v2 2/2] generic: test mount fails on physical device with configured dm volume

[Anand Jain]

On 3/16/24 22:32, Anand Jain wrote:
> When a dm Flakey device is configured, (or similar dm where both physical
> and dm devices are accessible) we have access to both the physical device
> and the dm flakey device, ensure that the physical device mount fails.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> Remove redirect for rm
> Add _require_test
> Add _filter_error_mount
> Change log - make it more sound generic with a dm device
> Add quick group
> 
  Thanks for the review comments; they have been accepted in this patch.
  This patch is now applied for the upcoming PR.

Thanks, Anand

>   tests/generic/741     | 60 +++++++++++++++++++++++++++++++++++++++++++
>   tests/generic/741.out |  3 +++
>   2 files changed, 63 insertions(+)
>   create mode 100755 tests/generic/741
>   create mode 100644 tests/generic/741.out
> 
> diff --git a/tests/generic/741 b/tests/generic/741
> new file mode 100755
> index 000000000000..f8f9a7be7619
> --- /dev/null
> +++ b/tests/generic/741
> @@ -0,0 +1,60 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> +#
> +# FS QA Test 741
> +#
> +# Attempt to mount both the DM physical device and the DM flakey device.
> +# Verify the returned error message.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick volume tempfsid
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	umount $extra_mnt &> /dev/null
> +	rm -rf $extra_mnt
> +	_unmount_flakey
> +	_cleanup_flakey
> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/dmflakey
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_test
> +_require_scratch
> +_require_dm_target flakey
> +
> +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit XXXXXXXXXXXX \
> +			"btrfs: return accurate error code on open failure"
> +
> +_scratch_mkfs >> $seqres.full
> +_init_flakey
> +_mount_flakey
> +
> +extra_mnt=$TEST_DIR/extra_mnt
> +rm -rf $extra_mnt
> +mkdir -p $extra_mnt
> +
> +# Mount must fail because the physical device has a dm created on it.
> +# Filters alter the return code of the mount.
> +_mount $SCRATCH_DEV $extra_mnt 2>&1 | \
> +			_filter_testdir_and_scratch | _filter_error_mount
> +
> +# Try again with flakey unmounted, must fail.
> +_unmount_flakey
> +_mount $SCRATCH_DEV $extra_mnt 2>&1 | \
> +			_filter_testdir_and_scratch | _filter_error_mount
> +
> +# Removing dm should make mount successful.
> +_cleanup_flakey
> +_scratch_mount
> +
> +status=0
> +exit
> diff --git a/tests/generic/741.out b/tests/generic/741.out
> new file mode 100644
> index 000000000000..b694f5fad6b8
> --- /dev/null
> +++ b/tests/generic/741.out
> @@ -0,0 +1,3 @@
> +QA output created by 741
> +mount: TEST_DIR/extra_mnt: SCRATCH_DEV already mounted or mount point busy
> +mount: TEST_DIR/extra_mnt: SCRATCH_DEV already mounted or mount point busy

Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

[Anand Jain]

Thanks for the comments and opinions. In v3, the patch is back to
the generic group. It is also limited by:

    supported_fs generic
    require_duplicate_fsid

Thanks, Anand

On 3/17/24 01:02, Anand Jain wrote:
> Given that ext4 also allows mounting of a cloned filesystem, the btrfs
> test case btrfs/312, which assesses the functionality of cloned filesystem
> support, can be refactored to be under the shared group.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2:
> Move to shared testcase instead of generic.
> Add _require_block_device $TEST_DEV.
> Add _require_duplicated_fsid.
> 
>   common/rc            | 14 +++++++
>   tests/btrfs/312      | 78 --------------------------------------
>   tests/btrfs/312.out  | 19 ----------
>   tests/shared/001     | 89 ++++++++++++++++++++++++++++++++++++++++++++
>   tests/shared/001.out |  4 ++
>   5 files changed, 107 insertions(+), 97 deletions(-)
>   delete mode 100755 tests/btrfs/312
>   delete mode 100644 tests/btrfs/312.out
>   create mode 100755 tests/shared/001
>   create mode 100644 tests/shared/001.out
> 
> diff --git a/common/rc b/common/rc
> index 36cad89cfc5d..2638dfb8e9b3 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5408,6 +5408,20 @@ _random_file() {
>   	echo "$basedir/$(ls -U $basedir | shuf -n 1)"
>   }
>   
> +_require_duplicate_fsid()
> +{
> +	case "$FSTYP" in
> +	"btrfs")
> +		_require_btrfs_fs_feature temp_fsid
> +		;;
> +	"ext4")
> +		;;
> +	*)
> +		_notrun "$FSTYP cannot support mounting with duplicate fsid"
> +		;;
> +	esac
> +}
> +
>   init_rc
>   
>   ################################################################################
> diff --git a/tests/btrfs/312 b/tests/btrfs/312
> deleted file mode 100755
> index eedcf11a2308..000000000000
> --- a/tests/btrfs/312
> +++ /dev/null
> @@ -1,78 +0,0 @@
> -#! /bin/bash
> -# SPDX-License-Identifier: GPL-2.0
> -# Copyright (c) 2024 Oracle.  All Rights Reserved.
> -#
> -# FS QA Test 312
> -#
> -# On a clone a device check to see if tempfsid is activated.
> -#
> -. ./common/preamble
> -_begin_fstest auto quick clone tempfsid
> -
> -_cleanup()
> -{
> -	cd /
> -	$UMOUNT_PROG $mnt1 > /dev/null 2>&1
> -	rm -r -f $tmp.*
> -	rm -r -f $mnt1
> -}
> -
> -. ./common/filter.btrfs
> -. ./common/reflink
> -
> -_supported_fs btrfs
> -_require_scratch_dev_pool 2
> -_scratch_dev_pool_get 2
> -_require_btrfs_fs_feature temp_fsid
> -
> -mnt1=$TEST_DIR/$seq/mnt1
> -mkdir -p $mnt1
> -
> -create_cloned_devices()
> -{
> -	local dev1=$1
> -	local dev2=$2
> -
> -	echo -n Creating cloned device...
> -	_mkfs_dev -fq -b $((1024 * 1024 * 300)) $dev1
> -
> -	_mount $dev1 $SCRATCH_MNT
> -
> -	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
> -								_filter_xfs_io
> -	$UMOUNT_PROG $SCRATCH_MNT
> -	# device dump of $dev1 to $dev2
> -	dd if=$dev1 of=$dev2 bs=300M count=1 conv=fsync status=none || \
> -							_fail "dd failed: $?"
> -	echo done
> -}
> -
> -mount_cloned_device()
> -{
> -	echo ---- $FUNCNAME ----
> -	create_cloned_devices ${SCRATCH_DEV_NAME[0]} ${SCRATCH_DEV_NAME[1]}
> -
> -	echo Mounting original device
> -	_mount ${SCRATCH_DEV_NAME[0]} $SCRATCH_MNT
> -	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
> -								_filter_xfs_io
> -	check_fsid ${SCRATCH_DEV_NAME[0]}
> -
> -	echo Mounting cloned device
> -	_mount ${SCRATCH_DEV_NAME[1]} $mnt1 || \
> -				_fail "mount failed, tempfsid didn't work"
> -
> -	echo cp reflink must fail
> -	_cp_reflink $SCRATCH_MNT/foo $mnt1/bar 2>&1 | \
> -						_filter_testdir_and_scratch
> -
> -	check_fsid ${SCRATCH_DEV_NAME[1]}
> -}
> -
> -mount_cloned_device
> -
> -_scratch_dev_pool_put
> -
> -# success, all done
> -status=0
> -exit
> diff --git a/tests/btrfs/312.out b/tests/btrfs/312.out
> deleted file mode 100644
> index b7de6ce3cc6e..000000000000
> --- a/tests/btrfs/312.out
> +++ /dev/null
> @@ -1,19 +0,0 @@
> -QA output created by 312
> ----- mount_cloned_device ----
> -Creating cloned device...wrote 9000/9000 bytes at offset 0
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -done
> -Mounting original device
> -wrote 9000/9000 bytes at offset 0
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -On disk fsid:		FSID
> -Metadata uuid:		FSID
> -Temp fsid:		FSID
> -Tempfsid status:	0
> -Mounting cloned device
> -cp reflink must fail
> -cp: failed to clone 'TEST_DIR/312/mnt1/bar' from 'SCRATCH_MNT/foo': Invalid cross-device link
> -On disk fsid:		FSID
> -Metadata uuid:		FSID
> -Temp fsid:		TEMPFSID
> -Tempfsid status:	1
> diff --git a/tests/shared/001 b/tests/shared/001
> new file mode 100755
> index 000000000000..3f2b85a41099
> --- /dev/null
> +++ b/tests/shared/001
> @@ -0,0 +1,89 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 001
> +#
> +# Set up a filesystem, create a clone, mount both, and verify if the cp reflink
> +# operation between these two mounts fails.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick clone volume tempfsid
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -r -f $tmp.*
> +
> +	$UMOUNT_PROG $mnt2 &> /dev/null
> +	rm -r -f $mnt2
> +	_destroy_loop_device $loop_dev2 &> /dev/null
> +	rm -r -f $loop_file2
> +
> +	$UMOUNT_PROG $mnt1 &> /dev/null
> +	rm -r -f $mnt1
> +	_destroy_loop_device $loop_dev1 &> /dev/null
> +	rm -r -f $loop_file1
> +}
> +
> +. ./common/filter
> +. ./common/reflink
> +
> +# Modify as appropriate.
> +_supported_fs btrfs ext4
> +_require_duplicate_fsid
> +_require_cp_reflink
> +_require_test
> +_require_block_device $TEST_DEV
> +_require_loop
> +
> +[[ $FSTYP == "btrfs" ]] && _require_btrfs_fs_feature temp_fsid
> +
> +clone_filesystem()
> +{
> +	local dev1=$1
> +	local dev2=$2
> +
> +	_mkfs_dev $dev1
> +
> +	_mount $dev1 $mnt1
> +	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $mnt1/foo >> $seqres.full
> +	$UMOUNT_PROG $mnt1
> +
> +	# device dump of $dev1 to $dev2
> +	dd if=$dev1 of=$dev2 conv=fsync status=none || _fail "dd failed: $?"
> +}
> +
> +mnt1=$TEST_DIR/$seq/mnt1
> +rm -r -f $mnt1
> +mkdir -p $mnt1
> +
> +mnt2=$TEST_DIR/$seq/mnt2
> +rm -r -f $mnt2
> +mkdir -p $mnt2
> +
> +loop_file1="$TEST_DIR/$seq/image1"
> +rm -r -f $loop_file1
> +truncate -s 300m "$loop_file1"
> +loop_dev1=$(_create_loop_device "$loop_file1")
> +
> +loop_file2="$TEST_DIR/$seq/image2"
> +rm -r -f $loop_file2
> +truncate -s 300m "$loop_file2"
> +loop_dev2=$(_create_loop_device "$loop_file2")
> +
> +clone_filesystem ${loop_dev1} ${loop_dev2}
> +
> +# Mounting original device
> +_mount $loop_dev1 $mnt1
> +$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $mnt1/foo | _filter_xfs_io
> +
> +# Mounting cloned device
> +_mount $loop_dev2 $mnt2 || _fail "mount of cloned device failed"
> +
> +# cp reflink across two different filesystems must fail
> +_cp_reflink $mnt1/foo $mnt2/bar 2>&1 | _filter_test_dir
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/shared/001.out b/tests/shared/001.out
> new file mode 100644
> index 000000000000..56b697ca3972
> --- /dev/null
> +++ b/tests/shared/001.out
> @@ -0,0 +1,4 @@
> +QA output created by 001
> +wrote 9000/9000 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +cp: failed to clone 'TEST_DIR/001/mnt2/bar' from 'TEST_DIR/001/mnt1/foo': Invalid cross-device link