diff mbox series

btrfs: Add test for the single-dev feature

Message ID 20230830221943.3375955-1-gpiccoli@igalia.com (mailing list archive)
State New, archived
Headers show
Series btrfs: Add test for the single-dev feature | expand

Commit Message

Guilherme G. Piccoli Aug. 30, 2023, 10:17 p.m. UTC 
The SINGLE_DEV btrfs feature allows to mount the same filesystem
multiple times, at the same time. This is the fstests counter-part,
which checks both mkfs/btrfstune (by mounting the FS twice), and
also unsupported scenarios, like device replace / remove.

Suggested-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---

Hi folks, this test was a suggestion by Josef for the SINGLE_DEV feature
we're trying to introduce in the kernel [0].

Some design decisions:

(a) The number 300 was selected since seems we already skipped 298 in
the code base, given that the test btrfs/298 is being worked upstream.

(b) About mounting twice the same filesystem - how to accomplish that
in a proper fashion? I went with the naive / trivial approach, just created
temporary directories on top of $SCRATCH_MNT, but I'm not sure if that is
the way to go. Is it guaranteed that $SCRATCH_MNT is writable prior to
any mount command?

(c) The way to be sure we have two different devices holding the same
image was using SCRATCH_DEV_POOL and dd'ing the first device to the 2nd
one - is there any better alternative than this one?

Suggestions / reviews much appreciated!
Cheers,

Guilherme

[0] https://lore.kernel.org/linux-btrfs/20230803154453.1488248-1-gpiccoli@igalia.com/


 tests/btrfs/300     | 107 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/300.out |   5 +++
 2 files changed, 112 insertions(+)
 create mode 100755 tests/btrfs/300
 create mode 100644 tests/btrfs/300.out

Comments

Anand Jain Sept. 4, 2023, 6:05 a.m. UTC | #1 
Thanks for the test cases. These test cases might need to be updated as
per the final changes in the kernel patches.

> 
> [0] https://lore.kernel.org/linux-btrfs/20230803154453.1488248-1-gpiccoli@igalia.com/
> 
> 
>   tests/btrfs/300     | 107 ++++++++++++++++++++++++++++++++++++++++++++
>   tests/btrfs/300.out |   5 +++
>   2 files changed, 112 insertions(+)
>   create mode 100755 tests/btrfs/300
>   create mode 100644 tests/btrfs/300.out
> 
> diff --git a/tests/btrfs/300 b/tests/btrfs/300
> new file mode 100755
> index 000000000000..120e790906ea
> --- /dev/null
> +++ b/tests/btrfs/300
> @@ -0,0 +1,107 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2023 Guilherme G. Piccoli (Igalia S.L.).  All Rights Reserved.
> +#
> +# FS QA Test 300
> +#
> +# Test for the btrfs single-dev feature - both mkfs and btrfstune are
> +# validated, as well as explicitly unsupported commands, like device
> +# removal / replacement.
> +#
> +. ./common/preamble
> +_begin_fstest auto mkfs quick
> +. ./common/filter
> +_supported_fs btrfs
> +
> +_require_btrfs_mkfs_feature single-dev
> +_require_btrfs_fs_feature single_dev
> +
> +_require_scratch_dev_pool 2
> +_scratch_dev_pool_get 2

We can use

_scratch_dev_pool_get 1
_spare_dev_get

So scratch devices are set to SCRATCH_DEV and SPARE_DEV.


> +
> +_require_command "$BTRFS_TUNE_PROG" btrfstune
> +_require_command "$WIPEFS_PROG" wipefs
> +
> +# Used random hash to avoid name collision.
> +DIR_a="563b01de1f_a"
> +DIR_b="563b01de1f_b"
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	cd /
> +	rm -r -f $tmp.*
> +	rm -rf $SCRATCH_MNT/$DIR_a
> +	rm -rf $SCRATCH_MNT/$DIR_b
> +}
> +
> +# Helper to mount a btrfs fs
> +# Arg 1: device
> +# Arg 2: mount point
> +mount_btrfs()
> +{
> +	$MOUNT_PROG -t btrfs $1 $2
> +	[ $? -ne 0 ] && _fail "mounting $1 on $2 failed"
> +}
> +
> +mkdir $SCRATCH_MNT/$DIR_a
> +mkdir $SCRATCH_MNT/$DIR_b

