[PATCH 0/9] some backlog patches and new test and fixes

[PATCH 0/9] some backlog patches and new test and fixes

[Eryu Guan]

First five patches are backlogs from last fstests update(but rebased on current
master and renumbered). The sixth patch is a new test case. And the last three
patches are random fixes.

Any reviews are welcomed!

Thanks,
Eryu

Eryu Guan (9):
  generic: test some mount/umount corner cases
  generic: test quota handling on remount ro failure
  generic: test hardlink to unlinked file
  shared: test truncate orphan inodes when mounting extN
  generic: test fs freeze/unfreeze and mount/umount race
  generic: test I/O error path by fully filling dm snapshot
  common: recognise NFS export over IPv6 in _require_scratch_nocheck()
  xfs/014: replace df with $DF_PROG
  generic/077: add missing _require_scratch

 common/config         |   1 +
 common/rc             |   6 ++-
 tests/generic/067     | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/067.out |   2 +
 tests/generic/072     |  69 ++++++++++++++++++++++++
 tests/generic/072.out |   3 ++
 tests/generic/073     |  99 ++++++++++++++++++++++++++++++++++
 tests/generic/073.out |   2 +
 tests/generic/077     |   1 +
 tests/generic/080     |  98 ++++++++++++++++++++++++++++++++++
 tests/generic/080.out |   2 +
 tests/generic/081     |  79 ++++++++++++++++++++++++++++
 tests/generic/081.out |   2 +
 tests/generic/group   |   5 ++
 tests/shared/001      |  71 +++++++++++++++++++++++++
 tests/shared/001.out  |   2 +
 tests/shared/group    |   1 +
 tests/xfs/014         |   2 +-
 18 files changed, 585 insertions(+), 3 deletions(-)
 create mode 100755 tests/generic/067
 create mode 100644 tests/generic/067.out
 create mode 100755 tests/generic/072
 create mode 100644 tests/generic/072.out
 create mode 100755 tests/generic/073
 create mode 100644 tests/generic/073.out
 create mode 100755 tests/generic/080
 create mode 100644 tests/generic/080.out
 create mode 100755 tests/generic/081
 create mode 100644 tests/generic/081.out
 create mode 100755 tests/shared/001
 create mode 100644 tests/shared/001.out

-- 
2.1.0

[PATCH 1/9 RESEND] generic: test some mount/umount corner cases

[Eryu Guan]

There're six test cases:
 - mount at a nonexistent mount point
 - mount a free loop device
 - mount with a wrong fs type
 - umount an symlink to device which is not mounted
 - umount a path with too long name
 - lazy umount a symlink

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/generic/067     | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/067.out |   2 +
 tests/generic/group   |   1 +
 3 files changed, 146 insertions(+)
 create mode 100755 tests/generic/067
 create mode 100644 tests/generic/067.out

diff --git a/tests/generic/067 b/tests/generic/067
new file mode 100755
index 0000000..7a4840b
--- /dev/null
+++ b/tests/generic/067
@@ -0,0 +1,143 @@
+#! /bin/bash
+# FS QA Test No. 067
+#
+# Some random mount/umount corner case tests
+#
+# - mount at a nonexistent mount point
+# - mount a free loop device
+# - mount with a wrong fs type specified
+# - umount an symlink to device which is not mounted
+# - umount a path with too long name
+# - lazy umount a symlink
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_test
+_require_scratch
+_require_loop
+
+rm -f $seqres.full
+
+_scratch_mkfs >>$seqres.full 2>&1
+
+# kernel should not hang nor oops when mounting fs to nonexistent mount point
+mount_nonexistent_mnt()
+{
+	echo "# mount to nonexistent mount point" >>$seqres.full
+	rm -rf $TEST_DIR/nosuchdir
+	$MOUNT_PROG $SCRATCH_DEV $TEST_DIR/nosuchdir >>$seqres.full 2>&1
+}
+
+# fs driver should be able to handle mounting a free loop device gracefully
+# xfs ever hung, "ec53d1d xfs: don't block on buffer read errors" fixed it
+mount_free_loopdev()
+{
+	echo "# mount a free loop device" >>$seqres.full
+	loopdev=`losetup -f`
+	$MOUNT_PROG -t $FSTYP $loopdev $SCRATCH_MNT >>$seqres.full 2>&1
+}
+
+# mount with wrong fs type specified.
+# This should fail gracefully, no hang no oops are expected
+mount_wrong_fstype()
+{
+	local fs=ext4
+	if [ "$FSTYP" == "ext4" ]; then
+		fs=xfs
+	fi
+	echo "# mount with wrong fs type" >>$seqres.full
+	$MOUNT_PROG -t $fs $SCRATCH_DEV $SCRATCH_MNT >>$seqres.full 2>&1
+}
+
+# umount a symlink to device, which is not mounted.
+# This should fail gracefully, no hang no oops are expected
+umount_symlink_device()
+{
+	local symlink=$TEST_DIR/$seq.scratch_dev_symlink
+	rm -f $symlink
+	echo "# umount symlink to device, which is not mounted" >>$seqres.full
+	ln -s $SCRATCH_DEV $symlink
+	$UMOUNT_PROG $symlink >>$seqres.full 2>&1
+}
+
+# umount a path name that is 256 bytes long, this should fail gracefully,
+# and the following umount should not hang nor oops
+umount_toolong_name()
+{
+	local longname=$SCRATCH_MNT/`$PERL_PROG -e 'print "a"x256;'`
+
+	_scratch_mount 2>&1 | tee -a $seqres.full
+
+	echo "# umount a too-long name" >>$seqres.full
+	$UMOUNT_PROG $longname >>$seqres.full 2>&1
+	_scratch_unmount 2>&1 | tee -a $seqres.full
+}
+
+# lazy umount a symlink should not block following umount.
+# This is the test case described in https://lkml.org/lkml/2014/7/21/98
+lazy_umount_symlink()
+{
+	local symlink=$SCRATCH_MNT/$seq.testdir_symlink
+	echo "# lazy umount a symlink" >>$seqres.full
+	_scratch_mount 2>&1 | tee -a $seqres.full
+	mkdir -p $SCRATCH_MNT/testdir
+	rm -f $symlink
+	ln -s $SCRATCH_MNT/testdir $symlink
+
+	$UMOUNT_PROG -l $symlink >>$seqres.full 2>&1
+	# umount $SCRATCH_MNT should not be blocked
+	_scratch_unmount 2>&1 | tee -a $seqres.full
+}
+
+echo "Silence is golden"
+
+mount_nonexistent_mnt
+mount_free_loopdev
+mount_wrong_fstype
+
+umount_symlink_device
+umount_toolong_name
+lazy_umount_symlink
+
+status=0
+exit
diff --git a/tests/generic/067.out b/tests/generic/067.out
new file mode 100644
index 0000000..daa1545
--- /dev/null
+++ b/tests/generic/067.out
@@ -0,0 +1,2 @@
+QA output created by 067
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index d56d3ce..03ad7b8 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -69,6 +69,7 @@
 064 auto quick prealloc
 065 metadata auto quick
 066 metadata auto quick
+067 auto quick mount
 068 other auto freeze dangerous stress
 069 rw udf auto quick
 070 attr udf auto quick stress
-- 
2.1.0

Re: [PATCH 1/9 RESEND] generic: test some mount/umount corner cases

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:50PM +0800, Eryu Guan wrote:
> There're six test cases:
>  - mount at a nonexistent mount point
>  - mount a free loop device
>  - mount with a wrong fs type
>  - umount an symlink to device which is not mounted
>  - umount a path with too long name
>  - lazy umount a symlink
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---

Looks good to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  tests/generic/067     | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/067.out |   2 +
>  tests/generic/group   |   1 +
>  3 files changed, 146 insertions(+)
>  create mode 100755 tests/generic/067
>  create mode 100644 tests/generic/067.out
> 
> diff --git a/tests/generic/067 b/tests/generic/067
> new file mode 100755
> index 0000000..7a4840b
> --- /dev/null
> +++ b/tests/generic/067
> @@ -0,0 +1,143 @@
> +#! /bin/bash
> +# FS QA Test No. 067
> +#
> +# Some random mount/umount corner case tests
> +#
> +# - mount at a nonexistent mount point
> +# - mount a free loop device
> +# - mount with a wrong fs type specified
> +# - umount an symlink to device which is not mounted
> +# - umount a path with too long name
> +# - lazy umount a symlink
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_test
> +_require_scratch
> +_require_loop
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +
> +# kernel should not hang nor oops when mounting fs to nonexistent mount point
> +mount_nonexistent_mnt()
> +{
> +	echo "# mount to nonexistent mount point" >>$seqres.full
> +	rm -rf $TEST_DIR/nosuchdir
> +	$MOUNT_PROG $SCRATCH_DEV $TEST_DIR/nosuchdir >>$seqres.full 2>&1
> +}
> +
> +# fs driver should be able to handle mounting a free loop device gracefully
> +# xfs ever hung, "ec53d1d xfs: don't block on buffer read errors" fixed it
> +mount_free_loopdev()
> +{
> +	echo "# mount a free loop device" >>$seqres.full
> +	loopdev=`losetup -f`
> +	$MOUNT_PROG -t $FSTYP $loopdev $SCRATCH_MNT >>$seqres.full 2>&1
> +}
> +
> +# mount with wrong fs type specified.
> +# This should fail gracefully, no hang no oops are expected
> +mount_wrong_fstype()
> +{
> +	local fs=ext4
> +	if [ "$FSTYP" == "ext4" ]; then
> +		fs=xfs
> +	fi
> +	echo "# mount with wrong fs type" >>$seqres.full
> +	$MOUNT_PROG -t $fs $SCRATCH_DEV $SCRATCH_MNT >>$seqres.full 2>&1
> +}
> +
> +# umount a symlink to device, which is not mounted.
> +# This should fail gracefully, no hang no oops are expected
> +umount_symlink_device()
> +{
> +	local symlink=$TEST_DIR/$seq.scratch_dev_symlink
> +	rm -f $symlink
> +	echo "# umount symlink to device, which is not mounted" >>$seqres.full
> +	ln -s $SCRATCH_DEV $symlink
> +	$UMOUNT_PROG $symlink >>$seqres.full 2>&1
> +}
> +
> +# umount a path name that is 256 bytes long, this should fail gracefully,
> +# and the following umount should not hang nor oops
> +umount_toolong_name()
> +{
> +	local longname=$SCRATCH_MNT/`$PERL_PROG -e 'print "a"x256;'`
> +
> +	_scratch_mount 2>&1 | tee -a $seqres.full
> +
> +	echo "# umount a too-long name" >>$seqres.full
> +	$UMOUNT_PROG $longname >>$seqres.full 2>&1
> +	_scratch_unmount 2>&1 | tee -a $seqres.full
> +}
> +
> +# lazy umount a symlink should not block following umount.
> +# This is the test case described in https://lkml.org/lkml/2014/7/21/98
> +lazy_umount_symlink()
> +{
> +	local symlink=$SCRATCH_MNT/$seq.testdir_symlink
> +	echo "# lazy umount a symlink" >>$seqres.full
> +	_scratch_mount 2>&1 | tee -a $seqres.full
> +	mkdir -p $SCRATCH_MNT/testdir
> +	rm -f $symlink
> +	ln -s $SCRATCH_MNT/testdir $symlink
> +
> +	$UMOUNT_PROG -l $symlink >>$seqres.full 2>&1
> +	# umount $SCRATCH_MNT should not be blocked
> +	_scratch_unmount 2>&1 | tee -a $seqres.full
> +}
> +
> +echo "Silence is golden"
> +
> +mount_nonexistent_mnt
> +mount_free_loopdev
> +mount_wrong_fstype
> +
> +umount_symlink_device
> +umount_toolong_name
> +lazy_umount_symlink
> +
> +status=0
> +exit
> diff --git a/tests/generic/067.out b/tests/generic/067.out
> new file mode 100644
> index 0000000..daa1545
> --- /dev/null
> +++ b/tests/generic/067.out
> @@ -0,0 +1,2 @@
> +QA output created by 067
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index d56d3ce..03ad7b8 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -69,6 +69,7 @@
>  064 auto quick prealloc
>  065 metadata auto quick
>  066 metadata auto quick
> +067 auto quick mount
>  068 other auto freeze dangerous stress
>  069 rw udf auto quick
>  070 attr udf auto quick stress
> -- 
> 2.1.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 2/9 RESEND] generic: test quota handling on remount ro failure

[Eryu Guan]

Remount ro should not turn qouta off unconditionally, even remount ro
failed, also kernel should not oops on the next succeeded remount ro.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/generic/072     | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/072.out |  3 +++
 tests/generic/group   |  1 +
 3 files changed, 73 insertions(+)
 create mode 100755 tests/generic/072
 create mode 100644 tests/generic/072.out

diff --git a/tests/generic/072 b/tests/generic/072
new file mode 100755
index 0000000..ef40822
--- /dev/null
+++ b/tests/generic/072
@@ -0,0 +1,69 @@
+#! /bin/bash
+# FS QA Test No. 072
+#
+# Test quota handling on remount ro failure
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/quota
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_test
+_require_scratch
+_require_quota
+
+rm -f $seqres.full
+
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount "-o usrquota,grpquota"
+
+quotacheck -ug $SCRATCH_MNT >/dev/null 2>&1
+quotaon $SCRATCH_MNT >/dev/null 2>&1
+
+# first remount ro with a bad option, a failed remount ro should not disable quota
+_scratch_mount "-o remount,ro,nosuchopt" >>$seqres.full 2>&1
+quotaon -p $SCRATCH_MNT | _filter_scratch
+# second remount should succeed, no oops or hang expected
+_scratch_mount "-o remount,ro"
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/072.out b/tests/generic/072.out
new file mode 100644
index 0000000..ff0e180
--- /dev/null
+++ b/tests/generic/072.out
@@ -0,0 +1,3 @@
+QA output created by 072
+group quota on SCRATCH_MNT (SCRATCH_DEV) is on
+user quota on SCRATCH_MNT (SCRATCH_DEV) is on
diff --git a/tests/generic/group b/tests/generic/group
index 03ad7b8..c0210d2 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -74,6 +74,7 @@
 069 rw udf auto quick
 070 attr udf auto quick stress
 071 auto quick prealloc
+072 auto quick quota
 074 rw udf auto
 075 rw udf auto quick
 076 metadata rw udf auto quick stress
-- 
2.1.0

Re: [PATCH 2/9 RESEND] generic: test quota handling on remount ro failure

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:51PM +0800, Eryu Guan wrote:
> Remount ro should not turn qouta off unconditionally, even remount ro
> failed, also kernel should not oops on the next succeeded remount ro.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  tests/generic/072     | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/072.out |  3 +++
>  tests/generic/group   |  1 +
>  3 files changed, 73 insertions(+)
>  create mode 100755 tests/generic/072
>  create mode 100644 tests/generic/072.out
> 
> diff --git a/tests/generic/072 b/tests/generic/072
> new file mode 100755
> index 0000000..ef40822
> --- /dev/null
> +++ b/tests/generic/072
> @@ -0,0 +1,69 @@
> +#! /bin/bash
> +# FS QA Test No. 072
> +#
> +# Test quota handling on remount ro failure
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/quota
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_test
> +_require_scratch
> +_require_quota
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount "-o usrquota,grpquota"
> +
> +quotacheck -ug $SCRATCH_MNT >/dev/null 2>&1
> +quotaon $SCRATCH_MNT >/dev/null 2>&1
> +

What's the purpose of the above two commands? The quotacheck seems to
fail (masked by the redirect to /dev/null) and shouldn't quota be on by
virtue of the mount options?

> +# first remount ro with a bad option, a failed remount ro should not disable quota
> +_scratch_mount "-o remount,ro,nosuchopt" >>$seqres.full 2>&1
> +quotaon -p $SCRATCH_MNT | _filter_scratch
> +# second remount should succeed, no oops or hang expected
> +_scratch_mount "-o remount,ro"
> +

FWIW, when I '-o remount,ro,nosuchopt' on XFS the remount ro actually
succeeds and the bad option is dropped. Does that differ from other fs
behavior? If not, I wonder whether the second remount,ro is necessary
here as well.

Brian

> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/072.out b/tests/generic/072.out
> new file mode 100644
> index 0000000..ff0e180
> --- /dev/null
> +++ b/tests/generic/072.out
> @@ -0,0 +1,3 @@
> +QA output created by 072
> +group quota on SCRATCH_MNT (SCRATCH_DEV) is on
> +user quota on SCRATCH_MNT (SCRATCH_DEV) is on
> diff --git a/tests/generic/group b/tests/generic/group
> index 03ad7b8..c0210d2 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -74,6 +74,7 @@
>  069 rw udf auto quick
>  070 attr udf auto quick stress
>  071 auto quick prealloc
> +072 auto quick quota
>  074 rw udf auto
>  075 rw udf auto quick
>  076 metadata rw udf auto quick stress
> -- 
> 2.1.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 2/9 RESEND] generic: test quota handling on remount ro failure

