Re: [PATCH v2] btrfs: add test for btrfstune squota enable/disable

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Fri, Jul 12, 2024 at 6:26 PM Boris Burkov <boris@xxxxxx> wrote:
>
> On Fri, Jul 12, 2024 at 05:56:15PM +0100, Filipe Manana wrote:
> > On Fri, Jul 12, 2024 at 4:53 PM Boris Burkov <boris@xxxxxx> wrote:
> > >
> > > btrfstune supports enabling simple quotas on a fleshed out filesystem
> > > (without adding owner refs) and clearing squotas entirely from a
> > > filesystem that ran under squotas (clearing the incompat bit)
> > >
> > > Test that these operations work on a relatively complicated filesystem
> > > populated by fsstress while preserving fssum.
> > >
> > > Signed-off-by: Boris Burkov <boris@xxxxxx>
> > > ---
> > >  tests/btrfs/332     | 69 +++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/btrfs/332.out |  2 ++
> > >  2 files changed, 71 insertions(+)
> > >  create mode 100755 tests/btrfs/332
> > >  create mode 100644 tests/btrfs/332.out
> > >
> > > diff --git a/tests/btrfs/332 b/tests/btrfs/332
> > > new file mode 100755
> > > index 000000000..d5cf32664
> > > --- /dev/null
> > > +++ b/tests/btrfs/332
> > > @@ -0,0 +1,69 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2024 Meta Platforms, Inc.  All Rights Reserved.
> > > +#
> > > +# FS QA Test No. btrfs/332
> > > +#
> > > +# Test tune enabling and removing squotas on a live filesystem
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick qgroup
> > > +
> > > +# Import common functions.
> > > +. ./common/filter.btrfs
> > > +
> > > +# real QA test starts here
> > > +_supported_fs btrfs
> > > +_require_scratch_enable_simple_quota
> > > +_require_no_compress
> > > +_require_command "$BTRFS_TUNE_PROG" btrfstune
> > > +_require_fssum
> > > +_require_btrfs_dump_super
> > > +_require_btrfs_command inspect-internal dump-tree
> > > +$BTRFS_TUNE_PROG --help 2>&1 | grep -wq -- '--enable-simple-quota' || \
> > > +       _notrun "$BTRFS_TUNE_PROG too old (must support --enable-simple-quota)"
> > > +$BTRFS_TUNE_PROG --help 2>&1 | grep -wq -- '--remove-simple-quota' || \
> > > +       _notrun "$BTRFS_TUNE_PROG too old (must support --remove-simple-quota)"
> > > +
> > > +_scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed"
> > > +_scratch_mount
> > > +
> > > +# do some stuff
> > > +d1=$SCRATCH_MNT/d1
> > > +d2=$SCRATCH_MNT/d2
> > > +mkdir $d1
> > > +mkdir $d2
> > > +run_check $FSSTRESS_PROG -d $d1 -w -n 2000 $FSSTRESS_AVOID
> > > +fssum_pre=$($FSSUM_PROG -A $SCRATCH_MNT)
> > > +
> > > +# enable squotas
> > > +_scratch_unmount
> > > +$BTRFS_TUNE_PROG --enable-simple-quota $SCRATCH_DEV >> $seqres.full 2>&1 || \
> > > +       _fail "enable simple quota failed"
> >
> > Instead of doing a "|| _fail ..." everywhere, can't we simply not
> > redirect stderr to the .full file and instead have a golden output
> > mismatch in case an error happens?
> > Not only that makes the test shorter and easier to read, it goes along
> > with the most common practice in fstests.
> >
>
> That's what I wanted to do, since you have given me that feedback in the
> past, but unfortunately --enable-simple-quota currently spits out a line
> for each qgroup it creates, which we can't predict in the output, since
> I don't think the fsstress run is deterministic?

But are those messages printed to stderr?
As they aren't errors, I would expect them to be sent to stdout only.

>
> Options I considered where:
> - grep -v or otherwise filter out those lines
> - check the failure
>
> I am happy to switch to the grep.

If those non-error messages are sent to stderr, then I would say just
leave the test as it is.

Thanks.

>
> > > +_check_btrfs_filesystem $SCRATCH_DEV
> > > +_scratch_mount
> > > +fssum_post=$($FSSUM_PROG -A $SCRATCH_MNT)
> > > +[ "$fssum_pre" == "$fssum_post" ] \
> > > +       || echo "fssum $fssum_pre does not match $fssum_post after enabling squota"
> > > +
> > > +# do some more stuff
> > > +run_check $FSSTRESS_PROG -d $d2 -w -n 2000 $FSSTRESS_AVOID
> > > +fssum_pre=$($FSSUM_PROG -A $SCRATCH_MNT)
> > > +_scratch_unmount
> > > +_check_btrfs_filesystem $SCRATCH_DEV
> > > +
> > > +$BTRFS_TUNE_PROG --remove-simple-quota $SCRATCH_DEV >> $seqres.full 2>&1 || \
> > > +       _fail "remove simple quota failed"
> >
> > Same here.
> >
> > With that fixed (if it can be done):
>
> I think here, the command does generally work how we want: silent on
> success, spews on failure, but I wanted it to be consistent with the
> enable, not have to look in different files (or stick in a tee) etc..
>
> I'll play around with it and re-send if the filtered version looks
> better.
>
> >
> > Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > Thanks.
> >
> > > +_check_btrfs_filesystem $SCRATCH_DEV
> > > +$BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV | grep 'SIMPLE_QUOTA'
> > > +$BTRFS_UTIL_PROG inspect-internal dump-tree $SCRATCH_DEV  | grep -e 'QUOTA' -e 'QGROUP'
> > > +
> > > +_scratch_mount
> > > +fssum_post=$($FSSUM_PROG -A $SCRATCH_MNT)
> > > +_scratch_unmount
> > > +[ "$fssum_pre" == "$fssum_post" ] \
> > > +       || echo "fssum $fssum_pre does not match $fssum_post after disabling squota"
> > > +
> > > +echo Silence is golden
> > > +status=0
> > > +exit
> > > diff --git a/tests/btrfs/332.out b/tests/btrfs/332.out
> > > new file mode 100644
> > > index 000000000..adb316136
> > > --- /dev/null
> > > +++ b/tests/btrfs/332.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 332
> > > +Silence is golden
> > > --
> > > 2.45.2
> > >
> > >





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux