Re: [PATCH v4 1/2] common/rc: add functions to check or write objects under /sys/fs/$FSTYP

[Eryu Guan <eguan@redhat.com>]

On Mon, Jun 20, 2016 at 09:24:32PM +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,
> 
> V4 did below changes:
> 1) function name from *_fs_sys_fs to *_fs_sysfs
> 2) use $UMOUNT_PROG to instead of umount
> 3) use _short_dev
> 4) add comments for new functions
> 5) other little changes
> 
> Thanks,
> Zorro
> 
>  common/rc | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 51092a0..88c3777 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3556,6 +3556,48 @@ run_fsx()
>  	fi
>  }
>  
> +# Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ENTRY
> +_require_fs_sysfs()
> +{
> +	local dev=$1
> +	local entry=$2
> +	local tmp_mnt=`mktemp -d`
> +
> +	if [ ! -b "$dev" -o -z "$entry" ];then
> +		_fail "Usage: _require_fs_sysfs <device> <sysfs_path>"
> +        fi

Mixed tab and space in above line.

> +
> +	local dname=$(_short_dev $dev)
> +	_mount -t $FSTYP `_common_dev_mount_options` $dev $tmp_mnt
> +	if [ $? -ne 0 ];then
> +		rm -f $tmp_mnt
> +		_fail "_require_fs_sysfs: could not mount, mkfs first in your test?"

I'd like to see which device failed to mount in error message.

> +	elif [ ! -e /sys/fs/${FSTYP}/${dname}/${entry} ];then
> +		$UMOUNT_PROG $tmp_mnt
> +		rm -f $tmp_mnt
> +		_notrun "/sys/fs/${FSTYP}/${dname}/${entry}: No such file or directory"
> +	fi
> +	$UMOUNT_PROG $tmp_mnt
> +	rm -rf $tmp_mnt
> +}
> +
> +# Write "content" into /sys/fs/$FSTYP/$DEV/$ENTRY
> +_set_fs_sysfs_param()

I think we can have a _get_fs_sysfs_param() helper to ensure the value
is correct.

Maybe naming them as _set|get_fs_sysfs_attr()?

Thanks,
Eryu

> +{
> +	local dev=$1
> +	shift
> +	local entry=$1
> +	shift
> +	local content="$*"
> +
> +	if [ ! -b "$dev" -o -z "$entry" -o -z "$content" ];then
> +		_fail "Usage: _set_fs_sysfs_param <mounted_device> <entry> <content>"
> +	fi
> +
> +	local dname=$(_short_dev $dev)
> +	echo "$content" > /sys/fs/${FSTYP}/${dname}/${entry}
> +}
> +
>  init_rc
>  
>  ################################################################################
> -- 
> 2.5.5
>