diff mbox series

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

Message ID 440eff6d16407f12ec55df69db283ba6eb9b278c.1710599671.git.anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show
Series fstests: new test cases for generic group | expand

Commit Message

Anand Jain March 16, 2024, 5:02 p.m. UTC 
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

Comments

David Sterba March 18, 2024, 10:02 p.m. UTC | #1 
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.
Christoph Hellwig March 18, 2024, 10:15 p.m. UTC | #2 
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.
Zorro Lang March 19, 2024, 4:16 a.m. UTC | #3 
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

>
Anand Jain March 19, 2024, 5:17 p.m. UTC | #4 
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
David Sterba March 19, 2024, 5:27 p.m. UTC | #5 
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
Christoph Hellwig March 19, 2024, 8:43 p.m. UTC | #6 
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..
David Sterba March 20, 2024, 4:08 p.m. UTC | #7 
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.
Christoph Hellwig March 21, 2024, 9:41 p.m. UTC | #8 
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).
Anand Jain April 9, 2024, 2:43 p.m. UTC | #9 
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
diff mbox series

Patch

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