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

Filipe Manana
2024-02-20T16:45:36Z

On Mon, Feb 19, 2024 at 7:49 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>
> ---
> v2:
> Bring create_cloned_devices() into the testcase.
> Just use _cp_reflink output to match with golden output.
>
> tests/btrfs/312 | 85 +++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/312.out | 19 ++++++++++
> 2 files changed, 104 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..6dd5811ddaa5
> --- /dev/null
> +++ b/tests/btrfs/312
> @@ -0,0 +1,85 @@
> +#! /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

This exercises reflinks, so it should have the 'clone' group as well.

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

Same as mentioned before, 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

Instead of requiring here the inspect-internal command, it should be
inside check_fsid()

That's the pattern we usually do, common functions have the _require_*
calls, avoiding the caller test to have to know each dependency and
copy-paste it.

Other than that, it looks fine.
Thanks.

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