[PATCH v3 00/10] btrfs: functional test cases for tempfsid

[PATCH v3 00/10] btrfs: functional test cases for tempfsid

[Anand Jain]

v3: Mainly, move the prerequisite checks
  _require_btrfs_command inspect-internal dump-super
  _require_btrfs_mkfs_uuid_option
to the common/btrfs function mkfs_clone() and move
  _require_btrfs_command inspect-internal dump-super
to check_fsid() from each individual testcase.
A few more changes as in each individual testcase.

v2: Each individual patch has undergone numerous fixes based on the
feedback received. Please refer to the individual patches.

This patch set validates the tempfsid feature in Btrfs, testing its
functionality and limitations. Also, has one minor bug fix.

Anand Jain (10):
  assign SCRATCH_DEV_POOL to an array
  btrfs: introduce tempfsid test group
  btrfs: create a helper function, check_fsid(), to verify the tempfsid
  btrfs: verify that subvolume mounts are unaffected by tempfsid
  btrfs: check if cloned device mounts with tempfsid
  btrfs: test case prerequisite _require_btrfs_mkfs_uuid_option
  btrfs: introduce helper for creating cloned devices with mkfs
  btrfs: verify tempfsid clones using mkfs
  btrfs: validate send-receive operation with tempfsid.
  btrfs: test tempfsid with device add, seed, and balance

 common/btrfs        | 71 ++++++++++++++++++++++++++++++++++++
 common/rc           | 18 +++++++---
 doc/group-names.txt |  1 +
 tests/btrfs/311     | 88 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/311.out | 24 +++++++++++++
 tests/btrfs/312     | 84 +++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/312.out | 19 ++++++++++
 tests/btrfs/313     | 53 +++++++++++++++++++++++++++
 tests/btrfs/313.out | 16 +++++++++
 tests/btrfs/314     | 79 ++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/314.out | 23 ++++++++++++
 tests/btrfs/315     | 78 ++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/315.out | 10 ++++++
 13 files changed, 560 insertions(+), 4 deletions(-)
 create mode 100755 tests/btrfs/311
 create mode 100644 tests/btrfs/311.out
 create mode 100755 tests/btrfs/312
 create mode 100644 tests/btrfs/312.out
 create mode 100755 tests/btrfs/313
 create mode 100644 tests/btrfs/313.out
 create mode 100755 tests/btrfs/314
 create mode 100644 tests/btrfs/314.out
 create mode 100755 tests/btrfs/315
 create mode 100644 tests/btrfs/315.out

-- 
2.39.3

[PATCH v3 01/10] assign SCRATCH_DEV_POOL to an array

[Anand Jain]

Many test cases use local variables to manage the names of each device in
SCRATCH_DEV_POOL. Let _scratch_dev_pool_get set an array, SCRATCH_DEV_NAME,
for it.

Usage:

	_scratch_dev_pool_get <n>

	# device names are in the array SCRATCH_DEV_NAME.
	${SCRATCH_DEV_NAME[0]} ${SCRATCH_DEV_NAME[1]} ...

	_scratch_dev_pool_put

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
---
v3: add rb
v2:
 Fix typo in the commit log.
 Fix array SCRATCH_DEV_POOL_SAVED handling.

 common/rc | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/common/rc b/common/rc
index e475c890d902..50dde313b851 100644
--- a/common/rc
+++ b/common/rc
@@ -835,8 +835,9 @@ _spare_dev_put()
 # to make sure it has the enough scratch devices including
 # replace-target and spare device. Now arg1 here is the
 # required number of scratch devices by a-test-case excluding
-# the replace-target and spare device. So this function will
-# set SCRATCH_DEV_POOL to the specified number of devices.
+# the replace-target and spare device. So, this function sets
+# SCRATCH_DEV_POOL to the specified number of devices and also
+# sets a SCRATCH_DEV_NAME array with the names of the devices.
 #
 # Usage:
 #  _scratch_dev_pool_get() <ndevs>
@@ -867,19 +868,28 @@ _scratch_dev_pool_get()
 	export SCRATCH_DEV_POOL_SAVED
 	SCRATCH_DEV_POOL=${devs[@]:0:$test_ndevs}
 	export SCRATCH_DEV_POOL
+	SCRATCH_DEV_NAME=( $SCRATCH_DEV_POOL )
+	export SCRATCH_DEV_NAME
 }
 
 _scratch_dev_pool_put()
 {
+	local ret1
+	local ret2
+
 	typeset -p SCRATCH_DEV_POOL_SAVED >/dev/null 2>&1
-	if [ $? -ne 0 ]; then
+	ret1=$?
+	typeset -p SCRATCH_DEV_NAME >/dev/null 2>&1
+	ret2=$?
+	if [[ $ret1 -ne 0 || $ret2 -ne 0 ]]; then
 		_fail "Bug: unset val, must call _scratch_dev_pool_get before _scratch_dev_pool_put"
 	fi
 
-	if [ -z "$SCRATCH_DEV_POOL_SAVED" ]; then
+	if [[ -z "$SCRATCH_DEV_POOL_SAVED" || -z "${SCRATCH_DEV_NAME[@]}" ]]; then
 		_fail "Bug: str empty, must call _scratch_dev_pool_get before _scratch_dev_pool_put"
 	fi
 
+	export SCRATCH_DEV_NAME=()
 	export SCRATCH_DEV_POOL=$SCRATCH_DEV_POOL_SAVED
 	export SCRATCH_DEV_POOL_SAVED=""
 }
-- 
2.39.3

[PATCH v3 02/10] btrfs: introduce tempfsid test group

[Anand Jain]

Introducing a new test group named tempfsid.

Tempfsid is a feature of the Btrfs filesystem. When encountering another
device with the same fsid as one already mounted, the system will mount
the new device with a temporary, randomly generated in-memory fsid.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v3: -
v2: add rb.

 doc/group-names.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/group-names.txt b/doc/group-names.txt
index 2ac95ac83a79..50262e02f681 100644
--- a/doc/group-names.txt
+++ b/doc/group-names.txt
@@ -131,6 +131,7 @@ swap			swap files
 swapext			XFS_IOC_SWAPEXT ioctl
 symlink			symbolic links
 tape			dump and restore with a tape
+tempfsid		temporary fsid
 thin			thin provisioning
 trim			FITRIM ioctl
 udf			UDF functionality tests
-- 
2.39.3

[PATCH v3 03/10] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Anand Jain]

check_fsid() provides a method to verify if the given device is mounted
with the tempfsid in the kernel. Function sb() is an internal only
function.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
---
v3: 
 add rb
 add  _require_btrfs_command inspect-internal dump-super
v2:
 Fix typo in the commit log.
 Fix array SCRATCH_DEV_POOL_SAVED handling.

 common/btrfs | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/common/btrfs b/common/btrfs
index e1b29c613767..406be9574e32 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -792,3 +792,40 @@ _has_btrfs_sysfs_feature_attr()
 
 	test -e /sys/fs/btrfs/features/$feature_attr
 }
+
+# Print the fsid and metadata uuid replaced with constant strings FSID and
+# METADATA_UUID. Compare temp_fsid with fsid and metadata_uuid, then echo what
+# it matches to or TEMP_FSID. This helps in comparing with the golden output.
+check_fsid()
+{
+	local dev1=$1
+	local fsid
+	local metadata_uuid
+
+	_require_btrfs_command inspect-internal dump-super
+
+	# on disk fsid
+	fsid=$($BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | \
+				grep ^fsid | $AWK_PROG -d" " '{print $2}')
+	echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
+
+	# Print FSID even if it is not the same as metadata_uuid because it has
+	# to match in the golden output.
+	metadata_uuid=$(cat /sys/fs/btrfs/$fsid/metadata_uuid)
+	echo -e "Metadata uuid:\t\tFSID"
+
+	# This returns the temp_fsid if set
+	tempfsid=$(_btrfs_get_fsid $dev1)
+	if [[ $tempfsid == $fsid ]]; then
+		echo -e "Temp fsid:\t\tFSID"
+	elif [[ $tempfsid == $metadata_uuid ]]; then
+		# If we are here, it means there is a bug; let it not match with
+		# the golden output.
+		echo -e "Temp fsid:\t\t$metadata_uuid"
+	else
+		echo -e "Temp fsid:\t\tTEMPFSID"
+	fi
+
+	echo -e -n "Tempfsid status:\t"
+	cat /sys/fs/btrfs/$tempfsid/temp_fsid
+}
-- 
2.39.3

