[v4,1/6] common: refactor sysfs_attr functions

Commit Message

Boris Burkov Sept. 28, 2023, 11:16 p.m. UTC

Expand the has/get/require functions to allow passing a dev by
parameter, and implement the test_dev specific one in terms of the new
generic one.

Signed-off-by: Boris Burkov <[e-mail]>
---
 common/rc | 127 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 82 insertions(+), 45 deletions(-)

Comments

Anand Jain Sept. 29, 2023, 8:23 a.m. UTC | #1

On 29/09/2023 07:16, Boris Burkov wrote:
> Expand the has/get/require functions to allow passing a dev by
> parameter, and implement the test_dev specific one in terms of the new
> generic one.
> 


> Signed-off-by: Boris Burkov <[e-mail]>

looks good. The diff is confusing.

Reviewed-by: Anand Jain <[e-mail]>

a small nit below.

>   
> +# Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ATTR
> +#
> +# All arguments are necessary, and in this order:
> +#  - dev: device name, e.g. $SCRATCH_DEV
> +#  - attr: path name under /sys/fs/$FSTYP/$dev
> +#
> +# Usage example:
> +#   _has_fs_sysfs_attr /dev/mapper/scratch-dev error/fail_at_unmount
> +_has_fs_sysfs_attr()
> +{
> +	local dev=$1
> +	local attr=$2
> +
> +	if [ ! -b "$dev" -o -z "$attr" ];then
> +		_fail "Usage: _get_fs_sysfs_attr <mounted_device> <attr>"
                               ^^^
				_has_fs_sysfs_attr

No need to resend.

Thanks, Anand


> +	fi
> +
> +	local dname=$(_fs_sysfs_dname $dev)
> +
> +	test -e /sys/fs/${FSTYP}/${dname}/${attr}
> +}
> +
> +# Require the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ATTR
> +# All arguments are necessary, and in this order:
> +#  - dev: device name, e.g. $SCRATCH_DEV
> +#  - attr: path name under /sys/fs/$FSTYP/$dev
> +#
> +# Usage example:
> +#   _require_fs_sysfs_attr /dev/mapper/scratch-dev error/fail_at_unmount
> +_require_fs_sysfs_attr()
> +{
> +	_has_fs_sysfs_attr "$@" && return
> +
> +	local dev=$1
> +	local attr=$2
> +	local dname=$(_fs_sysfs_dname $dev)
> +
> +	_notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr}"
> +}
> +
> +# Test for the existence of a sysfs entry at /sys/fs/$FSTYP/DEV/$ATTR
> +#
> +# Only one argument is needed:
> +#  - attr: path name under /sys/fs/$FSTYP/DEV
> +#
> +# Usage example:
> +#   _has_fs_sysfs error/fail_at_unmount
> +_has_fs_sysfs()
> +{
> +	_has_fs_sysfs_attr $TEST_DEV "$@"
> +}
> +
> +# Require the existence of a sysfs entry at /sys/fs/$FSTYP/DEV/$ATTR
> +_require_fs_sysfs()
> +{
> +	_has_fs_sysfs "$@" && return
> +
> +	local attr=$1
> +	local dname=$(_short_dev $TEST_DEV)
> +
> +	_notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr}"
> +}
> +
>   # Generic test for specific filesystem feature.
>   # Currently only implemented to test overlayfs features.
>   _require_scratch_feature()

Patch

diff --git a/common/rc b/common/rc
index 1618ded54..c3cd53546 100644
--- a/common/rc
+++ b/common/rc
@@ -4689,47 +4689,33 @@  run_fsx()
 	rm -f $tmp.fsx
 }
 
-# Test for the existence of a sysfs entry at /sys/fs/$FSTYP/DEV/$ATTR
+_require_statx()
+{
+	$here/src/stat_test --check-statx ||
+	_notrun "This test requires the statx system call"
+}
+
+# Get the path to the sysfs directory for the fs on a device
 #
 # Only one argument is needed:
-#  - attr: path name under /sys/fs/$FSTYP/DEV
+#  - dev: mounted block device for the fs
 #
 # Usage example:
-#   _require_fs_sysfs error/fail_at_unmount
-_has_fs_sysfs()
+#   _fs_sysfs_dname /dev/mapper/scratch-dev
+_fs_sysfs_dname()
 {
-	local attr=$1
-	local dname
+	local dev=$1
+
+	if [ ! -b "$dev" ]; then
+		_fail "Usage: _fs_sysfs_dname <mounted_device>"
+	fi
 
 	case "$FSTYP" in
 	btrfs)
-		dname=$(findmnt -n -o UUID $TEST_DEV) ;;
+		findmnt -n -o UUID ${dev} ;;
 	*)
-		dname=$(_short_dev $TEST_DEV) ;;
+		_short_dev $dev ;;
 	esac
-
-	if [ -z "$attr" -o -z "$dname" ];then
-		_fail "Usage: _require_fs_sysfs <sysfs_attr_path>"
-	fi
-
-	test -e /sys/fs/${FSTYP}/${dname}/${attr}
-}
-
-# Require the existence of a sysfs entry at /sys/fs/$FSTYP/DEV/$ATTR
-_require_fs_sysfs()
-{
-	_has_fs_sysfs "$@" && return
-
-	local attr=$1
-	local dname=$(_short_dev $TEST_DEV)
-
-	_notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr}"
-}
-
-_require_statx()
-{
-	$here/src/stat_test --check-statx ||
-	_notrun "This test requires the statx system call"
 }
 
 # Write "content" into /sys/fs/$FSTYP/$DEV/$ATTR
@@ -4753,13 +4739,7 @@  _set_fs_sysfs_attr()
 		_fail "Usage: _set_fs_sysfs_attr <mounted_device> <attr> <content>"
 	fi
 
-	local dname
-	case "$FSTYP" in
-	btrfs)
-		dname=$(findmnt -n -o UUID ${dev}) ;;
-	*)
-		dname=$(_short_dev $dev) ;;
-	esac
+	local dname=$(_fs_sysfs_dname $dev)
 
 	echo "$content" > /sys/fs/${FSTYP}/${dname}/${attr}
 }
@@ -4781,17 +4761,74 @@  _get_fs_sysfs_attr()
 		_fail "Usage: _get_fs_sysfs_attr <mounted_device> <attr>"
 	fi
 
-	local dname
-	case "$FSTYP" in
-	btrfs)
-		dname=$(findmnt -n -o UUID ${dev}) ;;
-	*)
-		dname=$(_short_dev $dev) ;;
-	esac
+	local dname=$(_fs_sysfs_dname $dev)
 
 	cat /sys/fs/${FSTYP}/${dname}/${attr}
 }
 
+# Test for the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ATTR
+#
+# All arguments are necessary, and in this order:
+#  - dev: device name, e.g. $SCRATCH_DEV
+#  - attr: path name under /sys/fs/$FSTYP/$dev
+#
+# Usage example:
+#   _has_fs_sysfs_attr /dev/mapper/scratch-dev error/fail_at_unmount
+_has_fs_sysfs_attr()
+{
+	local dev=$1
+	local attr=$2
+
+	if [ ! -b "$dev" -o -z "$attr" ];then
+		_fail "Usage: _get_fs_sysfs_attr <mounted_device> <attr>"
+	fi
+
+	local dname=$(_fs_sysfs_dname $dev)
+
+	test -e /sys/fs/${FSTYP}/${dname}/${attr}
+}
+
+# Require the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ATTR
+# All arguments are necessary, and in this order:
+#  - dev: device name, e.g. $SCRATCH_DEV
+#  - attr: path name under /sys/fs/$FSTYP/$dev
+#
+# Usage example:
+#   _require_fs_sysfs_attr /dev/mapper/scratch-dev error/fail_at_unmount
+_require_fs_sysfs_attr()
+{
+	_has_fs_sysfs_attr "$@" && return
+
+	local dev=$1
+	local attr=$2
+	local dname=$(_fs_sysfs_dname $dev)
+
+	_notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr}"
+}
+
+# Test for the existence of a sysfs entry at /sys/fs/$FSTYP/DEV/$ATTR
+#
+# Only one argument is needed:
+#  - attr: path name under /sys/fs/$FSTYP/DEV
+#
+# Usage example:
+#   _has_fs_sysfs error/fail_at_unmount
+_has_fs_sysfs()
+{
+	_has_fs_sysfs_attr $TEST_DEV "$@"
+}
+
+# Require the existence of a sysfs entry at /sys/fs/$FSTYP/DEV/$ATTR
+_require_fs_sysfs()
+{
+	_has_fs_sysfs "$@" && return
+
+	local attr=$1
+	local dname=$(_short_dev $TEST_DEV)
+
+	_notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr}"
+}
+
 # Generic test for specific filesystem feature.
 # Currently only implemented to test overlayfs features.
 _require_scratch_feature()