[Eryu Guan]

On Wed, Mar 25, 2015 at 03:32:27PM -0400, Brian Foster wrote:
> On Fri, Mar 20, 2015 at 06:16:51PM +0800, Eryu Guan wrote:
> > Remount ro should not turn qouta off unconditionally, even remount ro
> > failed, also kernel should not oops on the next succeeded remount ro.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> >  tests/generic/072     | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/072.out |  3 +++
> >  tests/generic/group   |  1 +
> >  3 files changed, 73 insertions(+)
> >  create mode 100755 tests/generic/072
> >  create mode 100644 tests/generic/072.out
> > 
> > diff --git a/tests/generic/072 b/tests/generic/072
> > new file mode 100755
> > index 0000000..ef40822
> > --- /dev/null
> > +++ b/tests/generic/072
> > @@ -0,0 +1,69 @@
> > +#! /bin/bash
> > +# FS QA Test No. 072
> > +#
> > +# Test quota handling on remount ro failure
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/quota
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_test
> > +_require_scratch
> > +_require_quota
> > +
> > +rm -f $seqres.full
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_scratch_mount "-o usrquota,grpquota"
> > +
> > +quotacheck -ug $SCRATCH_MNT >/dev/null 2>&1
> > +quotaon $SCRATCH_MNT >/dev/null 2>&1
> > +
> 
> What's the purpose of the above two commands? The quotacheck seems to
> fail (masked by the redirect to /dev/null) and shouldn't quota be on by
> virtue of the mount options?

extN needs quotacheck and quotaon first to enable quota, xfs doesn't
need these commands and as you said quotacheck also fails on xfs. So
output of these commands are redirected to /dev/null

> 
> > +# first remount ro with a bad option, a failed remount ro should not disable quota
> > +_scratch_mount "-o remount,ro,nosuchopt" >>$seqres.full 2>&1
> > +quotaon -p $SCRATCH_MNT | _filter_scratch
> > +# second remount should succeed, no oops or hang expected
> > +_scratch_mount "-o remount,ro"
> > +
> 
> FWIW, when I '-o remount,ro,nosuchopt' on XFS the remount ro actually
> succeeds and the bad option is dropped. Does that differ from other fs
> behavior? If not, I wonder whether the second remount,ro is necessary
> here as well.

The first remount (with unsupported option) fails on extN.

Maybe this test should be in shared, and only support extN?

Thanks,
Eryu
> 
> Brian
> 
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/072.out b/tests/generic/072.out
> > new file mode 100644
> > index 0000000..ff0e180
> > --- /dev/null
> > +++ b/tests/generic/072.out
> > @@ -0,0 +1,3 @@
> > +QA output created by 072
> > +group quota on SCRATCH_MNT (SCRATCH_DEV) is on
> > +user quota on SCRATCH_MNT (SCRATCH_DEV) is on
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 03ad7b8..c0210d2 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -74,6 +74,7 @@
> >  069 rw udf auto quick
> >  070 attr udf auto quick stress
> >  071 auto quick prealloc
> > +072 auto quick quota
> >  074 rw udf auto
> >  075 rw udf auto quick
> >  076 metadata rw udf auto quick stress
> > -- 
> > 2.1.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
> --
> 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 2/9 RESEND] generic: test quota handling on remount ro failure

[Brian Foster]

On Wed, Apr 01, 2015 at 02:06:30PM +0800, Eryu Guan wrote:
> On Wed, Mar 25, 2015 at 03:32:27PM -0400, Brian Foster wrote:
> > On Fri, Mar 20, 2015 at 06:16:51PM +0800, Eryu Guan wrote:
> > > Remount ro should not turn qouta off unconditionally, even remount ro
> > > failed, also kernel should not oops on the next succeeded remount ro.
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > >  tests/generic/072     | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/072.out |  3 +++
> > >  tests/generic/group   |  1 +
> > >  3 files changed, 73 insertions(+)
> > >  create mode 100755 tests/generic/072
> > >  create mode 100644 tests/generic/072.out
> > > 
> > > diff --git a/tests/generic/072 b/tests/generic/072
> > > new file mode 100755
> > > index 0000000..ef40822
> > > --- /dev/null
> > > +++ b/tests/generic/072
> > > @@ -0,0 +1,69 @@
> > > +#! /bin/bash
> > > +# FS QA Test No. 072
> > > +#
> > > +# Test quota handling on remount ro failure
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +# 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"
> > > +
> > > +here=`pwd`
> > > +tmp=/tmp/$$
> > > +status=1	# failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -f $tmp.*
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +. ./common/quota
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
> > > +_supported_os Linux
> > > +_require_test
> > > +_require_scratch
> > > +_require_quota
> > > +
> > > +rm -f $seqres.full
> > > +
> > > +_scratch_mkfs >>$seqres.full 2>&1
> > > +_scratch_mount "-o usrquota,grpquota"
> > > +
> > > +quotacheck -ug $SCRATCH_MNT >/dev/null 2>&1
> > > +quotaon $SCRATCH_MNT >/dev/null 2>&1
> > > +
> > 
> > What's the purpose of the above two commands? The quotacheck seems to
> > fail (masked by the redirect to /dev/null) and shouldn't quota be on by
> > virtue of the mount options?
> 
> extN needs quotacheck and quotaon first to enable quota, xfs doesn't
> need these commands and as you said quotacheck also fails on xfs. So
> output of these commands are redirected to /dev/null
> 

Ok, comment please. :) We should point out that these commands are not
necessary for all filesystems and could even fail, and therefore the
output is thrown away and exit status is not important. We print the
quota status at some point below, and ultimately that's all we care
about.

> > 
> > > +# first remount ro with a bad option, a failed remount ro should not disable quota
> > > +_scratch_mount "-o remount,ro,nosuchopt" >>$seqres.full 2>&1
> > > +quotaon -p $SCRATCH_MNT | _filter_scratch
> > > +# second remount should succeed, no oops or hang expected
> > > +_scratch_mount "-o remount,ro"
> > > +
> > 
> > FWIW, when I '-o remount,ro,nosuchopt' on XFS the remount ro actually
> > succeeds and the bad option is dropped. Does that differ from other fs
> > behavior? If not, I wonder whether the second remount,ro is necessary
> > here as well.
> 
> The first remount (with unsupported option) fails on extN.
> 
> Maybe this test should be in shared, and only support extN?
> 

Perhaps the test is less interesting for xfs, but I don't think it
necessarily precludes running it there. That behavior could change in
the future. It's just that the test is ambiguous and could lead to
confusion if somebody happens to look at this sometime down the road and
doesn't have that context from author.

We should have a comment for the first remount that explains the remount
may or may not return failure depending on fs, but regardless, quota
should remain enabled. We should probably have '|| _fail "failed to
remount ro"' for the final remount to indicate that this should always
succeed (and not rely on whether _scratch_mount fails the test as a side
effect).

Brian

> Thanks,
> Eryu
> > 
> > Brian
> > 
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/072.out b/tests/generic/072.out
> > > new file mode 100644
> > > index 0000000..ff0e180
> > > --- /dev/null
> > > +++ b/tests/generic/072.out
> > > @@ -0,0 +1,3 @@
> > > +QA output created by 072
> > > +group quota on SCRATCH_MNT (SCRATCH_DEV) is on
> > > +user quota on SCRATCH_MNT (SCRATCH_DEV) is on
> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index 03ad7b8..c0210d2 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -74,6 +74,7 @@
> > >  069 rw udf auto quick
> > >  070 attr udf auto quick stress
> > >  071 auto quick prealloc
> > > +072 auto quick quota
> > >  074 rw udf auto
> > >  075 rw udf auto quick
> > >  076 metadata rw udf auto quick stress
> > > -- 
> > > 2.1.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
> > --
> > 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
> --
> 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 3/9 RESEND] generic: test hardlink to unlinked file

[Eryu Guan]

Kernel commit

aae8a97 fs: Don't allow to create hardlink for deleted file

disabled hardlink to unlinked file.

Test the race between link and unlink, which could end up adding link
count to an unlinked file and leading to fs corruption on xfs.

Test case was originally written by Eric Sandeen.

Cc: Eric Sandeen <esandeen@redhat.com>
Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/generic/073     | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/073.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 102 insertions(+)
 create mode 100755 tests/generic/073
 create mode 100644 tests/generic/073.out

diff --git a/tests/generic/073 b/tests/generic/073
new file mode 100755
index 0000000..6a5261c
--- /dev/null
+++ b/tests/generic/073
@@ -0,0 +1,99 @@
+#! /bin/bash
+# FS QA Test No. 073
+#
+# Test hardlink to unlinked file.
+#
+# Regression test for commit:
+# aae8a97 fs: Don't allow to create hardlink for deleted file
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_test
+
+link_unlink_storm()
+{
+	local src=$1
+	local target=$2
+
+	while true; do
+		ln -f $src $target >/dev/null 2>&1
+		rm -f $target >/dev/null 2>&1
+	done
+}
+
+rm -f $seqres.full
+nr_cpu=`$here/src/feature -o`
+echo "Silence is golden"
+
+# create, open & unlinked files so unlinked inode list is not empty
+for i in `seq 1 64`; do
+	testfile=$TEST_DIR/$seq.unlinked.$i
+	touch $testfile
+	tail -f $testfile &
+	tail_pids="$tail_pids $!"
+	rm -f $testfile
+done
+
+# start link/unlink storm
+src=$TEST_DIR/$seq.target
+touch $testfile
+for i in `seq 1 $nr_cpu`; do
+	target=$TEST_DIR/$seq.target.link.$i
+	link_unlink_storm $src $target &
+	link_pids="$link_pids $!"
+done
+
+# remove & re-create target to race with link/unlink
+while true; do
+	rm -f $src
+	touch $src
+done &
+sleep 5
+kill $! >/dev/null 2>&1
+
+kill $tail_pids $link_pids >/dev/null 2>&1
+wait $tail_pids $link_pids
+
+# all done, no oops/hang expected, _check_filesystems checks TEST_DEV after test
+status=0
+exit
diff --git a/tests/generic/073.out b/tests/generic/073.out
new file mode 100644
index 0000000..d107704
--- /dev/null
+++ b/tests/generic/073.out
@@ -0,0 +1,2 @@
+QA output created by 073
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index c0210d2..ee5b642 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -75,6 +75,7 @@
 070 attr udf auto quick stress
 071 auto quick prealloc
 072 auto quick quota
+073 auto metadata
 074 rw udf auto
 075 rw udf auto quick
 076 metadata rw udf auto quick stress
-- 
2.1.0

Re: [PATCH 3/9 RESEND] generic: test hardlink to unlinked file

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:52PM +0800, Eryu Guan wrote:
> Kernel commit
> 
> aae8a97 fs: Don't allow to create hardlink for deleted file
> 
> disabled hardlink to unlinked file.
> 
> Test the race between link and unlink, which could end up adding link
> count to an unlinked file and leading to fs corruption on xfs.
> 
> Test case was originally written by Eric Sandeen.
> 
> Cc: Eric Sandeen <esandeen@redhat.com>
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  tests/generic/073     | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/073.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 102 insertions(+)
>  create mode 100755 tests/generic/073
>  create mode 100644 tests/generic/073.out
> 
> diff --git a/tests/generic/073 b/tests/generic/073
> new file mode 100755
> index 0000000..6a5261c
> --- /dev/null
> +++ b/tests/generic/073
> @@ -0,0 +1,99 @@
> +#! /bin/bash
> +# FS QA Test No. 073
> +#
> +# Test hardlink to unlinked file.
> +#
> +# Regression test for commit:
> +# aae8a97 fs: Don't allow to create hardlink for deleted file
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_test
> +
> +link_unlink_storm()
> +{
> +	local src=$1
> +	local target=$2
> +
> +	while true; do
> +		ln -f $src $target >/dev/null 2>&1
> +		rm -f $target >/dev/null 2>&1
> +	done
> +}

This seems slightly different from the original reproducer in that it
recreates the same link and unlinks/recreates the $src entry many times
as well. That seems Ok, but out of curiousity, is this test confirmed to
reproduce the original problem?

> +
> +rm -f $seqres.full
> +nr_cpu=`$here/src/feature -o`
> +echo "Silence is golden"
> +
> +# create, open & unlinked files so unlinked inode list is not empty
> +for i in `seq 1 64`; do

		   $nr_cpu

> +	testfile=$TEST_DIR/$seq.unlinked.$i
> +	touch $testfile
> +	tail -f $testfile &
> +	tail_pids="$tail_pids $!"
> +	rm -f $testfile

I get some file not found errors from 'tail' when I run this test. They
go away if I add a sleep after the 'tail -f ... &,' so I suspect the rm
-f could be racing with the file removal.

That might be unnecessarily slow, however. Perhaps create and tail all
of the files in one loop, sleep for a bit, then remove them?

> +done
> +
> +# start link/unlink storm
> +src=$TEST_DIR/$seq.target
> +touch $testfile

touch $src ?

> +for i in `seq 1 $nr_cpu`; do
> +	target=$TEST_DIR/$seq.target.link.$i
> +	link_unlink_storm $src $target &
> +	link_pids="$link_pids $!"
> +done
> +
> +# remove & re-create target to race with link/unlink
> +while true; do
> +	rm -f $src
> +	touch $src
> +done &
> +sleep 5
> +kill $! >/dev/null 2>&1
> +
> +kill $tail_pids $link_pids >/dev/null 2>&1
> +wait $tail_pids $link_pids
> +
> +# all done, no oops/hang expected, _check_filesystems checks TEST_DEV after test
> +status=0
> +exit
> diff --git a/tests/generic/073.out b/tests/generic/073.out
> new file mode 100644
> index 0000000..d107704
> --- /dev/null
> +++ b/tests/generic/073.out
> @@ -0,0 +1,2 @@
> +QA output created by 073
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index c0210d2..ee5b642 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -75,6 +75,7 @@
>  070 attr udf auto quick stress
>  071 auto quick prealloc
>  072 auto quick quota
> +073 auto metadata

quick seems fine as well.

Brian

>  074 rw udf auto
>  075 rw udf auto quick
>  076 metadata rw udf auto quick stress
> -- 
> 2.1.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 3/9 RESEND] generic: test hardlink to unlinked file

[Eryu Guan]

On Wed, Mar 25, 2015 at 03:32:36PM -0400, Brian Foster wrote:
> On Fri, Mar 20, 2015 at 06:16:52PM +0800, Eryu Guan wrote:
> > Kernel commit
> > 
> > aae8a97 fs: Don't allow to create hardlink for deleted file
> > 
> > disabled hardlink to unlinked file.
> > 
> > Test the race between link and unlink, which could end up adding link
> > count to an unlinked file and leading to fs corruption on xfs.
> > 
> > Test case was originally written by Eric Sandeen.
> > 
> > Cc: Eric Sandeen <esandeen@redhat.com>
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> >  tests/generic/073     | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/073.out |  2 ++
> >  tests/generic/group   |  1 +
> >  3 files changed, 102 insertions(+)
> >  create mode 100755 tests/generic/073
> >  create mode 100644 tests/generic/073.out
> > 
> > diff --git a/tests/generic/073 b/tests/generic/073
> > new file mode 100755
> > index 0000000..6a5261c
> > --- /dev/null
> > +++ b/tests/generic/073
> > @@ -0,0 +1,99 @@
> > +#! /bin/bash
> > +# FS QA Test No. 073
> > +#
> > +# Test hardlink to unlinked file.
> > +#
> > +# Regression test for commit:
> > +# aae8a97 fs: Don't allow to create hardlink for deleted file
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_test
> > +
> > +link_unlink_storm()
> > +{
> > +	local src=$1
> > +	local target=$2
> > +
> > +	while true; do
> > +		ln -f $src $target >/dev/null 2>&1
> > +		rm -f $target >/dev/null 2>&1
> > +	done
> > +}
> 
> This seems slightly different from the original reproducer in that it
> recreates the same link and unlinks/recreates the $src entry many times
> as well. That seems Ok, but out of curiousity, is this test confirmed to
> reproduce the original problem?

The answer seems to be no..

> 
> > +
> > +rm -f $seqres.full
> > +nr_cpu=`$here/src/feature -o`
> > +echo "Silence is golden"
> > +
> > +# create, open & unlinked files so unlinked inode list is not empty
> > +for i in `seq 1 64`; do
> 
> 		   $nr_cpu
> 

Yes, typo here

> > +	testfile=$TEST_DIR/$seq.unlinked.$i
> > +	touch $testfile
> > +	tail -f $testfile &
> > +	tail_pids="$tail_pids $!"
> > +	rm -f $testfile
> 
> I get some file not found errors from 'tail' when I run this test. They
> go away if I add a sleep after the 'tail -f ... &,' so I suspect the rm
> -f could be racing with the file removal.
> 
> That might be unnecessarily slow, however. Perhaps create and tail all
> of the files in one loop, sleep for a bit, then remove them?

