public inbox for fstests@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP
@ 2016-05-29  4:28 Zorro Lang
  2016-05-29  4:28 ` [PATCH v3 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang
  2016-06-11 14:18 ` [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP Eryu Guan
  0 siblings, 2 replies; 4+ messages in thread
From: Zorro Lang @ 2016-05-29  4:28 UTC (permalink / raw)
  To: fstests; +Cc: sandeen, Zorro Lang

XFS add more configurations in /sys/fs/xfs recently. For use
them, this patch add some common functions for:
  1. "require" a file/dir in /sys/fs/${FSTYP}.
  2. write a file in /sys/fs/${FSTYP}.

For common use, these functions can be used by other filesystems.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

V3 revert dname=$(basename $(readlink -f $dev)) line.

I think we don't need to check if /sys/fs/${FSTYP}/${dname}/$target
is exist in _set_fs_sys_fs_param(). Because if no this file,
*echo* will report error, and it will disturb golden file.

Thanks,
Zorro

 common/rc | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/common/rc b/common/rc
index 51092a0..759ec30 100644
--- a/common/rc
+++ b/common/rc
@@ -3556,6 +3556,46 @@ run_fsx()
 	fi
 }
 
+_require_fs_sys_fs()
+{
+	local dev=$1
+	local target=$2
+	local tmp_mnt=`mktemp -d`
+
+	if [ ! -b "$dev" -o -z "$target" ];then
+		_fail "Usage: _require_fs_sys_fs <device> <sysfs_path>"
+        fi
+
+	local dname=$(basename $(readlink -f $dev))
+	_mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt
+	if [ $? -ne 0 ];then
+		rm -f $tmp_mnt
+		_fail "could not mount; mkfs first in your test?"
+	elif [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then
+		umount $tmp_mnt
+		rm -f $tmp_mnt
+		_notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory"
+	fi
+	umount $tmp_mnt
+	rm -rf $tmp_mnt
+}
+
+_set_fs_sys_fs_param()
+{
+	local dev=$1
+	shift
+	local target=$1
+	shift
+	local content="$*"
+
+	if [ ! -b "$dev" -o -z "$target" -o -z "$content" ];then
+		_fail "Usage: _set_sys_fs_param <mounted_device> <target> <content>"
+	fi
+
+	local dname=$(basename $(readlink -f $dev))
+	echo "$content" > /sys/fs/${FSTYP}/${dname}/$target
+}
+
 init_rc
 
 ################################################################################
-- 
2.5.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] xfs/006: new case to test xfs fail_at_unmount error handling
  2016-05-29  4:28 [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP Zorro Lang
@ 2016-05-29  4:28 ` Zorro Lang
  2016-06-11 14:39   ` Eryu Guan
  2016-06-11 14:18 ` [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP Eryu Guan
  1 sibling, 1 reply; 4+ messages in thread
From: Zorro Lang @ 2016-05-29  4:28 UTC (permalink / raw)
  To: fstests; +Cc: sandeen, Zorro Lang

XFS bring in a new configuration under /sys/fs/xfs/error, named
fail_at_unmount. It's used to stop unmount retrying forever when
it hit IO error.

This case try to unmount an faulty dm device, and to sure unmount
won't retry forever if fail_at_unmount=1.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

Nothing changed in this V3. Due to I modify [PATCH v3 1/2], so I think
if I send this 2/2 patch with 1/2 together, maybe easy to review and
merge? :)

Thanks,
Zorro

 tests/xfs/006     | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/006.out |  2 ++
 tests/xfs/group   |  1 +
 3 files changed, 84 insertions(+)
 create mode 100755 tests/xfs/006
 create mode 100644 tests/xfs/006.out

diff --git a/tests/xfs/006 b/tests/xfs/006
new file mode 100755
index 0000000..c134b1c
--- /dev/null
+++ b/tests/xfs/006
@@ -0,0 +1,81 @@
+#! /bin/bash
+# FS QA Test 006
+#
+# Test xfs' "fail at unmount" error handling configuration. Stop
+# XFS retrying to writeback forever when unmount.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016 YOUR NAME HERE.  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.*
+	_dmerror_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmerror
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs xfs
+_supported_os Linux
+_require_dm_target error
+_require_scratch
+
+_scratch_mkfs > $seqres.full 2>&1
+_require_fs_sys_fs $SCRATCH_DEV error/fail_at_unmount
+
+# The device is still a linear device at here
+_dmerror_init
+_dmerror_mount
+
+# Enable fail_at_unmount
+_set_fs_sys_fs_param $DMERROR_DEV error/fail_at_unmount 1
+
+# make some IO at here. If these data still not writeback from
+# journal to data, it will hit writeback error after load error
+# tabel in next step.
+_pwrite_byte 0x61 0 100M $SCRATCH_MNT/file >/dev/null
+_dmerror_load_error_table
+
+# Even above data already all writeback, umount will cause XFS
+# try to writeback something to root inode. It still can trigger
+# umount fail.
+_dmerror_unmount
+
+echo "Silence is golden"
+_dmerror_cleanup
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/006.out b/tests/xfs/006.out
new file mode 100644
index 0000000..675c1b7
--- /dev/null
+++ b/tests/xfs/006.out
@@ -0,0 +1,2 @@
+QA output created by 006
+Silence is golden
diff --git a/tests/xfs/group b/tests/xfs/group
index f4c6816..39169ea 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -4,6 +4,7 @@
 004 db auto quick
 005 auto quick
 007 auto quota quick
+006 auto quick unmount
 008 rw ioctl auto quick
 009 rw ioctl auto prealloc quick
 010 auto quick repair
-- 
2.5.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP
  2016-05-29  4:28 [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP Zorro Lang
  2016-05-29  4:28 ` [PATCH v3 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang
@ 2016-06-11 14:18 ` Eryu Guan
  1 sibling, 0 replies; 4+ messages in thread
From: Eryu Guan @ 2016-06-11 14:18 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, sandeen

On Sun, May 29, 2016 at 12:28:40PM +0800, Zorro Lang wrote:
> XFS add more configurations in /sys/fs/xfs recently. For use
> them, this patch add some common functions for:
>   1. "require" a file/dir in /sys/fs/${FSTYP}.
>   2. write a file in /sys/fs/${FSTYP}.
> 
> For common use, these functions can be used by other filesystems.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> V3 revert dname=$(basename $(readlink -f $dev)) line.
> 
> I think we don't need to check if /sys/fs/${FSTYP}/${dname}/$target
> is exist in _set_fs_sys_fs_param(). Because if no this file,
> *echo* will report error, and it will disturb golden file.
> 
> Thanks,
> Zorro
> 
>  common/rc | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 51092a0..759ec30 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3556,6 +3556,46 @@ run_fsx()
>  	fi
>  }
>  
> +_require_fs_sys_fs()

_require_fs_sysfs seems better to me :)

And a comment about the purpose of this helper would be fine, e.g. the
comment Eric wrote in his review:

"Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY"

> +{
> +	local dev=$1
> +	local target=$2
> +	local tmp_mnt=`mktemp -d`
> +
> +	if [ ! -b "$dev" -o -z "$target" ];then
> +		_fail "Usage: _require_fs_sys_fs <device> <sysfs_path>"
> +        fi
> +
> +	local dname=$(basename $(readlink -f $dev))

We do have a helper "_short_dev" to do this, e.g.

	local dname=`_short_dev $dev`

> +	_mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt
> +	if [ $? -ne 0 ];then
> +		rm -f $tmp_mnt
> +		_fail "could not mount; mkfs first in your test?"

Having function name as prefix in error message makes debugging easier,
I think.

> +	elif [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then
> +		umount $tmp_mnt

Use $UMOUNT_PROG here, and in all other umount calls.

> +		rm -f $tmp_mnt
> +		_notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory"
> +	fi
> +	umount $tmp_mnt
> +	rm -rf $tmp_mnt
> +}
> +
> +_set_fs_sys_fs_param()

_set_fs_sysfs_param :) And a comment about its purpose.

Thanks,
Eryu

> +{
> +	local dev=$1
> +	shift
> +	local target=$1
> +	shift
> +	local content="$*"
> +
> +	if [ ! -b "$dev" -o -z "$target" -o -z "$content" ];then
> +		_fail "Usage: _set_sys_fs_param <mounted_device> <target> <content>"
> +	fi
> +
> +	local dname=$(basename $(readlink -f $dev))
> +	echo "$content" > /sys/fs/${FSTYP}/${dname}/$target
> +}
> +
>  init_rc
>  
>  ################################################################################
> -- 
> 2.5.5
> 
> --
> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 2/2] xfs/006: new case to test xfs fail_at_unmount error handling
  2016-05-29  4:28 ` [PATCH v3 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang
@ 2016-06-11 14:39   ` Eryu Guan
  0 siblings, 0 replies; 4+ messages in thread
From: Eryu Guan @ 2016-06-11 14:39 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, sandeen

On Sun, May 29, 2016 at 12:28:41PM +0800, Zorro Lang wrote:
> XFS bring in a new configuration under /sys/fs/xfs/error, named
> fail_at_unmount. It's used to stop unmount retrying forever when
> it hit IO error.

Refering to the kernel commit IDs in commit log would be good for future
reference.

> 
> This case try to unmount an faulty dm device, and to sure unmount
> won't retry forever if fail_at_unmount=1.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> Nothing changed in this V3. Due to I modify [PATCH v3 1/2], so I think
> if I send this 2/2 patch with 1/2 together, maybe easy to review and
> merge? :)
> 
> Thanks,
> Zorro
> 
>  tests/xfs/006     | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/006.out |  2 ++
>  tests/xfs/group   |  1 +
>  3 files changed, 84 insertions(+)
>  create mode 100755 tests/xfs/006
>  create mode 100644 tests/xfs/006.out
> 
> diff --git a/tests/xfs/006 b/tests/xfs/006
> new file mode 100755
> index 0000000..c134b1c
> --- /dev/null
> +++ b/tests/xfs/006
> @@ -0,0 +1,81 @@
> +#! /bin/bash
> +# FS QA Test 006
> +#
> +# Test xfs' "fail at unmount" error handling configuration. Stop
> +# XFS retrying to writeback forever when unmount.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2016 YOUR NAME HERE.  All Rights Reserved.
                        ^^^^^^^^^^^^^^ Red Hat Inc. :)
> +#
> +# 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.*
> +	_dmerror_cleanup
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmerror
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_supported_os Linux
> +_require_dm_target error
> +_require_scratch
> +
> +_scratch_mkfs > $seqres.full 2>&1
> +_require_fs_sys_fs $SCRATCH_DEV error/fail_at_unmount
> +
> +# The device is still a linear device at here
> +_dmerror_init
> +_dmerror_mount
> +
> +# Enable fail_at_unmount
> +_set_fs_sys_fs_param $DMERROR_DEV error/fail_at_unmount 1

I tested this by setting "fail_at_unmount 0" on 4.7-rc1 kernel, I think
test should hang at umount time in this case, but test still passed.
Seems the case didn't test what it supposed to test.

> +
> +# make some IO at here. If these data still not writeback from
> +# journal to data, it will hit writeback error after load error
> +# tabel in next step.
> +_pwrite_byte 0x61 0 100M $SCRATCH_MNT/file >/dev/null
> +_dmerror_load_error_table
> +
> +# Even above data already all writeback, umount will cause XFS
> +# try to writeback something to root inode. It still can trigger
> +# umount fail.
> +_dmerror_unmount
> +
> +echo "Silence is golden"
> +_dmerror_cleanup

It's called in _cleanup, not needed here.

Thanks,
Eryu

> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/006.out b/tests/xfs/006.out
> new file mode 100644
> index 0000000..675c1b7
> --- /dev/null
> +++ b/tests/xfs/006.out
> @@ -0,0 +1,2 @@
> +QA output created by 006
> +Silence is golden
> diff --git a/tests/xfs/group b/tests/xfs/group
> index f4c6816..39169ea 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -4,6 +4,7 @@
>  004 db auto quick
>  005 auto quick
>  007 auto quota quick
> +006 auto quick unmount
>  008 rw ioctl auto quick
>  009 rw ioctl auto prealloc quick
>  010 auto quick repair
> -- 
> 2.5.5
> 
> --
> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-06-11 14:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-29  4:28 [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP Zorro Lang
2016-05-29  4:28 ` [PATCH v3 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang
2016-06-11 14:39   ` Eryu Guan
2016-06-11 14:18 ` [PATCH v3 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP Eryu Guan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox