[PATCH] xfs: tests extent size hint size overflows

[PATCH] xfs: tests extent size hint size overflows

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

In certain cases, the extent size hints can cause maximum extent
size overflows resulting in extent tree corruptions. This test
exercises the original reproducer, and another corner case
demonstrated to expose problems on 1k block size filesystems.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
--
Version 2:
- TESTDIR->TEST_DIR
- append output to $seqres.full
---
 tests/xfs/074     | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/074.out |  2 ++
 tests/xfs/group   |  1 +
 3 files changed, 96 insertions(+)
 create mode 100755 tests/xfs/074
 create mode 100644 tests/xfs/074.out

diff --git a/tests/xfs/074 b/tests/xfs/074
new file mode 100755
index 0000000..a571a34
--- /dev/null
+++ b/tests/xfs/074
@@ -0,0 +1,93 @@
+#! /bin/bash
+# FS QA Test 074-extsz-hints-vs-maxextlen
+#
+# Check some extent size hint boundary conditions that can result in
+# MAXEXTLEN overflows.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Red Hat, Inc. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+_cleanup()
+{
+	cd /
+	_destroy_loop_device $LOOP_DEV
+	rm -f $tmp.* $LOOP_FILE
+}
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+_supported_os Linux
+
+rm -f $seqres.full
+
+_require_test
+_require_xfs_io_command "falloc"
+
+# we use loop devices for this so that we can create large files for prealloc
+# without having to care about the underlying device size.
+_require_loop
+
+LOOP_FILE=$TEST_DIR/$seq.img
+LOOP_MNT=$TEST_DIR/$seq.mnt
+mkdir -p $LOOP_MNT
+$XFS_IO_PROG -ft -c "truncate 1t" $LOOP_FILE >> $seqres.full
+LOOP_DEV=`_create_loop_device $LOOP_FILE`
+
+_mkfs_dev -d size=40051712b,agcount=4 -l size=32m $LOOP_DEV
+_mount $LOOP_DEV $LOOP_MNT
+
+# Corrupt the BMBT by creating extents larger than MAXEXTLEN
+$XFS_IO_PROG -ft \
+	-c "extsize 16m" \
+	-c "falloc 0 30g" \
+	$LOOP_MNT/foo >> $seqres.full
+
+umount $LOOP_MNT
+_check_xfs_filesystem $LOOP_DEV none none
+
+_mkfs_dev -f -l size=32m $LOOP_DEV
+_mount $LOOP_DEV $LOOP_MNT
+
+# check we trim both ends of the extent approproiately; this will fail
+# on 1k block size filesystems without the correct fixes in place.
+$XFS_IO_PROG -ft \
+	-c "extsize 1g" \
+	-c "falloc 1023m 2g" \
+	$LOOP_MNT/foo >> $seqres.full
+
+umount $LOOP_MNT
+_check_xfs_filesystem $LOOP_DEV none none
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/xfs/074.out b/tests/xfs/074.out
new file mode 100644
index 0000000..380f065
--- /dev/null
+++ b/tests/xfs/074.out
@@ -0,0 +1,2 @@
+QA output created by 074
+Silence is golden
diff --git a/tests/xfs/group b/tests/xfs/group
index 848a1bd..c211fc1 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -71,6 +71,7 @@
 071 rw auto
 072 rw auto prealloc quick
 073 copy auto
+074 quick auto prealloc rw
 078 growfs auto quick
 080 rw ioctl
 081 deprecated # log logprint quota
-- 
2.0.0

Re: [PATCH] xfs: tests extent size hint size overflows

[Eryu Guan]

On Fri, May 29, 2015 at 01:52:50PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> In certain cases, the extent size hints can cause maximum extent
> size overflows resulting in extent tree corruptions. This test
> exercises the original reproducer, and another corner case
> demonstrated to expose problems on 1k block size filesystems.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> --
> Version 2:
> - TESTDIR->TEST_DIR
> - append output to $seqres.full

Looks good to me.

Reviewed-by: Eryu Guan <eguan@redhat.com>

> ---
>  tests/xfs/074     | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/074.out |  2 ++
>  tests/xfs/group   |  1 +
>  3 files changed, 96 insertions(+)
>  create mode 100755 tests/xfs/074
>  create mode 100644 tests/xfs/074.out
> 
> diff --git a/tests/xfs/074 b/tests/xfs/074
> new file mode 100755
> index 0000000..a571a34
> --- /dev/null
> +++ b/tests/xfs/074
> @@ -0,0 +1,93 @@
> +#! /bin/bash
> +# FS QA Test 074-extsz-hints-vs-maxextlen
> +#
> +# Check some extent size hint boundary conditions that can result in
> +# MAXEXTLEN overflows.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2015 Red Hat, Inc. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +_cleanup()
> +{
> +	cd /
> +	_destroy_loop_device $LOOP_DEV
> +	rm -f $tmp.* $LOOP_FILE
> +}
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_supported_os Linux
> +
> +rm -f $seqres.full
> +
> +_require_test
> +_require_xfs_io_command "falloc"
> +
> +# we use loop devices for this so that we can create large files for prealloc
> +# without having to care about the underlying device size.
> +_require_loop
> +
> +LOOP_FILE=$TEST_DIR/$seq.img
> +LOOP_MNT=$TEST_DIR/$seq.mnt
> +mkdir -p $LOOP_MNT
> +$XFS_IO_PROG -ft -c "truncate 1t" $LOOP_FILE >> $seqres.full
> +LOOP_DEV=`_create_loop_device $LOOP_FILE`
> +
> +_mkfs_dev -d size=40051712b,agcount=4 -l size=32m $LOOP_DEV
> +_mount $LOOP_DEV $LOOP_MNT
> +
> +# Corrupt the BMBT by creating extents larger than MAXEXTLEN
> +$XFS_IO_PROG -ft \
> +	-c "extsize 16m" \
> +	-c "falloc 0 30g" \
> +	$LOOP_MNT/foo >> $seqres.full
> +
> +umount $LOOP_MNT
> +_check_xfs_filesystem $LOOP_DEV none none
> +
> +_mkfs_dev -f -l size=32m $LOOP_DEV
> +_mount $LOOP_DEV $LOOP_MNT
> +
> +# check we trim both ends of the extent approproiately; this will fail
> +# on 1k block size filesystems without the correct fixes in place.
> +$XFS_IO_PROG -ft \
> +	-c "extsize 1g" \
> +	-c "falloc 1023m 2g" \
> +	$LOOP_MNT/foo >> $seqres.full
> +
> +umount $LOOP_MNT
> +_check_xfs_filesystem $LOOP_DEV none none
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/074.out b/tests/xfs/074.out
> new file mode 100644
> index 0000000..380f065
> --- /dev/null
> +++ b/tests/xfs/074.out
> @@ -0,0 +1,2 @@
> +QA output created by 074
> +Silence is golden
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 848a1bd..c211fc1 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -71,6 +71,7 @@
>  071 rw auto
>  072 rw auto prealloc quick
>  073 copy auto
> +074 quick auto prealloc rw
>  078 growfs auto quick
>  080 rw ioctl
>  081 deprecated # log logprint quota
> -- 
> 2.0.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] xfs: tests extent size hint size overflows

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

in certain cases, the extent size hints can cause maximum extent
size overflows resulting in extent tree corruptions. This test
exercises the original reproducer, and another corner case
demonstrated to expose problems on 1k block size filesystems.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tests/xfs/074     | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/074.out |  2 ++
 tests/xfs/group   |  1 +
 3 files changed, 96 insertions(+)
 create mode 100755 tests/xfs/074
 create mode 100644 tests/xfs/074.out