[PATCH v3 04/10] btrfs: verify that subvolume mounts are unaffected by tempfsid

[Anand Jain]

The tempfsid logic must determine whether the incoming mount request
is for a device already mounted or a new device mount. Verify that it
recognizes the device already mounted well by creating reflink across
the subvolume mount points.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v3:
Fix subvolume create output with _filter_scratch and its golden output
add rb
remove  _require_btrfs_command inspect-internal dump-super
v2:
add subvol group
use $UMOUNT_PROG 
remove _fail for _cp_reflink

 tests/btrfs/311     | 88 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/311.out | 24 +++++++++++++
 2 files changed, 112 insertions(+)
 create mode 100755 tests/btrfs/311
 create mode 100644 tests/btrfs/311.out

diff --git a/tests/btrfs/311 b/tests/btrfs/311
new file mode 100755
index 000000000000..bdabcf6a9814
--- /dev/null
+++ b/tests/btrfs/311
@@ -0,0 +1,88 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle. All Rights Reserved.
+#
+# FS QA Test 311
+#
+# Mount the device twice check if the reflink works, this helps to
+# ensure device is mounted as the same device.
+#
+. ./common/preamble
+_begin_fstest auto quick subvol tempfsid
+
+# Override the default cleanup function.
+_cleanup()
+{
+	cd /
+	$UMOUNT_PROG $mnt1 > /dev/null 2>&1
+	rm -r -f $tmp.*
+	rm -r -f $mnt1
+}
+
+. ./common/filter.btrfs
+. ./common/reflink
+
+# Modify as appropriate.
+_supported_fs btrfs
+_require_cp_reflink
+_require_btrfs_sysfs_fsid
+_require_btrfs_fs_feature temp_fsid
+_require_scratch
+
+mnt1=$TEST_DIR/$seq/mnt1
+mkdir -p $mnt1
+
+same_dev_mount()
+{
+	echo ---- $FUNCNAME ----
+
+	_scratch_mkfs >> $seqres.full 2>&1
+
+	_scratch_mount
+	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
+								_filter_xfs_io
+
+	echo Mount the device again to a different mount point
+	_mount $SCRATCH_DEV $mnt1
+
+	_cp_reflink $SCRATCH_MNT/foo $mnt1/bar
+	echo Checksum of reflinked files
+	md5sum $SCRATCH_MNT/foo | _filter_scratch
+	md5sum $mnt1/bar | _filter_test_dir
+
+	check_fsid $SCRATCH_DEV
+}
+
+same_dev_subvol_mount()
+{
+	echo ---- $FUNCNAME ----
+	_scratch_mkfs >> $seqres.full 2>&1
+
+	_scratch_mount
+	$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol | _filter_scratch
+
+	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/subvol/foo | \
+								_filter_xfs_io
+
+	echo Mounting a subvol
+	_mount -o subvol=subvol $SCRATCH_DEV $mnt1
+
+	_cp_reflink $SCRATCH_MNT/subvol/foo $mnt1/bar
+	echo Checksum of reflinked files
+	md5sum $SCRATCH_MNT/subvol/foo | _filter_scratch
+	md5sum $mnt1/bar | _filter_test_dir
+
+	check_fsid $SCRATCH_DEV
+}
+
+same_dev_mount
+
+_scratch_unmount
+_cleanup
+mkdir -p $mnt1
+
+same_dev_subvol_mount
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/311.out b/tests/btrfs/311.out
new file mode 100644
index 000000000000..4ea46eab3c72
--- /dev/null
+++ b/tests/btrfs/311.out
@@ -0,0 +1,24 @@
+QA output created by 311
+---- same_dev_mount ----
+wrote 9000/9000 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Mount the device again to a different mount point
+Checksum of reflinked files
+42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/foo
+42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/311/mnt1/bar
+On disk fsid:		FSID
+Metadata uuid:		FSID
+Temp fsid:		FSID
+Tempfsid status:	0
+---- same_dev_subvol_mount ----
+Create subvolume 'SCRATCH_MNT/subvol'
+wrote 9000/9000 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Mounting a subvol
+Checksum of reflinked files
+42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/subvol/foo
+42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/311/mnt1/bar
+On disk fsid:		FSID
+Metadata uuid:		FSID
+Temp fsid:		FSID
+Tempfsid status:	0
-- 
2.39.3

[PATCH v3 05/10] btrfs: check if cloned device mounts with tempfsid

[Anand Jain]

If another device with the same fsid and uuid would mount then verify if
it mounts with a temporary fsid.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v3:
add clone group
fix use $UMOUNT_PROG
remove (_require_btrfs_command inspect-internal dump-super)
v2:
Bring create_cloned_devices() into the testcase.
Just use _cp_reflink output to match with golden output.

 tests/btrfs/312     | 84 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/312.out | 19 ++++++++++
 2 files changed, 103 insertions(+)
 create mode 100755 tests/btrfs/312
 create mode 100644 tests/btrfs/312.out

