Re: [PATCH] xfs: test mkfs.xfs sizing of internal logs that

Dave Chinner
2022-05-24T23:44:36Z

On Tue, May 24, 2022 at 12:52:57PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> This is a regression test that exercises the mkfs.xfs code that creates
> log sizes that are very close to the AG size when stripe units are in
> play and/or when the log is forced to be in AG 0.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> tests/xfs/843 | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/843.out | 2 ++
> 2 files changed, 58 insertions(+)
> create mode 100755 tests/xfs/843
> create mode 100644 tests/xfs/843.out
>
> diff --git a/tests/xfs/843 b/tests/xfs/843
> new file mode 100755
> index 00000000..3384b1aa
> --- /dev/null
> +++ b/tests/xfs/843
> @@ -0,0 +1,56 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 843
> +#
> +# Now that we've increased the default log size calculation, test mkfs with
> +# various stripe units and filesystem sizes to see if we can provoke mkfs into
> +# breaking.
> +#
> +. ./common/preamble
> +_begin_fstest auto mkfs
> +
> +_cleanup()
> +{
> + cd /
> + rm -r -f $tmp.* $testfile
> +}

I'd omit this completely.

> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_require_test
> +
> +testfile=$TEST_DIR/a
> +rm -f $testfile
> +
> +test_format() {
> + local tag="$1"
> + shift
> +
> + echo "$tag" >> $seqres.full
> + $MKFS_XFS_PROG $@ -d file,name=$testfile &>> $seqres.full
> + local res=$?
> + test $res -eq 0 || echo "$tag FAIL $res" | tee -a $seqres.full

What breakage are you trying to provoke? Just the log size
calculation? If so, why do we need to actually write the filesystem
to disk? Won't "-N" still calculate everything and fail if it's
broken or quit with success without needing to write anything to
disk?

> +}
> +
> +# First we try various small filesystems and stripe sizes.
> +for M in `seq 298 302` `seq 490 520`; do
> + for S in `seq 32 4 64`; do
> + test_format "M=$M S=$S" -dsu=${S}k,sw=1,size=${M}m
> + done
> +done
> +
> +# log so large it pushes the root dir into AG 1
> +test_format "log pushes rootdir into AG 1" -d agcount=3200,size=6366g -lagnum=0
> +
> +# log end rounded beyond EOAG due to stripe unit
> +test_format "log end beyond eoag" -d agcount=3200,size=6366g -d su=256k,sw=4
> +
> +echo Silence is golden

Put this at the top where the test is being set up (i.e. where you
define testfile). That tells the reader straight away that no output
is expected on a successful run before they start reading the test
code....