we can use something like:

test_dir="${TEST_DIR}/${seq}"
scratch_mnt_copy=$test_dir/mnt_copy

> +
> +DEV1=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{ print $1 }')
> +DEV2=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{ print $2 }')


A general rule of thumb is to have one test case per file.


> +# Part 1
> +# First test involves a mkfs with single-dev feature enabled.
> +# If it succeeds and mounting that FS *twice* also succeeds,
> +# we're good and continue.
> +$WIPEFS_PROG -a $DEV1 >> $seqres.full 2>&1
> +$WIPEFS_PROG -a $DEV2 >> $seqres.full 2>&1
> +
> +$MKFS_BTRFS_PROG -O single-dev $DEV1 >>$seqres.full 2>&1

_scratch_mkfs "-b 300m -O single-dev"

The '-b 300m' option prevents the below 'dd' operation from taking
too long to complete on larger devices.

> +dd if=$DEV1 of=$DEV2 bs=64M conv=fsync >> $seqres.full 2>&1
> +

dd if=$SCRATCH_DEV of=SPARE_DEV bs=300m count=1 conv=fsync >> 
$seqres.full 2>&1


> +mount_btrfs $DEV1 $SCRATCH_MNT/$DIR_a
> +mount_btrfs $DEV2 $SCRATCH_MNT/$DIR_b


_scratch_mount
_mount $SPARE_DEV $scratch_mnt_copy

_spare_dev_put
_scratch_dev_pool_put


status=0
exit

> +
> +$UMOUNT_PROG $SCRATCH_MNT/$DIR_b
> +$UMOUNT_PROG $SCRATCH_MNT/$DIR_a


> +
> +
> +# Part 2
> +# Second test is similar to the first with the difference we
> +# run mkfs with no single-dev mention, and make use of btrfstune
> +# to set such feature.
> +$WIPEFS_PROG -a $DEV1 >> $seqres.full 2>&1
> +$WIPEFS_PROG -a $DEV2 >> $seqres.full 2>&1
> +


> +$MKFS_BTRFS_PROG $DEV1 >>$seqres.full 2>&1
> +$BTRFS_TUNE_PROG --convert-to-single-device $DEV1
> +dd if=$DEV1 of=$DEV2 bs=64M conv=fsync >> $seqres.full 2>&1
> +
> +mount_btrfs $DEV1 $SCRATCH_MNT/$DIR_a
> +mount_btrfs $DEV2 $SCRATCH_MNT/$DIR_b
> +
> +$UMOUNT_PROG $SCRATCH_MNT/$DIR_b
> +$UMOUNT_PROG $SCRATCH_MNT/$DIR_a
> +
> +
> +# Part 3
> +# Final part attempts to run some single-dev unsupported commands,
> +# like device replace/remove - it they fail, test succeeds!
> +mount_btrfs $DEV1 $SCRATCH_MNT
> +
> +$BTRFS_UTIL_PROG device replace start $DEV1 $DEV1 $SCRATCH_MNT 2>&1 \
> +	| _filter_scratch
> +
> +$BTRFS_UTIL_PROG device remove $DEV1 $SCRATCH_MNT 2>&1 \
> +	| sed -e "s,\B$DEV1,DEV1,g"
> +
> +$UMOUNT_PROG $SCRATCH_MNT
> +_scratch_dev_pool_put 2
> +
> +# success, all done
> +status=0
> +echo "Finished"
> +
> +exit
> diff --git a/tests/btrfs/300.out b/tests/btrfs/300.out
> new file mode 100644
> index 000000000000..5f13cfb228fb
> --- /dev/null
> +++ b/tests/btrfs/300.out
> @@ -0,0 +1,5 @@
> +QA output created by 300
> +ERROR: ioctl(DEV_REPLACE_STATUS) failed on "SCRATCH_MNT": Invalid argument
> +
> +ERROR: error removing device 'DEV1': Invalid argument
> +Finished
Guilherme G. Piccoli Sept. 4, 2023, 12:35 p.m. UTC | #2 
On 04/09/2023 03:05, Anand Jain wrote:
> 
> 
> Thanks for the test cases. These test cases might need to be updated as
> per the final changes in the kernel patches.
> 