diff --git a/tests/xfs/074 b/tests/xfs/074
new file mode 100755
index 0000000..810a1a0
--- /dev/null
+++ b/tests/xfs/074
@@ -0,0 +1,93 @@
+#! /bin/bash
+# FS QA Test 074-extsz-hints-vs-maxextlen
+#
+# Check some extent size hint boundary conditions that can result in
+# MAXEXTLEN overflows.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Red Hat, Inc. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+_cleanup()
+{
+	cd /
+	_destroy_loop_device $LOOP_DEV
+	rm -f $tmp.* $LOOP_FILE
+}
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+_supported_os Linux
+
+rm -f $seqres.full
+
+_require_test
+_require_xfs_io_command "falloc"
+
+# we use loop devices for this so that we can create large files for prealloc
+# without having to care about the underlying device size.
+_require_loop
+
+LOOP_FILE=$TESTDIR/$seq.img
+LOOP_MNT=$TESTDIR/$seq.mnt
+mkdir -p $LOOP_MNT
+$XFS_IO_PROG -ft -c "truncate 1t" $LOOP_FILE 2>&1 > $seqres.full
+LOOP_DEV=`_create_loop_device $LOOP_FILE`
+
+_mkfs_dev -d size=40051712b,agcount=4 -l size=32m $LOOP_DEV
+_mount $LOOP_DEV $LOOP_MNT
+
+# Corrupt the BMBT by creating extents larger than MAXEXTLEN
+$XFS_IO_PROG -ft \
+	-c "extsize 16m" \
+	-c "falloc 0 30g" \
+	$LOOP_MNT/foo 2>&1 > $seqres.full
+
+umount $LOOP_MNT
+_check_xfs_filesystem $LOOP_DEV none none
+
+_mkfs_dev -f -l size=32m $LOOP_DEV
+_mount $LOOP_DEV $LOOP_MNT
+
+# check we trim both ends of the extent approproiately; this will fail
+# on 1k block size filesystems without the correct fixes in place.
+$XFS_IO_PROG -ft \
+	-c "extsize 1g" \
+	-c "falloc 1023m 2g" \
+	$LOOP_MNT/foo 2>&1 > $seqres.full
+
+umount $LOOP_MNT
+_check_xfs_filesystem $LOOP_DEV none none
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/xfs/074.out b/tests/xfs/074.out
new file mode 100644
index 0000000..81a0d24
--- /dev/null
+++ b/tests/xfs/074.out
@@ -0,0 +1,2 @@
+QA output created by 074-extsz-hints-vs-maxextlen
+Silence is golden
diff --git a/tests/xfs/group b/tests/xfs/group
index 848a1bd..c211fc1 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -71,6 +71,7 @@
 071 rw auto
 072 rw auto prealloc quick
 073 copy auto
+074 quick auto prealloc rw
 078 growfs auto quick
 080 rw ioctl
 081 deprecated # log logprint quota
-- 
2.0.0

Re: [PATCH] xfs: tests extent size hint size overflows

[Eryu Guan]

On Wed, May 27, 2015 at 12:43:37PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> in certain cases, the extent size hints can cause maximum extent
> size overflows resulting in extent tree corruptions. This test
> exercises the original reproducer, and another corner case
> demonstrated to expose problems on 1k block size filesystems.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  tests/xfs/074     | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/074.out |  2 ++
>  tests/xfs/group   |  1 +
>  3 files changed, 96 insertions(+)
>  create mode 100755 tests/xfs/074
>  create mode 100644 tests/xfs/074.out
> 
> diff --git a/tests/xfs/074 b/tests/xfs/074
> new file mode 100755
> index 0000000..810a1a0
> --- /dev/null
> +++ b/tests/xfs/074
> @@ -0,0 +1,93 @@
> +#! /bin/bash
> +# FS QA Test 074-extsz-hints-vs-maxextlen
> +#
> +# Check some extent size hint boundary conditions that can result in
> +# MAXEXTLEN overflows.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2015 Red Hat, Inc. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +_cleanup()
> +{
> +	cd /
> +	_destroy_loop_device $LOOP_DEV
> +	rm -f $tmp.* $LOOP_FILE
> +}
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_supported_os Linux
> +
> +rm -f $seqres.full
> +
> +_require_test
> +_require_xfs_io_command "falloc"
> +
> +# we use loop devices for this so that we can create large files for prealloc
> +# without having to care about the underlying device size.
> +_require_loop
> +
> +LOOP_FILE=$TESTDIR/$seq.img
> +LOOP_MNT=$TESTDIR/$seq.mnt

This should be $TEST_DIR, TESTDIR is empty and ends up in /074.mnt

> +mkdir -p $LOOP_MNT
> +$XFS_IO_PROG -ft -c "truncate 1t" $LOOP_FILE 2>&1 > $seqres.full

This will leave stderr to stdout, then only stdout goes to $seqres.full
I see "+ fallocate: No space left on device" when testing.

"command >$seqres.full 2>&1" will do the work.

> +LOOP_DEV=`_create_loop_device $LOOP_FILE`
> +
> +_mkfs_dev -d size=40051712b,agcount=4 -l size=32m $LOOP_DEV
> +_mount $LOOP_DEV $LOOP_MNT
> +
> +# Corrupt the BMBT by creating extents larger than MAXEXTLEN
> +$XFS_IO_PROG -ft \
> +	-c "extsize 16m" \
> +	-c "falloc 0 30g" \
> +	$LOOP_MNT/foo 2>&1 > $seqres.full

Here too, and should append to $seqres.full with ">>"

> +
> +umount $LOOP_MNT
> +_check_xfs_filesystem $LOOP_DEV none none

I think '_check_scratch_fs $LOOP_DEV' is eaiser, it accepts optional
device argument to check and deal with SCRATCH_LOG and SCRATCH_RT
internally.

> +
> +_mkfs_dev -f -l size=32m $LOOP_DEV
> +_mount $LOOP_DEV $LOOP_MNT
> +
> +# check we trim both ends of the extent approproiately; this will fail
> +# on 1k block size filesystems without the correct fixes in place.
> +$XFS_IO_PROG -ft \
> +	-c "extsize 1g" \
> +	-c "falloc 1023m 2g" \
> +	$LOOP_MNT/foo 2>&1 > $seqres.full

And here.

> +
> +umount $LOOP_MNT
> +_check_xfs_filesystem $LOOP_DEV none none
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/074.out b/tests/xfs/074.out
> new file mode 100644
> index 0000000..81a0d24
> --- /dev/null
> +++ b/tests/xfs/074.out
> @@ -0,0 +1,2 @@
> +QA output created by 074-extsz-hints-vs-maxextlen

This should be "QA output created by 074"

Thanks,
Eryu
> +Silence is golden
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 848a1bd..c211fc1 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -71,6 +71,7 @@
>  071 rw auto
>  072 rw auto prealloc quick
>  073 copy auto
> +074 quick auto prealloc rw
>  078 growfs auto quick
>  080 rw ioctl
>  081 deprecated # log logprint quota
> -- 
> 2.0.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfs: tests extent size hint size overflows

[Dave Chinner]

On Wed, May 27, 2015 at 11:34:04AM +0800, Eryu Guan wrote:
> On Wed, May 27, 2015 at 12:43:37PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > in certain cases, the extent size hints can cause maximum extent
> > size overflows resulting in extent tree corruptions. This test
> > exercises the original reproducer, and another corner case
> > demonstrated to expose problems on 1k block size filesystems.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
....
> > +rm -f $seqres.full
> > +
> > +_require_test
> > +_require_xfs_io_command "falloc"
> > +
> > +# we use loop devices for this so that we can create large files for prealloc
> > +# without having to care about the underlying device size.
> > +_require_loop
> > +
> > +LOOP_FILE=$TESTDIR/$seq.img
> > +LOOP_MNT=$TESTDIR/$seq.mnt
> 
> This should be $TEST_DIR, TESTDIR is empty and ends up in /074.mnt

Oops, will fix.

> > +mkdir -p $LOOP_MNT
> > +$XFS_IO_PROG -ft -c "truncate 1t" $LOOP_FILE 2>&1 > $seqres.full
> 
> This will leave stderr to stdout, then only stdout goes to $seqres.full
> I see "+ fallocate: No space left on device" when testing.

Why would the truncate command give an "fallocate" failure?

And, besides, allowing stderr to be exposed is done on purpose - an
error in the execution of the truncate command will trigger a test
failure because stderr is captured by the test harness...