diff --git a/tests/btrfs/312 b/tests/btrfs/312
new file mode 100755
index 000000000000..90ca7a30d3e2
--- /dev/null
+++ b/tests/btrfs/312
@@ -0,0 +1,84 @@
+#! /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_btrfs_sysfs_fsid
+_require_btrfs_fs_feature temp_fsid
+_require_scratch_dev_pool 2
+_scratch_dev_pool_get 2
+
+mnt1=$TEST_DIR/$seq/mnt1
+mkdir -p $mnt1
+
+create_cloned_devices()
+{
+	local dev1=$1
+	local dev2=$2
+
+	[[ -z $dev1 || -z $dev2 ]] && \
+		_fail "create_cloned_devices() requires two devices as arguments"
+
+	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()
+{
+	local ret
+
+	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
new file mode 100644
index 000000000000..b7de6ce3cc6e
--- /dev/null
+++ b/tests/btrfs/312.out
@@ -0,0 +1,19 @@
+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
-- 
2.39.3

[PATCH v3 06/10] btrfs: test case prerequisite _require_btrfs_mkfs_uuid_option

[Anand Jain]

For easier and more effective testing of btrfs tempfsid, newer versions
of mkfs.btrfs contain options such as --device-uuid. Check if the
currently running mkfs.btrfs contains this option.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
---
v3:
 add rb
v2:
 Fix coding style, add space before grep
 Fix typp option -> options

 common/btrfs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/common/btrfs b/common/btrfs
index 406be9574e32..55847733b20e 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -88,6 +88,17 @@ _require_btrfs_mkfs_feature()
 		_notrun "Feature $feat not supported in the available version of mkfs.btrfs"
 }
 
+_require_btrfs_mkfs_uuid_option()
+{
+	local cnt
+
+	cnt=$($MKFS_BTRFS_PROG --help 2>&1 | \
+				grep -E --count "\-\-uuid|\-\-device-uuid")
+	if [ $cnt != 2 ]; then
+		_notrun "Require $MKFS_BTRFS_PROG with --uuid and --device-uuid options"
+	fi
+}
+
 _require_btrfs_fs_feature()
 {
 	if [ -z $1 ]; then
-- 
2.39.3

[PATCH v3 07/10] btrfs: introduce helper for creating cloned devices with mkfs

[Anand Jain]

Use newer mkfs.btrfs option to generate two cloned devices,
used in test cases.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v3:
 call
 _require_btrfs_command inspect-internal dump-super
 _require_btrfs_mkfs_uuid_option
 in the function mkfs_clone()
 Remove the conflict fix metadata line

v2:
 Organize changes to its right patch.
 Fix _fail erorr message.
 Declare local variables for fsid and uuid.

 common/btrfs | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/common/btrfs b/common/btrfs
index 55847733b20e..04ecff6ada71 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -840,3 +840,26 @@ check_fsid()
 	echo -e -n "Tempfsid status:\t"
 	cat /sys/fs/btrfs/$tempfsid/temp_fsid
 }
+
+mkfs_clone()
+{
+	local fsid
+	local uuid
+	local dev1=$1
+	local dev2=$2
+
+	_require_btrfs_command inspect-internal dump-super
+	_require_btrfs_mkfs_uuid_option
+
+	[[ -z $dev1 || -z $dev2 ]] && \
+		_fail "mkfs_clone requires two devices as arguments"
+
+	_mkfs_dev -fq $dev1
+
+	fsid=$($BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | \
+					grep -E ^fsid | $AWK_PROG '{print $2}')
+	uuid=$($BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | \
+				grep -E ^dev_item.uuid | $AWK_PROG '{print $2}')
+
+	_mkfs_dev -fq --uuid $fsid --device-uuid $uuid $dev2
+}
-- 
2.39.3

[PATCH v3 08/10] btrfs: verify tempfsid clones using mkfs

[Anand Jain]

Create appearing to be a clone using the mkfs.btrfs option and test if
the tempfsid is active.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v3:
prerequisite checks are in the function mkfs_clone(), remove from the
testcase.

v2:
 Remove unnecessary function.
 Add clone group
 use $UMOUNT_PROG
 Let _cp_reflink fail on the stdout.

 tests/btrfs/313     | 53 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/313.out | 16 ++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100755 tests/btrfs/313
 create mode 100644 tests/btrfs/313.out

diff --git a/tests/btrfs/313 b/tests/btrfs/313
new file mode 100755
index 000000000000..1f50ee78ab99
--- /dev/null
+++ b/tests/btrfs/313
@@ -0,0 +1,53 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle.  All Rights Reserved.
+#
+# FS QA Test 313
+#
+# Functional test for the tempfsid, clone devices created using the mkfs option.
+#
+. ./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_cp_reflink
+_require_btrfs_sysfs_fsid
+_require_scratch_dev_pool 2
+_require_btrfs_fs_feature temp_fsid
+
+_scratch_dev_pool_get 2
+
+mnt1=$TEST_DIR/$seq/mnt1
+mkdir -p $mnt1
+
+echo ---- clone_uuids_verify_tempfsid ----
+mkfs_clone ${SCRATCH_DEV_NAME[0]} ${SCRATCH_DEV_NAME[1]}
+
+echo Mounting original device
+_mount ${SCRATCH_DEV_NAME[0]} $SCRATCH_MNT
+check_fsid ${SCRATCH_DEV_NAME[0]}
+
+echo Mounting cloned device
+_mount ${SCRATCH_DEV_NAME[1]} $mnt1
+check_fsid ${SCRATCH_DEV_NAME[1]}
+
+$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | _filter_xfs_io
+echo cp reflink must fail
+_cp_reflink $SCRATCH_MNT/foo $mnt1/bar 2>&1 | _filter_testdir_and_scratch
+
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/313.out b/tests/btrfs/313.out
new file mode 100644
index 000000000000..7a089d2c29c5
--- /dev/null
+++ b/tests/btrfs/313.out
@@ -0,0 +1,16 @@
+QA output created by 313
+---- clone_uuids_verify_tempfsid ----
+Mounting original device
+On disk fsid:		FSID
+Metadata uuid:		FSID
+Temp fsid:		FSID
+Tempfsid status:	0
+Mounting cloned device
+On disk fsid:		FSID
+Metadata uuid:		FSID
+Temp fsid:		TEMPFSID
+Tempfsid status:	1
+wrote 9000/9000 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+cp reflink must fail
+cp: failed to clone 'TEST_DIR/313/mnt1/bar' from 'SCRATCH_MNT/foo': Invalid cross-device link
-- 
2.39.3

[PATCH v3 09/10] btrfs: validate send-receive operation with tempfsid.

[Anand Jain]

Given concurrent mounting of both the original and its clone device on
the same system, this test confirms the integrity of send and receive
operations in the presence of active tempfsid.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v3:
 Drop prerequisite check in the testcase

v2:
 Organize changes to its right patch.
 Fix _fail erorr message.
 Declare local variables for fsid and uuid.

 tests/btrfs/314     | 79 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/314.out | 23 +++++++++++++
 2 files changed, 102 insertions(+)
 create mode 100755 tests/btrfs/314
 create mode 100644 tests/btrfs/314.out

diff --git a/tests/btrfs/314 b/tests/btrfs/314
new file mode 100755
index 000000000000..4a5b1ed2c06f
--- /dev/null
+++ b/tests/btrfs/314
@@ -0,0 +1,79 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle.  All Rights Reserved.
+#
+# FS QA Test 314
+#
+# Send and receive functionality test between a normal and
+# tempfsid filesystem.
+#
+. ./common/preamble
+_begin_fstest auto quick snapshot send tempfsid
+
+_cleanup()
+{
+	cd /
+	$UMOUNT_PROG $tempfsid_mnt 2>/dev/null
+	rm -r -f $tmp.*
+	rm -r -f $sendfile
+	rm -r -f $tempfsid_mnt
+}
+
+. ./common/filter.btrfs
+
+_supported_fs btrfs
+_require_btrfs_sysfs_fsid
+_require_scratch_dev_pool 2
+_require_btrfs_fs_feature temp_fsid
+
+_scratch_dev_pool_get 2
+
+# mount point for the tempfsid device
+tempfsid_mnt=$TEST_DIR/$seq/tempfsid_mnt
+sendfile=$TEST_DIR/$seq/replicate.send
+
+send_receive_tempfsid()
+{
+	local src=$1
+	local dst=$2
+
+	# Use first 2 devices from the SCRATCH_DEV_POOL
+	mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
+	_scratch_mount
+	_mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
+
+	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' ${src}/foo | _filter_xfs_io
+	$BTRFS_UTIL_PROG subvolume snapshot -r ${src} ${src}/snap1 | \
+						_filter_testdir_and_scratch
+
+	echo Send ${src} | _filter_testdir_and_scratch
+	$BTRFS_UTIL_PROG send -f ${sendfile} ${src}/snap1 2>&1 | \
+						_filter_testdir_and_scratch
+	echo Receive ${dst} | _filter_testdir_and_scratch
+	$BTRFS_UTIL_PROG receive -f ${sendfile} ${dst} | \
+						_filter_testdir_and_scratch
+	echo -e -n "Send:\t"
+	md5sum  ${src}/foo | _filter_testdir_and_scratch
+	echo -e -n "Recv:\t"
+	md5sum ${dst}/snap1/foo | _filter_testdir_and_scratch
+}
+
+mkdir -p $tempfsid_mnt
+
+echo -e \\nFrom non-tempfsid ${SCRATCH_MNT} to tempfsid ${tempfsid_mnt} | \
+						_filter_testdir_and_scratch
+send_receive_tempfsid $SCRATCH_MNT $tempfsid_mnt
+
+_scratch_unmount
+_cleanup
+mkdir -p $tempfsid_mnt
+
+echo -e \\nFrom tempfsid ${tempfsid_mnt} to non-tempfsid ${SCRATCH_MNT} | \
+						_filter_testdir_and_scratch
+send_receive_tempfsid $tempfsid_mnt $SCRATCH_MNT
+
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/314.out b/tests/btrfs/314.out
new file mode 100644
index 000000000000..21963899c2b2
--- /dev/null
+++ b/tests/btrfs/314.out
@@ -0,0 +1,23 @@
+QA output created by 314
+
+From non-tempfsid SCRATCH_MNT to tempfsid TEST_DIR/314/tempfsid_mnt
+wrote 9000/9000 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Create a readonly snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap1'
+Send SCRATCH_MNT
+At subvol SCRATCH_MNT/snap1
+Receive TEST_DIR/314/tempfsid_mnt
+At subvol snap1
+Send:	42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/foo
+Recv:	42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/314/tempfsid_mnt/snap1/foo
+
+From tempfsid TEST_DIR/314/tempfsid_mnt to non-tempfsid SCRATCH_MNT
+wrote 9000/9000 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Create a readonly snapshot of 'TEST_DIR/314/tempfsid_mnt' in 'TEST_DIR/314/tempfsid_mnt/snap1'
+Send TEST_DIR/314/tempfsid_mnt
+At subvol TEST_DIR/314/tempfsid_mnt/snap1
+Receive SCRATCH_MNT
+At subvol snap1
+Send:	42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/314/tempfsid_mnt/foo
+Recv:	42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/snap1/foo
-- 
2.39.3

[PATCH v3 10/10] btrfs: test tempfsid with device add, seed, and balance

[Anand Jain]

Make sure that basic functions such as seeding and device add fail,
while balance runs successfully with tempfsid.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v3:
 Comment updated.
 Add balance group.
 Drop prerequisite checks.
 Use error (from subvol create) in the golden output instead of calling _fail.

v2:
 Remove unnecessary function.
 Add clone group
 use $UMOUNT_PROG
 Let _cp_reflink fail on the stdout.

 tests/btrfs/315     | 78 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/315.out | 10 ++++++
 2 files changed, 88 insertions(+)
 create mode 100755 tests/btrfs/315
 create mode 100644 tests/btrfs/315.out

diff --git a/tests/btrfs/315 b/tests/btrfs/315
new file mode 100755
index 000000000000..696e26fe339c
--- /dev/null
+++ b/tests/btrfs/315
@@ -0,0 +1,78 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle. All Rights Reserved.
+#
+# FS QA Test 315
+#
+# Verify if the seed and device add to a tempfsid filesystem fails
+# and balance devices is successful.
+#
+. ./common/preamble
+_begin_fstest auto quick volume seed balance tempfsid
+
+_cleanup()
+{
+	cd /
+	$UMOUNT_PROG $tempfsid_mnt 2>/dev/null
+	rm -r -f $tmp.*
+	rm -r -f $tempfsid_mnt
+}
+
+. ./common/filter.btrfs
+
+_supported_fs btrfs
+_require_btrfs_sysfs_fsid
+_require_scratch_dev_pool 3
+_require_btrfs_fs_feature temp_fsid
+
+_scratch_dev_pool_get 3
+
+# mount point for the tempfsid device
+tempfsid_mnt=$TEST_DIR/$seq/tempfsid_mnt
+
+seed_device_must_fail()
+{
+	echo ---- $FUNCNAME ----
+
+	mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
+
+	$BTRFS_TUNE_PROG -S 1 ${SCRATCH_DEV}
+	$BTRFS_TUNE_PROG -S 1 ${SCRATCH_DEV_NAME[1]}
+
+	_scratch_mount 2>&1 | _filter_scratch
+	_mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt} 2>&1 | _filter_test_dir
+}
+
+device_add_must_fail()
+{
+	echo ---- $FUNCNAME ----
+
+	mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
+	_scratch_mount
+	_mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
+
+	$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
+							_filter_xfs_io
+
+$BTRFS_UTIL_PROG device add -f ${SCRATCH_DEV_NAME[2]} ${tempfsid_mnt} 2>&1 | \
+		grep -v "Performing full device TRIM" | _filter_scratch_pool
+
+	echo Balance must be successful
+	_run_btrfs_balance_start ${tempfsid_mnt}
+}
+
+mkdir -p $tempfsid_mnt
+
+seed_device_must_fail
+
+_scratch_unmount
+_cleanup
+mkdir -p $tempfsid_mnt
+
+device_add_must_fail
+
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/315.out b/tests/btrfs/315.out
new file mode 100644
index 000000000000..56301f9f069e
--- /dev/null
+++ b/tests/btrfs/315.out
@@ -0,0 +1,10 @@
+QA output created by 315
+---- seed_device_must_fail ----
+mount: SCRATCH_MNT: WARNING: source write-protected, mounted read-only.
+mount: TEST_DIR/315/tempfsid_mnt: mount(2) system call failed: File exists.
+---- device_add_must_fail ----
+wrote 9000/9000 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+ERROR: error adding device 'SCRATCH_DEV': Invalid argument
+Balance must be successful
+Done, had to relocate 3 out of 3 chunks
-- 
2.39.3

