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. 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. > > 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 > > > > > > > >