> "command >$seqres.full 2>&1" will do the work.
> 
> > +LOOP_DEV=`_create_loop_device $LOOP_FILE`
> > +
> > +_mkfs_dev -d size=40051712b,agcount=4 -l size=32m $LOOP_DEV
> > +_mount $LOOP_DEV $LOOP_MNT
> > +
> > +# Corrupt the BMBT by creating extents larger than MAXEXTLEN
> > +$XFS_IO_PROG -ft \
> > +	-c "extsize 16m" \
> > +	-c "falloc 0 30g" \
> > +	$LOOP_MNT/foo 2>&1 > $seqres.full
> 
> Here too, and should append to $seqres.full with ">>"

Will fix.

> 
> > +
> > +umount $LOOP_MNT
> > +_check_xfs_filesystem $LOOP_DEV none none
> 
> I think '_check_scratch_fs $LOOP_DEV' is eaiser, it accepts optional
> device argument to check and deal with SCRATCH_LOG and SCRATCH_RT
> internally.

But it's not the scratch device I'm using. i.e. it doesn't use
SCRATCH_LOG or SCRATCH_RT because I didn't use _scratch_mkfs to
create the filesystem, nor am I using _scratch_mount, so it would
fail to mount if mkfs used external devices....

> > --- /dev/null
> > +++ b/tests/xfs/074.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 074-extsz-hints-vs-maxextlen
> 
> This should be "QA output created by 074"

Hmm - the patch is stale then. That's strange, because I pulled it
from the git tree to my test machines, and they didn't fail. I'll
fix it.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs: tests extent size hint size overflows

[Eryu Guan]

On Fri, May 29, 2015 at 11:20:31AM +1000, Dave Chinner wrote:
> On Wed, May 27, 2015 at 11:34:04AM +0800, Eryu Guan wrote:
> > On Wed, May 27, 2015 at 12:43:37PM +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > in certain cases, the extent size hints can cause maximum extent
> > > size overflows resulting in extent tree corruptions. This test
> > > exercises the original reproducer, and another corner case
> > > demonstrated to expose problems on 1k block size filesystems.
> > > 
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ....
> > > +rm -f $seqres.full
> > > +
> > > +_require_test
> > > +_require_xfs_io_command "falloc"
> > > +
> > > +# we use loop devices for this so that we can create large files for prealloc
> > > +# without having to care about the underlying device size.
> > > +_require_loop
> > > +
> > > +LOOP_FILE=$TESTDIR/$seq.img
> > > +LOOP_MNT=$TESTDIR/$seq.mnt
> > 
> > This should be $TEST_DIR, TESTDIR is empty and ends up in /074.mnt
> 
> Oops, will fix.
> 
> > > +mkdir -p $LOOP_MNT
> > > +$XFS_IO_PROG -ft -c "truncate 1t" $LOOP_FILE 2>&1 > $seqres.full
> > 
> > This will leave stderr to stdout, then only stdout goes to $seqres.full
> > I see "+ fallocate: No space left on device" when testing.
> 
> Why would the truncate command give an "fallocate" failure?

Oh, I mean at the "falloc" time, I should refer to the next xfs_io
command below..

> 
> And, besides, allowing stderr to be exposed is done on purpose - an
> error in the execution of the truncate command will trigger a test
> failure because stderr is captured by the test harness...

Then I think a comment should be added, because "2>&1 >$seqres.full" is
not a usual usage, usually it indicates a programming mistake. Or just
">$seqres.full" and leave stderr untouched? which indicates clearly we
leave stderr unfiltered on purpose.

> 
> > "command >$seqres.full 2>&1" will do the work.
> > 
> > > +LOOP_DEV=`_create_loop_device $LOOP_FILE`
> > > +
> > > +_mkfs_dev -d size=40051712b,agcount=4 -l size=32m $LOOP_DEV
> > > +_mount $LOOP_DEV $LOOP_MNT
> > > +
> > > +# Corrupt the BMBT by creating extents larger than MAXEXTLEN
> > > +$XFS_IO_PROG -ft \
> > > +	-c "extsize 16m" \
> > > +	-c "falloc 0 30g" \
> > > +	$LOOP_MNT/foo 2>&1 > $seqres.full