Re: [PATCH v3 03/10] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Filipe Manana]

On Sat, Feb 24, 2024 at 4:43 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> check_fsid() provides a method to verify if the given device is mounted
> with the tempfsid in the kernel. Function sb() is an internal only
> function.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> ---
> v3:
>  add rb
>  add  _require_btrfs_command inspect-internal dump-super
> v2:
>  Fix typo in the commit log.
>  Fix array SCRATCH_DEV_POOL_SAVED handling.
>
>  common/btrfs | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/common/btrfs b/common/btrfs
> index e1b29c613767..406be9574e32 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -792,3 +792,40 @@ _has_btrfs_sysfs_feature_attr()
>
>         test -e /sys/fs/btrfs/features/$feature_attr
>  }
> +
> +# Print the fsid and metadata uuid replaced with constant strings FSID and
> +# METADATA_UUID. Compare temp_fsid with fsid and metadata_uuid, then echo what
> +# it matches to or TEMP_FSID. This helps in comparing with the golden output.
> +check_fsid()
> +{
> +       local dev1=$1
> +       local fsid
> +       local metadata_uuid
> +
> +       _require_btrfs_command inspect-internal dump-super

So, as pointed out in the previous version of the patchset, the
function should do the require for everything it needs that may not be
available.
It's doing for the inspect-internal command, but it's missing a:

_require_btrfs_sysfs_fsid

Instead this is being called for every test case that calls this new
helper function, when those requirements should be hidden from the
tests themselves.

Thanks.

> +
> +       # on disk fsid
> +       fsid=$($BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | \
> +                               grep ^fsid | $AWK_PROG -d" " '{print $2}')
> +       echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
> +
> +       # Print FSID even if it is not the same as metadata_uuid because it has
> +       # to match in the golden output.
> +       metadata_uuid=$(cat /sys/fs/btrfs/$fsid/metadata_uuid)
> +       echo -e "Metadata uuid:\t\tFSID"
> +
> +       # This returns the temp_fsid if set
> +       tempfsid=$(_btrfs_get_fsid $dev1)
> +       if [[ $tempfsid == $fsid ]]; then
> +               echo -e "Temp fsid:\t\tFSID"
> +       elif [[ $tempfsid == $metadata_uuid ]]; then
> +               # If we are here, it means there is a bug; let it not match with
> +               # the golden output.
> +               echo -e "Temp fsid:\t\t$metadata_uuid"
> +       else
> +               echo -e "Temp fsid:\t\tTEMPFSID"
> +       fi
> +
> +       echo -e -n "Tempfsid status:\t"
> +       cat /sys/fs/btrfs/$tempfsid/temp_fsid
> +}
> --
> 2.39.3
>

Re: [PATCH v3 04/10] btrfs: verify that subvolume mounts are unaffected by tempfsid

[Filipe Manana]