This case has more issues than I thought, I haven't seen such errors in
the test run before posting patch, but now I see, that's weird.

I think I should have just followed Eric's orginal reproducer (with a c
program to do the link/unlink storm).

Sorry for the premature post, and thanks for your review!

Eryu
> 
> > +done
> > +
> > +# start link/unlink storm
> > +src=$TEST_DIR/$seq.target
> > +touch $testfile
> 
> touch $src ?
> 
> > +for i in `seq 1 $nr_cpu`; do
> > +	target=$TEST_DIR/$seq.target.link.$i
> > +	link_unlink_storm $src $target &
> > +	link_pids="$link_pids $!"
> > +done
> > +
> > +# remove & re-create target to race with link/unlink
> > +while true; do
> > +	rm -f $src
> > +	touch $src
> > +done &
> > +sleep 5
> > +kill $! >/dev/null 2>&1
> > +
> > +kill $tail_pids $link_pids >/dev/null 2>&1
> > +wait $tail_pids $link_pids
> > +
> > +# all done, no oops/hang expected, _check_filesystems checks TEST_DEV after test
> > +status=0
> > +exit
> > diff --git a/tests/generic/073.out b/tests/generic/073.out
> > new file mode 100644
> > index 0000000..d107704
> > --- /dev/null
> > +++ b/tests/generic/073.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 073
> > +Silence is golden
> > diff --git a/tests/generic/group b/tests/generic/group
> > index c0210d2..ee5b642 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -75,6 +75,7 @@
> >  070 attr udf auto quick stress
> >  071 auto quick prealloc
> >  072 auto quick quota
> > +073 auto metadata
> 
> quick seems fine as well.
> 
> Brian
> 
> >  074 rw udf auto
> >  075 rw udf auto quick
> >  076 metadata rw udf auto quick stress
> > -- 
> > 2.1.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 v2 3/9] generic: test hardlink to unlinked file

[Eryu Guan]

Kernel commit

aae8a97 fs: Don't allow to create hardlink for deleted file

disabled hardlink to unlinked file.

Test the race between link and unlink, which could end up adding link
count to an unlinked file and leading to fs corruption on xfs.

Test case was originally written by Eric Sandeen.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
v2:
- create different link file in each iteration in link_unlink_storm(), which
  is confirmed to be able to reproduce the original corruption
- remove all newly created & tail'ed files at last, out of the loop, to avoid
  tail & rm race
- iterate $nr_cpu times to do the link_unlink_storm, not a fixed number
- add to quick group

 tests/generic/073     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/073.out |   2 +
 tests/generic/group   |   1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/generic/073
 create mode 100644 tests/generic/073.out

diff --git a/tests/generic/073 b/tests/generic/073
new file mode 100755
index 0000000..6fd8c89
--- /dev/null
+++ b/tests/generic/073
@@ -0,0 +1,102 @@
+#! /bin/bash
+# FS QA Test No. 073
+#
+# Test hardlink to unlinked file.
+#
+# Regression test for commit:
+# aae8a97 fs: Don't allow to create hardlink for deleted file
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+link_unlink_storm()
+{
+	local src=$1
+	local target=$2
+	local i=0
+
+	while true; do
+		ln -f $src $target.$i >/dev/null 2>&1
+		rm -f $target.$i >/dev/null 2>&1
+		let i=i+1
+	done
+}
+
+rm -f $seqres.full
+nr_cpu=`$here/src/feature -o`
+echo "Silence is golden"
+
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
+# create, open & unlinked files so unlinked inode list is not empty
+for i in `seq 1 $nr_cpu`; do
+	testfile=$SCRATCH_MNT/$seq.unlinked.$i
+	touch $testfile
+	tail -f $testfile &
+	tail_pids="$tail_pids $!"
+done
+rm -f $SCRATCH_MNT/$seq.unlinked.*
+
+# start link/unlink storm
+src=$SCRATCH_MNT/$seq.target
+touch $src
+for i in `seq 1 $nr_cpu`; do
+	target=$SCRATCH_MNT/$seq.target.link.$i
+	link_unlink_storm $src $target &
+	link_pids="$link_pids $!"
+done
+
+# remove & re-create target to race with link/unlink
+while true; do
+	rm -f $src
+	touch $src
+done &
+sleep 5
+kill $! >/dev/null 2>&1
+
+kill $tail_pids $link_pids >/dev/null 2>&1
+wait $tail_pids $link_pids
+
+# all done, no oops/hang expected, _check_filesystems checks SCRATCH_DEV after test
+status=0
+exit
diff --git a/tests/generic/073.out b/tests/generic/073.out
new file mode 100644
index 0000000..d107704
--- /dev/null
+++ b/tests/generic/073.out
@@ -0,0 +1,2 @@
+QA output created by 073
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index c0210d2..ee5b642 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -75,6 +75,7 @@
 070 attr udf auto quick stress
 071 auto quick prealloc
 072 auto quick quota
+073 auto metadata quick
 074 rw udf auto
 075 rw udf auto quick
 076 metadata rw udf auto quick stress
-- 
2.1.0

Re: [PATCH v2 3/9] generic: test hardlink to unlinked file

[Brian Foster]

On Wed, Apr 01, 2015 at 09:48:40PM +0800, Eryu Guan wrote:
> Kernel commit
> 
> aae8a97 fs: Don't allow to create hardlink for deleted file
> 
> disabled hardlink to unlinked file.
> 
> Test the race between link and unlink, which could end up adding link
> count to an unlinked file and leading to fs corruption on xfs.
> 
> Test case was originally written by Eric Sandeen.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> v2:
> - create different link file in each iteration in link_unlink_storm(), which
>   is confirmed to be able to reproduce the original corruption
> - remove all newly created & tail'ed files at last, out of the loop, to avoid
>   tail & rm race
> - iterate $nr_cpu times to do the link_unlink_storm, not a fixed number
> - add to quick group
> 
>  tests/generic/073     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/073.out |   2 +
>  tests/generic/group   |   1 +
>  3 files changed, 105 insertions(+)
>  create mode 100755 tests/generic/073
>  create mode 100644 tests/generic/073.out
> 
> diff --git a/tests/generic/073 b/tests/generic/073
> new file mode 100755
> index 0000000..6fd8c89
> --- /dev/null
> +++ b/tests/generic/073
> @@ -0,0 +1,102 @@
> +#! /bin/bash
> +# FS QA Test No. 073
> +#
> +# Test hardlink to unlinked file.
> +#
> +# Regression test for commit:
> +# aae8a97 fs: Don't allow to create hardlink for deleted file
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +
> +link_unlink_storm()
> +{
> +	local src=$1
> +	local target=$2
> +	local i=0
> +
> +	while true; do
> +		ln -f $src $target.$i >/dev/null 2>&1
> +		rm -f $target.$i >/dev/null 2>&1
> +		let i=i+1
> +	done
> +}
> +
> +rm -f $seqres.full
> +nr_cpu=`$here/src/feature -o`
> +echo "Silence is golden"
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +# create, open & unlinked files so unlinked inode list is not empty
> +for i in `seq 1 $nr_cpu`; do
> +	testfile=$SCRATCH_MNT/$seq.unlinked.$i
> +	touch $testfile
> +	tail -f $testfile &
> +	tail_pids="$tail_pids $!"
> +done
> +rm -f $SCRATCH_MNT/$seq.unlinked.*
> +

Still fails:

generic/073      - output mismatch (see /root/xfstests-dev/results//generic/073.out.bad)
    --- tests/generic/073.out   2015-04-01 12:45:25.633900350 -0400
    +++ /root/xfstests-dev/results//generic/073.out.bad 2015-04-01 12:46:39.129900350 -0400
    @@ -1,2 +1,10 @@
     QA output created by 073
     Silence is golden
    +tail: cannot open '/mnt/scratch/073.unlinked.2' for reading: No such file or directory
    +tail: cannot open '/mnt/scratch/073.unlinked.4' for readingtail: : No such file or directory
    +cannot open '/mnt/scratch/073.unlinked.3' for reading: No such file or directory
    +tail: cannot open '/mnt/scratch/073.unlinked.1' for reading: No such file or directory
    +tail: no files remaining
    ...
    (Run 'diff -u tests/generic/073.out /root/xfstests-dev/results//generic/073.out.bad'  to see the entire diff)

We need a 'sleep 1' between the loop that runs the tail instances and
actually removing the files to trigger the background tasks to run and
open the files before they are removed.

The rest looks pretty good to me.

Brian

> +# start link/unlink storm
> +src=$SCRATCH_MNT/$seq.target
> +touch $src
> +for i in `seq 1 $nr_cpu`; do
> +	target=$SCRATCH_MNT/$seq.target.link.$i
> +	link_unlink_storm $src $target &
> +	link_pids="$link_pids $!"
> +done
> +
> +# remove & re-create target to race with link/unlink
> +while true; do
> +	rm -f $src
> +	touch $src
> +done &
> +sleep 5
> +kill $! >/dev/null 2>&1
> +
> +kill $tail_pids $link_pids >/dev/null 2>&1
> +wait $tail_pids $link_pids
> +
> +# all done, no oops/hang expected, _check_filesystems checks SCRATCH_DEV after test
> +status=0
> +exit
> diff --git a/tests/generic/073.out b/tests/generic/073.out
> new file mode 100644
> index 0000000..d107704
> --- /dev/null
> +++ b/tests/generic/073.out
> @@ -0,0 +1,2 @@
> +QA output created by 073
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index c0210d2..ee5b642 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -75,6 +75,7 @@
>  070 attr udf auto quick stress
>  071 auto quick prealloc
>  072 auto quick quota
> +073 auto metadata quick
>  074 rw udf auto
>  075 rw udf auto quick
>  076 metadata rw udf auto quick stress
> -- 
> 2.1.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 v2 3/9] generic: test hardlink to unlinked file

[Eryu Guan]

On Wed, Apr 01, 2015 at 01:54:47PM -0400, Brian Foster wrote:
> On Wed, Apr 01, 2015 at 09:48:40PM +0800, Eryu Guan wrote:
> > Kernel commit
> > 
> > aae8a97 fs: Don't allow to create hardlink for deleted file
> > 
> > disabled hardlink to unlinked file.
> > 
> > Test the race between link and unlink, which could end up adding link
> > count to an unlinked file and leading to fs corruption on xfs.
> > 
> > Test case was originally written by Eric Sandeen.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > v2:
> > - create different link file in each iteration in link_unlink_storm(), which
> >   is confirmed to be able to reproduce the original corruption
> > - remove all newly created & tail'ed files at last, out of the loop, to avoid
> >   tail & rm race
> > - iterate $nr_cpu times to do the link_unlink_storm, not a fixed number
> > - add to quick group
> > 
> >  tests/generic/073     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/073.out |   2 +
> >  tests/generic/group   |   1 +
> >  3 files changed, 105 insertions(+)
> >  create mode 100755 tests/generic/073
> >  create mode 100644 tests/generic/073.out
> > 
> > diff --git a/tests/generic/073 b/tests/generic/073
> > new file mode 100755
> > index 0000000..6fd8c89
> > --- /dev/null
> > +++ b/tests/generic/073
> > @@ -0,0 +1,102 @@
> > +#! /bin/bash
> > +# FS QA Test No. 073
> > +#
> > +# Test hardlink to unlinked file.
> > +#
> > +# Regression test for commit:
> > +# aae8a97 fs: Don't allow to create hardlink for deleted file
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_scratch
> > +
> > +link_unlink_storm()
> > +{
> > +	local src=$1
> > +	local target=$2
> > +	local i=0
> > +
> > +	while true; do
> > +		ln -f $src $target.$i >/dev/null 2>&1
> > +		rm -f $target.$i >/dev/null 2>&1
> > +		let i=i+1
> > +	done
> > +}
> > +
> > +rm -f $seqres.full
> > +nr_cpu=`$here/src/feature -o`
> > +echo "Silence is golden"
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_scratch_mount
> > +
> > +# create, open & unlinked files so unlinked inode list is not empty
> > +for i in `seq 1 $nr_cpu`; do
> > +	testfile=$SCRATCH_MNT/$seq.unlinked.$i
> > +	touch $testfile
> > +	tail -f $testfile &
> > +	tail_pids="$tail_pids $!"
> > +done
> > +rm -f $SCRATCH_MNT/$seq.unlinked.*
> > +
> 
> Still fails:
> 
> generic/073      - output mismatch (see /root/xfstests-dev/results//generic/073.out.bad)
>     --- tests/generic/073.out   2015-04-01 12:45:25.633900350 -0400
>     +++ /root/xfstests-dev/results//generic/073.out.bad 2015-04-01 12:46:39.129900350 -0400
>     @@ -1,2 +1,10 @@
>      QA output created by 073
>      Silence is golden
>     +tail: cannot open '/mnt/scratch/073.unlinked.2' for reading: No such file or directory
>     +tail: cannot open '/mnt/scratch/073.unlinked.4' for readingtail: : No such file or directory
>     +cannot open '/mnt/scratch/073.unlinked.3' for reading: No such file or directory
>     +tail: cannot open '/mnt/scratch/073.unlinked.1' for reading: No such file or directory
>     +tail: no files remaining
>     ...
>     (Run 'diff -u tests/generic/073.out /root/xfstests-dev/results//generic/073.out.bad'  to see the entire diff)
> 
> We need a 'sleep 1' between the loop that runs the tail instances and
> actually removing the files to trigger the background tasks to run and
> open the files before they are removed.

This version passed on the host where originally I can see it fail.
Seems it doesn't fail every time.

You mean adding 'sleep 1' before removing all the files?

	for ; do
	done
	sleep 1
	rm -f $SCRATCH_MNT/$seq.unlinked.*

And what's your test vm setup? I want to see if I can reproduce the
failure myself. I'm testing on a vm with 4 vcpus and 8G mem, running
RHEL6.

Thanks for your review!

Eryu
> 
> The rest looks pretty good to me.
> 
> Brian
> 
> > +# start link/unlink storm
> > +src=$SCRATCH_MNT/$seq.target
> > +touch $src
> > +for i in `seq 1 $nr_cpu`; do
> > +	target=$SCRATCH_MNT/$seq.target.link.$i
> > +	link_unlink_storm $src $target &
> > +	link_pids="$link_pids $!"
> > +done
> > +
> > +# remove & re-create target to race with link/unlink
> > +while true; do
> > +	rm -f $src
> > +	touch $src
> > +done &
> > +sleep 5
> > +kill $! >/dev/null 2>&1
> > +
> > +kill $tail_pids $link_pids >/dev/null 2>&1
> > +wait $tail_pids $link_pids
> > +
> > +# all done, no oops/hang expected, _check_filesystems checks SCRATCH_DEV after test
> > +status=0
> > +exit
> > diff --git a/tests/generic/073.out b/tests/generic/073.out
> > new file mode 100644
> > index 0000000..d107704
> > --- /dev/null
> > +++ b/tests/generic/073.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 073
> > +Silence is golden
> > diff --git a/tests/generic/group b/tests/generic/group
> > index c0210d2..ee5b642 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -75,6 +75,7 @@
> >  070 attr udf auto quick stress
> >  071 auto quick prealloc
> >  072 auto quick quota
> > +073 auto metadata quick
> >  074 rw udf auto
> >  075 rw udf auto quick
> >  076 metadata rw udf auto quick stress
> > -- 
> > 2.1.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 v2 3/9] generic: test hardlink to unlinked file

[Brian Foster]