The "+ fallocate: No space left on device" error comes from this
command. Is this on purpose too? So on patched kernel the error
shouldn't be there?

Thanks,
Eryu

Re: [PATCH] xfs: tests extent size hint size overflows

[Dave Chinner]

On Fri, May 29, 2015 at 10:51:05AM +0800, Eryu Guan wrote:
> On Fri, May 29, 2015 at 11:20:31AM +1000, Dave Chinner wrote:
> > On Wed, May 27, 2015 at 11:34:04AM +0800, Eryu Guan wrote:
> > > On Wed, May 27, 2015 at 12:43:37PM +1000, Dave Chinner wrote:
> > > > From: Dave Chinner <dchinner@redhat.com>
> > > > 
> > > > in certain cases, the extent size hints can cause maximum extent
> > > > size overflows resulting in extent tree corruptions. This test
> > > > exercises the original reproducer, and another corner case
> > > > demonstrated to expose problems on 1k block size filesystems.
> > > > 
> > > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ....
> > > > +rm -f $seqres.full
> > > > +
> > > > +_require_test
> > > > +_require_xfs_io_command "falloc"
> > > > +
> > > > +# we use loop devices for this so that we can create large files for prealloc
> > > > +# without having to care about the underlying device size.
> > > > +_require_loop
> > > > +
> > > > +LOOP_FILE=$TESTDIR/$seq.img
> > > > +LOOP_MNT=$TESTDIR/$seq.mnt
> > > 
> > > This should be $TEST_DIR, TESTDIR is empty and ends up in /074.mnt
> > 
> > Oops, will fix.
> > 
> > > > +mkdir -p $LOOP_MNT
> > > > +$XFS_IO_PROG -ft -c "truncate 1t" $LOOP_FILE 2>&1 > $seqres.full
> > > 
> > > This will leave stderr to stdout, then only stdout goes to $seqres.full
> > > I see "+ fallocate: No space left on device" when testing.
> > 
> > Why would the truncate command give an "fallocate" failure?
> 
> Oh, I mean at the "falloc" time, I should refer to the next xfs_io
> command below..
> 
> > 
> > And, besides, allowing stderr to be exposed is done on purpose - an
> > error in the execution of the truncate command will trigger a test
> > failure because stderr is captured by the test harness...
> 
> Then I think a comment should be added, because "2>&1 >$seqres.full" is
> not a usual usage, usually it indicates a programming mistake. Or just
> ">$seqres.full" and leave stderr untouched? which indicates clearly we
> leave stderr unfiltered on purpose.

If we had to comment on every time we are capturing stderr as the
failure indication for the test, then we'd be adding comments
everywhere.  This is how xfstests is supposed to work - we capture
*error messages* from tools because that's what is used to inform the
user that there was an error. Those error messages cause golden
image match failures, and that causes the test to fail.

See the comments I've made previously about why "run_check" is
considered harmful - it encourages tools to be silent on error and
only set exit values, because that is all it checks. Users *never*
check return values - there need to be error messages when an error
occurs...

> > > "command >$seqres.full 2>&1" will do the work.
> > > 
> > > > +LOOP_DEV=`_create_loop_device $LOOP_FILE`
> > > > +
> > > > +_mkfs_dev -d size=40051712b,agcount=4 -l size=32m $LOOP_DEV
> > > > +_mount $LOOP_DEV $LOOP_MNT
> > > > +
> > > > +# Corrupt the BMBT by creating extents larger than MAXEXTLEN
> > > > +$XFS_IO_PROG -ft \
> > > > +	-c "extsize 16m" \
> > > > +	-c "falloc 0 30g" \
> > > > +	$LOOP_MNT/foo 2>&1 > $seqres.full
> 
> The "+ fallocate: No space left on device" error comes from this
> command. Is this on purpose too? So on patched kernel the error
> shouldn't be there?

That's right - the falloc completes without error when the bug is
fixed, so stderr is silent and the test passes.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com