On Sat, Feb 24, 2024 at 4:44 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> The tempfsid logic must determine whether the incoming mount request
> is for a device already mounted or a new device mount. Verify that it
> recognizes the device already mounted well by creating reflink across
> the subvolume mount points.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v3:
> Fix subvolume create output with _filter_scratch and its golden output
> add rb
> remove  _require_btrfs_command inspect-internal dump-super
> v2:
> add subvol group
> use $UMOUNT_PROG
> remove _fail for _cp_reflink
>
>  tests/btrfs/311     | 88 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/311.out | 24 +++++++++++++
>  2 files changed, 112 insertions(+)
>  create mode 100755 tests/btrfs/311
>  create mode 100644 tests/btrfs/311.out
>
> diff --git a/tests/btrfs/311 b/tests/btrfs/311
> new file mode 100755
> index 000000000000..bdabcf6a9814
> --- /dev/null
> +++ b/tests/btrfs/311
> @@ -0,0 +1,88 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 311
> +#
> +# Mount the device twice check if the reflink works, this helps to
> +# ensure device is mounted as the same device.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick subvol tempfsid
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +       cd /
> +       $UMOUNT_PROG $mnt1 > /dev/null 2>&1
> +       rm -r -f $tmp.*
> +       rm -r -f $mnt1
> +}
> +
> +. ./common/filter.btrfs
> +. ./common/reflink
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_require_cp_reflink
> +_require_btrfs_sysfs_fsid

This requirement should be inside the check_fsid() helper, as pointed before.

Thanks.

> +_require_btrfs_fs_feature temp_fsid
> +_require_scratch
> +
> +mnt1=$TEST_DIR/$seq/mnt1
> +mkdir -p $mnt1
> +
> +same_dev_mount()
> +{
> +       echo ---- $FUNCNAME ----
> +
> +       _scratch_mkfs >> $seqres.full 2>&1
> +
> +       _scratch_mount
> +       $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
> +                                                               _filter_xfs_io
> +
> +       echo Mount the device again to a different mount point
> +       _mount $SCRATCH_DEV $mnt1
> +
> +       _cp_reflink $SCRATCH_MNT/foo $mnt1/bar
> +       echo Checksum of reflinked files
> +       md5sum $SCRATCH_MNT/foo | _filter_scratch
> +       md5sum $mnt1/bar | _filter_test_dir
> +
> +       check_fsid $SCRATCH_DEV
> +}
> +
> +same_dev_subvol_mount()
> +{
> +       echo ---- $FUNCNAME ----
> +       _scratch_mkfs >> $seqres.full 2>&1
> +
> +       _scratch_mount
> +       $BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol | _filter_scratch
> +
> +       $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/subvol/foo | \
> +                                                               _filter_xfs_io
> +
> +       echo Mounting a subvol
> +       _mount -o subvol=subvol $SCRATCH_DEV $mnt1
> +
> +       _cp_reflink $SCRATCH_MNT/subvol/foo $mnt1/bar
> +       echo Checksum of reflinked files
> +       md5sum $SCRATCH_MNT/subvol/foo | _filter_scratch
> +       md5sum $mnt1/bar | _filter_test_dir
> +
> +       check_fsid $SCRATCH_DEV
> +}
> +
> +same_dev_mount
> +
> +_scratch_unmount
> +_cleanup
> +mkdir -p $mnt1
> +
> +same_dev_subvol_mount
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/311.out b/tests/btrfs/311.out
> new file mode 100644
> index 000000000000..4ea46eab3c72
> --- /dev/null
> +++ b/tests/btrfs/311.out
> @@ -0,0 +1,24 @@
> +QA output created by 311
> +---- same_dev_mount ----
> +wrote 9000/9000 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Mount the device again to a different mount point
> +Checksum of reflinked files
> +42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/foo
> +42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/311/mnt1/bar
> +On disk fsid:          FSID
> +Metadata uuid:         FSID
> +Temp fsid:             FSID
> +Tempfsid status:       0
> +---- same_dev_subvol_mount ----
> +Create subvolume 'SCRATCH_MNT/subvol'
> +wrote 9000/9000 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Mounting a subvol
> +Checksum of reflinked files
> +42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/subvol/foo
> +42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/311/mnt1/bar
> +On disk fsid:          FSID
> +Metadata uuid:         FSID
> +Temp fsid:             FSID
> +Tempfsid status:       0
> --
> 2.39.3
>

Re: [PATCH v3 05/10] btrfs: check if cloned device mounts with tempfsid

[Filipe Manana]

On Sat, Feb 24, 2024 at 4:44 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> If another device with the same fsid and uuid would mount then verify if
> it mounts with a temporary fsid.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v3:
> add clone group
> fix use $UMOUNT_PROG
> remove (_require_btrfs_command inspect-internal dump-super)
> v2:
> Bring create_cloned_devices() into the testcase.
> Just use _cp_reflink output to match with golden output.
>
>  tests/btrfs/312     | 84 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/312.out | 19 ++++++++++
>  2 files changed, 103 insertions(+)
>  create mode 100755 tests/btrfs/312
>  create mode 100644 tests/btrfs/312.out
>
> diff --git a/tests/btrfs/312 b/tests/btrfs/312
> new file mode 100755
> index 000000000000..90ca7a30d3e2
> --- /dev/null
> +++ b/tests/btrfs/312
> @@ -0,0 +1,84 @@
> +#! /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_btrfs_sysfs_fsid

This requirement should be inside the check_fsid() helper, as pointed before.

