Re: [PATCH 1/5] btrfs/400: new test for simple quotas

[Boris Burkov <boris@bur.io>]

On Fri, Sep 22, 2023 at 11:40:46PM +0800, Anand Jain wrote:
> 
> 
> 
> 
> 
> On 06/07/2023 07:42, Boris Burkov wrote:
> > Test some interesting basic and edge cases of simple quotas.
> > 
> > To some extent, this is redundant with the alternate testing strategy of
> > using MKFS_OPTIONS to enable simple quotas, running the full suite and
> > relying on kernel warnings and fsck to surface issues.
> > 
> > Signed-off-by: Boris Burkov <boris@bur.io>
> > ---
> >   tests/btrfs/400     | 439 ++++++++++++++++++++++++++++++++++++++++++++
> >   tests/btrfs/400.out |   2 +
> >   2 files changed, 441 insertions(+)
> >   create mode 100755 tests/btrfs/400
> >   create mode 100644 tests/btrfs/400.out
> > 
> > diff --git a/tests/btrfs/400 b/tests/btrfs/400
> > new file mode 100755
> > index 000000000..c3548d42e
> > --- /dev/null
> > +++ b/tests/btrfs/400
> > @@ -0,0 +1,439 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
> > +#
> > +# FS QA Test 400
> > +#
> > +# Test common btrfs simple quotas scenarios involving sharing extents and
> > +# removing them in various orders.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick qgroup copy_range snapshot
> > +
> > +# Import common functions.
> > +# . ./common/filter
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs btrfs
> > +_require_scratch
> 
> 
> I don't see any prerequisite checking and call of notrun() on the
> systems without the kernel or progs simple-quota support. Is it not
> required?
> 
> 
> > +
> > +SUBV=$SCRATCH_MNT/subv
> > +NESTED=$SCRATCH_MNT/subv/nested
> > +SNAP=$SCRATCH_MNT/snap
> > +K=1024
> > +M=$(($K * $K))
> > +NR_FILL=1024
> > +FILL_SZ=$((8 * $K))
> > +TOTAL_FILL=$(($NR_FILL * $FILL_SZ))
> > +EB_SZ=$((16 * $K))
> > +EXT_SZ=$((128 * M))
> > +LIMIT_NR=8
> > +LIMIT=$(($EXT_SZ * $LIMIT_NR))
> 
> Style consistency requires the use of lowercase for test local
> variables.
> 

Is this documented? There are lots of counterexamples.
Will fix, of course.

> 
> > +
> > +prepare()
> > +{
> > +	echo "preparing" > /dev/kmsg
> 
> 
>  Please use $seqres.full or stdout for debugging purpose.

These messages are only helpful in dmesg, as they help to sequence test
actions with kernel logs while debugging test failures.

I personally find it to be a great pattern for writing debuggable tests
that don't end up with annoying fragile test.out files, so I would
encourage you to play with the pattern and see what you think.
Especially in vm-like environments there's really no downside, that I
can see, it's just super nice to have the test actions in dmesg.

However, I appreciate that I'm the one doing the weird thing, so I'll
remove the messages or throw them in seqres.full.

> 
> 
> > +	_scratch_mkfs >> $seqres.full
> > +	_scratch_mount
> > +	enable_quota "s"
> > +	$BTRFS_UTIL_PROG subvolume create $SUBV >> $seqres.full
> > +	set_subvol_limit 256 $LIMIT
> > +	check_subvol_usage 256 0
> > +
> > +	echo "filling" > /dev/kmsg
> > +	# Create a bunch of little filler files to generate several levels in
> > +	# the btree, to make snapshotting sharing scenarios complex enough.
> > +	$FIO_PROG --name=filler --directory=$SUBV --rw=randwrite --nrfiles=$NR_FILL --filesize=$FILL_SZ >/dev/null 2>&1
> > +	echo "filled" > /dev/kmsg
> > +	check_subvol_usage 256 $TOTAL_FILL
> > +
> > +	# Create a single file whose extents we will explicitly share/unshare.
> > +	do_write $SUBV/f $EXT_SZ
> > +	check_subvol_usage 256 $(($TOTAL_FILL + $EXT_SZ))
> > +	echo "prepared" > /dev/kmsg
> > +}
> > +
> 
> 
> 
> > +
> > +echo "Silence is golden"
> 
> We can have the echo part, like (echo 'prepared' > /dev/kmsg), directed
> to stdout; this will be useful for verification and debugging as well.
> 
> 
> Thanks, Anand
> 
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/btrfs/400.out b/tests/btrfs/400.out
> > new file mode 100644
> > index 000000000..c940c6206
> > --- /dev/null
> > +++ b/tests/btrfs/400.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 400
> > +Silence is golden
>