diff mbox series

xfs: test xfs_growfs with too-small size expansion

Message ID 20240128155653.1533493-1-zlang@kernel.org (mailing list archive)
State New, archived
Headers show
Series xfs: test xfs_growfs with too-small size expansion | expand

Commit Message

Zorro Lang Jan. 28, 2024, 3:56 p.m. UTC 
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@redhat.com>
---
 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

Comments

David Disseldorp Jan. 29, 2024, 4:02 a.m. UTC | #1 
Looks fine.
Reviewed-by: David Disseldorp <ddiss@suse.de>

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@redhat.com>
> ---
>  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...

> +#
> +# 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

> +}
> +
> +# 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
Zorro Lang Feb. 2, 2024, 12:23 p.m. UTC | #2 
On Mon, Jan 29, 2024 at 03:02:47PM +1100, David Disseldorp wrote:
> Looks fine.
> Reviewed-by: David Disseldorp <ddiss@suse.de>

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@redhat.com>
> > ---
> >  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

>
diff mbox series

Patch

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
+#
+# 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.*
+}
+
+# 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"
+
+LOOP_IMG=$TEST_DIR/$seq.dev
+LOOP_MNT=$TEST_DIR/$seq.mnt
+rm -rf $LOOP_IMG $LOOP_MNT
+mkdir -p $LOOP_MNT
+# 1G image
+$XFS_IO_PROG -f -c "truncate 1073741824" $LOOP_IMG
+$MKFS_XFS_PROG -f $LOOP_IMG >$seqres.full
+# extend by just 8K
+$XFS_IO_PROG -f -c "truncate 1073750016" $LOOP_IMG
+_mount -oloop $LOOP_IMG $LOOP_MNT
+# A known bug shows "XFS_IOC_FSGROWFSDATA xfsctl failed: No space left on
+# device" at here, refer to _fixed_by_kernel_commit above
+$XFS_GROWFS_PROG $LOOP_MNT >$seqres.full
+if [ $? -ne 0 ];then
+	echo "xfs_growfs fails!"
+fi
+
+echo "Silence is golden"
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/999.out b/tests/xfs/999.out
new file mode 100644
index 00000000..3b276ca8
--- /dev/null
+++ b/tests/xfs/999.out
@@ -0,0 +1,2 @@ 
+QA output created by 999
+Silence is golden