On Thu, Apr 02, 2015 at 10:59:23AM +0800, Eryu Guan wrote:
> On Wed, Apr 01, 2015 at 01:54:47PM -0400, Brian Foster wrote:
> > On Wed, Apr 01, 2015 at 09:48:40PM +0800, Eryu Guan wrote:
> > > Kernel commit
> > > 
> > > aae8a97 fs: Don't allow to create hardlink for deleted file
> > > 
> > > disabled hardlink to unlinked file.
> > > 
> > > Test the race between link and unlink, which could end up adding link
> > > count to an unlinked file and leading to fs corruption on xfs.
> > > 
> > > Test case was originally written by Eric Sandeen.
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > > v2:
> > > - create different link file in each iteration in link_unlink_storm(), which
> > >   is confirmed to be able to reproduce the original corruption
> > > - remove all newly created & tail'ed files at last, out of the loop, to avoid
> > >   tail & rm race
> > > - iterate $nr_cpu times to do the link_unlink_storm, not a fixed number
> > > - add to quick group
> > > 
> > >  tests/generic/073     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/073.out |   2 +
> > >  tests/generic/group   |   1 +
> > >  3 files changed, 105 insertions(+)
> > >  create mode 100755 tests/generic/073
> > >  create mode 100644 tests/generic/073.out
> > > 
> > > diff --git a/tests/generic/073 b/tests/generic/073
> > > new file mode 100755
> > > index 0000000..6fd8c89
> > > --- /dev/null
> > > +++ b/tests/generic/073
> > > @@ -0,0 +1,102 @@
> > > +#! /bin/bash
> > > +# FS QA Test No. 073
> > > +#
> > > +# Test hardlink to unlinked file.
> > > +#
> > > +# Regression test for commit:
> > > +# aae8a97 fs: Don't allow to create hardlink for deleted file
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +# 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"
> > > +
> > > +here=`pwd`
> > > +tmp=/tmp/$$
> > > +status=1	# failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -f $tmp.*
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +_supported_fs generic
> > > +_supported_os Linux
> > > +_require_scratch
> > > +
> > > +link_unlink_storm()
> > > +{
> > > +	local src=$1
> > > +	local target=$2
> > > +	local i=0
> > > +
> > > +	while true; do
> > > +		ln -f $src $target.$i >/dev/null 2>&1
> > > +		rm -f $target.$i >/dev/null 2>&1
> > > +		let i=i+1
> > > +	done
> > > +}
> > > +
> > > +rm -f $seqres.full
> > > +nr_cpu=`$here/src/feature -o`
> > > +echo "Silence is golden"
> > > +
> > > +_scratch_mkfs >>$seqres.full 2>&1
> > > +_scratch_mount
> > > +
> > > +# create, open & unlinked files so unlinked inode list is not empty
> > > +for i in `seq 1 $nr_cpu`; do
> > > +	testfile=$SCRATCH_MNT/$seq.unlinked.$i
> > > +	touch $testfile
> > > +	tail -f $testfile &
> > > +	tail_pids="$tail_pids $!"
> > > +done
> > > +rm -f $SCRATCH_MNT/$seq.unlinked.*
> > > +
> > 
> > Still fails:
> > 
> > generic/073      - output mismatch (see /root/xfstests-dev/results//generic/073.out.bad)
> >     --- tests/generic/073.out   2015-04-01 12:45:25.633900350 -0400
> >     +++ /root/xfstests-dev/results//generic/073.out.bad 2015-04-01 12:46:39.129900350 -0400
> >     @@ -1,2 +1,10 @@
> >      QA output created by 073
> >      Silence is golden
> >     +tail: cannot open '/mnt/scratch/073.unlinked.2' for reading: No such file or directory
> >     +tail: cannot open '/mnt/scratch/073.unlinked.4' for readingtail: : No such file or directory
> >     +cannot open '/mnt/scratch/073.unlinked.3' for reading: No such file or directory
> >     +tail: cannot open '/mnt/scratch/073.unlinked.1' for reading: No such file or directory
> >     +tail: no files remaining
> >     ...
> >     (Run 'diff -u tests/generic/073.out /root/xfstests-dev/results//generic/073.out.bad'  to see the entire diff)
> > 
> > We need a 'sleep 1' between the loop that runs the tail instances and
> > actually removing the files to trigger the background tasks to run and
> > open the files before they are removed.
> 
> This version passed on the host where originally I can see it fail.
> Seems it doesn't fail every time.
> 
> You mean adding 'sleep 1' before removing all the files?
> 
> 	for ; do
> 	done
> 	sleep 1
> 	rm -f $SCRATCH_MNT/$seq.unlinked.*
> 

Yes... the for loop creates a bunch of background tasks to open the
files we're about to unlink. There's no guarantee those background tasks
actually even begin execution by the time the parent gets to removing
the files. E.g., it's a race, sometimes it might work, sometimes not. A
sleep 1 is probably enough to make sure the parent can't progress until
the children have started.

> And what's your test vm setup? I want to see if I can reproduce the
> failure myself. I'm testing on a vm with 4 vcpus and 8G mem, running
> RHEL6.
> 

My basic testing vms are usually 2-4cpu with 4GB ram running various
operating systems. I'm running Fedora rawhide in this case...

Brian

> Thanks for your review!
> 
> Eryu
> > 
> > The rest looks pretty good to me.
> > 
> > Brian
> > 
> > > +# start link/unlink storm
> > > +src=$SCRATCH_MNT/$seq.target
> > > +touch $src
> > > +for i in `seq 1 $nr_cpu`; do
> > > +	target=$SCRATCH_MNT/$seq.target.link.$i
> > > +	link_unlink_storm $src $target &
> > > +	link_pids="$link_pids $!"
> > > +done
> > > +
> > > +# remove & re-create target to race with link/unlink
> > > +while true; do
> > > +	rm -f $src
> > > +	touch $src
> > > +done &
> > > +sleep 5
> > > +kill $! >/dev/null 2>&1
> > > +
> > > +kill $tail_pids $link_pids >/dev/null 2>&1
> > > +wait $tail_pids $link_pids
> > > +
> > > +# all done, no oops/hang expected, _check_filesystems checks SCRATCH_DEV after test
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/073.out b/tests/generic/073.out
> > > new file mode 100644
> > > index 0000000..d107704
> > > --- /dev/null
> > > +++ b/tests/generic/073.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 073
> > > +Silence is golden
> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index c0210d2..ee5b642 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -75,6 +75,7 @@
> > >  070 attr udf auto quick stress
> > >  071 auto quick prealloc
> > >  072 auto quick quota
> > > +073 auto metadata quick
> > >  074 rw udf auto
> > >  075 rw udf auto quick
> > >  076 metadata rw udf auto quick stress
> > > -- 
> > > 2.1.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
> --
> 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 4/9 RESEND] shared: test truncate orphan inodes when mounting extN

[Eryu Guan]

ext4 should hold i_mutex when truncating orhpan inodes, or a WARNING
would be triggered. This commit fixed this issue.

721e3eb ext4: lock i_mutex when truncating orphan inodes

Though it's an ext4 specific issue, there's no harm to test on ext2/3
too, as debugfs is used to set orphan inode list.

This test is based on a script from Lukas Czerner.

Cc: Lukas Czerner <lczerner@redhat.com>
Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/shared/001     | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/shared/001.out |  2 ++
 tests/shared/group   |  1 +
 3 files changed, 74 insertions(+)
 create mode 100755 tests/shared/001
 create mode 100644 tests/shared/001.out

diff --git a/tests/shared/001 b/tests/shared/001
new file mode 100755
index 0000000..7a4bc68
--- /dev/null
+++ b/tests/shared/001
@@ -0,0 +1,71 @@
+#! /bin/bash
+# FS QA Test No. 005
+#
+# Test truncate orphan inodes when mounting extN.
+# ext4 used to hit WARNING, this commit fixed the issue
+#
+# 721e3eb ext4: lock i_mutex when truncating orphan inodes
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs ext2 ext3 ext4
+_supported_os Linux
+_require_scratch
+
+rm -f $seqres.full
+echo "Silence is golden"
+
+_scratch_mkfs_sized $((16*1024*1024)) >>$seqres.full 2>&1
+_scratch_mount
+
+# create a file and get its inode number, usually it's 12, but to be accurate
+testfile=$SCRATCH_MNT/testfile
+touch $testfile
+inode=`ls -i $testfile | awk '{print $1}'`
+
+# add the inode in orphan inode list
+_scratch_unmount
+debugfs -w -R "set_super_value last_orphan $inode" $SCRATCH_DEV \
+	>>$seqres.full 2>&1
+
+# mount again to truncate orphan inode, _check_dmesg will catch the WARNING
+_scratch_mount
+
+status=0
+exit
diff --git a/tests/shared/001.out b/tests/shared/001.out
new file mode 100644
index 0000000..88678b8
--- /dev/null
+++ b/tests/shared/001.out
@@ -0,0 +1,2 @@
+QA output created by 001
+Silence is golden
diff --git a/tests/shared/group b/tests/shared/group
index 429f2b4..0134f81 100644
--- a/tests/shared/group
+++ b/tests/shared/group
@@ -3,6 +3,7 @@
 # - do not start group names with a digit
 # - comment line before each group is "new" description
 #
+001 auto quick
 006 auto enospc
 032 mkfs auto quick
 051 acl udf auto quick
-- 
2.1.0

[PATCH 5/9 RESEND] generic: test fs freeze/unfreeze and mount/umount race

[Eryu Guan]

Exercise fs freeze/unfreeze and mount/umount race, which could lead to
use-after-free oops.

This commit fixed the issue:
1494583 fix get_active_super()/umount() race

This test case is based on a script from Monakhov Dmitriy.

Cc: Monakhov Dmitriy <dmonakhov@openvz.org>
Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/generic/080     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/080.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 101 insertions(+)
 create mode 100755 tests/generic/080
 create mode 100644 tests/generic/080.out

diff --git a/tests/generic/080 b/tests/generic/080
new file mode 100755
index 0000000..7d9fa5c
--- /dev/null
+++ b/tests/generic/080
@@ -0,0 +1,98 @@
+#! /bin/bash
+# FS QA Test No. 080
+#
+# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
+# use-after-free oops.
+#
+# This commit fixed the issue:
+# 1494583 fix get_active_super()/umount() race
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	cleanup_dmdev
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_block_device $SCRATCH_DEV
+_require_command $DMSETUP_PROG
+_require_freeze
+
+setup_dmdev()
+{
+	table="0 $size_in_sector linear $SCRATCH_DEV 0"
+	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1
+	$DMSETUP_PROG mknodes >/dev/null 2>&1
+}
+
+cleanup_dmdev()
+{
+	# in case it's still suspended and/or mounted
+	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
+	$UMOUNT_PROG $lvdev >/dev/null 2>&1
+
+	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
+	$DMSETUP_PROG mknodes >/dev/null 2>&1
+}
+
+rm -f $seqres.full
+echo "Silence is golden"
+
+size=$((256 * 1024 * 1024))
+size_in_sector=$((size / 512))
+_scratch_mkfs_sized $size >>$seqres.full 2>&1
+
+node=$seq-test
+lvdev=/dev/mapper/$node
+setup_dmdev
+
+for ((i=0; i<100; i++)); do
+	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
+	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
+done &
+pid=$!
+for ((i=0; i<100; i++)); do
+	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
+	$UMOUNT_PROG $lvdev >/dev/null 2>&1
+done &
+pid="$pid $!"
+
+wait $pid
+
+status=0
+exit
diff --git a/tests/generic/080.out b/tests/generic/080.out
new file mode 100644
index 0000000..11a4a1a
--- /dev/null
+++ b/tests/generic/080.out
@@ -0,0 +1,2 @@
+QA output created by 080
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index ee5b642..cf7408c 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -82,6 +82,7 @@
 077 acl attr auto enospc
 078 auto quick metadata
 079 acl attr ioctl metadata auto quick
+080 auto freeze mount
 083 rw auto enospc stress
 088 perms auto quick
 089 metadata auto
-- 
2.1.0

Re: [PATCH 5/9 RESEND] generic: test fs freeze/unfreeze and mount/umount race

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:54PM +0800, Eryu Guan wrote:
> Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> use-after-free oops.
> 
> This commit fixed the issue:
> 1494583 fix get_active_super()/umount() race
> 
> This test case is based on a script from Monakhov Dmitriy.
> 
> Cc: Monakhov Dmitriy <dmonakhov@openvz.org>
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  tests/generic/080     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/080.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 101 insertions(+)
>  create mode 100755 tests/generic/080
>  create mode 100644 tests/generic/080.out
> 
> diff --git a/tests/generic/080 b/tests/generic/080
> new file mode 100755
> index 0000000..7d9fa5c
> --- /dev/null
> +++ b/tests/generic/080
> @@ -0,0 +1,98 @@
> +#! /bin/bash
> +# FS QA Test No. 080
> +#
> +# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> +# use-after-free oops.
> +#
> +# This commit fixed the issue:
> +# 1494583 fix get_active_super()/umount() race
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +	cleanup_dmdev
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +_require_block_device $SCRATCH_DEV
> +_require_command $DMSETUP_PROG
> +_require_freeze
> +
> +setup_dmdev()
> +{
> +	table="0 $size_in_sector linear $SCRATCH_DEV 0"
> +	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1
> +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> +}
> +
> +cleanup_dmdev()
> +{
> +	# in case it's still suspended and/or mounted
> +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> +
> +	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
> +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> +}
> +
> +rm -f $seqres.full
> +echo "Silence is golden"
> +
> +size=$((256 * 1024 * 1024))
> +size_in_sector=$((size / 512))
> +_scratch_mkfs_sized $size >>$seqres.full 2>&1
> +
> +node=$seq-test
> +lvdev=/dev/mapper/$node
> +setup_dmdev
> +
> +for ((i=0; i<100; i++)); do
> +	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
> +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> +done &

Any reason we can't use xfs_freeze/fsfreeze for this test? That seems
more simple (avoids the dm stuff), but I don't know enough about the
original problem to know if that still reproduces it.

Brian

> +pid=$!
> +for ((i=0; i<100; i++)); do
> +	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
> +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> +done &
> +pid="$pid $!"
> +
> +wait $pid
> +
> +status=0
> +exit
> diff --git a/tests/generic/080.out b/tests/generic/080.out
> new file mode 100644
> index 0000000..11a4a1a
> --- /dev/null
> +++ b/tests/generic/080.out
> @@ -0,0 +1,2 @@
> +QA output created by 080
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index ee5b642..cf7408c 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -82,6 +82,7 @@
>  077 acl attr auto enospc
>  078 auto quick metadata
>  079 acl attr ioctl metadata auto quick
> +080 auto freeze mount
>  083 rw auto enospc stress
>  088 perms auto quick
>  089 metadata auto
> -- 
> 2.1.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 5/9 RESEND] generic: test fs freeze/unfreeze and mount/umount race

[Eryu Guan]

On Wed, Mar 25, 2015 at 03:44:29PM -0400, Brian Foster wrote:
> On Fri, Mar 20, 2015 at 06:16:54PM +0800, Eryu Guan wrote:
> > Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > use-after-free oops.
> > 
> > This commit fixed the issue:
> > 1494583 fix get_active_super()/umount() race
> > 
> > This test case is based on a script from Monakhov Dmitriy.
> > 
> > Cc: Monakhov Dmitriy <dmonakhov@openvz.org>
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> >  tests/generic/080     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/080.out |  2 ++
> >  tests/generic/group   |  1 +
> >  3 files changed, 101 insertions(+)
> >  create mode 100755 tests/generic/080
> >  create mode 100644 tests/generic/080.out
> > 
> > diff --git a/tests/generic/080 b/tests/generic/080
> > new file mode 100755
> > index 0000000..7d9fa5c
> > --- /dev/null
> > +++ b/tests/generic/080
> > @@ -0,0 +1,98 @@
> > +#! /bin/bash
> > +# FS QA Test No. 080
> > +#
> > +# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > +# use-after-free oops.
> > +#
> > +# This commit fixed the issue:
> > +# 1494583 fix get_active_super()/umount() race
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +	cleanup_dmdev
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_scratch
> > +_require_block_device $SCRATCH_DEV
> > +_require_command $DMSETUP_PROG
> > +_require_freeze
> > +
> > +setup_dmdev()
> > +{
> > +	table="0 $size_in_sector linear $SCRATCH_DEV 0"
> > +	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1
> > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > +}
> > +
> > +cleanup_dmdev()
> > +{
> > +	# in case it's still suspended and/or mounted
> > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > +
> > +	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
> > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > +}
> > +
> > +rm -f $seqres.full
> > +echo "Silence is golden"
> > +
> > +size=$((256 * 1024 * 1024))
> > +size_in_sector=$((size / 512))
> > +_scratch_mkfs_sized $size >>$seqres.full 2>&1
> > +
> > +node=$seq-test
> > +lvdev=/dev/mapper/$node
> > +setup_dmdev
> > +
> > +for ((i=0; i<100; i++)); do
> > +	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
> > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > +done &
> 
> Any reason we can't use xfs_freeze/fsfreeze for this test? That seems
> more simple (avoids the dm stuff), but I don't know enough about the
> original problem to know if that still reproduces it.

Yes, fsfreeze/unfreeze can reproduce the problem too, and that was
how Monakhov Dmitriy reproduced it at first, running fstests with
xfs_freeze -f/xfs_freeze -u loop in background.

The problem is it's racing with umount so it's possible SCRATCH_DEV is
not mounted at SCRATCH_MNT when xfs_freeze tries to freeze it, then the
root fs is freezed instead, then the host just hangs there.

Using dmsetup is safer, just freezes the device we want to test.

