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

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

[Anand Jain]

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

Anand Jain (12):
  add t_reflink_read_race to .gitignore file
  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
  create a helper to clone devices
  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

 .gitignore          |  1 +
 common/btrfs        | 84 +++++++++++++++++++++++++++++++++++++++++
 common/filter.btrfs |  6 +++
 common/rc           | 15 +++++++-
 doc/group-names.txt |  1 +
 tests/btrfs/311     | 91 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/311.out | 24 ++++++++++++
 tests/btrfs/312     | 67 +++++++++++++++++++++++++++++++++
 tests/btrfs/312.out | 19 ++++++++++
 tests/btrfs/313     | 66 ++++++++++++++++++++++++++++++++
 tests/btrfs/313.out | 16 ++++++++
 tests/btrfs/314     | 85 ++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/314.out | 30 +++++++++++++++
 tests/btrfs/315     | 79 +++++++++++++++++++++++++++++++++++++++
 tests/btrfs/315.out | 11 ++++++
 15 files changed, 593 insertions(+), 2 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 01/12] add t_reflink_read_race to .gitignore file

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 5cc3a5e4ae57..d930b9cc8404 100644
--- a/.gitignore
+++ b/.gitignore
@@ -204,6 +204,7 @@ tags
 /src/vfs/mount-idmapped
 /src/log-writes/replay-log
 /src/perf/*.pyc
+/src/t_reflink_read_race
 
 # Symlinked files
 /tests/generic/035.out
-- 
2.39.3

Re: [PATCH 01/12] add t_reflink_read_race to .gitignore file

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:34 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Please always add a changelog...

Also, you need to update your local checkout...
This was already fixed before:

https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?h=for-next&id=acff198213e3d874e76f6a133a816c8dee5e128d

It's also odd to include this in a patchset for the tempfsid feature,
as it's completely unrelated, it should go as a separate patch.

Thanks.

> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.gitignore b/.gitignore
> index 5cc3a5e4ae57..d930b9cc8404 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -204,6 +204,7 @@ tags
>  /src/vfs/mount-idmapped
>  /src/log-writes/replay-log
>  /src/perf/*.pyc
> +/src/t_reflink_read_race
>
>  # Symlinked files
>  /tests/generic/035.out
> --
> 2.39.3
>
>

Re: [PATCH 01/12] add t_reflink_read_race to .gitignore file

[Anand Jain]

On 2/15/24 17:15, Filipe Manana wrote:
> On Thu, Feb 15, 2024 at 6:34 AM Anand Jain <anand.jain@oracle.com> wrote:
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> Please always add a changelog...
> 
> Also, you need to update your local checkout...

Oh no. I thought I did.


> This was already fixed before:
> 
> https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?h=for-next&id=acff198213e3d874e76f6a133a816c8dee5e128d


> 
> It's also odd to include this in a patchset for the tempfsid feature,
> as it's completely unrelated, it should go as a separate patch.
> 

I should have.

I'll drop this patch in v2 (if any).

Thanks, Anand

> Thanks.
> 
>> ---
>>   .gitignore | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/.gitignore b/.gitignore
>> index 5cc3a5e4ae57..d930b9cc8404 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -204,6 +204,7 @@ tags
>>   /src/vfs/mount-idmapped
>>   /src/log-writes/replay-log
>>   /src/perf/*.pyc
>> +/src/t_reflink_read_race
>>
>>   # Symlinked files
>>   /tests/generic/035.out
>> --
>> 2.39.3
>>
>>

[PATCH 02/12] assign SCRATCH_DEV_POOL to an array

[Anand Jain]

Many test cases uses local variable to manage the names of each devices in
SCRATCH_DEV_POOL. Let _scratch_dev_pool_get set an array 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>
---
 common/rc | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 524ffa02aa6a..5e4afb2cd484 100644
--- a/common/rc
+++ b/common/rc
@@ -830,6 +830,8 @@ _spare_dev_put()
 # 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.
+# Also, this functions assigns array SCRATCH_DEV_NAME to the
+# array SCRATCH_DEV_POOL.
 #
 # Usage:
 #  _scratch_dev_pool_get() <ndevs>
@@ -860,19 +862,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

Re: [PATCH 02/12] assign SCRATCH_DEV_POOL to an array

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:34 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> Many test cases uses local variable to manage the names of each devices in

uses -> use
variable -> variables
devices -> device

> SCRATCH_DEV_POOL. Let _scratch_dev_pool_get set an array 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>
> ---
>  common/rc | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 524ffa02aa6a..5e4afb2cd484 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -830,6 +830,8 @@ _spare_dev_put()
>  # 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.
> +# Also, this functions assigns array SCRATCH_DEV_NAME to the
> +# array SCRATCH_DEV_POOL.
>  #
>  # Usage:
>  #  _scratch_dev_pool_get() <ndevs>
> @@ -860,19 +862,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

missing dollar before SCRATCH_DEV_NAME... and should all be inside
double quotes, as -z is meant to test strings.
And as it's supposed to be an array, test it like:

-z "${SCRATCH_DEV_NAME[@]}"

>                 _fail "Bug: str empty, must call _scratch_dev_pool_get before _scratch_dev_pool_put"
>         fi
>
> +       export SCRATCH_DEV_NAME=""

Would rather assign an empty array (), as the variable is set to an
array in the get function.

Thanks.

>         export SCRATCH_DEV_POOL=$SCRATCH_DEV_POOL_SAVED
>         export SCRATCH_DEV_POOL_SAVED=""
>  }
> --
> 2.39.3
>
>

[PATCH 03/12] 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.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 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

Re: [PATCH 03/12] btrfs: introduce tempfsid test group

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:37 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> 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.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

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

Thanks.

> ---
>  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 04/12] 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>
---
 common/btrfs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/common/btrfs b/common/btrfs
index e1b29c613767..5cba9b16b4de 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -792,3 +792,37 @@ _has_btrfs_sysfs_feature_attr()
 
 	test -e /sys/fs/btrfs/features/$feature_attr
 }
+
+# Dump key members of the on-disk super-block from the given disk; helps debug
+sb()
+{
+	local dev1=$1
+	local parameters="device|devid|^metadata_uuid|^fsid|^incom|^generation|\
+		^flags| \|$| \)$|compat_flags|compat_ro_flags|dev_item.uuid"
+
+	$BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | egrep "$parameters"
+}
+
+check_fsid()
+{
+	local dev1=$1
+	local fsid
+
+	# on disk fsid
+	fsid=$(sb $dev1 | grep ^fsid | awk -d" " '{print $2}')
+	echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
+
+	echo -e -n "Metadata uuid:\t\t"
+	cat /sys/fs/btrfs/$fsid/metadata_uuid | sed -e "s/$fsid/FSID/g"
+
+	# This returns the temp_fsid if set
+	tempfsid=$(_btrfs_get_fsid $dev1)
+	if [[ $tempfsid == $fsid ]]; then
+		echo -e "Temp fsid:\t\tFSID"
+	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 04/12] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:35 AM 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>
> ---
>  common/btrfs | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/common/btrfs b/common/btrfs
> index e1b29c613767..5cba9b16b4de 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -792,3 +792,37 @@ _has_btrfs_sysfs_feature_attr()
>
>         test -e /sys/fs/btrfs/features/$feature_attr
>  }
> +
> +# Dump key members of the on-disk super-block from the given disk; helps debug
> +sb()
> +{
> +       local dev1=$1
> +       local parameters="device|devid|^metadata_uuid|^fsid|^incom|^generation|\
> +               ^flags| \|$| \)$|compat_flags|compat_ro_flags|dev_item.uuid"
> +
> +       $BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | egrep "$parameters"
> +}

Don't add such a short function name that doesn't minimally indicate
what it does.... especially taking into account that it's polluting
the global namespace and isn't used anywhere else.
So this code should be in the function below, as it's only used once....

> +
> +check_fsid()
> +{
> +       local dev1=$1
> +       local fsid
> +
> +       # on disk fsid
> +       fsid=$(sb $dev1 | grep ^fsid | awk -d" " '{print $2}')

Please use $AWK_PROG instead.

Again this sb() function is pointless, even because here we only care
about the fsid line, yet the function is extracting a lot of other
lines without any users.

> +       echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
> +
> +       echo -e -n "Metadata uuid:\t\t"
> +       cat /sys/fs/btrfs/$fsid/metadata_uuid | sed -e "s/$fsid/FSID/g"

Ok, so why do we care about printing the fsid and metadata uuid lines
if we always replace the IDs with the constant FSID?
It seems pointless to print them... At the very least this deserves a
comment, a rationale on what this accomplishes.

Thanks.

> +
> +       # This returns the temp_fsid if set
> +       tempfsid=$(_btrfs_get_fsid $dev1)
> +       if [[ $tempfsid == $fsid ]]; then
> +               echo -e "Temp fsid:\t\tFSID"
> +       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 04/12] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Zorro Lang]

On Thu, Feb 15, 2024 at 02:34:07PM +0800, Anand Jain 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>
> ---
>  common/btrfs | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/common/btrfs b/common/btrfs
> index e1b29c613767..5cba9b16b4de 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -792,3 +792,37 @@ _has_btrfs_sysfs_feature_attr()
>  
>  	test -e /sys/fs/btrfs/features/$feature_attr
>  }
> +
> +# Dump key members of the on-disk super-block from the given disk; helps debug
> +sb()
> +{
> +	local dev1=$1
> +	local parameters="device|devid|^metadata_uuid|^fsid|^incom|^generation|\
> +		^flags| \|$| \)$|compat_flags|compat_ro_flags|dev_item.uuid"
> +
> +	$BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | egrep "$parameters"

Please don't use "egrep", it's deprecated, might hit a warning output.
Replace it with "grep -E" :)

> +}
> +
> +check_fsid()
> +{
> +	local dev1=$1
> +	local fsid
> +
> +	# on disk fsid
> +	fsid=$(sb $dev1 | grep ^fsid | awk -d" " '{print $2}')
> +	echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
> +
> +	echo -e -n "Metadata uuid:\t\t"
> +	cat /sys/fs/btrfs/$fsid/metadata_uuid | sed -e "s/$fsid/FSID/g"
> +
> +	# This returns the temp_fsid if set
> +	tempfsid=$(_btrfs_get_fsid $dev1)
> +	if [[ $tempfsid == $fsid ]]; then
> +		echo -e "Temp fsid:\t\tFSID"
> +	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 05/12] 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>
---
 tests/btrfs/311     | 91 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/311.out | 24 ++++++++++++
 2 files changed, 115 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..71c26055fa1e
--- /dev/null
+++ b/tests/btrfs/311
@@ -0,0 +1,91 @@
+#! /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 tempfsid
+
+# Override the default cleanup function.
+_cleanup()
+{
+	cd /
+	umount $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_btrfs_command inspect-internal dump-super
+_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 || \
+		_fail "reflink failed, check if mounted as the same device"
+	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
+
+	$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 || \
+			_fail "reflink failed, not the same device?"
+	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..8787f24ab867
--- /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 '/mnt/scratch/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 05/12] btrfs: verify that subvolume mounts are unaffected by tempfsid

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:35 AM 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>
> ---
>  tests/btrfs/311     | 91 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/311.out | 24 ++++++++++++
>  2 files changed, 115 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..71c26055fa1e
> --- /dev/null
> +++ b/tests/btrfs/311
> @@ -0,0 +1,91 @@
> +#! /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 tempfsid

Also 'subvol' group, as the test creates a subvolume and makes
something with it.

> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +       cd /
> +       umount $mnt1 > /dev/null 2>&1

Use $UMOUNT_PROG please.

> +       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_btrfs_command inspect-internal dump-super
> +_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 || \
> +               _fail "reflink failed, check if mounted as the same device"

What's the _fail for?
If cp fails it outputs an error and causes a mismatch with the golden
output, automatically failing the
test and in an easy way to notice the failure was due to a cp failure...

> +       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
> +
> +       $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 || \
> +                       _fail "reflink failed, not the same device?"

Same as above.

Thanks.

> +       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..8787f24ab867
> --- /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 '/mnt/scratch/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 06/12] create a helper to clone devices

[Anand Jain]

The function create_cloned_devices() takes two devices, creates
a filesystem using mkfs, and clones the content from one device
to another using device dump.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/btrfs | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/common/btrfs b/common/btrfs
index 5cba9b16b4de..61d5812d49d9 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -826,3 +826,25 @@ check_fsid()
 	echo -e -n "Tempfsid status:\t"
 	cat /sys/fs/btrfs/$tempfsid/temp_fsid
 }
+
+create_cloned_devices()
+{
+	local dev1=$1
+	local dev2=$2
+
+	[[ -z $dev1 || -z $dev2 ]] && \
+		_fail "BUGGY code, create_cloned_devices needs arg1 arg3"
+
+	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 $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
+}
-- 
2.39.3

Re: [PATCH 06/12] create a helper to clone devices

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:35 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> The function create_cloned_devices() takes two devices, creates
> a filesystem using mkfs, and clones the content from one device
> to another using device dump.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  common/btrfs | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/common/btrfs b/common/btrfs
> index 5cba9b16b4de..61d5812d49d9 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -826,3 +826,25 @@ check_fsid()
>         echo -e -n "Tempfsid status:\t"
>         cat /sys/fs/btrfs/$tempfsid/temp_fsid
>  }
> +
> +create_cloned_devices()
> +{
> +       local dev1=$1
> +       local dev2=$2
> +
> +       [[ -z $dev1 || -z $dev2 ]] && \
> +               _fail "BUGGY code, create_cloned_devices needs arg1 arg3"

arg3 should be arg2.

But rather than such a cryptic message, I would for something more
clear and informative such as:

"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 $SCRATCH_MNT

$UMOUNT_PROG

So we have a generic function that is named create_cloned_devices()
that is too specific for
a single test. The whole thing of creating exactly a 300M fs and with
a single file, it's anything but generic.

Really as it's only used in a single test case and given its
specificity, it should not be made generic by having it in
common/btrfs.
I'd rather have it in the test case that is using it.

Thanks.

> +       # device dump of $dev1 to $dev2
> +       dd if=$dev1 of=$dev2 bs=300M count=1 conv=fsync status=none || \
> +                                                       _fail "dd failed: $?"
> +       echo done
> +}
> --
> 2.39.3
>
>

[PATCH 07/12] 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>
---
 tests/btrfs/312     | 67 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/312.out | 19 +++++++++++++
 2 files changed, 86 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..782490b1c62f
--- /dev/null
+++ b/tests/btrfs/312
@@ -0,0 +1,67 @@
+#! /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 tempfsid
+
+_cleanup()
+{
+	cd /
+	umount $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_btrfs_command inspect-internal dump-super
+_require_scratch_dev_pool 2
+_scratch_dev_pool_get 2
+
+mnt1=$TEST_DIR/$seq/mnt1
+mkdir -p $mnt1
+
+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 > $tmp.cp.out 2>&1
+	ret=$?
+	cat $tmp.cp.out | _filter_testdir_and_scratch
+	if [ $ret -ne 1 ]; then
+		_fail "reflink failed to fail"
+	fi
+
+	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 07/12] btrfs: check if cloned device mounts with tempfsid

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:35 AM 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>
> ---
>  tests/btrfs/312     | 67 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/312.out | 19 +++++++++++++
>  2 files changed, 86 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..782490b1c62f
> --- /dev/null
> +++ b/tests/btrfs/312
> @@ -0,0 +1,67 @@
> +#! /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 tempfsid

Also add the 'clone' group, as it uses reflinks.

> +
> +_cleanup()
> +{
> +       cd /
> +       umount $mnt1 > /dev/null 2>&1

Use $UMOUNT_PROG.

> +       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_btrfs_command inspect-internal dump-super
> +_require_scratch_dev_pool 2
> +_scratch_dev_pool_get 2
> +
> +mnt1=$TEST_DIR/$seq/mnt1
> +mkdir -p $mnt1
> +
> +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 > $tmp.cp.out 2>&1
> +       ret=$?
> +       cat $tmp.cp.out | _filter_testdir_and_scratch
> +       if [ $ret -ne 1 ]; then
> +               _fail "reflink failed to fail"
> +       fi

Such complexity to check if a reflink fails...

All this could be accomplished with a single line:

_cp_reflink $SCRATCH_MNT/foo $mnt1/bar

And then have the golden output expect an error message. That's the
most standard and prefered way to do things in fstests.
No need to redirect stdout and stderr to a temporary file, check
return value, check the temporary file, etc...

> +
> +       check_fsid ${SCRATCH_DEV_NAME[1]}
> +}
> +
> +mount_cloned_device

Really, why have all the test code inside a function that is called only once?
Get rid of the function...

Thanks.

> +
> +_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 08/12] 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>
---
 common/btrfs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/common/btrfs b/common/btrfs
index 61d5812d49d9..9a7fa2c71ec5 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -88,6 +88,22 @@ _require_btrfs_mkfs_feature()
 		_notrun "Feature $feat not supported in the available version of mkfs.btrfs"
 }
 
+_require_btrfs_mkfs_uuid_option()
+{
+	local cnt
+	local feature
+
+	if [ -z $1 ]; then
+		echo "Missing option name argument for _require_btrfs_mkfs_option"
+		exit 1
+	fi
+	feature=$1
+	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 option"
+	fi
+}
+
 _require_btrfs_fs_feature()
 {
 	if [ -z $1 ]; then
-- 
2.39.3

Re: [PATCH 08/12] btrfs: test case prerequisite _require_btrfs_mkfs_uuid_option

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:35 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> 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>
> ---
>  common/btrfs | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/common/btrfs b/common/btrfs
> index 61d5812d49d9..9a7fa2c71ec5 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -88,6 +88,22 @@ _require_btrfs_mkfs_feature()
>                 _notrun "Feature $feat not supported in the available version of mkfs.btrfs"
>  }
>
> +_require_btrfs_mkfs_uuid_option()
> +{
> +       local cnt
> +       local feature
> +
> +       if [ -z $1 ]; then
> +               echo "Missing option name argument for _require_btrfs_mkfs_option"
> +               exit 1
> +       fi
> +       feature=$1
> +       cnt=$($MKFS_BTRFS_PROG --help 2>&1 |grep -E --count "\-\-uuid|\-\-device-uuid")

A space between | and grep would make things more readable and match
the general coding style.

> +       if [ $cnt != 2 ]; then
> +               _notrun "Require $MKFS_BTRFS_PROG with --uuid and --device-uuid option"

option -> options.

Otherwise it looks fine, thanks.

> +       fi
> +}
> +
>  _require_btrfs_fs_feature()
>  {
>         if [ -z $1 ]; then
> --
> 2.39.3
>
>

[PATCH 09/12] 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>
---
 common/btrfs | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/common/btrfs b/common/btrfs
index 9a7fa2c71ec5..8ffce3c39695 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -91,13 +91,7 @@ _require_btrfs_mkfs_feature()
 _require_btrfs_mkfs_uuid_option()
 {
 	local cnt
-	local feature
 
-	if [ -z $1 ]; then
-		echo "Missing option name argument for _require_btrfs_mkfs_option"
-		exit 1
-	fi
-	feature=$1
 	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 option"
@@ -864,3 +858,21 @@ create_cloned_devices()
 							_fail "dd failed: $?"
 	echo done
 }
+
+mkfs_clone()
+{
+	local dev1=$1
+	local dev2=$2
+
+	[[ -z $dev1 || -z $dev2 ]] && \
+		_fail "BUGGY code, mkfs_clone needs arg1 arg2"
+
+	_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 09/12] btrfs: introduce helper for creating cloned devices with mkfs

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:37 AM 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>
> ---
>  common/btrfs | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/common/btrfs b/common/btrfs
> index 9a7fa2c71ec5..8ffce3c39695 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -91,13 +91,7 @@ _require_btrfs_mkfs_feature()
>  _require_btrfs_mkfs_uuid_option()
>  {
>         local cnt
> -       local feature
>
> -       if [ -z $1 ]; then
> -               echo "Missing option name argument for _require_btrfs_mkfs_option"
> -               exit 1
> -       fi
> -       feature=$1

This is confusing... It was just introduced in the previous patch that
introduced this function (_require_btrfs_mkfs_uuid_option).

If there was never any need for this code, why was it added in the
previous patch and removed in this one, without any users in between?

>         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 option"
> @@ -864,3 +858,21 @@ create_cloned_devices()
>                                                         _fail "dd failed: $?"
>         echo done
>  }
> +
> +mkfs_clone()
> +{
> +       local dev1=$1
> +       local dev2=$2
> +
> +       [[ -z $dev1 || -z $dev2 ]] && \
> +               _fail "BUGGY code, mkfs_clone needs arg1 arg2"

Rather more clear and informative to say "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}')

Make the variables local please.

Thanks.

> +
> +       _mkfs_dev -fq --uuid $fsid --device-uuid $uuid $dev2
> +}
> --
> 2.39.3
>
>

Re: [PATCH 09/12] btrfs: introduce helper for creating cloned devices with mkfs

[Anand Jain]

On 2/15/24 18:12, Filipe Manana wrote:
> On Thu, Feb 15, 2024 at 6:37 AM 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>
>> ---
>>   common/btrfs | 24 ++++++++++++++++++------
>>   1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/common/btrfs b/common/btrfs
>> index 9a7fa2c71ec5..8ffce3c39695 100644
>> --- a/common/btrfs
>> +++ b/common/btrfs
>> @@ -91,13 +91,7 @@ _require_btrfs_mkfs_feature()
>>   _require_btrfs_mkfs_uuid_option()
>>   {
>>          local cnt
>> -       local feature
>>
>> -       if [ -z $1 ]; then
>> -               echo "Missing option name argument for _require_btrfs_mkfs_option"
>> -               exit 1
>> -       fi
>> -       feature=$1
> 
> This is confusing... It was just introduced in the previous patch that
> introduced this function (_require_btrfs_mkfs_uuid_option).
> 
> If there was never any need for this code, why was it added in the
> previous patch and removed in this one, without any users in between?
> 

This change was mistakenly introduced here; it should have been in
the previous patch. I will move it. Thx.

>>          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 option"
>> @@ -864,3 +858,21 @@ create_cloned_devices()
>>                                                          _fail "dd failed: $?"
>>          echo done
>>   }
>> +
>> +mkfs_clone()
>> +{
>> +       local dev1=$1
>> +       local dev2=$2
>> +
>> +       [[ -z $dev1 || -z $dev2 ]] && \
>> +               _fail "BUGGY code, mkfs_clone needs arg1 arg2"
> 
> Rather more clear and informative to say "mkfs_clone requires two
> devices as arguments".

Sure.

> 
>> +
>> +       _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}')
> 
> Make the variables local please.
> 

Sure.

Thanks, Anand
> Thanks.
> 
>> +
>> +       _mkfs_dev -fq --uuid $fsid --device-uuid $uuid $dev2
>> +}
>> --
>> 2.39.3
>>
>>

[PATCH 10/12] 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>
---
 tests/btrfs/313     | 66 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/313.out | 16 +++++++++++
 2 files changed, 82 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..45811320e596
--- /dev/null
+++ b/tests/btrfs/313
@@ -0,0 +1,66 @@
+#! /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 tempfsid
+
+_cleanup()
+{
+	cd /
+	umount $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
+_require_btrfs_command inspect-internal dump-super
+_require_btrfs_mkfs_uuid_option
+
+_scratch_dev_pool_get 2
+
+mnt1=$TEST_DIR/$seq/mnt1
+mkdir -p $mnt1
+
+clone_uuids_verify_tempfsid()
+{
+	echo ---- $FUNCNAME ----
+	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 || \
+				_fail "mount failed, tempfsid didn't work"
+	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 > $tmp.cp.out 2>&1
+	ret=$?
+	cat $tmp.cp.out | _filter_testdir_and_scratch
+	if [ $ret -ne 1 ]; then
+		_fail "reflink failed to fail"
+	fi
+}
+
+clone_uuids_verify_tempfsid
+_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 10/12] btrfs: verify tempfsid clones using mkfs

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:36 AM 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>
> ---
>  tests/btrfs/313     | 66 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/313.out | 16 +++++++++++
>  2 files changed, 82 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..45811320e596
> --- /dev/null
> +++ b/tests/btrfs/313
> @@ -0,0 +1,66 @@
> +#! /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 tempfsid

clone group as well, it uses reflinks.

> +
> +_cleanup()
> +{
> +       cd /
> +       umount $mnt1 > /dev/null 2>&1

$UMOUNT_PROG

> +       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
> +_require_btrfs_command inspect-internal dump-super
> +_require_btrfs_mkfs_uuid_option
> +
> +_scratch_dev_pool_get 2
> +
> +mnt1=$TEST_DIR/$seq/mnt1
> +mkdir -p $mnt1
> +
> +clone_uuids_verify_tempfsid()
> +{
> +       echo ---- $FUNCNAME ----
> +       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 || \
> +                               _fail "mount failed, tempfsid didn't work"
> +       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 > $tmp.cp.out 2>&1
> +       ret=$?
> +       cat $tmp.cp.out | _filter_testdir_and_scratch
> +       if [ $ret -ne 1 ]; then
> +               _fail "reflink failed to fail"
> +       fi

Same comment as in a previous patch.

So much complexity to check if a reflink fails...

All this could be accomplished with a single line:

_cp_reflink $SCRATCH_MNT/foo $mnt1/bar

And then have the golden output expect an error message. That's the
most standard and prefered way to do things in fstests.
No need to redirect stdout and stderr to a temporary file, check
return value, check the temporary file, etc...


> +}
> +
> +clone_uuids_verify_tempfsid

Really, why have all the test code inside a function that is called only once?
Get rid of the function...

Thanks.

> +_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 11/12] 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>
---
 tests/btrfs/314     | 85 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/314.out | 30 ++++++++++++++++
 2 files changed, 115 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..1ceb448d2a5e
--- /dev/null
+++ b/tests/btrfs/314
@@ -0,0 +1,85 @@
+#! /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 tempfsid
+
+_cleanup()
+{
+	cd /
+	umount $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
+_require_btrfs_command inspect-internal dump-super
+_require_btrfs_mkfs_uuid_option
+
+_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
+
+	echo ---- $FUNCNAME ----
+
+	# 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:\n"
+	sha256sum ${src}/foo | _filter_testdir_and_scratch
+	echo -e -n "Receive:\n"
+	sha256sum ${dst}/snap1/foo | _filter_testdir_and_scratch
+}
+
+mkdir -p $tempfsid_mnt
+
+echo Test Send and Receive
+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..eb0010da264e
--- /dev/null
+++ b/tests/btrfs/314.out
@@ -0,0 +1,30 @@
+QA output created by 314
+Test Send and Receive
+
+From non-tempfsid SCRATCH_MNT to tempfsid TEST_DIR/314/tempfsid_mnt
+---- send_receive_tempfsid ----
+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:
+0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  SCRATCH_MNT/foo
+Receive:
+0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  TEST_DIR/314/tempfsid_mnt/snap1/foo
+
+From tempfsid TEST_DIR/314/tempfsid_mnt to non-tempfsid SCRATCH_MNT
+---- send_receive_tempfsid ----
+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:
+0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  TEST_DIR/314/tempfsid_mnt/foo
+Receive:
+0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  SCRATCH_MNT/snap1/foo
-- 
2.39.3

Re: [PATCH 11/12] btrfs: validate send-receive operation with tempfsid.

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:35 AM 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>
> ---
>  tests/btrfs/314     | 85 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/314.out | 30 ++++++++++++++++
>  2 files changed, 115 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..1ceb448d2a5e
> --- /dev/null
> +++ b/tests/btrfs/314
> @@ -0,0 +1,85 @@
> +#! /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 tempfsid

Missing 'send' group.

> +
> +_cleanup()
> +{
> +       cd /
> +       umount $tempfsid_mnt 2>/dev/null

$UMOUNT_PROG

> +       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
> +_require_btrfs_command inspect-internal dump-super
> +_require_btrfs_mkfs_uuid_option
> +
> +_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
> +
> +       echo ---- $FUNCNAME ----

What value do we get from echoing the function's name?
It's the only test function, plus the callers of the function already
echo something more useful.
Can be removed.

> +
> +       # 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:\n"

What's -e and -n for?
This could be just:

echo Send:


> +       sha256sum ${src}/foo | _filter_testdir_and_scratch

Previous tests use md5... here sha256.
What's the reason? This is highly inconsistent...

Also we have _md5_checksum(), which prints just the checksum and
filters out the file path.
Or  just call _hexdump and match with the golden output.


> +       echo -e -n "Receive:\n"

Same here. This can be just:

echo Receive:

> +       sha256sum ${dst}/snap1/foo | _filter_testdir_and_scratch
> +}
> +
> +mkdir -p $tempfsid_mnt
> +
> +echo Test Send and Receive

Useless message too, we know this is a send/receive test, doesn't test
other features.

Thanks.

> +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..eb0010da264e
> --- /dev/null
> +++ b/tests/btrfs/314.out
> @@ -0,0 +1,30 @@
> +QA output created by 314
> +Test Send and Receive
> +
> +From non-tempfsid SCRATCH_MNT to tempfsid TEST_DIR/314/tempfsid_mnt
> +---- send_receive_tempfsid ----
> +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:
> +0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  SCRATCH_MNT/foo
> +Receive:
> +0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  TEST_DIR/314/tempfsid_mnt/snap1/foo
> +
> +From tempfsid TEST_DIR/314/tempfsid_mnt to non-tempfsid SCRATCH_MNT
> +---- send_receive_tempfsid ----
> +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:
> +0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  TEST_DIR/314/tempfsid_mnt/foo
> +Receive:
> +0598aa54768194ade580b9806ac98ace43a0310aeceae95762f62491625eee52  SCRATCH_MNT/snap1/foo
> --
> 2.39.3
>
>

[PATCH 12/12] 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>
---
 common/filter.btrfs |  6 ++++
 tests/btrfs/315     | 79 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/315.out | 11 +++++++
 3 files changed, 96 insertions(+)
 create mode 100755 tests/btrfs/315
 create mode 100644 tests/btrfs/315.out

diff --git a/common/filter.btrfs b/common/filter.btrfs
index 8ab76fcb193a..d48e96c6f66b 100644
--- a/common/filter.btrfs
+++ b/common/filter.btrfs
@@ -68,6 +68,12 @@ _filter_btrfs_device_stats()
 	sed -e "s/ *$NUMDEVS /<NUMDEVS> /g"
 }
 
+_filter_btrfs_device_add()
+{
+	_filter_scratch_pool | \
+		sed -E 's/\(([0-9]+(\.[0-9]+)?)[a-zA-Z]+B\)/\(NUM\)/'
+}
+
 _filter_transaction_commit() {
 	sed -e "/Transaction commit: none (default)/d" \
 	    -e "s/Delete subvolume [0-9]\+ (.*commit):/Delete subvolume/g" \
diff --git a/tests/btrfs/315 b/tests/btrfs/315
new file mode 100755
index 000000000000..7ad0dfbc9c32
--- /dev/null
+++ b/tests/btrfs/315
@@ -0,0 +1,79 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 YOUR NAME HERE.  All Rights Reserved.
+#
+# FS QA Test 315
+#
+# Verify if the seed and device add to a tempfsid filesystem fails.
+#
+. ./common/preamble
+_begin_fstest auto quick volume seed tempfsid
+
+_cleanup()
+{
+	cd /
+	umount $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
+_require_btrfs_command inspect-internal dump-super
+_require_btrfs_mkfs_uuid_option
+
+_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 |\
+							_filter_btrfs_device_add
+
+	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..32149972beb4
--- /dev/null
+++ b/tests/btrfs/315.out
@@ -0,0 +1,11 @@
+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
+Performing full device TRIM SCRATCH_DEV (NUM) ...
+Balance must be successful
+Done, had to relocate 3 out of 3 chunks
-- 
2.39.3

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

[Filipe Manana]

On Thu, Feb 15, 2024 at 6:35 AM 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>
> ---
>  common/filter.btrfs |  6 ++++
>  tests/btrfs/315     | 79 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/315.out | 11 +++++++
>  3 files changed, 96 insertions(+)
>  create mode 100755 tests/btrfs/315
>  create mode 100644 tests/btrfs/315.out
>
> diff --git a/common/filter.btrfs b/common/filter.btrfs
> index 8ab76fcb193a..d48e96c6f66b 100644
> --- a/common/filter.btrfs
> +++ b/common/filter.btrfs
> @@ -68,6 +68,12 @@ _filter_btrfs_device_stats()
>         sed -e "s/ *$NUMDEVS /<NUMDEVS> /g"
>  }
>
> +_filter_btrfs_device_add()
> +{
> +       _filter_scratch_pool | \
> +               sed -E 's/\(([0-9]+(\.[0-9]+)?)[a-zA-Z]+B\)/\(NUM\)/'

Why do we need this new filter?
We are testing for a failure, where none of this is relevant except
filtering device names.

The test can just filter with  _filter_scratch_pool only.

> +}
> +
>  _filter_transaction_commit() {
>         sed -e "/Transaction commit: none (default)/d" \
>             -e "s/Delete subvolume [0-9]\+ (.*commit):/Delete subvolume/g" \
> diff --git a/tests/btrfs/315 b/tests/btrfs/315
> new file mode 100755
> index 000000000000..7ad0dfbc9c32
> --- /dev/null
> +++ b/tests/btrfs/315
> @@ -0,0 +1,79 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 YOUR NAME HERE.  All Rights Reserved.
> +#
> +# FS QA Test 315
> +#
> +# Verify if the seed and device add to a tempfsid filesystem fails.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick volume seed tempfsid
> +
> +_cleanup()
> +{
> +       cd /
> +       umount $tempfsid_mnt 2>/dev/null

$UMOUNT_PROG

> +       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
> +_require_btrfs_command inspect-internal dump-super
> +_require_btrfs_mkfs_uuid_option
> +
> +_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 |\
> +                                                       _filter_btrfs_device_add

We are testing for failure, so no need for the new filter
_filter_btrfs_device_add.
Just filter through  _filter_scratch_pool here and nothing more.

Thanks.

> +
> +       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..32149972beb4
> --- /dev/null
> +++ b/tests/btrfs/315.out
> @@ -0,0 +1,11 @@
> +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
> +Performing full device TRIM SCRATCH_DEV (NUM) ...
> +Balance must be successful
> +Done, had to relocate 3 out of 3 chunks
> --
> 2.39.3
>
>

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

[Anand Jain]

On 2/15/24 18:33, Filipe Manana wrote:
> On Thu, Feb 15, 2024 at 6:35 AM 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>
>> ---
>>   common/filter.btrfs |  6 ++++
>>   tests/btrfs/315     | 79 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/btrfs/315.out | 11 +++++++
>>   3 files changed, 96 insertions(+)
>>   create mode 100755 tests/btrfs/315
>>   create mode 100644 tests/btrfs/315.out
>>
>> diff --git a/common/filter.btrfs b/common/filter.btrfs
>> index 8ab76fcb193a..d48e96c6f66b 100644
>> --- a/common/filter.btrfs
>> +++ b/common/filter.btrfs
>> @@ -68,6 +68,12 @@ _filter_btrfs_device_stats()
>>          sed -e "s/ *$NUMDEVS /<NUMDEVS> /g"
>>   }
>>
>> +_filter_btrfs_device_add()
>> +{
>> +       _filter_scratch_pool | \
>> +               sed -E 's/\(([0-9]+(\.[0-9]+)?)[a-zA-Z]+B\)/\(NUM\)/'
> 
> Why do we need this new filter?
> We are testing for a failure, where none of this is relevant except
> filtering device names.
> 

2nd part filters out the size part as seen in the raw
btrfs device add output below.

$ btrfs device add /dev/sdb2 /btrfs
Performing full device TRIM /dev/sdb2 (731.00MiB) ...

I will add a comment.

> The test can just filter with  _filter_scratch_pool only.
> 
>> +}
>> +
>>   _filter_transaction_commit() {
>>          sed -e "/Transaction commit: none (default)/d" \
>>              -e "s/Delete subvolume [0-9]\+ (.*commit):/Delete subvolume/g" \
>> diff --git a/tests/btrfs/315 b/tests/btrfs/315
>> new file mode 100755
>> index 000000000000..7ad0dfbc9c32
>> --- /dev/null
>> +++ b/tests/btrfs/315
>> @@ -0,0 +1,79 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 YOUR NAME HERE.  All Rights Reserved.
>> +#
>> +# FS QA Test 315
>> +#
>> +# Verify if the seed and device add to a tempfsid filesystem fails.
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick volume seed tempfsid
>> +
>> +_cleanup()
>> +{
>> +       cd /
>> +       umount $tempfsid_mnt 2>/dev/null
> 
> $UMOUNT_PROG
> 

got it.

>> +       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
>> +_require_btrfs_command inspect-internal dump-super
>> +_require_btrfs_mkfs_uuid_option
>> +
>> +_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 |\
>> +                                                       _filter_btrfs_device_add
> 
> We are testing for failure, so no need for the new filter
> _filter_btrfs_device_add.
> Just filter through  _filter_scratch_pool here and nothing more.
> 

As shown above, we need to filter out the size part too.

Thanks, Anand

> Thanks.
> 
>> +
>> +       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..32149972beb4
>> --- /dev/null
>> +++ b/tests/btrfs/315.out
>> @@ -0,0 +1,11 @@
>> +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
>> +Performing full device TRIM SCRATCH_DEV (NUM) ...
>> +Balance must be successful
>> +Done, had to relocate 3 out of 3 chunks
>> --
>> 2.39.3
>>
>>

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

[Filipe Manana]

On Mon, Feb 19, 2024 at 1:18 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> On 2/15/24 18:33, Filipe Manana wrote:
> > On Thu, Feb 15, 2024 at 6:35 AM 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>
> >> ---
> >>   common/filter.btrfs |  6 ++++
> >>   tests/btrfs/315     | 79 +++++++++++++++++++++++++++++++++++++++++++++
> >>   tests/btrfs/315.out | 11 +++++++
> >>   3 files changed, 96 insertions(+)
> >>   create mode 100755 tests/btrfs/315
> >>   create mode 100644 tests/btrfs/315.out
> >>
> >> diff --git a/common/filter.btrfs b/common/filter.btrfs
> >> index 8ab76fcb193a..d48e96c6f66b 100644
> >> --- a/common/filter.btrfs
> >> +++ b/common/filter.btrfs
> >> @@ -68,6 +68,12 @@ _filter_btrfs_device_stats()
> >>          sed -e "s/ *$NUMDEVS /<NUMDEVS> /g"
> >>   }
> >>
> >> +_filter_btrfs_device_add()
> >> +{
> >> +       _filter_scratch_pool | \
> >> +               sed -E 's/\(([0-9]+(\.[0-9]+)?)[a-zA-Z]+B\)/\(NUM\)/'
> >
> > Why do we need this new filter?
> > We are testing for a failure, where none of this is relevant except
> > filtering device names.
> >
>
> 2nd part filters out the size part as seen in the raw
> btrfs device add output below.
>
> $ btrfs device add /dev/sdb2 /btrfs
> Performing full device TRIM /dev/sdb2 (731.00MiB) ...
>
> I will add a comment.

So that means the test will fail the golden output if the device does
not support trim,
as in that case progs does not do the trim.

That line about performing TRIM must be filtered out. Replacing the
size with NUM is irrelevant.

Thanks.


>
> > The test can just filter with  _filter_scratch_pool only.
> >
> >> +}
> >> +
> >>   _filter_transaction_commit() {
> >>          sed -e "/Transaction commit: none (default)/d" \
> >>              -e "s/Delete subvolume [0-9]\+ (.*commit):/Delete subvolume/g" \
> >> diff --git a/tests/btrfs/315 b/tests/btrfs/315
> >> new file mode 100755
> >> index 000000000000..7ad0dfbc9c32
> >> --- /dev/null
> >> +++ b/tests/btrfs/315
> >> @@ -0,0 +1,79 @@
> >> +#! /bin/bash
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (c) 2024 YOUR NAME HERE.  All Rights Reserved.
> >> +#
> >> +# FS QA Test 315
> >> +#
> >> +# Verify if the seed and device add to a tempfsid filesystem fails.
> >> +#
> >> +. ./common/preamble
> >> +_begin_fstest auto quick volume seed tempfsid
> >> +
> >> +_cleanup()
> >> +{
> >> +       cd /
> >> +       umount $tempfsid_mnt 2>/dev/null
> >
> > $UMOUNT_PROG
> >
>
> got it.
>
> >> +       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
> >> +_require_btrfs_command inspect-internal dump-super
> >> +_require_btrfs_mkfs_uuid_option
> >> +
> >> +_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 |\
> >> +                                                       _filter_btrfs_device_add
> >
> > We are testing for failure, so no need for the new filter
> > _filter_btrfs_device_add.
> > Just filter through  _filter_scratch_pool here and nothing more.
> >
>
> As shown above, we need to filter out the size part too.
>
> Thanks, Anand
>
> > Thanks.
> >
> >> +
> >> +       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..32149972beb4
> >> --- /dev/null
> >> +++ b/tests/btrfs/315.out
> >> @@ -0,0 +1,11 @@
> >> +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
> >> +Performing full device TRIM SCRATCH_DEV (NUM) ...
> >> +Balance must be successful
> >> +Done, had to relocate 3 out of 3 chunks
> >> --
> >> 2.39.3
> >>
> >>
>

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

[Anand Jain]

On 2/19/24 18:59, Filipe Manana wrote:
> On Mon, Feb 19, 2024 at 1:18 PM Anand Jain <anand.jain@oracle.com> wrote:
>>
>> On 2/15/24 18:33, Filipe Manana wrote:
>>> On Thu, Feb 15, 2024 at 6:35 AM 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>
>>>> ---
>>>>    common/filter.btrfs |  6 ++++
>>>>    tests/btrfs/315     | 79 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    tests/btrfs/315.out | 11 +++++++
>>>>    3 files changed, 96 insertions(+)
>>>>    create mode 100755 tests/btrfs/315
>>>>    create mode 100644 tests/btrfs/315.out
>>>>
>>>> diff --git a/common/filter.btrfs b/common/filter.btrfs
>>>> index 8ab76fcb193a..d48e96c6f66b 100644
>>>> --- a/common/filter.btrfs
>>>> +++ b/common/filter.btrfs
>>>> @@ -68,6 +68,12 @@ _filter_btrfs_device_stats()
>>>>           sed -e "s/ *$NUMDEVS /<NUMDEVS> /g"
>>>>    }
>>>>
>>>> +_filter_btrfs_device_add()
>>>> +{
>>>> +       _filter_scratch_pool | \
>>>> +               sed -E 's/\(([0-9]+(\.[0-9]+)?)[a-zA-Z]+B\)/\(NUM\)/'
>>>
>>> Why do we need this new filter?
>>> We are testing for a failure, where none of this is relevant except
>>> filtering device names.
>>>
>>
>> 2nd part filters out the size part as seen in the raw
>> btrfs device add output below.
>>
>> $ btrfs device add /dev/sdb2 /btrfs
>> Performing full device TRIM /dev/sdb2 (731.00MiB) ...
>>
>> I will add a comment.
> 
> So that means the test will fail the golden output if the device does
> not support trim,
> as in that case progs does not do the trim.
> 
> That line about performing TRIM must be filtered out. Replacing the
> size with NUM is irrelevant.
> 

Right. I missed those devices that don't support trim.
Thanks for pointing that out.

Anand

> Thanks.
> 
> 
>>
>>> The test can just filter with  _filter_scratch_pool only.
>>>
>>>> +}
>>>> +
>>>>    _filter_transaction_commit() {
>>>>           sed -e "/Transaction commit: none (default)/d" \
>>>>               -e "s/Delete subvolume [0-9]\+ (.*commit):/Delete subvolume/g" \
>>>> diff --git a/tests/btrfs/315 b/tests/btrfs/315
>>>> new file mode 100755
>>>> index 000000000000..7ad0dfbc9c32
>>>> --- /dev/null
>>>> +++ b/tests/btrfs/315
>>>> @@ -0,0 +1,79 @@
>>>> +#! /bin/bash
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +# Copyright (c) 2024 YOUR NAME HERE.  All Rights Reserved.
>>>> +#
>>>> +# FS QA Test 315
>>>> +#
>>>> +# Verify if the seed and device add to a tempfsid filesystem fails.
>>>> +#
>>>> +. ./common/preamble
>>>> +_begin_fstest auto quick volume seed tempfsid
>>>> +
>>>> +_cleanup()
>>>> +{
>>>> +       cd /
>>>> +       umount $tempfsid_mnt 2>/dev/null
>>>
>>> $UMOUNT_PROG
>>>
>>
>> got it.
>>
>>>> +       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
>>>> +_require_btrfs_command inspect-internal dump-super
>>>> +_require_btrfs_mkfs_uuid_option
>>>> +
>>>> +_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 |\
>>>> +                                                       _filter_btrfs_device_add
>>>
>>> We are testing for failure, so no need for the new filter
>>> _filter_btrfs_device_add.
>>> Just filter through  _filter_scratch_pool here and nothing more.
>>>
>>
>> As shown above, we need to filter out the size part too.
>>
>> Thanks, Anand
>>
>>> Thanks.
>>>
>>>> +
>>>> +       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..32149972beb4
>>>> --- /dev/null
>>>> +++ b/tests/btrfs/315.out
>>>> @@ -0,0 +1,11 @@
>>>> +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
>>>> +Performing full device TRIM SCRATCH_DEV (NUM) ...
>>>> +Balance must be successful
>>>> +Done, had to relocate 3 out of 3 chunks
>>>> --
>>>> 2.39.3
>>>>
>>>>
>>

Re: [PATCH 00/12] btrfs: functional test cases for tempfsid

[Anand Jain]

   All review comments were accepted in V2 and sent out.

Thank You.
Anand