On Mon, Jan 29, 2024 at 03:02:47PM +1100, David Disseldorp wrote:
> Looks fine.
> Reviewed-by: David Disseldorp <ddiss@xxxxxxx>
Thanks for reviewing :)
>
> A couple of minor comments below...
>
> On Sun, 28 Jan 2024 23:56:53 +0800, Zorro Lang wrote:
>
> > This's a regression test of 84712492e6da ("xfs: short circuit
> > xfs_growfs_data_private() if delta is zero").
> >
> > If try to do growfs with "too-small" size expansion, might lead to a
> > delta of "0" in xfs_growfs_data_private(), then end up in the shrink
> > case and emit the EXPERIMENTAL warning even if we're not changing
> > anything at all.
> >
> > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> > ---
> > tests/xfs/999 | 53 +++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/999.out | 2 ++
> > 2 files changed, 55 insertions(+)
> > create mode 100755 tests/xfs/999
> > create mode 100644 tests/xfs/999.out
> >
> > diff --git a/tests/xfs/999 b/tests/xfs/999
> > new file mode 100755
> > index 00000000..09192ba3
> > --- /dev/null
> > +++ b/tests/xfs/999
> > @@ -0,0 +1,53 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2024 Red Hat, Inc. All Rights Reserved.
> > +#
> > +# FS QA Test No. 999
>
> I'll assume 999 is just a placeholder...
Yeah, I'll replace it when I merge it.
>
> > +#
> > +# Test xfs_growfs with "too-small" size expansion, which lead to a delta of "0"
> > +# in xfs_growfs_data_private. This's a regression test of 84712492e6da ("xfs:
> > +# short circuit xfs_growfs_data_private() if delta is zero").
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick growfs
> > +
> > +_cleanup()
> > +{
> > + local dev
> > + $UMOUNT_PROG $LOOP_MNT 2>/dev/null
> > + dev=$(losetup -j testfile | cut -d: -f1)
> > + losetup -d $dev 2>/dev/null
> > + rm -rf $LOOP_IMG $LOOP_MNT
> > + cd /
> > + rm -f $tmp.*
>
> nit: tabs and spaces mixed above
Oh, mistake, I'll change all of them to tabs.
>
> > +}
> > +
> > +# real QA test starts here
> > +_supported_fs xfs
> > +_fixed_by_kernel_commit 84712492e6da \
> > + "xfs: short circuit xfs_growfs_data_private() if delta is zero"
> > +_require_test
> > +_require_loop
> > +_require_xfs_io_command "truncate"
>
> nit: it doesn't seem common for growfs, but you might want to add a:
> _require_command "$XFS_GROWFS_PROG" xfs_growfs
Sure, will do it. If you don't need to check V2, I'll merge this patch
directly with above changes, as you've given a RVB :)
Thanks,
Zorro
>