Thanks,
Eryu
> 
> Brian
> 
> > +pid=$!
> > +for ((i=0; i<100; i++)); do
> > +	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
> > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > +done &
> > +pid="$pid $!"
> > +
> > +wait $pid
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/080.out b/tests/generic/080.out
> > new file mode 100644
> > index 0000000..11a4a1a
> > --- /dev/null
> > +++ b/tests/generic/080.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 080
> > +Silence is golden
> > diff --git a/tests/generic/group b/tests/generic/group
> > index ee5b642..cf7408c 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -82,6 +82,7 @@
> >  077 acl attr auto enospc
> >  078 auto quick metadata
> >  079 acl attr ioctl metadata auto quick
> > +080 auto freeze mount
> >  083 rw auto enospc stress
> >  088 perms auto quick
> >  089 metadata auto
> > -- 
> > 2.1.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 5/9 RESEND] generic: test fs freeze/unfreeze and mount/umount race

[Brian Foster]

On Fri, Mar 27, 2015 at 04:46:51PM +0800, Eryu Guan wrote:
> On Wed, Mar 25, 2015 at 03:44:29PM -0400, Brian Foster wrote:
> > On Fri, Mar 20, 2015 at 06:16:54PM +0800, Eryu Guan wrote:
> > > Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > > use-after-free oops.
> > > 
> > > This commit fixed the issue:
> > > 1494583 fix get_active_super()/umount() race
> > > 
> > > This test case is based on a script from Monakhov Dmitriy.
> > > 
> > > Cc: Monakhov Dmitriy <dmonakhov@openvz.org>
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > >  tests/generic/080     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/080.out |  2 ++
> > >  tests/generic/group   |  1 +
> > >  3 files changed, 101 insertions(+)
> > >  create mode 100755 tests/generic/080
> > >  create mode 100644 tests/generic/080.out
> > > 
> > > diff --git a/tests/generic/080 b/tests/generic/080
> > > new file mode 100755
> > > index 0000000..7d9fa5c
> > > --- /dev/null
> > > +++ b/tests/generic/080
> > > @@ -0,0 +1,98 @@
> > > +#! /bin/bash
> > > +# FS QA Test No. 080
> > > +#
> > > +# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > > +# use-after-free oops.
> > > +#
> > > +# This commit fixed the issue:
> > > +# 1494583 fix get_active_super()/umount() race
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +# 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"
> > > +
> > > +here=`pwd`
> > > +tmp=/tmp/$$
> > > +status=1	# failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -f $tmp.*
> > > +	cleanup_dmdev
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +_supported_fs generic
> > > +_supported_os Linux
> > > +_require_scratch
> > > +_require_block_device $SCRATCH_DEV
> > > +_require_command $DMSETUP_PROG
> > > +_require_freeze
> > > +
> > > +setup_dmdev()
> > > +{
> > > +	table="0 $size_in_sector linear $SCRATCH_DEV 0"
> > > +	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1
> > > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > > +}
> > > +
> > > +cleanup_dmdev()
> > > +{
> > > +	# in case it's still suspended and/or mounted
> > > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > > +
> > > +	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
> > > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > > +}
> > > +
> > > +rm -f $seqres.full
> > > +echo "Silence is golden"
> > > +
> > > +size=$((256 * 1024 * 1024))
> > > +size_in_sector=$((size / 512))
> > > +_scratch_mkfs_sized $size >>$seqres.full 2>&1
> > > +
> > > +node=$seq-test
> > > +lvdev=/dev/mapper/$node
> > > +setup_dmdev
> > > +
> > > +for ((i=0; i<100; i++)); do
> > > +	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
> > > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > > +done &
> > 
> > Any reason we can't use xfs_freeze/fsfreeze for this test? That seems
> > more simple (avoids the dm stuff), but I don't know enough about the
> > original problem to know if that still reproduces it.
> 
> Yes, fsfreeze/unfreeze can reproduce the problem too, and that was
> how Monakhov Dmitriy reproduced it at first, running fstests with
> xfs_freeze -f/xfs_freeze -u loop in background.
> 
> The problem is it's racing with umount so it's possible SCRATCH_DEV is
> not mounted at SCRATCH_MNT when xfs_freeze tries to freeze it, then the
> root fs is freezed instead, then the host just hangs there.
> 
> Using dmsetup is safer, just freezes the device we want to test.
> 

Ah, I see. Sounds reasonable. Could you add some comments to explain why
the test is implemented this way?

I'd also recommend some error checking for this test (e.g., the various
setup commands, the suspend/resume commands, etc.).

Brian

> Thanks,
> Eryu
> > 
> > Brian
> > 
> > > +pid=$!
> > > +for ((i=0; i<100; i++)); do
> > > +	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
> > > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > > +done &
> > > +pid="$pid $!"
> > > +
> > > +wait $pid
> > > +
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/080.out b/tests/generic/080.out
> > > new file mode 100644
> > > index 0000000..11a4a1a
> > > --- /dev/null
> > > +++ b/tests/generic/080.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 080
> > > +Silence is golden
> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index ee5b642..cf7408c 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -82,6 +82,7 @@
> > >  077 acl attr auto enospc
> > >  078 auto quick metadata
> > >  079 acl attr ioctl metadata auto quick
> > > +080 auto freeze mount
> > >  083 rw auto enospc stress
> > >  088 perms auto quick
> > >  089 metadata auto
> > > -- 
> > > 2.1.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 5/9 RESEND] generic: test fs freeze/unfreeze and mount/umount race

[Eryu Guan]

On Fri, Mar 27, 2015 at 08:10:02AM -0400, Brian Foster wrote:
> On Fri, Mar 27, 2015 at 04:46:51PM +0800, Eryu Guan wrote:
> > On Wed, Mar 25, 2015 at 03:44:29PM -0400, Brian Foster wrote:
> > > On Fri, Mar 20, 2015 at 06:16:54PM +0800, Eryu Guan wrote:
> > > > Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > > > use-after-free oops.
> > > > 
> > > > This commit fixed the issue:
> > > > 1494583 fix get_active_super()/umount() race
> > > > 
> > > > This test case is based on a script from Monakhov Dmitriy.
> > > > 
> > > > Cc: Monakhov Dmitriy <dmonakhov@openvz.org>
> > > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > > ---
> > > >  tests/generic/080     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/generic/080.out |  2 ++
> > > >  tests/generic/group   |  1 +
> > > >  3 files changed, 101 insertions(+)
> > > >  create mode 100755 tests/generic/080
> > > >  create mode 100644 tests/generic/080.out
> > > > 
> > > > diff --git a/tests/generic/080 b/tests/generic/080
> > > > new file mode 100755
> > > > index 0000000..7d9fa5c
> > > > --- /dev/null
> > > > +++ b/tests/generic/080
> > > > @@ -0,0 +1,98 @@
> > > > +#! /bin/bash
> > > > +# FS QA Test No. 080
> > > > +#
> > > > +# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > > > +# use-after-free oops.
> > > > +#
> > > > +# This commit fixed the issue:
> > > > +# 1494583 fix get_active_super()/umount() race
> > > > +#
> > > > +#-----------------------------------------------------------------------
> > > > +# 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"
> > > > +
> > > > +here=`pwd`
> > > > +tmp=/tmp/$$
> > > > +status=1	# failure is the default!
> > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > > +
> > > > +_cleanup()
> > > > +{
> > > > +	cd /
> > > > +	rm -f $tmp.*
> > > > +	cleanup_dmdev
> > > > +}
> > > > +
> > > > +# get standard environment, filters and checks
> > > > +. ./common/rc
> > > > +. ./common/filter
> > > > +
> > > > +# real QA test starts here
> > > > +_supported_fs generic
> > > > +_supported_os Linux
> > > > +_require_scratch
> > > > +_require_block_device $SCRATCH_DEV
> > > > +_require_command $DMSETUP_PROG
> > > > +_require_freeze
> > > > +
> > > > +setup_dmdev()
> > > > +{
> > > > +	table="0 $size_in_sector linear $SCRATCH_DEV 0"
> > > > +	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1
> > > > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > > > +}
> > > > +
> > > > +cleanup_dmdev()
> > > > +{
> > > > +	# in case it's still suspended and/or mounted
> > > > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > > > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > > > +
> > > > +	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
> > > > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > > > +}
> > > > +
> > > > +rm -f $seqres.full
> > > > +echo "Silence is golden"
> > > > +
> > > > +size=$((256 * 1024 * 1024))
> > > > +size_in_sector=$((size / 512))
> > > > +_scratch_mkfs_sized $size >>$seqres.full 2>&1
> > > > +
> > > > +node=$seq-test
> > > > +lvdev=/dev/mapper/$node
> > > > +setup_dmdev
> > > > +
> > > > +for ((i=0; i<100; i++)); do
> > > > +	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
> > > > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > > > +done &
> > > 
> > > Any reason we can't use xfs_freeze/fsfreeze for this test? That seems
> > > more simple (avoids the dm stuff), but I don't know enough about the
> > > original problem to know if that still reproduces it.
> > 
> > Yes, fsfreeze/unfreeze can reproduce the problem too, and that was
> > how Monakhov Dmitriy reproduced it at first, running fstests with
> > xfs_freeze -f/xfs_freeze -u loop in background.
> > 
> > The problem is it's racing with umount so it's possible SCRATCH_DEV is
> > not mounted at SCRATCH_MNT when xfs_freeze tries to freeze it, then the
> > root fs is freezed instead, then the host just hangs there.
> > 
> > Using dmsetup is safer, just freezes the device we want to test.
> > 
> 
> Ah, I see. Sounds reasonable. Could you add some comments to explain why
> the test is implemented this way?

Sure, will add in v2.

> 
> I'd also recommend some error checking for this test (e.g., the various
> setup commands, the suspend/resume commands, etc.).

OK, I'm fine with either way. Also I'll update the other test using lvm.

Thanks!

Eryu

P.S. I still don't have a chance to look into the comments deeply in the
"quota remount,ro" test, but will do.

> 
> Brian
> 
> > Thanks,
> > Eryu
> > > 
> > > Brian
> > > 
> > > > +pid=$!
> > > > +for ((i=0; i<100; i++)); do
> > > > +	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
> > > > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > > > +done &
> > > > +pid="$pid $!"
> > > > +
> > > > +wait $pid
> > > > +
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/generic/080.out b/tests/generic/080.out
> > > > new file mode 100644
> > > > index 0000000..11a4a1a
> > > > --- /dev/null
> > > > +++ b/tests/generic/080.out
> > > > @@ -0,0 +1,2 @@
> > > > +QA output created by 080
> > > > +Silence is golden
> > > > diff --git a/tests/generic/group b/tests/generic/group
> > > > index ee5b642..cf7408c 100644
> > > > --- a/tests/generic/group
> > > > +++ b/tests/generic/group
> > > > @@ -82,6 +82,7 @@
> > > >  077 acl attr auto enospc
> > > >  078 auto quick metadata
> > > >  079 acl attr ioctl metadata auto quick
> > > > +080 auto freeze mount
> > > >  083 rw auto enospc stress
> > > >  088 perms auto quick
> > > >  089 metadata auto
> > > > -- 
> > > > 2.1.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 v2 5/9] generic: test fs freeze/unfreeze and mount/umount race

[Eryu Guan]

Exercise fs freeze/unfreeze and mount/umount race, which could lead to
use-after-free oops.

This commit fixed the issue:
1494583 fix get_active_super()/umount() race

This test case is based on a script from Monakhov Dmitriy.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
v2:
- add comments to explain dmsetup is used not xfs_freeze to freeze filesystem
- detect failures in setup phase, so test exits not continus with errors

 tests/generic/080     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/080.out |   2 +
 tests/generic/group   |   1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/generic/080
 create mode 100644 tests/generic/080.out

diff --git a/tests/generic/080 b/tests/generic/080
new file mode 100755
index 0000000..fd460b1
--- /dev/null
+++ b/tests/generic/080
@@ -0,0 +1,102 @@
+#! /bin/bash
+# FS QA Test No. 080
+#
+# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
+# use-after-free oops.
+#
+# This commit fixed the issue:
+# 1494583 fix get_active_super()/umount() race
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	cleanup_dmdev
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_block_device $SCRATCH_DEV
+_require_command $DMSETUP_PROG
+_require_freeze
+
+setup_dmdev()
+{
+	table="0 $size_in_sector linear $SCRATCH_DEV 0"
+	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1 && \
+	$DMSETUP_PROG mknodes >/dev/null 2>&1
+	return $?
+}
+
+cleanup_dmdev()
+{
+	# in case it's still suspended and/or mounted
+	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
+	$UMOUNT_PROG $lvdev >/dev/null 2>&1
+
+	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
+	$DMSETUP_PROG mknodes >/dev/null 2>&1
+}
+
+rm -f $seqres.full
+echo "Silence is golden"
+
+size=$((256 * 1024 * 1024))
+size_in_sector=$((size / 512))
+_scratch_mkfs_sized $size >>$seqres.full 2>&1
+
+node=$seq-test
+lvdev=/dev/mapper/$node
+setup_dmdev || _fail "setup dm device failed"
+
+# take use of dmsetup suspend to freeze the fs.
+# xfs_freeze/fsfreeze cannot be used in this test, because it can possibly
+# freeze the root fs of the host when SCRATCH_MNT is not mounted
+for ((i=0; i<100; i++)); do
+	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
+	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
+done &
+pid=$!
+for ((i=0; i<100; i++)); do
+	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
+	$UMOUNT_PROG $lvdev >/dev/null 2>&1
+done &
+pid="$pid $!"
+
+wait $pid
+
+status=0
+exit
diff --git a/tests/generic/080.out b/tests/generic/080.out
new file mode 100644
index 0000000..11a4a1a
--- /dev/null
+++ b/tests/generic/080.out
@@ -0,0 +1,2 @@
+QA output created by 080
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index ee5b642..cf7408c 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -82,6 +82,7 @@
 077 acl attr auto enospc
 078 auto quick metadata
 079 acl attr ioctl metadata auto quick
+080 auto freeze mount
 083 rw auto enospc stress
 088 perms auto quick
 089 metadata auto
-- 
2.1.0

Re: [PATCH v2 5/9] generic: test fs freeze/unfreeze and mount/umount race

[Brian Foster]

On Wed, Apr 01, 2015 at 09:53:25PM +0800, Eryu Guan wrote:
> Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> use-after-free oops.
> 
> This commit fixed the issue:
> 1494583 fix get_active_super()/umount() race
> 
> This test case is based on a script from Monakhov Dmitriy.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> v2:
> - add comments to explain dmsetup is used not xfs_freeze to freeze filesystem
> - detect failures in setup phase, so test exits not continus with errors
> 
>  tests/generic/080     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/080.out |   2 +
>  tests/generic/group   |   1 +
>  3 files changed, 105 insertions(+)
>  create mode 100755 tests/generic/080
>  create mode 100644 tests/generic/080.out
> 
> diff --git a/tests/generic/080 b/tests/generic/080
> new file mode 100755
> index 0000000..fd460b1
> --- /dev/null
> +++ b/tests/generic/080
> @@ -0,0 +1,102 @@
> +#! /bin/bash
> +# FS QA Test No. 080
> +#
> +# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> +# use-after-free oops.
> +#
> +# This commit fixed the issue:
> +# 1494583 fix get_active_super()/umount() race
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +	cleanup_dmdev
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +_require_block_device $SCRATCH_DEV
> +_require_command $DMSETUP_PROG
> +_require_freeze
> +
> +setup_dmdev()
> +{
> +	table="0 $size_in_sector linear $SCRATCH_DEV 0"
> +	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1 && \
> +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> +	return $?
> +}
> +
> +cleanup_dmdev()
> +{
> +	# in case it's still suspended and/or mounted
> +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> +
> +	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
> +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> +}
> +
> +rm -f $seqres.full
> +echo "Silence is golden"
> +
> +size=$((256 * 1024 * 1024))
> +size_in_sector=$((size / 512))
> +_scratch_mkfs_sized $size >>$seqres.full 2>&1
> +
> +node=$seq-test
> +lvdev=/dev/mapper/$node
> +setup_dmdev || _fail "setup dm device failed"
> +
> +# take use of dmsetup suspend to freeze the fs.
> +# xfs_freeze/fsfreeze cannot be used in this test, because it can possibly
> +# freeze the root fs of the host when SCRATCH_MNT is not mounted
> +for ((i=0; i<100; i++)); do
> +	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
> +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> +done &
> +pid=$!
> +for ((i=0; i<100; i++)); do
> +	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
> +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> +done &
> +pid="$pid $!"
> +

Still need error checks for the suspend/resume and mount/umount
commands. As it is, I reproduce mount errors if I add such checking to
this test. That needs to be either addressed or documented, if
expected..?

My primary concern is that as written, any or all of these commands
could fail/explode for whatever external reason and the test will just
silently pass. :)

Brian

> +wait $pid
> +
> +status=0
> +exit
> diff --git a/tests/generic/080.out b/tests/generic/080.out
> new file mode 100644
> index 0000000..11a4a1a
> --- /dev/null
> +++ b/tests/generic/080.out
> @@ -0,0 +1,2 @@
> +QA output created by 080
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index ee5b642..cf7408c 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -82,6 +82,7 @@
>  077 acl attr auto enospc
>  078 auto quick metadata
>  079 acl attr ioctl metadata auto quick
> +080 auto freeze mount
>  083 rw auto enospc stress
>  088 perms auto quick
>  089 metadata auto
> -- 
> 2.1.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 v2 5/9] generic: test fs freeze/unfreeze and mount/umount race