> +_require_btrfs_fs_feature temp_fsid
> +_require_scratch_dev_pool 2
> +_scratch_dev_pool_get 2
> +
> +mnt1=$TEST_DIR/$seq/mnt1
> +mkdir -p $mnt1
> +
> +create_cloned_devices()
> +{
> +       local dev1=$1
> +       local dev2=$2
> +
> +       [[ -z $dev1 || -z $dev2 ]] && \
> +               _fail "create_cloned_devices() requires two devices as arguments"

Now that the function is not generic, in common/btrfs, and used only
once in this test, this check doesn't make that much sense anymore.

> +
> +       echo -n Creating cloned device...

Wondering why the -n here, makes the golden output a bit weird.

> +       _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()
> +{
> +       local ret

Unused variable.

Thanks.

> +
> +       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
> new file mode 100644
> index 000000000000..b7de6ce3cc6e
> --- /dev/null
> +++ b/tests/btrfs/312.out
> @@ -0,0 +1,19 @@
> +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
> --
> 2.39.3
>

Re: [PATCH v3 07/10] btrfs: introduce helper for creating cloned devices with mkfs

[Filipe Manana]

On Sat, Feb 24, 2024 at 4:44 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> Use newer mkfs.btrfs option to generate two cloned devices,
> used in test cases.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, thanks.

> ---
> v3:
>  call
>  _require_btrfs_command inspect-internal dump-super
>  _require_btrfs_mkfs_uuid_option
>  in the function mkfs_clone()
>  Remove the conflict fix metadata line
>
> v2:
>  Organize changes to its right patch.
>  Fix _fail erorr message.
>  Declare local variables for fsid and uuid.
>
>  common/btrfs | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/common/btrfs b/common/btrfs
> index 55847733b20e..04ecff6ada71 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -840,3 +840,26 @@ check_fsid()
>         echo -e -n "Tempfsid status:\t"
>         cat /sys/fs/btrfs/$tempfsid/temp_fsid
>  }
> +
> +mkfs_clone()
> +{
> +       local fsid
> +       local uuid
> +       local dev1=$1
> +       local dev2=$2
> +
> +       _require_btrfs_command inspect-internal dump-super
> +       _require_btrfs_mkfs_uuid_option
> +
> +       [[ -z $dev1 || -z $dev2 ]] && \
> +               _fail "mkfs_clone requires two devices as arguments"
> +
> +       _mkfs_dev -fq $dev1
> +
> +       fsid=$($BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | \
> +                                       grep -E ^fsid | $AWK_PROG '{print $2}')
> +       uuid=$($BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | \
> +                               grep -E ^dev_item.uuid | $AWK_PROG '{print $2}')
> +
> +       _mkfs_dev -fq --uuid $fsid --device-uuid $uuid $dev2
> +}
> --
> 2.39.3
>

Re: [PATCH v3 08/10] btrfs: verify tempfsid clones using mkfs

[Filipe Manana]

On Sat, Feb 24, 2024 at 4:44 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> Create appearing to be a clone using the mkfs.btrfs option and test if
> the tempfsid is active.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v3:
> prerequisite checks are in the function mkfs_clone(), remove from the
> testcase.
>
> v2:
>  Remove unnecessary function.
>  Add clone group
>  use $UMOUNT_PROG
>  Let _cp_reflink fail on the stdout.
>
>  tests/btrfs/313     | 53 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/313.out | 16 ++++++++++++++
>  2 files changed, 69 insertions(+)
>  create mode 100755 tests/btrfs/313
>  create mode 100644 tests/btrfs/313.out
>
> diff --git a/tests/btrfs/313 b/tests/btrfs/313
> new file mode 100755
> index 000000000000..1f50ee78ab99
> --- /dev/null
> +++ b/tests/btrfs/313
> @@ -0,0 +1,53 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> +#
> +# FS QA Test 313
> +#
> +# Functional test for the tempfsid, clone devices created using the mkfs option.
> +#
> +. ./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_cp_reflink
> +_require_btrfs_sysfs_fsid

This requirement should be inside the check_fsid() helper, as pointed before.

Thanks.

> +_require_scratch_dev_pool 2
> +_require_btrfs_fs_feature temp_fsid
> +
> +_scratch_dev_pool_get 2
> +
> +mnt1=$TEST_DIR/$seq/mnt1
> +mkdir -p $mnt1
> +
> +echo ---- clone_uuids_verify_tempfsid ----
> +mkfs_clone ${SCRATCH_DEV_NAME[0]} ${SCRATCH_DEV_NAME[1]}
> +
> +echo Mounting original device
> +_mount ${SCRATCH_DEV_NAME[0]} $SCRATCH_MNT
> +check_fsid ${SCRATCH_DEV_NAME[0]}
> +
> +echo Mounting cloned device
> +_mount ${SCRATCH_DEV_NAME[1]} $mnt1
> +check_fsid ${SCRATCH_DEV_NAME[1]}
> +
> +$XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | _filter_xfs_io
> +echo cp reflink must fail
> +_cp_reflink $SCRATCH_MNT/foo $mnt1/bar 2>&1 | _filter_testdir_and_scratch
> +
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/313.out b/tests/btrfs/313.out
> new file mode 100644
> index 000000000000..7a089d2c29c5
> --- /dev/null
> +++ b/tests/btrfs/313.out
> @@ -0,0 +1,16 @@
> +QA output created by 313
> +---- clone_uuids_verify_tempfsid ----
> +Mounting original device
> +On disk fsid:          FSID
> +Metadata uuid:         FSID
> +Temp fsid:             FSID
> +Tempfsid status:       0
> +Mounting cloned device
> +On disk fsid:          FSID
> +Metadata uuid:         FSID
> +Temp fsid:             TEMPFSID
> +Tempfsid status:       1
> +wrote 9000/9000 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +cp reflink must fail
> +cp: failed to clone 'TEST_DIR/313/mnt1/bar' from 'SCRATCH_MNT/foo': Invalid cross-device link
> --
> 2.39.3
>

Re: [PATCH v3 09/10] btrfs: validate send-receive operation with tempfsid.

[Filipe Manana]

On Sat, Feb 24, 2024 at 4:44 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> Given concurrent mounting of both the original and its clone device on
> the same system, this test confirms the integrity of send and receive
> operations in the presence of active tempfsid.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v3:
>  Drop prerequisite check in the testcase
>
> v2:
>  Organize changes to its right patch.
>  Fix _fail erorr message.
>  Declare local variables for fsid and uuid.
>
>  tests/btrfs/314     | 79 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/314.out | 23 +++++++++++++
>  2 files changed, 102 insertions(+)
>  create mode 100755 tests/btrfs/314
>  create mode 100644 tests/btrfs/314.out
>
> diff --git a/tests/btrfs/314 b/tests/btrfs/314
> new file mode 100755
> index 000000000000..4a5b1ed2c06f
> --- /dev/null
> +++ b/tests/btrfs/314
> @@ -0,0 +1,79 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> +#
> +# FS QA Test 314
> +#
> +# Send and receive functionality test between a normal and
> +# tempfsid filesystem.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick snapshot send tempfsid
> +
> +_cleanup()
> +{
> +       cd /
> +       $UMOUNT_PROG $tempfsid_mnt 2>/dev/null
> +       rm -r -f $tmp.*
> +       rm -r -f $sendfile
> +       rm -r -f $tempfsid_mnt
> +}
> +
> +. ./common/filter.btrfs
> +
> +_supported_fs btrfs
> +_require_btrfs_sysfs_fsid

This requirement of the sysfs fsid path is not needed in the test, as
it's not used anywhere in this test (likely copy-pasted from other
tests in this patchset).

Thanks.

> +_require_scratch_dev_pool 2
> +_require_btrfs_fs_feature temp_fsid
> +
> +_scratch_dev_pool_get 2
> +
> +# mount point for the tempfsid device
> +tempfsid_mnt=$TEST_DIR/$seq/tempfsid_mnt
> +sendfile=$TEST_DIR/$seq/replicate.send
> +
> +send_receive_tempfsid()
> +{
> +       local src=$1
> +       local dst=$2
> +
> +       # Use first 2 devices from the SCRATCH_DEV_POOL
> +       mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
> +       _scratch_mount
> +       _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
> +
> +       $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' ${src}/foo | _filter_xfs_io
> +       $BTRFS_UTIL_PROG subvolume snapshot -r ${src} ${src}/snap1 | \
> +                                               _filter_testdir_and_scratch
> +
> +       echo Send ${src} | _filter_testdir_and_scratch
> +       $BTRFS_UTIL_PROG send -f ${sendfile} ${src}/snap1 2>&1 | \
> +                                               _filter_testdir_and_scratch
> +       echo Receive ${dst} | _filter_testdir_and_scratch
> +       $BTRFS_UTIL_PROG receive -f ${sendfile} ${dst} | \
> +                                               _filter_testdir_and_scratch
> +       echo -e -n "Send:\t"
> +       md5sum  ${src}/foo | _filter_testdir_and_scratch
> +       echo -e -n "Recv:\t"
> +       md5sum ${dst}/snap1/foo | _filter_testdir_and_scratch
> +}
> +
> +mkdir -p $tempfsid_mnt
> +
> +echo -e \\nFrom non-tempfsid ${SCRATCH_MNT} to tempfsid ${tempfsid_mnt} | \
> +                                               _filter_testdir_and_scratch
> +send_receive_tempfsid $SCRATCH_MNT $tempfsid_mnt
> +
> +_scratch_unmount
> +_cleanup
> +mkdir -p $tempfsid_mnt
> +
> +echo -e \\nFrom tempfsid ${tempfsid_mnt} to non-tempfsid ${SCRATCH_MNT} | \
> +                                               _filter_testdir_and_scratch
> +send_receive_tempfsid $tempfsid_mnt $SCRATCH_MNT
> +
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/314.out b/tests/btrfs/314.out
> new file mode 100644
> index 000000000000..21963899c2b2
> --- /dev/null
> +++ b/tests/btrfs/314.out
> @@ -0,0 +1,23 @@
> +QA output created by 314
> +
> +From non-tempfsid SCRATCH_MNT to tempfsid TEST_DIR/314/tempfsid_mnt
> +wrote 9000/9000 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Create a readonly snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap1'
> +Send SCRATCH_MNT
> +At subvol SCRATCH_MNT/snap1
> +Receive TEST_DIR/314/tempfsid_mnt
> +At subvol snap1
> +Send:  42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/foo
> +Recv:  42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/314/tempfsid_mnt/snap1/foo
> +
> +From tempfsid TEST_DIR/314/tempfsid_mnt to non-tempfsid SCRATCH_MNT
> +wrote 9000/9000 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Create a readonly snapshot of 'TEST_DIR/314/tempfsid_mnt' in 'TEST_DIR/314/tempfsid_mnt/snap1'
> +Send TEST_DIR/314/tempfsid_mnt
> +At subvol TEST_DIR/314/tempfsid_mnt/snap1
> +Receive SCRATCH_MNT
> +At subvol snap1
> +Send:  42d69d1a6d333a7ebdf64792a555e392  TEST_DIR/314/tempfsid_mnt/foo
> +Recv:  42d69d1a6d333a7ebdf64792a555e392  SCRATCH_MNT/snap1/foo
> --
> 2.39.3
>

Re: [PATCH v3 10/10] btrfs: test tempfsid with device add, seed, and balance

[Filipe Manana]

On Sat, Feb 24, 2024 at 4:44 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> Make sure that basic functions such as seeding and device add fail,
> while balance runs successfully with tempfsid.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v3:
>  Comment updated.
>  Add balance group.
>  Drop prerequisite checks.
>  Use error (from subvol create) in the golden output instead of calling _fail.
>
> v2:
>  Remove unnecessary function.
>  Add clone group
>  use $UMOUNT_PROG
>  Let _cp_reflink fail on the stdout.
>
>  tests/btrfs/315     | 78 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/315.out | 10 ++++++
>  2 files changed, 88 insertions(+)
>  create mode 100755 tests/btrfs/315
>  create mode 100644 tests/btrfs/315.out
>
> diff --git a/tests/btrfs/315 b/tests/btrfs/315
> new file mode 100755
> index 000000000000..696e26fe339c
> --- /dev/null
> +++ b/tests/btrfs/315
> @@ -0,0 +1,78 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 315
> +#
> +# Verify if the seed and device add to a tempfsid filesystem fails
> +# and balance devices is successful.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick volume seed balance tempfsid
> +
> +_cleanup()
> +{
> +       cd /
> +       $UMOUNT_PROG $tempfsid_mnt 2>/dev/null
> +       rm -r -f $tmp.*
> +       rm -r -f $tempfsid_mnt
> +}
> +
> +. ./common/filter.btrfs
> +
> +_supported_fs btrfs
> +_require_btrfs_sysfs_fsid

Same as in the previous test case. This requirement is not needed, the
sysfs fsid path is not used anywhere in this test, likely copy-pasted
from previous test cases in this patchset.

Thanks.

> +_require_scratch_dev_pool 3
> +_require_btrfs_fs_feature temp_fsid
> +
> +_scratch_dev_pool_get 3
> +
> +# mount point for the tempfsid device
> +tempfsid_mnt=$TEST_DIR/$seq/tempfsid_mnt
> +
> +seed_device_must_fail()
> +{
> +       echo ---- $FUNCNAME ----
> +
> +       mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
> +
> +       $BTRFS_TUNE_PROG -S 1 ${SCRATCH_DEV}
> +       $BTRFS_TUNE_PROG -S 1 ${SCRATCH_DEV_NAME[1]}
> +
> +       _scratch_mount 2>&1 | _filter_scratch
> +       _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt} 2>&1 | _filter_test_dir
> +}
> +
> +device_add_must_fail()
> +{
> +       echo ---- $FUNCNAME ----
> +
> +       mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
> +       _scratch_mount
> +       _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
> +
> +       $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
> +                                                       _filter_xfs_io
> +
> +$BTRFS_UTIL_PROG device add -f ${SCRATCH_DEV_NAME[2]} ${tempfsid_mnt} 2>&1 | \
> +               grep -v "Performing full device TRIM" | _filter_scratch_pool
> +
> +       echo Balance must be successful
> +       _run_btrfs_balance_start ${tempfsid_mnt}
> +}
> +
> +mkdir -p $tempfsid_mnt
> +
> +seed_device_must_fail
> +
> +_scratch_unmount
> +_cleanup
> +mkdir -p $tempfsid_mnt
> +
> +device_add_must_fail
> +
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/315.out b/tests/btrfs/315.out
> new file mode 100644
> index 000000000000..56301f9f069e
> --- /dev/null
> +++ b/tests/btrfs/315.out
> @@ -0,0 +1,10 @@
> +QA output created by 315
> +---- seed_device_must_fail ----
> +mount: SCRATCH_MNT: WARNING: source write-protected, mounted read-only.
> +mount: TEST_DIR/315/tempfsid_mnt: mount(2) system call failed: File exists.
> +---- device_add_must_fail ----
> +wrote 9000/9000 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +ERROR: error adding device 'SCRATCH_DEV': Invalid argument
> +Balance must be successful
> +Done, had to relocate 3 out of 3 chunks
> --
> 2.39.3
>

Re: [PATCH v3 03/10] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Anand Jain]

> function should do the require for everything it needs that may not be
> available.
> It's doing for the inspect-internal command, but it's missing a:
> 
> _require_btrfs_sysfs_fsid


Yes, it did. Actually, check_fsid() would need the following to
cover all the prerequisites.

  _require_btrfs_fs_sysfs
  _require_btrfs_fs_feature temp_fsid
  _require_btrfs_fs_feature metadata_uuid
  _require_btrfs_command inspect-internal dump-super


I already have v4 with what you just suggested, I am going to send it.


 > Instead this is being called for every test case that calls this new
 > helper function, when those requirements should be hidden from the
 > tests themselves.

However, I am a bit skeptical if we should move all prerequisites to
the helpers or only some major prerequisites.

Because returning _notrun() in the middle of the testcase is something
I am not sure is better than at the beginning of the testcase (I do not
have a specific example where it is not a good idea, though).

And, theoretically, figuring out if the test case would run/_notrun()
will be complicated.

Next, we shall end up checking the _require..() multiple times in
a test case, though one time is enough (the test cases 311, 312,
313 call check_fsid() two times).

Furthermore, it will inconsistent, as a lot of command wraps are
already missing such a requirement; I'm not sure if we shall ever
achieve consistency across fstests (For example: _cp_reflink()
missing _require_cp_reflink).

Lastly, if there are duplicating prerequisites across the helper
functions, then we call _require..() many more times (for example:
313 will call mkfs_clone() and check_fsid() two times, which
means we would verify the following three times in a testcase.

  _require_btrfs_fs_feature metadata_uuid
  _require_btrfs_command inspect-internal dump-super


So, how about prerequisites of the newer functions as comments
above the function to be copied into the test case?

Thanks, Anand

Re: [PATCH v3 03/10] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Filipe Manana]

