On Fri, Jul 12, 2024 at 7:02 PM Boris Burkov <boris@xxxxxx> wrote: > > On Fri, Jul 12, 2024 at 06:33:59PM +0100, Filipe Manana wrote: > > 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. > > They get sent to stdout, and adjusting the code to just grep them out > came out well enough, so I resent that as v3. I see, my idea was simple, no need to grep or do anything. Instead of : $BTRFS_TUNE_PROG --enable-simple-quota $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "enable simple quota failed" Just doing: $BTRFS_TUNE_PROG --enable-simple-quota $SCRATCH_DEV >> $seqres.full So those informative messages would be ignored, and in case an error happens, anything btrfstune prints to stderr makes the test automatically fail due to mismatch with the golden output. v3 does instead: $BTRFS_TUNE_PROG --enable-simple-quota $SCRATCH_DEV | grep -v 'created qgroup for' > > We crossed streams a little here, so hopefully I did a reasonable enough > thing and the v3 looks good to you :) > > Thanks for your time, and sorry for the churn on this simple stuff. No worries, thank you. > > > > > 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 > > > > > > > > > >