[Eryu Guan]

On Wed, Apr 01, 2015 at 01:56:09PM -0400, Brian Foster wrote:
> On Wed, Apr 01, 2015 at 09:53:25PM +0800, Eryu Guan wrote:
> > Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > use-after-free oops.
> > 
> > This commit fixed the issue:
> > 1494583 fix get_active_super()/umount() race
> > 
> > This test case is based on a script from Monakhov Dmitriy.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > v2:
> > - add comments to explain dmsetup is used not xfs_freeze to freeze filesystem
> > - detect failures in setup phase, so test exits not continus with errors
> > 
> >  tests/generic/080     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/080.out |   2 +
> >  tests/generic/group   |   1 +
> >  3 files changed, 105 insertions(+)
> >  create mode 100755 tests/generic/080
> >  create mode 100644 tests/generic/080.out
> > 
> > diff --git a/tests/generic/080 b/tests/generic/080
> > new file mode 100755
> > index 0000000..fd460b1
> > --- /dev/null
> > +++ b/tests/generic/080
> > @@ -0,0 +1,102 @@
> > +#! /bin/bash
> > +# FS QA Test No. 080
> > +#
> > +# Exercise fs freeze/unfreeze and mount/umount race, which could lead to
> > +# use-after-free oops.
> > +#
> > +# This commit fixed the issue:
> > +# 1494583 fix get_active_super()/umount() race
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +	cleanup_dmdev
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_scratch
> > +_require_block_device $SCRATCH_DEV
> > +_require_command $DMSETUP_PROG
> > +_require_freeze
> > +
> > +setup_dmdev()
> > +{
> > +	table="0 $size_in_sector linear $SCRATCH_DEV 0"
> > +	$DMSETUP_PROG create $node --table "$table" >>$seqres.full 2>&1 && \
> > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > +	return $?
> > +}
> > +
> > +cleanup_dmdev()
> > +{
> > +	# in case it's still suspended and/or mounted
> > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > +
> > +	$DMSETUP_PROG remove $node >>$seqres.full 2>&1
> > +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> > +}
> > +
> > +rm -f $seqres.full
> > +echo "Silence is golden"
> > +
> > +size=$((256 * 1024 * 1024))
> > +size_in_sector=$((size / 512))
> > +_scratch_mkfs_sized $size >>$seqres.full 2>&1
> > +
> > +node=$seq-test
> > +lvdev=/dev/mapper/$node
> > +setup_dmdev || _fail "setup dm device failed"
> > +
> > +# take use of dmsetup suspend to freeze the fs.
> > +# xfs_freeze/fsfreeze cannot be used in this test, because it can possibly
> > +# freeze the root fs of the host when SCRATCH_MNT is not mounted
> > +for ((i=0; i<100; i++)); do
> > +	$DMSETUP_PROG suspend $lvdev >/dev/null 2>&1
> > +	$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> > +done &
> > +pid=$!
> > +for ((i=0; i<100; i++)); do
> > +	$MOUNT_PROG $lvdev $SCRATCH_MNT >/dev/null 2>&1
> > +	$UMOUNT_PROG $lvdev >/dev/null 2>&1
> > +done &
> > +pid="$pid $!"
> > +
> 
> Still need error checks for the suspend/resume and mount/umount
> commands. As it is, I reproduce mount errors if I add such checking to
> this test. That needs to be either addressed or documented, if
> expected..?

I don't expect these racing commands to succeed all the time, and the
results are not important either, as long as they are racing with each
other.

I think I can add some comments to state the fact.

Thanks for your review!

Eryu
> 
> My primary concern is that as written, any or all of these commands
> could fail/explode for whatever external reason and the test will just
> silently pass. :)
> 
> Brian
> 
> > +wait $pid
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/080.out b/tests/generic/080.out
> > new file mode 100644
> > index 0000000..11a4a1a
> > --- /dev/null
> > +++ b/tests/generic/080.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 080
> > +Silence is golden
> > diff --git a/tests/generic/group b/tests/generic/group
> > index ee5b642..cf7408c 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -82,6 +82,7 @@
> >  077 acl attr auto enospc
> >  078 auto quick metadata
> >  079 acl attr ioctl metadata auto quick
> > +080 auto freeze mount
> >  083 rw auto enospc stress
> >  088 perms auto quick
> >  089 metadata auto
> > -- 
> > 2.1.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
> --
> 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 6/9] generic: test I/O error path by fully filling dm snapshot

[Eryu Guan]

xfs used to panic in this test, this xfs commit fix the bug

8d6c121 xfs: fix buffer use after free on IO error

ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 common/config         |  1 +
 tests/generic/081     | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/081.out |  2 ++
 tests/generic/group   |  1 +
 4 files changed, 83 insertions(+)
 create mode 100755 tests/generic/081
 create mode 100644 tests/generic/081.out

diff --git a/common/config b/common/config
index e5c3579..3732287 100644
--- a/common/config
+++ b/common/config
@@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`"
 export WIPEFS_PROG="`set_prog_path wipefs`"
 export DUMP_PROG="`set_prog_path dump`"
 export RESTORE_PROG="`set_prog_path restore`"
+export LVM_PROG="`set_prog_path lvm`"
 
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
diff --git a/tests/generic/081 b/tests/generic/081
new file mode 100755
index 0000000..476bfb9
--- /dev/null
+++ b/tests/generic/081
@@ -0,0 +1,79 @@
+#! /bin/bash
+# FS QA Test No. 081
+#
+# Test I/O error path by fully filling an dm snapshot.
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	# lvm may have umounted it on I/O error, but in case it does not
+	$UMOUNT_PROG $mnt >/dev/null 2>&1
+	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
+	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_test
+_require_scratch_nocheck
+_require_block_device $SCRATCH_DEV
+_require_command $LVM_PROG lvm
+
+echo "Silence is golden"
+rm -f $seqres.full
+
+vgname=vg_$seq
+lvname=base_$seq
+snapname=snap_$seq
+mnt=$TEST_DIR/mnt_$seq
+mkdir -p $mnt
+
+$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
+$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
+
+_mkfs_dev /dev/mapper/$vgname-$lvname
+
+# create a 4M snapshot
+$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1
+
+_mount /dev/mapper/$vgname-$snapname $mnt
+
+# write 5M data to the snapshot
+$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
+
+# _check_dmesg will check for WARNINGs/BUGs in dmesg
+status=0
+exit
diff --git a/tests/generic/081.out b/tests/generic/081.out
new file mode 100644
index 0000000..663a886
--- /dev/null
+++ b/tests/generic/081.out
@@ -0,0 +1,2 @@
+QA output created by 081
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index cf7408c..f5ebe48 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -83,6 +83,7 @@
 078 auto quick metadata
 079 acl attr ioctl metadata auto quick
 080 auto freeze mount
+081 auto quick
 083 rw auto enospc stress
 088 perms auto quick
 089 metadata auto
-- 
2.1.0

Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:55PM +0800, Eryu Guan wrote:
> xfs used to panic in this test, this xfs commit fix the bug
> 
> 8d6c121 xfs: fix buffer use after free on IO error
> 
> ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  common/config         |  1 +
>  tests/generic/081     | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/081.out |  2 ++
>  tests/generic/group   |  1 +
>  4 files changed, 83 insertions(+)
>  create mode 100755 tests/generic/081
>  create mode 100644 tests/generic/081.out
> 
> diff --git a/common/config b/common/config
> index e5c3579..3732287 100644
> --- a/common/config
> +++ b/common/config
> @@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`"
>  export WIPEFS_PROG="`set_prog_path wipefs`"
>  export DUMP_PROG="`set_prog_path dump`"
>  export RESTORE_PROG="`set_prog_path restore`"
> +export LVM_PROG="`set_prog_path lvm`"
>  
>  # Generate a comparable xfsprogs version number in the form of
>  # major * 10000 + minor * 100 + release
> diff --git a/tests/generic/081 b/tests/generic/081
> new file mode 100755
> index 0000000..476bfb9
> --- /dev/null
> +++ b/tests/generic/081
> @@ -0,0 +1,79 @@
> +#! /bin/bash
> +# FS QA Test No. 081
> +#
> +# Test I/O error path by fully filling an dm snapshot.
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +	# lvm may have umounted it on I/O error, but in case it does not
> +	$UMOUNT_PROG $mnt >/dev/null 2>&1
> +	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
> +	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_test
> +_require_scratch_nocheck
> +_require_block_device $SCRATCH_DEV
> +_require_command $LVM_PROG lvm
> +
> +echo "Silence is golden"
> +rm -f $seqres.full
> +
> +vgname=vg_$seq
> +lvname=base_$seq
> +snapname=snap_$seq
> +mnt=$TEST_DIR/mnt_$seq
> +mkdir -p $mnt
> +
> +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
> +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
> +

Hmm, I take it this introduces a block size size requirement of 256MB.
If so, perhaps we could use a sparse file and loop, seeing as we're not
actually using much space. It's not that big of a deal I suppose, even a
ramdisk probably has that much space. At the very least we should do
something like the following:

lvcreate -L 256M ... || _fail "lvcreate failed"

... to catch any failures from these commands.

> +_mkfs_dev /dev/mapper/$vgname-$lvname
> +
> +# create a 4M snapshot
> +$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1

Same thing here... I guess we're assuming dm-snapshot is available too.
We might want a require test for that...

Brian

> +
> +_mount /dev/mapper/$vgname-$snapname $mnt
> +
> +# write 5M data to the snapshot
> +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> +
> +# _check_dmesg will check for WARNINGs/BUGs in dmesg
> +status=0
> +exit
> diff --git a/tests/generic/081.out b/tests/generic/081.out
> new file mode 100644
> index 0000000..663a886
> --- /dev/null
> +++ b/tests/generic/081.out
> @@ -0,0 +1,2 @@
> +QA output created by 081
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index cf7408c..f5ebe48 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -83,6 +83,7 @@
>  078 auto quick metadata
>  079 acl attr ioctl metadata auto quick
>  080 auto freeze mount
> +081 auto quick
>  083 rw auto enospc stress
>  088 perms auto quick
>  089 metadata auto
> -- 
> 2.1.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 6/9] generic: test I/O error path by fully filling dm snapshot

[Eryu Guan]

On Wed, Mar 25, 2015 at 07:08:39PM -0400, Brian Foster wrote:
> On Fri, Mar 20, 2015 at 06:16:55PM +0800, Eryu Guan wrote:
> > xfs used to panic in this test, this xfs commit fix the bug
> > 
> > 8d6c121 xfs: fix buffer use after free on IO error
> > 
> > ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> >  common/config         |  1 +
> >  tests/generic/081     | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/081.out |  2 ++
> >  tests/generic/group   |  1 +
> >  4 files changed, 83 insertions(+)
> >  create mode 100755 tests/generic/081
> >  create mode 100644 tests/generic/081.out
> > 
> > diff --git a/common/config b/common/config
> > index e5c3579..3732287 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`"
> >  export WIPEFS_PROG="`set_prog_path wipefs`"
> >  export DUMP_PROG="`set_prog_path dump`"
> >  export RESTORE_PROG="`set_prog_path restore`"
> > +export LVM_PROG="`set_prog_path lvm`"
> >  
> >  # Generate a comparable xfsprogs version number in the form of
> >  # major * 10000 + minor * 100 + release
> > diff --git a/tests/generic/081 b/tests/generic/081
> > new file mode 100755
> > index 0000000..476bfb9
> > --- /dev/null
> > +++ b/tests/generic/081
> > @@ -0,0 +1,79 @@
> > +#! /bin/bash
> > +# FS QA Test No. 081
> > +#
> > +# Test I/O error path by fully filling an dm snapshot.
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +	# lvm may have umounted it on I/O error, but in case it does not
> > +	$UMOUNT_PROG $mnt >/dev/null 2>&1
> > +	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
> > +	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_test
> > +_require_scratch_nocheck
> > +_require_block_device $SCRATCH_DEV
> > +_require_command $LVM_PROG lvm
> > +
> > +echo "Silence is golden"
> > +rm -f $seqres.full
> > +
> > +vgname=vg_$seq
> > +lvname=base_$seq
> > +snapname=snap_$seq
> > +mnt=$TEST_DIR/mnt_$seq
> > +mkdir -p $mnt
> > +
> > +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
> > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
> > +
> 
> Hmm, I take it this introduces a block size size requirement of 256MB.
> If so, perhaps we could use a sparse file and loop, seeing as we're not
> actually using much space. It's not that big of a deal I suppose, even a
> ramdisk probably has that much space. At the very least we should do
> something like the following:
> 
> lvcreate -L 256M ... || _fail "lvcreate failed"
> 
> ... to catch any failures from these commands.

If vgcreate/lvcreate fails to create lv, the following _mkfs_dev fails
too and breaks the output match.

I think I can _require_fs_space <300M> of SCRATCH_DEV (in case lvm takes
some space to store metadata)

> 
> > +_mkfs_dev /dev/mapper/$vgname-$lvname
> > +
> > +# create a 4M snapshot
> > +$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1
> 
> Same thing here... I guess we're assuming dm-snapshot is available too.
> We might want a require test for that...

Yes, will do.

Thanks for the review!

Eryu
> 
> Brian
> 
> > +
> > +_mount /dev/mapper/$vgname-$snapname $mnt
> > +
> > +# write 5M data to the snapshot
> > +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> > +
> > +# _check_dmesg will check for WARNINGs/BUGs in dmesg
> > +status=0
> > +exit
> > diff --git a/tests/generic/081.out b/tests/generic/081.out
> > new file mode 100644
> > index 0000000..663a886
> > --- /dev/null
> > +++ b/tests/generic/081.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 081
> > +Silence is golden
> > diff --git a/tests/generic/group b/tests/generic/group
> > index cf7408c..f5ebe48 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -83,6 +83,7 @@
> >  078 auto quick metadata
> >  079 acl attr ioctl metadata auto quick
> >  080 auto freeze mount
> > +081 auto quick
> >  083 rw auto enospc stress
> >  088 perms auto quick
> >  089 metadata auto
> > -- 
> > 2.1.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 6/9] generic: test I/O error path by fully filling dm snapshot

[Brian Foster]

On Fri, Mar 27, 2015 at 03:38:26PM +0800, Eryu Guan wrote:
> On Wed, Mar 25, 2015 at 07:08:39PM -0400, Brian Foster wrote:
> > On Fri, Mar 20, 2015 at 06:16:55PM +0800, Eryu Guan wrote:
> > > xfs used to panic in this test, this xfs commit fix the bug
> > > 
> > > 8d6c121 xfs: fix buffer use after free on IO error
> > > 
> > > ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > >  common/config         |  1 +
> > >  tests/generic/081     | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/081.out |  2 ++
> > >  tests/generic/group   |  1 +
> > >  4 files changed, 83 insertions(+)
> > >  create mode 100755 tests/generic/081
> > >  create mode 100644 tests/generic/081.out
> > > 
> > > diff --git a/common/config b/common/config
> > > index e5c3579..3732287 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`"
> > >  export WIPEFS_PROG="`set_prog_path wipefs`"
> > >  export DUMP_PROG="`set_prog_path dump`"
> > >  export RESTORE_PROG="`set_prog_path restore`"
> > > +export LVM_PROG="`set_prog_path lvm`"
> > >  
> > >  # Generate a comparable xfsprogs version number in the form of
> > >  # major * 10000 + minor * 100 + release
> > > diff --git a/tests/generic/081 b/tests/generic/081
> > > new file mode 100755
> > > index 0000000..476bfb9
> > > --- /dev/null
> > > +++ b/tests/generic/081
> > > @@ -0,0 +1,79 @@
> > > +#! /bin/bash
> > > +# FS QA Test No. 081
> > > +#
> > > +# Test I/O error path by fully filling an dm snapshot.
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +# 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"
> > > +
> > > +here=`pwd`
> > > +tmp=/tmp/$$
> > > +status=1	# failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -f $tmp.*
> > > +	# lvm may have umounted it on I/O error, but in case it does not
> > > +	$UMOUNT_PROG $mnt >/dev/null 2>&1
> > > +	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
> > > +	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +_supported_fs generic
> > > +_supported_os Linux
> > > +_require_test
> > > +_require_scratch_nocheck
> > > +_require_block_device $SCRATCH_DEV
> > > +_require_command $LVM_PROG lvm
> > > +
> > > +echo "Silence is golden"
> > > +rm -f $seqres.full
> > > +
> > > +vgname=vg_$seq
> > > +lvname=base_$seq
> > > +snapname=snap_$seq
> > > +mnt=$TEST_DIR/mnt_$seq
> > > +mkdir -p $mnt
> > > +
> > > +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
> > > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
> > > +
> > 
> > Hmm, I take it this introduces a block size size requirement of 256MB.
> > If so, perhaps we could use a sparse file and loop, seeing as we're not
> > actually using much space. It's not that big of a deal I suppose, even a
> > ramdisk probably has that much space. At the very least we should do
> > something like the following:
> > 
> > lvcreate -L 256M ... || _fail "lvcreate failed"
> > 
> > ... to catch any failures from these commands.
> 
> If vgcreate/lvcreate fails to create lv, the following _mkfs_dev fails
> too and breaks the output match.
> 

