| Message ID | 20230913224545.3940971-1-gpiccoli@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] btrfs: Add test for the temp-fsid feature | expand |
On 14/09/2023 06:44, Guilherme G. Piccoli wrote: > The TEMP_FSID 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> This test case's integration will be timed alongside the kernel. Running this test case on older kernel/progs without the feature under test must terminate the test case with _notrun(). I find that part is missing here. > +_scratch_dev_pool_put 1 _scratch_dev_pool_put takes no argument. Thanks, Anand
On 15/09/2023 20:25, Anand Jain wrote: > [...] > This test case's integration will be timed alongside the kernel. > > Running this test case on older kernel/progs without the feature under > test must terminate the test case with _notrun(). I find that part is > missing here. > I'm confused about the relation between _notrun() and _require_btrfs_fs_feature(). I see that some tests (like mine) make use of the latter, but some tests do as you suggest, using _notrun. They intersect only on tests 125 and 192, and it seems they are aimed at different things, based on these two. The _require_btrfs_fs_feature() seems to be used with the same semantic I'm using, i.e., to check if a feature is present, given that the test requires it. Now the _notrun() thing is used like (in test 192): # We require a 4K nodesize to ensure the test isn't too slow if [ $(_get_page_size) -ne 4096 ]; then _notrun "This test doesn't support non-4K page size yet" fi So, there's a secondary condition here, and the test is prevented from running if such condition is not achieved. Do you / others think I should switch approaches and use _notrun()? Or should I somehow use both?! >> +_scratch_dev_pool_put 1 > > _scratch_dev_pool_put > > takes no argument. Thanks for noticing that! Will fix in next version =) Cheers!
On 16/09/2023 20:35, Guilherme G. Piccoli wrote: > On 15/09/2023 20:25, Anand Jain wrote: >> [...] >> This test case's integration will be timed alongside the kernel. >> >> Running this test case on older kernel/progs without the feature under >> test must terminate the test case with _notrun(). I find that part is >> missing here. >> > > I'm confused about the relation between _notrun() and > _require_btrfs_fs_feature(). I see that some tests (like mine) make use > of the latter, but some tests do as you suggest, using _notrun. They > intersect only on tests 125 and 192, and it seems they are aimed at > different things, based on these two. > > The _require_btrfs_fs_feature() seems to be used with the same semantic > I'm using, i.e., to check if a feature is present, given that the test > requires it. Now the _notrun() thing is used like (in test 192): > +_require_btrfs_mkfs_feature temp-fsid +_require_btrfs_fs_feature temp_fsid This will suffice for backward compatibility. My bad. I missed it. > > # We require a 4K nodesize to ensure the test isn't too slow > if [ $(_get_page_size) -ne 4096 ]; then > _notrun "This test doesn't support non-4K page size yet" > fi > > > So, there's a secondary condition here, and the test is prevented from > running if such condition is not achieved. > > Do you / others think I should switch approaches and use _notrun()? Or > should I somehow use both?! We don't need this because the temp_fsid feature is compatible with other page/sector sizes. > >>> +_scratch_dev_pool_put 1 >> >> _scratch_dev_pool_put >> >> takes no argument. > > Thanks for noticing that! Will fix in next version =) No worries. If this is the only change required, it can be fixed during the merge. Thanks, Anand > Cheers!
On 17/09/2023 05:38, Anand Jain wrote: > [...] > +_require_btrfs_mkfs_feature temp-fsid > +_require_btrfs_fs_feature temp_fsid > > This will suffice for backward compatibility. My bad. I missed it. Thanks for clarifying =) >>>> +_scratch_dev_pool_put 1 >>> >>> _scratch_dev_pool_put >>> >>> takes no argument. >> >> Thanks for noticing that! Will fix in next version =) > > No worries. If this is the only change required, it can be fixed during > the merge. Great then! Cheers, Guilherme
diff --git a/tests/btrfs/301 b/tests/btrfs/301 new file mode 100755 index 000000000000..2e3d55a3cd81 --- /dev/null +++ b/tests/btrfs/301 @@ -0,0 +1,83 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2023 Guilherme G. Piccoli (Igalia S.L.). All Rights Reserved. +# +# FS QA Test 301 +# +# Test for the btrfs temp-fsid feature - both mkfs and btrfstune are +# validated, as well as explicitly unsupported commands, like device +# removal / replacement. +# +. ./common/preamble +. ./common/filter +_begin_fstest auto mkfs quick +_supported_fs btrfs + +_require_btrfs_mkfs_feature temp-fsid +_require_btrfs_fs_feature temp_fsid +_require_scratch_dev_pool 2 + +_scratch_dev_pool_get 1 +_spare_dev_get + +_require_command "$BTRFS_TUNE_PROG" btrfstune +_require_command "$WIPEFS_PROG" wipefs + +spare_mnt="${TEST_DIR}/${seq}/spare_mnt" +mkdir -p $spare_mnt + + +# Part 1 +# First test involves a mkfs with temp-fsid feature enabled. +# If it succeeds and mounting that FS *twice* also succeeds, +# we're good and continue. +$WIPEFS_PROG -a $SCRATCH_DEV >> $seqres.full 2>&1 +$WIPEFS_PROG -a $SPARE_DEV >> $seqres.full 2>&1 + +_scratch_mkfs "-b 300M -O temp-fsid" >> $seqres.full 2>&1 +dd if=$SCRATCH_DEV of=$SPARE_DEV bs=300M count=1 conv=fsync >> $seqres.full 2>&1 + +_scratch_mount || _fail "failed to mount scratch dev (1)" +_mount $SPARE_DEV $spare_mnt || _fail "failed to mount spare dev (1)" + +$UMOUNT_PROG $spare_mnt +_scratch_unmount + + +# Part 2 +# Second test is similar to the first with the difference we +# run mkfs with no temp-fsid mention, and make use of btrfstune +# to set such feature. +$WIPEFS_PROG -a $SCRATCH_DEV >> $seqres.full 2>&1 +$WIPEFS_PROG -a $SPARE_DEV >> $seqres.full 2>&1 + +_scratch_mkfs "-b 300M" >> $seqres.full 2>&1 +$BTRFS_TUNE_PROG --convert-to-temp-fsid $SCRATCH_DEV +dd if=$SCRATCH_DEV of=$SPARE_DEV bs=300M count=1 conv=fsync >> $seqres.full 2>&1 + +_scratch_mount || _fail "failed to mount scratch dev (2)" +_mount $SPARE_DEV $spare_mnt || _fail "failed to mount spare dev (2)" + +$UMOUNT_PROG $spare_mnt +_scratch_unmount + + +# Part 3 +# Final part attempts to run some temp-fsid unsupported commands, +# like device replace/remove - it they fail, test succeeds! +_scratch_mount || _fail "failed to mount scratch dev (3)" + +$BTRFS_UTIL_PROG device replace start $SCRATCH_DEV $SCRATCH_DEV $SCRATCH_MNT 2>&1 \ + | _filter_scratch + +$BTRFS_UTIL_PROG device remove $SCRATCH_DEV $SCRATCH_MNT 2>&1 \ + | _filter_scratch + +_scratch_unmount + +_spare_dev_put +_scratch_dev_pool_put 1 + +# success, all done +status=0 +exit diff --git a/tests/btrfs/301.out b/tests/btrfs/301.out new file mode 100644 index 000000000000..f7f43d8c09c0 --- /dev/null +++ b/tests/btrfs/301.out @@ -0,0 +1,4 @@ +QA output created by 301 +ERROR: ioctl(DEV_REPLACE_STATUS) failed on "SCRATCH_MNT": Invalid argument + +ERROR: error removing device 'SCRATCH_DEV': Invalid argument
The TEMP_FSID 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> --- V3: - Renamed the feature to temp-fsid. - Group all requirements; - Remove the "Finished" echo; - Make use of helpers like _scratch_mount and _mount. (Thanks Josef!) - Use lower case for local vars (thanks Anand!). V2: https://lore.kernel.org/linux-btrfs/20230905200826.3605083-1-gpiccoli@igalia.com/ tests/btrfs/301 | 83 +++++++++++++++++++++++++++++++++++++++++++++ tests/btrfs/301.out | 4 +++ 2 files changed, 87 insertions(+) create mode 100755 tests/btrfs/301 create mode 100644 tests/btrfs/301.out