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@xxxxxxxxxx>
> ---
> 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
>
> ################################################################################
>
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