Perhaps, but I think it's better practice to fail the test outright if
we know it's broken, particularly with such setup commands that
create/format/mount devices. We don't run the risk of running subsequent
commands, generating more noise than necessary, etc.

Brian

> I think I can _require_fs_space <300M> of SCRATCH_DEV (in case lvm takes
> some space to store metadata)
> 
> > 
> > > +_mkfs_dev /dev/mapper/$vgname-$lvname
> > > +
> > > +# create a 4M snapshot
> > > +$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1
> > 
> > Same thing here... I guess we're assuming dm-snapshot is available too.
> > We might want a require test for that...
> 
> Yes, will do.
> 
> Thanks for the review!
> 
> Eryu
> > 
> > Brian
> > 
> > > +
> > > +_mount /dev/mapper/$vgname-$snapname $mnt
> > > +
> > > +# write 5M data to the snapshot
> > > +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> > > +
> > > +# _check_dmesg will check for WARNINGs/BUGs in dmesg
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/081.out b/tests/generic/081.out
> > > new file mode 100644
> > > index 0000000..663a886
> > > --- /dev/null
> > > +++ b/tests/generic/081.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 081
> > > +Silence is golden
> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index cf7408c..f5ebe48 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -83,6 +83,7 @@
> > >  078 auto quick metadata
> > >  079 acl attr ioctl metadata auto quick
> > >  080 auto freeze mount
> > > +081 auto quick
> > >  083 rw auto enospc stress
> > >  088 perms auto quick
> > >  089 metadata auto
> > > -- 
> > > 2.1.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 6/9] generic: test I/O error path by fully filling dm snapshot

[Eryu Guan]

xfs used to panic in this test, this xfs commit fix the bug

8d6c121 xfs: fix buffer use after free on IO error

ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
v2:
- add _require_dm_snapshot() function to require dm snapshot target
- make sure SCRATCH_DEV has enough space for the test
- fail the test directly when failures detected in setup phase

 common/config         |  1 +
 common/rc             | 10 ++++++
 tests/generic/081     | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/081.out |  2 ++
 tests/generic/group   |  1 +
 5 files changed, 100 insertions(+)
 create mode 100755 tests/generic/081
 create mode 100644 tests/generic/081.out

diff --git a/common/config b/common/config
index e5c3579..3732287 100644
--- a/common/config
+++ b/common/config
@@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`"
 export WIPEFS_PROG="`set_prog_path wipefs`"
 export DUMP_PROG="`set_prog_path dump`"
 export RESTORE_PROG="`set_prog_path restore`"
+export LVM_PROG="`set_prog_path lvm`"
 
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
diff --git a/common/rc b/common/rc
index 857308a..74df379 100644
--- a/common/rc
+++ b/common/rc
@@ -1311,6 +1311,16 @@ _require_dm_flakey()
     fi
 }
 
+_require_dm_snapshot()
+{
+	_require_command "$DMSETUP_PROG" dmsetup
+	modprobe dm-snapshot >/dev/null 2>&1
+	$DMSETUP_PROG targets | grep -q snapshot
+	if [ $? -ne 0 ]; then
+		_notrun "This test requires dm snapshot support"
+	fi
+}
+
 # this test requires the projid32bit feature to be available in mkfs.xfs.
 #
 _require_projid32bit()
diff --git a/tests/generic/081 b/tests/generic/081
new file mode 100755
index 0000000..3e17d34
--- /dev/null
+++ b/tests/generic/081
@@ -0,0 +1,86 @@
+#! /bin/bash
+# FS QA Test No. 081
+#
+# Test I/O error path by fully filling an dm snapshot.
+#
+#-----------------------------------------------------------------------
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	# lvm may have umounted it on I/O error, but in case it does not
+	$UMOUNT_PROG $mnt >/dev/null 2>&1
+	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
+	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_test
+_require_scratch_nocheck
+_require_dm_snapshot
+_require_block_device $SCRATCH_DEV
+_require_command $LVM_PROG lvm
+
+echo "Silence is golden"
+rm -f $seqres.full
+
+vgname=vg_$seq
+lvname=base_$seq
+snapname=snap_$seq
+mnt=$TEST_DIR/mnt_$seq
+mkdir -p $mnt
+
+# make sure there's enough disk space for 256M lv, test for 300M here in case
+# lvm uses some space for metadata
+_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1
+$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
+$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
+
+# _mkfs_dev exits the test on failure, this can make sure lv is created in
+# above vgcreate/lvcreate steps
+_mkfs_dev /dev/mapper/$vgname-$lvname
+
+# create a 4M snapshot
+$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1 || \
+	_fail "Failed to create snapshot"
+
+_mount /dev/mapper/$vgname-$snapname $mnt
+
+# write 5M data to the snapshot
+$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
+
+# _check_dmesg will check for WARNINGs/BUGs in dmesg
+status=0
+exit
diff --git a/tests/generic/081.out b/tests/generic/081.out
new file mode 100644
index 0000000..663a886
--- /dev/null
+++ b/tests/generic/081.out
@@ -0,0 +1,2 @@
+QA output created by 081
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index cf7408c..f5ebe48 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -83,6 +83,7 @@
 078 auto quick metadata
 079 acl attr ioctl metadata auto quick
 080 auto freeze mount
+081 auto quick
 083 rw auto enospc stress
 088 perms auto quick
 089 metadata auto
-- 
2.1.0

Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Brian Foster]

On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> xfs used to panic in this test, this xfs commit fix the bug
> 
> 8d6c121 xfs: fix buffer use after free on IO error
> 
> ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> v2:
> - add _require_dm_snapshot() function to require dm snapshot target
> - make sure SCRATCH_DEV has enough space for the test
> - fail the test directly when failures detected in setup phase
> 

FYI, the mail subject header hasn't changed so Dave might not notice
this is a new patch.

>  common/config         |  1 +
>  common/rc             | 10 ++++++
>  tests/generic/081     | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/081.out |  2 ++
>  tests/generic/group   |  1 +
>  5 files changed, 100 insertions(+)
>  create mode 100755 tests/generic/081
>  create mode 100644 tests/generic/081.out
> 
> diff --git a/common/config b/common/config
> index e5c3579..3732287 100644
> --- a/common/config
> +++ b/common/config
> @@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`"
>  export WIPEFS_PROG="`set_prog_path wipefs`"
>  export DUMP_PROG="`set_prog_path dump`"
>  export RESTORE_PROG="`set_prog_path restore`"
> +export LVM_PROG="`set_prog_path lvm`"
>  
>  # Generate a comparable xfsprogs version number in the form of
>  # major * 10000 + minor * 100 + release
> diff --git a/common/rc b/common/rc
> index 857308a..74df379 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1311,6 +1311,16 @@ _require_dm_flakey()
>      fi
>  }
>  
> +_require_dm_snapshot()
> +{
> +	_require_command "$DMSETUP_PROG" dmsetup
> +	modprobe dm-snapshot >/dev/null 2>&1
> +	$DMSETUP_PROG targets | grep -q snapshot
> +	if [ $? -ne 0 ]; then
> +		_notrun "This test requires dm snapshot support"
> +	fi
> +}
> +
>  # this test requires the projid32bit feature to be available in mkfs.xfs.
>  #
>  _require_projid32bit()
> diff --git a/tests/generic/081 b/tests/generic/081
> new file mode 100755
> index 0000000..3e17d34
> --- /dev/null
> +++ b/tests/generic/081
> @@ -0,0 +1,86 @@
> +#! /bin/bash
> +# FS QA Test No. 081
> +#
> +# Test I/O error path by fully filling an dm snapshot.
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +	# lvm may have umounted it on I/O error, but in case it does not
> +	$UMOUNT_PROG $mnt >/dev/null 2>&1
> +	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
> +	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_test
> +_require_scratch_nocheck
> +_require_dm_snapshot
> +_require_block_device $SCRATCH_DEV
> +_require_command $LVM_PROG lvm
> +
> +echo "Silence is golden"
> +rm -f $seqres.full
> +
> +vgname=vg_$seq
> +lvname=base_$seq
> +snapname=snap_$seq
> +mnt=$TEST_DIR/mnt_$seq
> +mkdir -p $mnt
> +
> +# make sure there's enough disk space for 256M lv, test for 300M here in case
> +# lvm uses some space for metadata
> +_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1
> +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
> +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
> +
> +# _mkfs_dev exits the test on failure, this can make sure lv is created in
> +# above vgcreate/lvcreate steps
> +_mkfs_dev /dev/mapper/$vgname-$lvname
> +
> +# create a 4M snapshot
> +$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1 || \
> +	_fail "Failed to create snapshot"
> +
> +_mount /dev/mapper/$vgname-$snapname $mnt
> +
> +# write 5M data to the snapshot
> +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> +

I noticed there were no errors in $seqres.full when running this test.
E.g., the pwrite succeeds because nothing is written back to disk at
that point. The fs does shutdown due to the flush on umount, but it's
kind of hidden away up in the _cleanup() function.

Kind of a nit, but we could be a bit more explicit and do a '-c fsync'
after the pwrite here? That way it's clear that writeback to disk is
part of the core test and we have a little feedback in $seqres.full that
I/O errors occurred, as expected.

Otherwise, the rest looks Ok to me.

Brian

> +# _check_dmesg will check for WARNINGs/BUGs in dmesg
> +status=0
> +exit
> diff --git a/tests/generic/081.out b/tests/generic/081.out
> new file mode 100644
> index 0000000..663a886
> --- /dev/null
> +++ b/tests/generic/081.out
> @@ -0,0 +1,2 @@
> +QA output created by 081
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index cf7408c..f5ebe48 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -83,6 +83,7 @@
>  078 auto quick metadata
>  079 acl attr ioctl metadata auto quick
>  080 auto freeze mount
> +081 auto quick
>  083 rw auto enospc stress
>  088 perms auto quick
>  089 metadata auto
> -- 
> 2.1.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 6/9] generic: test I/O error path by fully filling dm snapshot

[Dave Chinner]

On Wed, Apr 01, 2015 at 01:57:10PM -0400, Brian Foster wrote:
> On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> > xfs used to panic in this test, this xfs commit fix the bug
> > 
> > 8d6c121 xfs: fix buffer use after free on IO error
> > 
> > ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > v2:
> > - add _require_dm_snapshot() function to require dm snapshot target
> > - make sure SCRATCH_DEV has enough space for the test
> > - fail the test directly when failures detected in setup phase
> > 
> 
> FYI, the mail subject header hasn't changed so Dave might not notice
> this is a new patch.

Saw it.

> > +_mount /dev/mapper/$vgname-$snapname $mnt
> > +
> > +# write 5M data to the snapshot
> > +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> > +
> 
> I noticed there were no errors in $seqres.full when running this test.
> E.g., the pwrite succeeds because nothing is written back to disk at
> that point. The fs does shutdown due to the flush on umount, but it's
> kind of hidden away up in the _cleanup() function.
> 
> Kind of a nit, but we could be a bit more explicit and do a '-c fsync'
> after the pwrite here? That way it's clear that writeback to disk is
> part of the core test and we have a little feedback in $seqres.full that
> I/O errors occurred, as expected.

Added the -c fsync as I pulled it in.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Eryu Guan]

On Thu, Apr 02, 2015 at 09:29:54AM +1100, Dave Chinner wrote:
> On Wed, Apr 01, 2015 at 01:57:10PM -0400, Brian Foster wrote:
> > On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> > > xfs used to panic in this test, this xfs commit fix the bug
> > > 
> > > 8d6c121 xfs: fix buffer use after free on IO error
> > > 
> > > ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > > v2:
> > > - add _require_dm_snapshot() function to require dm snapshot target
> > > - make sure SCRATCH_DEV has enough space for the test
> > > - fail the test directly when failures detected in setup phase
> > > 
> > 
> > FYI, the mail subject header hasn't changed so Dave might not notice
> > this is a new patch.
> 
> Saw it.
> 
> > > +_mount /dev/mapper/$vgname-$snapname $mnt
> > > +
> > > +# write 5M data to the snapshot
> > > +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> > > +
> > 
> > I noticed there were no errors in $seqres.full when running this test.
> > E.g., the pwrite succeeds because nothing is written back to disk at
> > that point. The fs does shutdown due to the flush on umount, but it's
> > kind of hidden away up in the _cleanup() function.
> > 
> > Kind of a nit, but we could be a bit more explicit and do a '-c fsync'
> > after the pwrite here? That way it's clear that writeback to disk is
> > part of the core test and we have a little feedback in $seqres.full that
> > I/O errors occurred, as expected.
> 
> Added the -c fsync as I pulled it in.

I was thinking about adding fsync or sync at first, but it causes
problems in cleanup. For some reason(I don't know clearly) fsync
sometimes pins the snapshot in use and vgremove can't remove all lvs, so
SCRATCH_DEV is still used by the test vg when running next test

our local _scratch_mkfs routine ...                                                                                                                                              
mkfs.xfs: cannot open /dev/sda6: Device or resource busy                                                                                         
check: failed to mkfs $SCRATCH_DEV using specified options

>From 081.full I can see

Logical volume vg_081/snap_081 contains a filesystem in use.

The test vg/lv has to be removed manually after the test

vgremove -f vg_081

I don't find a proper way to fix it yet, but simply adding 'sleep 1'
before vgremove in _cleanup works for me

[root@hp-dl388eg8-01 xfstests]# git diff
diff --git a/tests/generic/081 b/tests/generic/081
index 3e17d34..29b0240 100755
--- a/tests/generic/081
+++ b/tests/generic/081
@@ -36,6 +36,7 @@ _cleanup()
        rm -f $tmp.*
        # lvm may have umounted it on I/O error, but in case it does not
        $UMOUNT_PROG $mnt >/dev/null 2>&1
+       sleep 1
        $LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
        $LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
 }

Thanks,
Eryu

Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Brian Foster]

On Thu, Apr 02, 2015 at 12:10:41PM +0800, Eryu Guan wrote:
> On Thu, Apr 02, 2015 at 09:29:54AM +1100, Dave Chinner wrote:
> > On Wed, Apr 01, 2015 at 01:57:10PM -0400, Brian Foster wrote:
> > > On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> > > > xfs used to panic in this test, this xfs commit fix the bug
> > > > 
> > > > 8d6c121 xfs: fix buffer use after free on IO error
> > > > 
> > > > ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> > > > 
> > > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > > ---
> > > > v2:
> > > > - add _require_dm_snapshot() function to require dm snapshot target
> > > > - make sure SCRATCH_DEV has enough space for the test
> > > > - fail the test directly when failures detected in setup phase
> > > > 
> > > 
> > > FYI, the mail subject header hasn't changed so Dave might not notice
> > > this is a new patch.
> > 
> > Saw it.
> > 
> > > > +_mount /dev/mapper/$vgname-$snapname $mnt
> > > > +
> > > > +# write 5M data to the snapshot
> > > > +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> > > > +
> > > 
> > > I noticed there were no errors in $seqres.full when running this test.
> > > E.g., the pwrite succeeds because nothing is written back to disk at
> > > that point. The fs does shutdown due to the flush on umount, but it's
> > > kind of hidden away up in the _cleanup() function.
> > > 
> > > Kind of a nit, but we could be a bit more explicit and do a '-c fsync'
> > > after the pwrite here? That way it's clear that writeback to disk is
> > > part of the core test and we have a little feedback in $seqres.full that
> > > I/O errors occurred, as expected.
> > 
> > Added the -c fsync as I pulled it in.
> 
> I was thinking about adding fsync or sync at first, but it causes
> problems in cleanup. For some reason(I don't know clearly) fsync
> sometimes pins the snapshot in use and vgremove can't remove all lvs, so
> SCRATCH_DEV is still used by the test vg when running next test
> 
> our local _scratch_mkfs routine ...                                                                                                                                              
> mkfs.xfs: cannot open /dev/sda6: Device or resource busy                                                                                         
> check: failed to mkfs $SCRATCH_DEV using specified options
> 
> From 081.full I can see
> 
> Logical volume vg_081/snap_081 contains a filesystem in use.
> 
> The test vg/lv has to be removed manually after the test
> 
> vgremove -f vg_081
> 
> I don't find a proper way to fix it yet, but simply adding 'sleep 1'
> before vgremove in _cleanup works for me
> 

Hmm, I haven't seen that one. Seems fine for me as a temporary
workaround though. Perhaps include it with the udev-settle thing that
Dave needs on the creation side of things..?

Brian

> [root@hp-dl388eg8-01 xfstests]# git diff
> diff --git a/tests/generic/081 b/tests/generic/081
> index 3e17d34..29b0240 100755
> --- a/tests/generic/081
> +++ b/tests/generic/081
> @@ -36,6 +36,7 @@ _cleanup()
>         rm -f $tmp.*
>         # lvm may have umounted it on I/O error, but in case it does not
>         $UMOUNT_PROG $mnt >/dev/null 2>&1
> +       sleep 1
>         $LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
>         $LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
>  }
> 
> Thanks,
> Eryu

Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Dave Chinner]

On Thu, Apr 02, 2015 at 07:11:45AM -0400, Brian Foster wrote:
> On Thu, Apr 02, 2015 at 12:10:41PM +0800, Eryu Guan wrote:
> > On Thu, Apr 02, 2015 at 09:29:54AM +1100, Dave Chinner wrote:
> > > On Wed, Apr 01, 2015 at 01:57:10PM -0400, Brian Foster wrote:
> > > > On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> > > > Kind of a nit, but we could be a bit more explicit and do a '-c fsync'
> > > > after the pwrite here? That way it's clear that writeback to disk is
> > > > part of the core test and we have a little feedback in $seqres.full that
> > > > I/O errors occurred, as expected.
> > > 
> > > Added the -c fsync as I pulled it in.
> > 
> > I was thinking about adding fsync or sync at first, but it causes
> > problems in cleanup. For some reason(I don't know clearly) fsync
> > sometimes pins the snapshot in use and vgremove can't remove all lvs, so
> > SCRATCH_DEV is still used by the test vg when running next test
> > 
> > our local _scratch_mkfs routine ...                                                                                                                                              
> > mkfs.xfs: cannot open /dev/sda6: Device or resource busy                                                                                         
> > check: failed to mkfs $SCRATCH_DEV using specified options
> > 
> > From 081.full I can see
> > 
> > Logical volume vg_081/snap_081 contains a filesystem in use.
> > 
> > The test vg/lv has to be removed manually after the test
> > 
> > vgremove -f vg_081
> > 
> > I don't find a proper way to fix it yet, but simply adding 'sleep 1'
> > before vgremove in _cleanup works for me
> 
> Hmm, I haven't seen that one. Seems fine for me as a temporary
> workaround though. Perhaps include it with the udev-settle thing that
> Dave needs on the creation side of things..?

More than likely. I hate udev at times....

FWIW, there's also other issues here, and I think I've finally also
worked out why dm-flakey tests have interesting problems when run on
ramdisks.

$ cat /home/dave/src/xfstests-dev/results//xfs/generic/081.full

[ snip scratch_mkfs output ]

  Physical volume "/dev/ram1" successfully created
  Volume group "vg_081" successfully created
  Logical volume "base_081" created
  Volume group "vg_081" not found
Failed to create snapshot
  Volume group "vg_081" not found
  Skipping volume group vg_081
  Can't open /dev/ram1 exclusively - not removing. Mounted filesystem?

Which is interesting, because:

$ sudo lvm vgcreate foobar /dev/ram1
  Physical volume "/dev/ram1" successfully created
  Volume group "foobar" successfully created
$ sudo lvm lvcreate -L 256M -n wtf foobar
  Logical volume "wtf" created
$ sudo lvm pvscan
  PV /dev/ram1   VG foobar   lvm2 [3.81 GiB / 3.56 GiB free]
  Total: 1 [3.81 GiB] / in use: 1 [3.81 GiB] / in no VG: 0 [0   ]
$ sudo lvm lvscan
  ACTIVE            '/dev/foobar/wtf' [256.00 MiB] inherit
(failed reverse-i-search)`hexd': sudo MKFS_OPTIONS="-m crc=1,finobt=1" ./c^Cck -g auto
$ sudo hexdump /dev/mapper/foobar-wtf
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
10000000
$ sudo hexdump /dev/ram1
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0000200 414c 4542 4f4c 454e 0001 0000 0000 0000
0000210 5d39 644e 0020 0000 564c 324d 3020 3130
0000220 6a54 6b6b 5a5a 7175 4c70 5434 5866 6450
0000230 4f73 6552 5330 3333 4c79 4157 6144 7678
0000240 0000 f424 0000 0000 0000 0010 0000 0000
.....
$ sudo mkfs.xfs /dev/mapper/foobar-wtf
meta-data=/dev/mapper/foobar-wtf isize=256    agcount=4, agsize=16384 blks
         =                       sectsz=4096  attr=2, projid32bit=1
         =                       crc=0        finobt=0
data     =                       bsize=4096   blocks=65536, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=0
log      =internal log           bsize=4096   blocks=1605, version=2
         =                       sectsz=4096  sunit=1 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
$ sudo hexdump /dev/ram1
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
f4240000
$ sudo lvm lvscan
  No volume groups found
$

because running mkfs.xfs on the LVM volume is causing the entire
underlying physical device to be erased. The machine has to be
rebooted at this point because /dev/ram1 is now stuck with
references that cannot be removed.

Oh, I bet the problem is the final flush ioctl that mkfs sends:

3420  pwrite(4, "XFSB\0\0\20\0\0\0\0\0\0\1\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096, 0) = 4096
3420  fsync(4)                          = 0
3420  ioctl(4, BLKFLSBUF, 0)            = 0
3420  close(4)                          = 0
3420  exit_group(0)                     = ?
3420  +++ exited with 0 +++

Now, when mkfs.xfs is run on the ramdisk, it has a reference to the
ramdisk and so brd ignores BLKFLSBUF. I bet that LVM doesn't hold a
reference to the underlying block device, and so when brd receives
the BLKFLSBUF passed down from /dev/mapper/foobar-wtf it sees no
references and so removes all pages...

This seems like a proper screwup - the brd code is abusing BLKFLSBUF
to mean "free all the memory" rather than "flush dirty buffers to
stable storage" like everyone else expects it to be.

Hence I suspect we need a new _requires rule here for DM based tests
to stop them from running on brd devices. I'll have a look at
rolling it into the DM requires statements and see how that goes...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Dave Chinner]

On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> xfs used to panic in this test, this xfs commit fix the bug
> 
> 8d6c121 xfs: fix buffer use after free on IO error
> 
> ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
....
> +# lvm uses some space for metadata
> +_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1
> +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
> +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
> +
> +# _mkfs_dev exits the test on failure, this can make sure lv is created in
> +# above vgcreate/lvcreate steps
> +_mkfs_dev /dev/mapper/$vgname-$lvname

So on my 1p test VM, this fails with

+mkfs.xfs: cannot open /dev/mapper/vg_081-base_081: Device or resource busy

The problem is that udev has not finished setting up the device
before mkfs is run. Hence we need a "udevadm settle" call after the
lvcreate call. This results in mkfs succeeding on this machine.

Eryu, I'm going to commit the test as it stands as it works on all
my other test systems - can you write a followup patch that does the
udev settle call in a portable manner? i.e. older systems used to
have a 'udev-settle' command, do we still care about that?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Eryu Guan]

On Thu, Apr 02, 2015 at 09:41:01AM +1100, Dave Chinner wrote:
> On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> > xfs used to panic in this test, this xfs commit fix the bug
> > 
> > 8d6c121 xfs: fix buffer use after free on IO error
> > 
> > ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> ....
> > +# lvm uses some space for metadata
> > +_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1
> > +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
> > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
> > +
> > +# _mkfs_dev exits the test on failure, this can make sure lv is created in
> > +# above vgcreate/lvcreate steps
> > +_mkfs_dev /dev/mapper/$vgname-$lvname
> 
> So on my 1p test VM, this fails with
> 
> +mkfs.xfs: cannot open /dev/mapper/vg_081-base_081: Device or resource busy
> 
> The problem is that udev has not finished setting up the device
> before mkfs is run. Hence we need a "udevadm settle" call after the
> lvcreate call. This results in mkfs succeeding on this machine.
> 
> Eryu, I'm going to commit the test as it stands as it works on all
> my other test systems - can you write a followup patch that does the
> udev settle call in a portable manner? i.e. older systems used to
> have a 'udev-settle' command, do we still care about that?

Sure, I'll do that, along with the fix of the cleanup error, as Brian
suggested.

As a distribution tester of RHEL, I don't care much about older systems
now like RHEL5, but I'll try to make it portable if possible.

Thanks,
Eryu

[PATCH 7/9] common: recognise NFS export over IPv6 in _require_scratch_nocheck()

[Eryu Guan]

This commit

73dfa4a common: Fixes for testing NFS over IPv6

adds NFS over IPv6 support, and commit

76c5f3c common: re-enable tests that require scratch dev on NFS

enables NFS tests on scratch device.

Now do the same updates to _require_scratch_nocheck() to enable NFS over
IPv6 support on scratch device.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 common/rc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 857308a..fbb19c2 100644
--- a/common/rc
+++ b/common/rc
@@ -1111,10 +1111,12 @@ _require_scratch_nocheck()
     esac
 
     # mounted?
-    if _mount | grep -q $SCRATCH_DEV
+    # Note that we use -F here so grep doesn't try to interpret an NFS over
+    # IPv6 server as a regular expression.
+    if _mount | grep -F -q $SCRATCH_DEV
     then
         # if it's mounted, make sure its on $SCRATCH_MNT
-        if ! _mount | grep $SCRATCH_DEV | grep -q $SCRATCH_MNT
+        if ! _mount | grep -F $SCRATCH_DEV | grep -q $SCRATCH_MNT
         then
             echo "\$SCRATCH_DEV is mounted but not on \$SCRATCH_MNT - aborting"
             exit 1
-- 
2.1.0

Re: [PATCH 7/9] common: recognise NFS export over IPv6 in _require_scratch_nocheck()

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:56PM +0800, Eryu Guan wrote:
> This commit
> 
> 73dfa4a common: Fixes for testing NFS over IPv6
> 
> adds NFS over IPv6 support, and commit
> 
> 76c5f3c common: re-enable tests that require scratch dev on NFS
> 
> enables NFS tests on scratch device.
> 
> Now do the same updates to _require_scratch_nocheck() to enable NFS over
> IPv6 support on scratch device.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---

Seems fine...

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  common/rc | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 857308a..fbb19c2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1111,10 +1111,12 @@ _require_scratch_nocheck()
>      esac
>  
>      # mounted?
> -    if _mount | grep -q $SCRATCH_DEV
> +    # Note that we use -F here so grep doesn't try to interpret an NFS over
> +    # IPv6 server as a regular expression.
> +    if _mount | grep -F -q $SCRATCH_DEV
>      then
>          # if it's mounted, make sure its on $SCRATCH_MNT
> -        if ! _mount | grep $SCRATCH_DEV | grep -q $SCRATCH_MNT
> +        if ! _mount | grep -F $SCRATCH_DEV | grep -q $SCRATCH_MNT
>          then
>              echo "\$SCRATCH_DEV is mounted but not on \$SCRATCH_MNT - aborting"
>              exit 1
> -- 
> 2.1.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 8/9] xfs/014: replace df with $DF_PROG

[Eryu Guan]

I've seen xfs/014 fails as

        [root@dhcp-66-86-3 xfstests]# diff -u tests/xfs/014.out /var/lib/xfstests/results//xfs/014.out.bad
        --- tests/xfs/014.out   2015-03-06 14:48:19.000000000 +0800
        +++ /var/lib/xfstests/results//xfs/014.out.bad  2015-03-09
        22:48:08.660001935 +0800
        @@ -1,2 +1,9 @@
         QA output created by 014
          Silence is golden.
          +falloc: invalid option -- '1'
          +falloc: invalid option -- '0'
          +falloc: invalid option -- 'M'
          +falloc [-k] off len -- allocates space associated with part of a file
          via fallocate
          +falloc [-k] off len -- allocates space associated with part of a file
          via fallocate
          +falloc [-k] off len -- allocates space associated with part of a file
          via fallocate
          +falloc [-k] off len -- allocates space associated with part of a file
          via fallocate

which is because output of "df -m" is split into two lines, and
freesp is 0, in _consume_free_space() function.

        Filesystem           1M-blocks  Used Available Use% Mounted on
        /mnt/testarea/scratch/014.fs
                                 10230  1061      9170  11% /mnt/testarea/scratch/014.mnt

Appending '-P' option to df fixes this issue. So use $DF_PROG which
already has -P option set.

$DF_PROG also sets -T option to print fs type in extra column, so change
the awk command accordingly to get the correct freespace.

Also replace awk with $AWK_PROG.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/xfs/014 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xfs/014 b/tests/xfs/014
index 8866bfe..ab3d85b 100755
--- a/tests/xfs/014
+++ b/tests/xfs/014
@@ -88,7 +88,7 @@ _consume_free_space()
 	dir=$1
 
 	# allocate all but 10MB of available space
-	freesp=`df -m $dir | awk '/^\// { print $4 - 10 }'`
+	freesp=`$DF_PROG -m $dir | $AWK_PROG '/^\// { print $5 - 10 }'`
 	$XFS_IO_PROG -f -c "falloc 0 ${freesp}M" $dir/spc
 }
 
-- 
2.1.0

Re: [PATCH 8/9] xfs/014: replace df with $DF_PROG

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:57PM +0800, Eryu Guan wrote:
> I've seen xfs/014 fails as
> 
>         [root@dhcp-66-86-3 xfstests]# diff -u tests/xfs/014.out /var/lib/xfstests/results//xfs/014.out.bad
>         --- tests/xfs/014.out   2015-03-06 14:48:19.000000000 +0800
>         +++ /var/lib/xfstests/results//xfs/014.out.bad  2015-03-09
>         22:48:08.660001935 +0800
>         @@ -1,2 +1,9 @@
>          QA output created by 014
>           Silence is golden.
>           +falloc: invalid option -- '1'
>           +falloc: invalid option -- '0'
>           +falloc: invalid option -- 'M'
>           +falloc [-k] off len -- allocates space associated with part of a file
>           via fallocate
>           +falloc [-k] off len -- allocates space associated with part of a file
>           via fallocate
>           +falloc [-k] off len -- allocates space associated with part of a file
>           via fallocate
>           +falloc [-k] off len -- allocates space associated with part of a file
>           via fallocate
> 
> which is because output of "df -m" is split into two lines, and
> freesp is 0, in _consume_free_space() function.
> 
>         Filesystem           1M-blocks  Used Available Use% Mounted on
>         /mnt/testarea/scratch/014.fs
>                                  10230  1061      9170  11% /mnt/testarea/scratch/014.mnt
> 
> Appending '-P' option to df fixes this issue. So use $DF_PROG which
> already has -P option set.
> 
> $DF_PROG also sets -T option to print fs type in extra column, so change
> the awk command accordingly to get the correct freespace.
> 
> Also replace awk with $AWK_PROG.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---

Looks good..

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  tests/xfs/014 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/014 b/tests/xfs/014
> index 8866bfe..ab3d85b 100755
> --- a/tests/xfs/014
> +++ b/tests/xfs/014
> @@ -88,7 +88,7 @@ _consume_free_space()
>  	dir=$1
>  
>  	# allocate all but 10MB of available space
> -	freesp=`df -m $dir | awk '/^\// { print $4 - 10 }'`
> +	freesp=`$DF_PROG -m $dir | $AWK_PROG '/^\// { print $5 - 10 }'`
>  	$XFS_IO_PROG -f -c "falloc 0 ${freesp}M" $dir/spc
>  }
>  
> -- 
> 2.1.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 9/9] generic/077: add missing _require_scratch

[Eryu Guan]

generic/077 uses $SCRATCH_DEV without _require_scratch. Add it back.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/generic/077 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/077 b/tests/generic/077
index 0d6aec7..8405b02 100755
--- a/tests/generic/077
+++ b/tests/generic/077
@@ -54,6 +54,7 @@ _supported_os Linux
 
 [ ! -d $filler ] && _notrun "No directory to source files from"
 
+_require_scratch
 _require_attrs
 _require_acls
 _require_user
-- 
2.1.0

Re: [PATCH 9/9] generic/077: add missing _require_scratch

[Brian Foster]

On Fri, Mar 20, 2015 at 06:16:58PM +0800, Eryu Guan wrote:
> generic/077 uses $SCRATCH_DEV without _require_scratch. Add it back.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---

Looks good...

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  tests/generic/077 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/077 b/tests/generic/077
> index 0d6aec7..8405b02 100755
> --- a/tests/generic/077
> +++ b/tests/generic/077
> @@ -54,6 +54,7 @@ _supported_os Linux
>  
>  [ ! -d $filler ] && _notrun "No directory to source files from"
>  
> +_require_scratch
>  _require_attrs
>  _require_acls
>  _require_user
> -- 
> 2.1.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