* [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs @ 2016-05-27 15:21 Zorro Lang 2016-05-27 15:21 ` [PATCH 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang 2016-05-27 18:05 ` [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Eric Sandeen 0 siblings, 2 replies; 10+ messages in thread From: Zorro Lang @ 2016-05-27 15:21 UTC (permalink / raw) To: fstests; +Cc: cem, 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. 2. write a file in /sys/fs. For common use, these functions can be used by other filesystems. Signed-off-by: Zorro Lang <zlang@redhat.com> --- common/rc | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/common/rc b/common/rc index 51092a0..9935f08 100644 --- a/common/rc +++ b/common/rc @@ -3556,6 +3556,87 @@ run_fsx() fi } +_require_sys_fs() +{ + local dev=$1 + local target=$2 + local dname="" + + if [ ! -b "$dev" -o -z "$target" ];then + echo "Usage: _require_sys_fs <device> <target_need_exist>" + exit 1 + fi + + dname=$(basename $(readlink -f $dev)) + _mkfs_dev $dev > /dev/null 2>&1 + _mount -t $FSTYP `_common_dev_mount_options` $dev $SCRATCH_MNT + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then + umount $SCRATCH_MNT + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" + fi + umount $SCRATCH_MNT +} + +_require_scratch_sys_fs() +{ + local target=$1 + local dname="" + + if [ ! -b "$SCRATCH_DEV" -o -z "$target" ];then + echo "Usage: _require_scratch_sys_fs <target_need_exist>" + exit 1 + fi + + dname=$(basename $(readlink -f $SCRATCH_DEV)) + _scratch_mkfs > /dev/null 2>&1 + _scratch_mount + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then + _scratch_unmount + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" + fi + _scratch_unmount +} + +_set_sys_fs_param() +{ + local dev=$1 + shift + local target=$1 + shift + local content="$*" + + if [ ! -b "$dev" -o -z "$target" -o -z "$content" ];then + echo "Usage: _set_sys_fs_param <mounted_device> <target> <content>" + exit 1 + fi + local dname=$(basename $(readlink -f $dev)) + echo "$content" > /sys/fs/${FSTYP}/${dname}/$target +} + +_enable_xfs_fail_at_unmount() +{ + local dev=$1 + + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" + exit 1 + fi + + _set_sys_fs_param $dev error/fail_at_unmount 1 +} + +_disable_xfs_fail_at_unmount() +{ + local dev=$1 + + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" + exit 1 + fi + + _set_sys_fs_param $dev error/fail_at_unmount 0 +} + init_rc ################################################################################ -- 2.5.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] xfs/006: new case to test xfs fail_at_unmount error handling 2016-05-27 15:21 [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Zorro Lang @ 2016-05-27 15:21 ` Zorro Lang 2016-05-27 18:26 ` Eric Sandeen 2016-05-27 18:05 ` [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Eric Sandeen 1 sibling, 1 reply; 10+ messages in thread From: Zorro Lang @ 2016-05-27 15:21 UTC (permalink / raw) To: fstests; +Cc: cem, 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, Thanks Carlos told this easy way(use dm error) to test this feature, or I will try to use dm thinp to reproduce unmount hit an ENOSPC error. That's more difficult to do. I haven't gotten some good ideas to test those /sys/fs/xfs/dm-X/error/ metadata/... configurations. If someone have good ideas, please let me know:) Thanks, Zorro tests/xfs/006 | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/006.out | 2 ++ tests/xfs/group | 1 + 3 files changed, 83 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..dafab50 --- /dev/null +++ b/tests/xfs/006 @@ -0,0 +1,80 @@ +#! /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.* +} + +# 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 +_require_scratch_sys_fs error/fail_at_unmount + +# Format and mount +# The device is still a linear device at here +_scratch_mkfs > $seqres.full 2>&1 +_dmerror_init +_dmerror_mount + +# Enable fail_at_unmount +_enable_xfs_fail_at_unmount $DMERROR_DEV + +# 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] 10+ messages in thread
* Re: [PATCH 2/2] xfs/006: new case to test xfs fail_at_unmount error handling 2016-05-27 15:21 ` [PATCH 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang @ 2016-05-27 18:26 ` Eric Sandeen 2016-05-28 16:56 ` Zorro Lang 0 siblings, 1 reply; 10+ messages in thread From: Eric Sandeen @ 2016-05-27 18:26 UTC (permalink / raw) To: Zorro Lang, fstests; +Cc: cem On 5/27/16 10:21 AM, 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. > > 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, > > Thanks Carlos told this easy way(use dm error) to test this feature, or > I will try to use dm thinp to reproduce unmount hit an ENOSPC error. That's > more difficult to do. right, different errors from dm_thinp. But I think my simple dm_thinp test and the infrastructure it uses should make this easy? It was merged recently. > I haven't gotten some good ideas to test those /sys/fs/xfs/dm-X/error/ > metadata/... configurations. If someone have good ideas, please let me > know:) > > Thanks, > Zorro > > tests/xfs/006 | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/006.out | 2 ++ > tests/xfs/group | 1 + > 3 files changed, 83 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..dafab50 > --- /dev/null > +++ b/tests/xfs/006 > @@ -0,0 +1,80 @@ > +#! /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.* > +} > + > +# 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 > +_require_scratch_sys_fs error/fail_at_unmount > + > +# Format and mount > +# The device is still a linear device at here > +_scratch_mkfs > $seqres.full 2>&1 I guess my earlier suggestion was to not mkfs inside the _require function for sysfs, so maybe move the test here: _require_fs_sys_fs $SCRATCH_DEV error/fail_at_unmount > +_dmerror_init > +_dmerror_mount > + > +# Enable fail_at_unmount > +_enable_xfs_fail_at_unmount $DMERROR_DEV _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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfs/006: new case to test xfs fail_at_unmount error handling 2016-05-27 18:26 ` Eric Sandeen @ 2016-05-28 16:56 ` Zorro Lang 0 siblings, 0 replies; 10+ messages in thread From: Zorro Lang @ 2016-05-28 16:56 UTC (permalink / raw) To: Eric Sandeen; +Cc: fstests On Fri, May 27, 2016 at 01:26:35PM -0500, Eric Sandeen wrote: > On 5/27/16 10:21 AM, 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. > > > > 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, > > > > Thanks Carlos told this easy way(use dm error) to test this feature, or > > I will try to use dm thinp to reproduce unmount hit an ENOSPC error. That's > > more difficult to do. > > right, different errors from dm_thinp. > > But I think my simple dm_thinp test and the infrastructure it uses > should make this easy? It was merged recently. Yeah, those dmthin functions looks fine for me, I haven't use them(maybe use them soon). I said use dmthin is "more difficult", not means use your functions is difficult, I mean use dmthin to test this "retry forever" issue is difficult. Because if use dmthinp, I should make sure when I do unmount, there're some journal have been recorded, but the data haven't be writeback. So when unmount try to flush these data, it will hit ENOSPC. right? I think it's too difficult to create this situation. But if use dmerror, it's easy to make unmount hit EIO error. Thanks, Zorro > > > I haven't gotten some good ideas to test those /sys/fs/xfs/dm-X/error/ > > metadata/... configurations. If someone have good ideas, please let me > > know:) > > > > Thanks, > > Zorro > > > > tests/xfs/006 | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/xfs/006.out | 2 ++ > > tests/xfs/group | 1 + > > 3 files changed, 83 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..dafab50 > > --- /dev/null > > +++ b/tests/xfs/006 > > @@ -0,0 +1,80 @@ > > +#! /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.* > > +} > > + > > +# 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 > > +_require_scratch_sys_fs error/fail_at_unmount > > + > > +# Format and mount > > +# The device is still a linear device at here > > +_scratch_mkfs > $seqres.full 2>&1 > > I guess my earlier suggestion was to not mkfs inside the > _require function for sysfs, so maybe move the test here: > > _require_fs_sys_fs $SCRATCH_DEV error/fail_at_unmount hmm... I think generally we do _require_* test before do real test. But do _require_* test after mkfs is still better than after mount:) > > > +_dmerror_init > > +_dmerror_mount > > + > > +# Enable fail_at_unmount > > +_enable_xfs_fail_at_unmount $DMERROR_DEV > > _set_fs_sys_fs_param $DMERROR_DEV error/fail_at_unmount 1 Sure Thanks again for your review, Zorro > > > + > > +# 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 > > > -- > 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] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs 2016-05-27 15:21 [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Zorro Lang 2016-05-27 15:21 ` [PATCH 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang @ 2016-05-27 18:05 ` Eric Sandeen 2016-05-27 18:27 ` Eric Sandeen 2016-05-28 16:36 ` Zorro Lang 1 sibling, 2 replies; 10+ messages in thread From: Eric Sandeen @ 2016-05-27 18:05 UTC (permalink / raw) To: Zorro Lang, fstests; +Cc: cem On 5/27/16 10:21 AM, 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. > 2. write a file in /sys/fs. More specifically, in /sys/fs/${FSTYP} > For common use, these functions can be used by other filesystems. > > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > common/rc | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 81 insertions(+) > > diff --git a/common/rc b/common/rc > index 51092a0..9935f08 100644 > --- a/common/rc > +++ b/common/rc > @@ -3556,6 +3556,87 @@ run_fsx() > fi > } > > +_require_sys_fs() > +{ > + local dev=$1 > + local target=$2 > + local dname="" > + > + if [ ! -b "$dev" -o -z "$target" ];then > + echo "Usage: _require_sys_fs <device> <target_need_exist>" > + exit 1 > + fi > + > + dname=$(basename $(readlink -f $dev)) > + _mkfs_dev $dev > /dev/null 2>&1 we don't want to mkfs the test dev just for a _require test; that device is supposed to age, and this could possibly take a very long time, as well. And in general a mkfs should not be required to test the existence of a sysfs tunable, I think? > + _mount -t $FSTYP `_common_dev_mount_options` $dev $SCRATCH_MNT > + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then > + umount $SCRATCH_MNT > + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" > + fi > + umount $SCRATCH_MNT > +} > + > +_require_scratch_sys_fs() > +{ > + local target=$1 > + local dname="" > + > + if [ ! -b "$SCRATCH_DEV" -o -z "$target" ];then > + echo "Usage: _require_scratch_sys_fs <target_need_exist>" > + exit 1 > + fi > + > + dname=$(basename $(readlink -f $SCRATCH_DEV)) > + _scratch_mkfs > /dev/null 2>&1 > + _scratch_mount > + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then > + _scratch_unmount > + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" > + fi > + _scratch_unmount > +} This seems overly complex; do you need both of these functions? We are only testing one filesystem type at a time, and $TEST_DEV is always required. What is the reason for the two differing functions? Isn't _require_sys_fs $SCRATCH_DEV exactly the same as _require_scratch_sys_fs ? Also, because $SCRATCH_DEV is not required, $SCRATCH_MNT isn't either, so I don't think you can use $SCRATCH_MNT. Also :) We may one day want to require sysfs files which are not in the fs/ namespace, so it should probably be named according to the namespace. So maybe something like this (not tested) + /* Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY */ +_require_fs_sys_fs() +{ + local dev=$1 + local target=$2 + local dname="" + local tmp_mnt=`mktemp -d` + + if [ ! -b "$dev" -o -z "$target" ];then + echo "Usage: _require_fs_sys_fs <device> <sysfs_path>" + exit 1 + fi + + dname=$(basename $(readlink -f $dev)) + _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt + if [ ! -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 $SCRATCH_MNT + rm -f $tmp_mnt +} and it would work for either SCRATCH or TEST? > +_set_sys_fs_param() > +{ > + local dev=$1 > + shift > + local target=$1 > + shift > + local content="$*" > + > + if [ ! -b "$dev" -o -z "$target" -o -z "$content" ];then > + echo "Usage: _set_sys_fs_param <mounted_device> <target> <content>" > + exit 1 > + fi > + local dname=$(basename $(readlink -f $dev)) > + echo "$content" > /sys/fs/${FSTYP}/${dname}/$target > +} This seems fine. Normally I'd say this could just be open coded in the test, but the dname=$(basename $(readlink -f $dev)) is worth encapsulating in the helper. > +_enable_xfs_fail_at_unmount() > +{ > + local dev=$1 > + > + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then > + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" > + exit 1 > + fi > + > + _set_sys_fs_param $dev error/fail_at_unmount 1 > +} > + > +_disable_xfs_fail_at_unmount() > +{ > + local dev=$1 > + > + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then > + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" > + exit 1 > + fi > + > + _set_sys_fs_param $dev error/fail_at_unmount 0 > +} I don't think this is needed; it's easy enough to just call: _set_fs_sys_fs_param $dev error/fail_at_unmount 0 _set_fs_sys_fs_param $dev error/fail_at_unmount 1 directly from tests. There are a lot of sysfs tunables, writing wrappers for each one isn't really that helpful. Thanks, -Eric > init_rc > > ################################################################################ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs 2016-05-27 18:05 ` [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Eric Sandeen @ 2016-05-27 18:27 ` Eric Sandeen 2016-05-28 16:42 ` Zorro Lang 2016-05-28 16:36 ` Zorro Lang 1 sibling, 1 reply; 10+ messages in thread From: Eric Sandeen @ 2016-05-27 18:27 UTC (permalink / raw) To: Zorro Lang, fstests; +Cc: cem On 5/27/16 1:05 PM, Eric Sandeen wrote: > So maybe something like this (not tested) > > + /* Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY */ > +_require_fs_sys_fs() > +{ > + local dev=$1 > + local target=$2 > + local dname="" > + local tmp_mnt=`mktemp -d` > + > + if [ ! -b "$dev" -o -z "$target" ];then > + echo "Usage: _require_fs_sys_fs <device> <sysfs_path>" > + exit 1 > + fi > + > + dname=$(basename $(readlink -f $dev)) > + _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt > + if [ ! -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 $SCRATCH_MNT > + rm -f $tmp_mnt > +} > > and it would work for either SCRATCH or TEST? I guess nothing guarantees that the device is already mkfs'd, at least for scratch, so maybe it also needs to catch a mount failure, with a message about "could not mount; mkfs first in your test?" or something like that. -Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs 2016-05-27 18:27 ` Eric Sandeen @ 2016-05-28 16:42 ` Zorro Lang 2016-05-28 17:54 ` Eric Sandeen 0 siblings, 1 reply; 10+ messages in thread From: Zorro Lang @ 2016-05-28 16:42 UTC (permalink / raw) To: Eric Sandeen; +Cc: fstests On Fri, May 27, 2016 at 01:27:56PM -0500, Eric Sandeen wrote: > On 5/27/16 1:05 PM, Eric Sandeen wrote: > > > So maybe something like this (not tested) > > > > + /* Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY */ > > +_require_fs_sys_fs() > > +{ > > + local dev=$1 > > + local target=$2 > > + local dname="" > > + local tmp_mnt=`mktemp -d` > > + > > + if [ ! -b "$dev" -o -z "$target" ];then > > + echo "Usage: _require_fs_sys_fs <device> <sysfs_path>" > > + exit 1 > > + fi > > + > > + dname=$(basename $(readlink -f $dev)) > > + _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt > > + if [ ! -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 $SCRATCH_MNT > > + rm -f $tmp_mnt > > +} > > > > and it would work for either SCRATCH or TEST? > > I guess nothing guarantees that the device is already mkfs'd, at least > for scratch, so maybe it also needs to catch a mount failure, > with a message about "could not mount; mkfs first in your test?" > or something like that. Yes, if we don't mkfs in _require_fs_sys_fs(), that means we need the caller make sure they do that. I will check the mount error likes: _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt if [ $? -ne 0 ];then rm -f $tmp_mnt _notrun "could not mount; mkfs first in your test?" elif [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then .... fi Thanks, Zorro > > -Eric > -- > 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] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs 2016-05-28 16:42 ` Zorro Lang @ 2016-05-28 17:54 ` Eric Sandeen 0 siblings, 0 replies; 10+ messages in thread From: Eric Sandeen @ 2016-05-28 17:54 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests On 5/28/16 11:42 AM, Zorro Lang wrote: > On Fri, May 27, 2016 at 01:27:56PM -0500, Eric Sandeen wrote: >> On 5/27/16 1:05 PM, Eric Sandeen wrote: >> >>> So maybe something like this (not tested) >>> >>> + /* Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY */ >>> +_require_fs_sys_fs() >>> +{ >>> + local dev=$1 >>> + local target=$2 >>> + local dname="" >>> + local tmp_mnt=`mktemp -d` >>> + >>> + if [ ! -b "$dev" -o -z "$target" ];then >>> + echo "Usage: _require_fs_sys_fs <device> <sysfs_path>" >>> + exit 1 >>> + fi >>> + >>> + dname=$(basename $(readlink -f $dev)) >>> + _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt >>> + if [ ! -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 $SCRATCH_MNT >>> + rm -f $tmp_mnt >>> +} >>> >>> and it would work for either SCRATCH or TEST? >> >> I guess nothing guarantees that the device is already mkfs'd, at least >> for scratch, so maybe it also needs to catch a mount failure, >> with a message about "could not mount; mkfs first in your test?" >> or something like that. > > Yes, if we don't mkfs in _require_fs_sys_fs(), that means we need the > caller make sure they do that. I will check the mount error likes: > > _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt > if [ $? -ne 0 ];then > rm -f $tmp_mnt > _notrun "could not mount; mkfs first in your test?" > elif [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then > .... > fi Yes, I think this is better. Any test using scratch should mkfs it anyway, and this way the test will not have to perform mkfs twice - once for the _require, and again for the test. Thanks, -Eric > Thanks, > Zorro ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs 2016-05-27 18:05 ` [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Eric Sandeen 2016-05-27 18:27 ` Eric Sandeen @ 2016-05-28 16:36 ` Zorro Lang 2016-05-28 18:06 ` Eric Sandeen 1 sibling, 1 reply; 10+ messages in thread From: Zorro Lang @ 2016-05-28 16:36 UTC (permalink / raw) To: Eric Sandeen; +Cc: fstests Hi Eric, Thanks for you can review this, as I know you're very busy recently:) On Fri, May 27, 2016 at 01:05:15PM -0500, Eric Sandeen wrote: > On 5/27/16 10:21 AM, 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. > > 2. write a file in /sys/fs. > > More specifically, in /sys/fs/${FSTYP} Sure, I should specify this:) > > > For common use, these functions can be used by other filesystems. > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > --- > > common/rc | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 81 insertions(+) > > > > diff --git a/common/rc b/common/rc > > index 51092a0..9935f08 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -3556,6 +3556,87 @@ run_fsx() > > fi > > } > > > > +_require_sys_fs() > > +{ > > + local dev=$1 > > + local target=$2 > > + local dname="" > > + > > + if [ ! -b "$dev" -o -z "$target" ];then > > + echo "Usage: _require_sys_fs <device> <target_need_exist>" > > + exit 1 > > + fi > > + > > + dname=$(basename $(readlink -f $dev)) > > + _mkfs_dev $dev > /dev/null 2>&1 > > we don't want to mkfs the test dev just for a _require test; that > device is supposed to age, and this could possibly take a very long > time, as well. And in general a mkfs should not be required to test > the existence of a sysfs tunable, I think? see below > > > + _mount -t $FSTYP `_common_dev_mount_options` $dev $SCRATCH_MNT > > + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then > > + umount $SCRATCH_MNT > > + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" > > + fi > > + umount $SCRATCH_MNT > > +} > > + > > +_require_scratch_sys_fs() > > +{ > > + local target=$1 > > + local dname="" > > + > > + if [ ! -b "$SCRATCH_DEV" -o -z "$target" ];then > > + echo "Usage: _require_scratch_sys_fs <target_need_exist>" > > + exit 1 > > + fi > > + > > + dname=$(basename $(readlink -f $SCRATCH_DEV)) > > + _scratch_mkfs > /dev/null 2>&1 > > + _scratch_mount > > + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then > > + _scratch_unmount > > + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" > > + fi > > + _scratch_unmount > > +} > > This seems overly complex; do you need both of these functions? We are only > testing one filesystem type at a time, and $TEST_DEV is always required. > What is the reason for the two differing functions? > > Isn't _require_sys_fs $SCRATCH_DEV exactly the same as > _require_scratch_sys_fs ? > Sorry for the "complex", let me explain: 1. At first, I write a function _require_sys_fs(), which used to check all kinds of device's /sys/fs/fstype configurations. Include dmerror, dmthinp and what ever other devices(not only TEST_DEV, SCRATCH_DEV). 2. Then I want to add a _require_scratch_sys_fs() function, which only used for SCRATCH_DEV. Because I feel generally if SCRATCH_DEV support a /sys/fs/fstype configurations, that means the working kernel support it. And _require_sys_fs() can be used by other devices write their _require_XXX_sys_fs() function(if needed... Looks like I thought too much....). 3. When I wrote _require_scratch_sys_fs(), I want to use _require_sys_fs $SCRATCH_DEV directly. But I saw _scratch_mkfs() use different options with common mkfs_dev() function. So I use _scratch_mkfs and _scratch_mount to write this _require_scratch_sys_fs(). 4. About why I need mkfs before in this _require function, at first those /sys/fs/fstype configurations only can be used after mount. The second is I hope _require_sys_fs() can be used by other devices, which maybe not mkfs... So.... > Also, because $SCRATCH_DEV is not required, $SCRATCH_MNT > isn't either, so I don't think you can use $SCRATCH_MNT. I did this according to _require_scratch_xfs_crc() function. > > Also :) We may one day want to require sysfs files which > are not in the fs/ namespace, so it should probably > be named according to the namespace. > > So maybe something like this (not tested) > > + /* Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY */ > +_require_fs_sys_fs() > +{ > + local dev=$1 > + local target=$2 > + local dname="" > + local tmp_mnt=`mktemp -d` > + > + if [ ! -b "$dev" -o -z "$target" ];then > + echo "Usage: _require_fs_sys_fs <device> <sysfs_path>" > + exit 1 > + fi > + > + dname=$(basename $(readlink -f $dev)) > + _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt > + if [ ! -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 $SCRATCH_MNT I think you mean "umount $tmp_mnt" at here > + rm -f $tmp_mnt > +} > > and it would work for either SCRATCH or TEST? hmm.. That's OK. I can't sure if mkfs before mount is better or not. If we don't mkfs in this function, we ask the caller keep sure the $dev has been mkfs. I think both are OK for me, I will follow your idea:) > > > +_set_sys_fs_param() > > +{ > > + local dev=$1 > > + shift > > + local target=$1 > > + shift > > + local content="$*" > > + > > + if [ ! -b "$dev" -o -z "$target" -o -z "$content" ];then > > + echo "Usage: _set_sys_fs_param <mounted_device> <target> <content>" > > + exit 1 > > + fi > > + local dname=$(basename $(readlink -f $dev)) > > + echo "$content" > /sys/fs/${FSTYP}/${dname}/$target > > +} > > This seems fine. Normally I'd say this could just be open coded > in the test, but the dname=$(basename $(readlink -f $dev)) is worth > encapsulating in the helper. Do you mean I should do?: local dname=$(readlink -f $dev) dname=`basename $dname` > > > +_enable_xfs_fail_at_unmount() > > +{ > > + local dev=$1 > > + > > + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then > > + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" > > + exit 1 > > + fi > > + > > + _set_sys_fs_param $dev error/fail_at_unmount 1 > > +} > > + > > +_disable_xfs_fail_at_unmount() > > +{ > > + local dev=$1 > > + > > + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then > > + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" > > + exit 1 > > + fi > > + > > + _set_sys_fs_param $dev error/fail_at_unmount 0 > > +} > > I don't think this is needed; it's easy enough to just call: > > _set_fs_sys_fs_param $dev error/fail_at_unmount 0 > _set_fs_sys_fs_param $dev error/fail_at_unmount 1 > > directly from tests. There are a lot of sysfs tunables, writing wrappers > for each one isn't really that helpful. Yeah, except they can check if "$FSTYP" = "xfs", they don't do more than use _set_fs_sys_fs_param directly. Thanks, Zorro > > Thanks, > -Eric > > > init_rc > > > > > ################################################################################ > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs 2016-05-28 16:36 ` Zorro Lang @ 2016-05-28 18:06 ` Eric Sandeen 0 siblings, 0 replies; 10+ messages in thread From: Eric Sandeen @ 2016-05-28 18:06 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests On 5/28/16 11:36 AM, Zorro Lang wrote: > Hi Eric, > > Thanks for you can review this, as I know you're very busy recently:) > > On Fri, May 27, 2016 at 01:05:15PM -0500, Eric Sandeen wrote: >> On 5/27/16 10:21 AM, 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. >>> 2. write a file in /sys/fs. >> >> More specifically, in /sys/fs/${FSTYP} > > Sure, I should specify this:) > >> >>> For common use, these functions can be used by other filesystems. >>> >>> Signed-off-by: Zorro Lang <zlang@redhat.com> >>> --- >>> common/rc | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 81 insertions(+) >>> >>> diff --git a/common/rc b/common/rc >>> index 51092a0..9935f08 100644 >>> --- a/common/rc >>> +++ b/common/rc >>> @@ -3556,6 +3556,87 @@ run_fsx() >>> fi >>> } >>> >>> +_require_sys_fs() >>> +{ >>> + local dev=$1 >>> + local target=$2 >>> + local dname="" >>> + >>> + if [ ! -b "$dev" -o -z "$target" ];then >>> + echo "Usage: _require_sys_fs <device> <target_need_exist>" >>> + exit 1 >>> + fi >>> + >>> + dname=$(basename $(readlink -f $dev)) >>> + _mkfs_dev $dev > /dev/null 2>&1 >> >> we don't want to mkfs the test dev just for a _require test; that >> device is supposed to age, and this could possibly take a very long >> time, as well. And in general a mkfs should not be required to test >> the existence of a sysfs tunable, I think? > > see below > >> >>> + _mount -t $FSTYP `_common_dev_mount_options` $dev $SCRATCH_MNT >>> + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then >>> + umount $SCRATCH_MNT >>> + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" >>> + fi >>> + umount $SCRATCH_MNT >>> +} >>> + >>> +_require_scratch_sys_fs() >>> +{ >>> + local target=$1 >>> + local dname="" >>> + >>> + if [ ! -b "$SCRATCH_DEV" -o -z "$target" ];then >>> + echo "Usage: _require_scratch_sys_fs <target_need_exist>" >>> + exit 1 >>> + fi >>> + >>> + dname=$(basename $(readlink -f $SCRATCH_DEV)) >>> + _scratch_mkfs > /dev/null 2>&1 >>> + _scratch_mount >>> + if [ ! -e /sys/fs/${FSTYP}/${dname}/$target ];then >>> + _scratch_unmount >>> + _notrun "/sys/fs/${FSTYP}/${dname}/$target: No such file or directory" >>> + fi >>> + _scratch_unmount >>> +} >> >> This seems overly complex; do you need both of these functions? We are only >> testing one filesystem type at a time, and $TEST_DEV is always required. >> What is the reason for the two differing functions? >> >> Isn't _require_sys_fs $SCRATCH_DEV exactly the same as >> _require_scratch_sys_fs ? >> > > Sorry for the "complex", let me explain: > 1. At first, I write a function _require_sys_fs(), which used to check > all kinds of device's /sys/fs/fstype configurations. Include dmerror, > dmthinp and what ever other devices(not only TEST_DEV, SCRATCH_DEV). Right, that one is generic. > 2. Then I want to add a _require_scratch_sys_fs() function, which only > used for SCRATCH_DEV. Because I feel generally if SCRATCH_DEV support > a /sys/fs/fstype configurations, that means the working kernel support > it. And _require_sys_fs() can be used by other devices write their > _require_XXX_sys_fs() function(if needed... Looks like I thought too > much....). ;) > 3. When I wrote _require_scratch_sys_fs(), I want to use _require_sys_fs > $SCRATCH_DEV directly. But I saw _scratch_mkfs() use different options > with common mkfs_dev() function. So I use _scratch_mkfs and _scratch_mount > to write this _require_scratch_sys_fs(). If available sysfs options depens on mkfs options, that could be an issue. But if we make the _require function check for a previous mkfs from the main test itself, then the test can control the mkfs options too. > 4. About why I need mkfs before in this _require function, at first those > /sys/fs/fstype configurations only can be used after mount. The second > is I hope _require_sys_fs() can be used by other devices, which maybe > not mkfs... So.... > >> Also, because $SCRATCH_DEV is not required, $SCRATCH_MNT >> isn't either, so I don't think you can use $SCRATCH_MNT. > > I did this according to _require_scratch_xfs_crc() function. right, but that is specific to "scratch." My point was that you were using $SCRATCH_MNT in the first generic function, which is not specific to scratch, and can operate on any device. >> >> Also :) We may one day want to require sysfs files which >> are not in the fs/ namespace, so it should probably >> be named according to the namespace. >> >> So maybe something like this (not tested) >> >> + /* Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY */ >> +_require_fs_sys_fs() >> +{ >> + local dev=$1 >> + local target=$2 >> + local dname="" >> + local tmp_mnt=`mktemp -d` >> + >> + if [ ! -b "$dev" -o -z "$target" ];then >> + echo "Usage: _require_fs_sys_fs <device> <sysfs_path>" >> + exit 1 >> + fi >> + >> + dname=$(basename $(readlink -f $dev)) >> + _mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt >> + if [ ! -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 $SCRATCH_MNT > > I think you mean "umount $tmp_mnt" at here yep! > >> + rm -f $tmp_mnt >> +} >> >> and it would work for either SCRATCH or TEST? > > hmm.. That's OK. I can't sure if mkfs before mount is better or not. If we > don't mkfs in this function, we ask the caller keep sure the $dev has been > mkfs. I think both are OK for me, I will follow your idea:) It does seem a little strange to require the mkfs I guess, but it seems better than performing mkfs twice in the test. I suppose one other way to do it might e to make a tiny loopback filesystem image in a tempfile, and test that. That way you could control the mkfs time? But I guess it's possible that sysfs options *could* even depend on the size of the fs, though I can't think of any today that do. >> >>> +_set_sys_fs_param() >>> +{ >>> + local dev=$1 >>> + shift >>> + local target=$1 >>> + shift >>> + local content="$*" >>> + >>> + if [ ! -b "$dev" -o -z "$target" -o -z "$content" ];then >>> + echo "Usage: _set_sys_fs_param <mounted_device> <target> <content>" >>> + exit 1 >>> + fi >>> + local dname=$(basename $(readlink -f $dev)) >>> + echo "$content" > /sys/fs/${FSTYP}/${dname}/$target >>> +} >> >> This seems fine. Normally I'd say this could just be open coded >> in the test, but the dname=$(basename $(readlink -f $dev)) is worth >> encapsulating in the helper. > > Do you mean I should do?: > local dname=$(readlink -f $dev) > dname=`basename $dname` No ... I mean, what you have is just fine, no change needed. >> >>> +_enable_xfs_fail_at_unmount() >>> +{ >>> + local dev=$1 >>> + >>> + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then >>> + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" >>> + exit 1 >>> + fi >>> + >>> + _set_sys_fs_param $dev error/fail_at_unmount 1 >>> +} >>> + >>> +_disable_xfs_fail_at_unmount() >>> +{ >>> + local dev=$1 >>> + >>> + if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then >>> + echo "Usage: _enable_xfs_fail_at_unmount <mounted_XFS_device>" >>> + exit 1 >>> + fi >>> + >>> + _set_sys_fs_param $dev error/fail_at_unmount 0 >>> +} >> >> I don't think this is needed; it's easy enough to just call: >> >> _set_fs_sys_fs_param $dev error/fail_at_unmount 0 >> _set_fs_sys_fs_param $dev error/fail_at_unmount 1 >> >> directly from tests. There are a lot of sysfs tunables, writing wrappers >> for each one isn't really that helpful. > > Yeah, except they can check if "$FSTYP" = "xfs", they don't do > more than use _set_fs_sys_fs_param directly. Well, it would be strange to call an xfs sysfs interface on a non-xfs test, but ... I guess it could happen. You could add something to _set_fs_sys_fs_param to also _fail with a warning if /sys/fs/${FSTYP}/${dname}/$target does not exist. -Eric > Thanks, > Zorro > >> >> Thanks, >> -Eric >> >>> init_rc >> >>> >>> ################################################################################ >>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-05-28 18:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-05-27 15:21 [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Zorro Lang 2016-05-27 15:21 ` [PATCH 2/2] xfs/006: new case to test xfs fail_at_unmount error handling Zorro Lang 2016-05-27 18:26 ` Eric Sandeen 2016-05-28 16:56 ` Zorro Lang 2016-05-27 18:05 ` [PATCH 1/2] common/rc: add functions to check or write objects under /sys/fs Eric Sandeen 2016-05-27 18:27 ` Eric Sandeen 2016-05-28 16:42 ` Zorro Lang 2016-05-28 17:54 ` Eric Sandeen 2016-05-28 16:36 ` Zorro Lang 2016-05-28 18:06 ` Eric Sandeen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).