Hi Anand, thanks a lot for the review! Very useful to me.

Now, what updates are missing? I've worked this fstests case after V3
was done and tested with that. Is there any missing piece you noticed?


>> [...]
>> +_require_scratch_dev_pool 2
>> +_scratch_dev_pool_get 2
> 
> We can use
> 
> _scratch_dev_pool_get 1
> _spare_dev_get
> 
> So scratch devices are set to SCRATCH_DEV and SPARE_DEV.
> 
> [...]
> we can use something like:
> 
> test_dir="${TEST_DIR}/${seq}"
> scratch_mnt_copy=$test_dir/mnt_copy
> 
>> +
>> +DEV1=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{ print $1 }')
>> +DEV2=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{ print $2 }')
> 

Great hints, I'll change and send V2 - thanks!


> 
> A general rule of thumb is to have one test case per file.
>

Well, I agree but what I'm doing in fact is testing single-dev feature
in this test case, so it's self-contained and single-scoped, only
single-dev stuff! I personally wouldn't see advantages having 3 tests
doing almost the same thing, but if that's a requirement, I can change.


> [...] 
>> +$MKFS_BTRFS_PROG -O single-dev $DEV1 >>$seqres.full 2>&1
> 
> _scratch_mkfs "-b 300m -O single-dev"
> 
> The '-b 300m' option prevents the below 'dd' operation from taking
> too long to complete on larger devices.
> 
>> +dd if=$DEV1 of=$DEV2 bs=64M conv=fsync >> $seqres.full 2>&1
>> +
>
> dd if=$SCRATCH_DEV of=SPARE_DEV bs=300m count=1 conv=fsync >> 
> $seqres.full 2>&1
> [...]
> 
>> +mount_btrfs $DEV1 $SCRATCH_MNT/$DIR_a
>> +mount_btrfs $DEV2 $SCRATCH_MNT/$DIR_b
> 
> 
> _scratch_mount
> _mount $SPARE_DEV $scratch_mnt_copy
> 
> _spare_dev_put
> _scratch_dev_pool_put
> 
> 
> status=0
> exit
> 

That is very useful, will change for sure! Thanks again.
diff mbox series

Patch

diff --git a/tests/btrfs/300 b/tests/btrfs/300
new file mode 100755
index 000000000000..120e790906ea
--- /dev/null
+++ b/tests/btrfs/300
@@ -0,0 +1,107 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2023 Guilherme G. Piccoli (Igalia S.L.).  All Rights Reserved.
+#
+# FS QA Test 300
+#
+# Test for the btrfs single-dev feature - both mkfs and btrfstune are
+# validated, as well as explicitly unsupported commands, like device
+# removal / replacement.
+#
+. ./common/preamble
+_begin_fstest auto mkfs quick
+. ./common/filter
+_supported_fs btrfs
+
+_require_btrfs_mkfs_feature single-dev
+_require_btrfs_fs_feature single_dev
+
+_require_scratch_dev_pool 2
+_scratch_dev_pool_get 2
+
+_require_command "$BTRFS_TUNE_PROG" btrfstune
+_require_command "$WIPEFS_PROG" wipefs
+
+# Used random hash to avoid name collision.
+DIR_a="563b01de1f_a"
+DIR_b="563b01de1f_b"
+
+# Override the default cleanup function.
+_cleanup()
+{
+	cd /
+	rm -r -f $tmp.*
+	rm -rf $SCRATCH_MNT/$DIR_a
+	rm -rf $SCRATCH_MNT/$DIR_b
+}
+
+# Helper to mount a btrfs fs
+# Arg 1: device
+# Arg 2: mount point
+mount_btrfs()
+{
+	$MOUNT_PROG -t btrfs $1 $2
+	[ $? -ne 0 ] && _fail "mounting $1 on $2 failed"
+}
+
+mkdir $SCRATCH_MNT/$DIR_a
+mkdir $SCRATCH_MNT/$DIR_b
+
+DEV1=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{ print $1 }')
+DEV2=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{ print $2 }')
+
+
+# Part 1
+# First test involves a mkfs with single-dev feature enabled.
+# If it succeeds and mounting that FS *twice* also succeeds,
+# we're good and continue.
+$WIPEFS_PROG -a $DEV1 >> $seqres.full 2>&1
+$WIPEFS_PROG -a $DEV2 >> $seqres.full 2>&1
+
+$MKFS_BTRFS_PROG -O single-dev $DEV1 >>$seqres.full 2>&1
+dd if=$DEV1 of=$DEV2 bs=64M conv=fsync >> $seqres.full 2>&1
+
+mount_btrfs $DEV1 $SCRATCH_MNT/$DIR_a
+mount_btrfs $DEV2 $SCRATCH_MNT/$DIR_b
+
+$UMOUNT_PROG $SCRATCH_MNT/$DIR_b
+$UMOUNT_PROG $SCRATCH_MNT/$DIR_a
+
+
+# Part 2
+# Second test is similar to the first with the difference we
+# run mkfs with no single-dev mention, and make use of btrfstune
+# to set such feature.
+$WIPEFS_PROG -a $DEV1 >> $seqres.full 2>&1
+$WIPEFS_PROG -a $DEV2 >> $seqres.full 2>&1
+
+$MKFS_BTRFS_PROG $DEV1 >>$seqres.full 2>&1
+$BTRFS_TUNE_PROG --convert-to-single-device $DEV1
+dd if=$DEV1 of=$DEV2 bs=64M conv=fsync >> $seqres.full 2>&1
+
+mount_btrfs $DEV1 $SCRATCH_MNT/$DIR_a
+mount_btrfs $DEV2 $SCRATCH_MNT/$DIR_b
+
+$UMOUNT_PROG $SCRATCH_MNT/$DIR_b
+$UMOUNT_PROG $SCRATCH_MNT/$DIR_a
+
+
+# Part 3
+# Final part attempts to run some single-dev unsupported commands,
+# like device replace/remove - it they fail, test succeeds!
+mount_btrfs $DEV1 $SCRATCH_MNT
+
+$BTRFS_UTIL_PROG device replace start $DEV1 $DEV1 $SCRATCH_MNT 2>&1 \
+	| _filter_scratch
+
+$BTRFS_UTIL_PROG device remove $DEV1 $SCRATCH_MNT 2>&1 \
+	| sed -e "s,\B$DEV1,DEV1,g"
+
+$UMOUNT_PROG $SCRATCH_MNT
+_scratch_dev_pool_put 2
+
+# success, all done
+status=0
+echo "Finished"
+
+exit
diff --git a/tests/btrfs/300.out b/tests/btrfs/300.out
new file mode 100644
index 000000000000..5f13cfb228fb
--- /dev/null
+++ b/tests/btrfs/300.out
@@ -0,0 +1,5 @@ 
+QA output created by 300
+ERROR: ioctl(DEV_REPLACE_STATUS) failed on "SCRATCH_MNT": Invalid argument
+
+ERROR: error removing device 'DEV1': Invalid argument
+Finished