On Wed, Feb 28, 2024 at 9:36 AM Anand Jain <anand.jain@oracle.com> wrote:
>
>
>
> > function should do the require for everything it needs that may not be
> > available.
> > It's doing for the inspect-internal command, but it's missing a:
> >
> > _require_btrfs_sysfs_fsid
>
>
> Yes, it did. Actually, check_fsid() would need the following to
> cover all the prerequisites.
>
>   _require_btrfs_fs_sysfs
>   _require_btrfs_fs_feature temp_fsid
>   _require_btrfs_fs_feature metadata_uuid
>   _require_btrfs_command inspect-internal dump-super
>
>
> I already have v4 with what you just suggested, I am going to send it.
>
>
>  > Instead this is being called for every test case that calls this new
>  > helper function, when those requirements should be hidden from the
>  > tests themselves.
>
> However, I am a bit skeptical if we should move all prerequisites to
> the helpers or only some major prerequisites.
>
> Because returning _notrun() in the middle of the testcase is something
> I am not sure is better than at the beginning of the testcase (I do not
> have a specific example where it is not a good idea, though).
>
> And, theoretically, figuring out if the test case would run/_notrun()
> will be complicated.
>
> Next, we shall end up checking the _require..() multiple times in
> a test case, though one time is enough (the test cases 311, 312,
> 313 call check_fsid() two times).
>
> Furthermore, it will inconsistent, as a lot of command wraps are
> already missing such a requirement; I'm not sure if we shall ever
> achieve consistency across fstests (For example: _cp_reflink()
> missing _require_cp_reflink).
>
> Lastly, if there are duplicating prerequisites across the helper
> functions, then we call _require..() many more times (for example:
> 313 will call mkfs_clone() and check_fsid() two times, which
> means we would verify the following three times in a testcase.
>
>   _require_btrfs_fs_feature metadata_uuid
>   _require_btrfs_command inspect-internal dump-super
>
>
> So, how about prerequisites of the newer functions as comments
> above the function to be copied into the test case?

Calling the require functions doesn't take that much time, I'm not
worried about more 1, 2, 3 or 10 milliseconds of test run time.

Now having each test that uses a common function to call all the
require functions is hard to maintain and messy.

Commenting the requirements on top of each function is not bullet
proof - test authors will have to do it and reviewers as well all the
time.
Not to mention that if a function's implementation changes and now it
has different requirements, we'll have to change every single test
that uses it.

Thanks.

>
> Thanks, Anand

Re: [PATCH v3 09/10] btrfs: validate send-receive operation with tempfsid.

[Anand Jain]

>> +. ./common/filter.btrfs
>> +
>> +_supported_fs btrfs
>> +_require_btrfs_sysfs_fsid
> 
> This requirement of the sysfs fsid path is not needed in the test, as
> it's not used anywhere in this test (likely copy-pasted from other
> tests in this patchset).
> 

Right. But it requires '_require_btrfs_fs_feature temp_fsid'.
Added.

Thanks, Anand

Re: [PATCH v3 05/10] btrfs: check if cloned device mounts with tempfsid

[Anand Jain]

>> +create_cloned_devices()
>> +{
>> +       local dev1=$1
>> +       local dev2=$2
>> +
>> +       [[ -z $dev1 || -z $dev2 ]] && \
>> +               _fail "create_cloned_devices() requires two devices as arguments"
> 
> Now that the function is not generic, in common/btrfs, and used only
> once in this test, this check doesn't make that much sense anymore.
> 

  Removed.

>> +
>> +       echo -n Creating cloned device...
> 
> Wondering why the -n here, makes the golden output a bit weird.
> 

  Overall it limits the output to a line; organized well w.r.t the
  helper function, deciding to keep it. Thx.


>> +       _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()
>> +{
>> +       local ret
> 
> Unused variable.
> 

  Removed.

Thanks. Anand

> Thanks.
> 
>> +
>> +       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
>> new file mode 100644
>> index 000000000000..b7de6ce3cc6e
>> --- /dev/null
>> +++ b/tests/btrfs/312.out
>> @@ -0,0 +1,19 @@
>> +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
>> --
>> 2.39.3
>>

Re: [PATCH v3 10/10] btrfs: test tempfsid with device add, seed, and balance

[Anand Jain]

>> +_require_btrfs_sysfs_fsid
> 
> Same as in the previous test case. This requirement is not needed, the
> sysfs fsid path is not used anywhere in this test, likely copy-pasted
> from previous test cases in this patchset.
> 

  Yep. Replaced with "_require_btrfs_fs_feature temp_fsid"

>> diff --git a/tests/btrfs/315.out b/tests/btrfs/315.out
>> new file mode 100644
>> index 000000000000..56301f9f069e
>> --- /dev/null
>> +++ b/tests/btrfs/315.out
>> @@ -0,0 +1,10 @@
>> +QA output created by 315
>> +---- seed_device_must_fail ----
>> +mount: SCRATCH_MNT: WARNING: source write-protected, mounted read-only.

>> +mount: TEST_DIR/315/tempfsid_mnt: mount(2) system call failed: File exists.

The error output has changed due to some external fixes.
V4 contains changes accordingly.

Old:
         mount: <mnt-point>: mount(2) system call failed: File exists.

New:
         mount: <mnt-point>: fsconfig system call failed: File exists.
         dmesg(1) may have more information after failed mount system call.

Thanks, Anand

Re: [PATCH v3 03/10] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Anand Jain]

On 2/28/24 15:58, Filipe Manana wrote:
> On Wed, Feb 28, 2024 at 9:36 AM Anand Jain <anand.jain@oracle.com> wrote:
>>
>>
>>
>>> function should do the require for everything it needs that may not be
>>> available.
>>> It's doing for the inspect-internal command, but it's missing a:
>>>
>>> _require_btrfs_sysfs_fsid
>>
>>
>> Yes, it did. Actually, check_fsid() would need the following to
>> cover all the prerequisites.
>>
>>    _require_btrfs_fs_sysfs
>>    _require_btrfs_fs_feature temp_fsid
>>    _require_btrfs_fs_feature metadata_uuid
>>    _require_btrfs_command inspect-internal dump-super
>>
>>
>> I already have v4 with what you just suggested, I am going to send it.
>>
>>
>>   > Instead this is being called for every test case that calls this new
>>   > helper function, when those requirements should be hidden from the
>>   > tests themselves.
>>
>> However, I am a bit skeptical if we should move all prerequisites to
>> the helpers or only some major prerequisites.
>>
>> Because returning _notrun() in the middle of the testcase is something
>> I am not sure is better than at the beginning of the testcase (I do not
>> have a specific example where it is not a good idea, though).
>>
>> And, theoretically, figuring out if the test case would run/_notrun()
>> will be complicated.
>>
>> Next, we shall end up checking the _require..() multiple times in
>> a test case, though one time is enough (the test cases 311, 312,
>> 313 call check_fsid() two times).
>>
>> Furthermore, it will inconsistent, as a lot of command wraps are
>> already missing such a requirement; I'm not sure if we shall ever
>> achieve consistency across fstests (For example: _cp_reflink()
>> missing _require_cp_reflink).
>>
>> Lastly, if there are duplicating prerequisites across the helper
>> functions, then we call _require..() many more times (for example:
>> 313 will call mkfs_clone() and check_fsid() two times, which
>> means we would verify the following three times in a testcase.
>>
>>    _require_btrfs_fs_feature metadata_uuid
>>    _require_btrfs_command inspect-internal dump-super
>>
>>
>> So, how about prerequisites of the newer functions as comments
>> above the function to be copied into the test case?
> 
> Calling the require functions doesn't take that much time, I'm not
> worried about more 1, 2, 3 or 10 milliseconds of test run time.
> 
> Now having each test that uses a common function to call all the
> require functions is hard to maintain and messy.
> 
> Commenting the requirements on top of each function is not bullet
> proof - test authors will have to do it and reviewers as well all the
> time.
> Not to mention that if a function's implementation changes and now it
> has different requirements, we'll have to change every single test
> that uses it.

I was trying to keep the code optimized and avoid duplicate '_require..'
statements as much as possible. Also, I aimed to avoid '_notrun' in the
middle of the testcase, which keeps it inline with the rest of the older
testcases. However, it seems not to be a big deal, so let the
'_require..' statements be in the helpers. This makes the test case
look more concise and further makes it easy to make changes. For
example, if a helper is deleted, the testcase will still be fine
without bugs. I have updated the rest of the test cases with this
idea in v4.

Thanks, Anand