* [PATCH v4 1/5] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout
2025-02-28 5:55 [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
@ 2025-02-28 5:55 ` Anand Jain
2025-03-31 13:48 ` Zorro Lang
2025-02-28 5:55 ` [PATCH v4 2/5] fstests: filter: helper for sysfs error filtering Anand Jain
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-02-28 5:55 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, david, djwong
Redirect sysfs write errors to stdout as a preparatory patch to enable
testing of expected sysfs write failures. Also, log the executed
sysfs write command and its failure if any to seqres.full for better
debugging and traceability.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
common/rc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/rc b/common/rc
index c7d7dbb8e93b..59af83a01e3a 100644
--- a/common/rc
+++ b/common/rc
@@ -5181,7 +5181,8 @@ _set_fs_sysfs_attr()
local dname=$(_fs_sysfs_dname $dev)
- echo "$content" > /sys/fs/${FSTYP}/${dname}/${attr}
+ echo "echo '$content' 2>&1 > /sys/fs/${FSTYP}/${dname}/${attr}" >> $seqres.full
+ echo "$content" 2>&1 > /sys/fs/${FSTYP}/${dname}/${attr} | tee -a $seqres.full
}
# Print the content of /sys/fs/$FSTYP/$DEV/$ATTR
--
2.47.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 1/5] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout
2025-02-28 5:55 ` [PATCH v4 1/5] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout Anand Jain
@ 2025-03-31 13:48 ` Zorro Lang
0 siblings, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-03-31 13:48 UTC (permalink / raw)
To: Anand Jain; +Cc: fstests, linux-btrfs, david, djwong
On Fri, Feb 28, 2025 at 01:55:19PM +0800, Anand Jain wrote:
> Redirect sysfs write errors to stdout as a preparatory patch to enable
> testing of expected sysfs write failures. Also, log the executed
> sysfs write command and its failure if any to seqres.full for better
> debugging and traceability.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> common/rc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index c7d7dbb8e93b..59af83a01e3a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5181,7 +5181,8 @@ _set_fs_sysfs_attr()
>
> local dname=$(_fs_sysfs_dname $dev)
>
> - echo "$content" > /sys/fs/${FSTYP}/${dname}/${attr}
> + echo "echo '$content' 2>&1 > /sys/fs/${FSTYP}/${dname}/${attr}" >> $seqres.full
> + echo "$content" 2>&1 > /sys/fs/${FSTYP}/${dname}/${attr} | tee -a $seqres.full
Actually I doubt the $seqres might be not defined before calling _set_fs_sysfs_attr.
For example, we call _set_fs_sysfs_attr in ./check as
_test_mount -> _prepare_for_eio_shutdown -> _xfs_prepare_for_eio_shutdown -> _set_fs_sysfs_attr
The _test_mount is called before $seqres is defined. At that time, the $seqres.full
will be ".full" if the FSTYP=xfs
Anyway, we can fix this problem in another patch, this
patch is good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>
> }
>
> # Print the content of /sys/fs/$FSTYP/$DEV/$ATTR
> --
> 2.47.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 2/5] fstests: filter: helper for sysfs error filtering
2025-02-28 5:55 [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
2025-02-28 5:55 ` [PATCH v4 1/5] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout Anand Jain
@ 2025-02-28 5:55 ` Anand Jain
2025-03-31 13:53 ` Zorro Lang
2025-02-28 5:55 ` [PATCH v4 3/5] fstests: common/rc: add sysfs argument verification helpers Anand Jain
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-02-28 5:55 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, david, djwong
Added filter helper to filter sysfs write errors, retain only the
error part.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
common/filter | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/common/filter b/common/filter
index 7e02ded377cc..44ba2b38c21d 100644
--- a/common/filter
+++ b/common/filter
@@ -671,5 +671,14 @@ _filter_flakey_EIO()
sed -e "s#.*: Input\/output error#$message#"
}
+# Filters
+# +./common/rc: line 5085: echo: write error: Invalid argument
+# to
+# Invalid argument
+_filter_sysfs_error()
+{
+ sed 's/.*: \(.*\)$/\1/'
+}
+
# make sure this script returns success
/bin/true
--
2.47.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/5] fstests: filter: helper for sysfs error filtering
2025-02-28 5:55 ` [PATCH v4 2/5] fstests: filter: helper for sysfs error filtering Anand Jain
@ 2025-03-31 13:53 ` Zorro Lang
0 siblings, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-03-31 13:53 UTC (permalink / raw)
To: Anand Jain; +Cc: fstests, linux-btrfs, david, djwong
On Fri, Feb 28, 2025 at 01:55:20PM +0800, Anand Jain wrote:
> Added filter helper to filter sysfs write errors, retain only the
> error part.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> common/filter | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/common/filter b/common/filter
> index 7e02ded377cc..44ba2b38c21d 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -671,5 +671,14 @@ _filter_flakey_EIO()
> sed -e "s#.*: Input\/output error#$message#"
> }
>
> +# Filters
> +# +./common/rc: line 5085: echo: write error: Invalid argument
> +# to
> +# Invalid argument
> +_filter_sysfs_error()
> +{
> + sed 's/.*: \(.*\)$/\1/'
> +}
Maybe output the "echo: write error: Invalid argument" or "write error:
Invalid argument" is better. But as there's not more requirement for
this helper, for later two cases, this helper is good. We can change
that when we have particular requirement.
Reviewed-by: Zorro Lang <zlang@redhat.com>
> +
> # make sure this script returns success
> /bin/true
> --
> 2.47.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 3/5] fstests: common/rc: add sysfs argument verification helpers
2025-02-28 5:55 [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
2025-02-28 5:55 ` [PATCH v4 1/5] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout Anand Jain
2025-02-28 5:55 ` [PATCH v4 2/5] fstests: filter: helper for sysfs error filtering Anand Jain
@ 2025-02-28 5:55 ` Anand Jain
2025-03-31 13:15 ` Zorro Lang
2025-03-31 13:58 ` Zorro Lang
2025-02-28 5:55 ` [PATCH v4 4/5] fstests: btrfs: testcase for sysfs policy syntax verification Anand Jain
` (2 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Anand Jain @ 2025-02-28 5:55 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, david, djwong
Introduce `verify_sysfs_syntax()` and `_require_fs_sysfs_attr_policy()` to verify
whether a sysfs attribute rejects invalid input arguments during writes.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
common/sysfs | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 142 insertions(+)
create mode 100644 common/sysfs
diff --git a/common/sysfs b/common/sysfs
new file mode 100644
index 000000000000..1362a1261dfc
--- /dev/null
+++ b/common/sysfs
@@ -0,0 +1,142 @@
+##/bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2025 Oracle. All Rights Reserved.
+#
+# Common/sysfs file for the sysfs related helper functions.
+
+# Test for the existence of a policy 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
+# - policy: policy within /sys/fs/$FSTYP/$dev
+#
+# Usage example:
+# _has_fs_sysfs_attr_policy /dev/mapper/scratch-dev read_policy round-robin
+_has_fs_sysfs_attr_policy()
+{
+ local dev=$1
+ local attr=$2
+ local policy=$3
+
+ if [ ! -b "$dev" -o -z "$attr" -o -z "$policy" ]; then
+ _fail \
+ "Usage: _has_fs_sysfs_attr_policy <mounted_device> <attr> <policy>"
+ fi
+
+ local dname=$(_fs_sysfs_dname $dev)
+ test -e /sys/fs/${FSTYP}/${dname}/${attr}
+
+ cat /sys/fs/${FSTYP}/${dname}/${attr} | grep -q ${policy}
+}
+
+# Require the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ATTR
+# and value in it contains $policy
+# All arguments are necessary, and in this order:
+# - dev: device name, e.g. $SCRATCH_DEV
+# - attr: path name under /sys/fs/$FSTYP/$dev
+# - policy: mentioned in /sys/fs/$FSTYP/$dev/$attr
+#
+# Usage example:
+# _require_fs_sysfs_attr_policy /dev/mapper/scratch-dev read_policy round-robin
+_require_fs_sysfs_attr_policy()
+{
+ _has_fs_sysfs_attr_policy "$@" && return
+
+ local dev=$1
+ local attr=$2
+ local policy=$3
+ local dname=$(_fs_sysfs_dname $dev)
+
+ _notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr} ${policy}"
+}
+
+set_sysfs_policy()
+{
+ local dev=$1
+ local attr=$2
+ shift
+ shift
+ local policy=$@
+
+ _set_fs_sysfs_attr $dev $attr ${policy}
+
+ case "$FSTYP" in
+ btrfs)
+ _get_fs_sysfs_attr $dev $attr | grep -q "[${policy}]"
+ if [[ $? != 0 ]]; then
+ echo "Setting sysfs $attr $policy failed"
+ fi
+ ;;
+ *)
+ _fail \
+"sysfs syntax verification for '${attr}' '${policy}' for '${FSTYP}' is not implemented"
+ ;;
+ esac
+}
+
+set_sysfs_policy_must_fail()
+{
+ local dev=$1
+ local attr=$2
+ shift
+ shift
+ local policy=$@
+
+ _set_fs_sysfs_attr $dev $attr ${policy} | _filter_sysfs_error \
+ | tee -a $seqres.full
+}
+
+# Verify sysfs attribute rejects invalid input.
+# Usage syntax:
+# verify_sysfs_syntax <$dev> <$attr> <$policy> [$value]
+# Examples:
+# verify_sysfs_syntax $TEST_DEV read_policy pid
+# verify_sysfs_syntax $TEST_DEV read_policy round-robin 4k
+# Note:
+# Process must call . ./common/filter
+verify_sysfs_syntax()
+{
+ local dev=$1
+ local attr=$2
+ local policy=$3
+ local value=$4
+
+ # Do this in the test case so that we know its prerequisites.
+ # '_require_fs_sysfs_attr_policy $TEST_DEV $attr $policy'
+
+ # Test policy specified wrongly. Must fail.
+ set_sysfs_policy_must_fail $dev $attr "'$policy $policy'"
+ set_sysfs_policy_must_fail $dev $attr "'$policy t'"
+ set_sysfs_policy_must_fail $dev $attr "' '"
+ set_sysfs_policy_must_fail $dev $attr "'${policy} n'"
+ set_sysfs_policy_must_fail $dev $attr "'n ${policy}'"
+ set_sysfs_policy_must_fail $dev $attr "' ${policy}'"
+ set_sysfs_policy_must_fail $dev $attr "' ${policy} '"
+ set_sysfs_policy_must_fail $dev $attr "'${policy} '"
+ set_sysfs_policy_must_fail $dev $attr _${policy}
+ set_sysfs_policy_must_fail $dev $attr ${policy}_
+ set_sysfs_policy_must_fail $dev $attr _${policy}_
+ set_sysfs_policy_must_fail $dev $attr ${policy}:
+ # Test policy longer than 32 chars fails stable.
+ set_sysfs_policy_must_fail $dev $attr 'jfdkkkkjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjffjfjfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff'
+
+ # Test policy specified correctly. Must pass.
+ set_sysfs_policy $dev $attr $policy
+
+ # If the policy has no value return
+ if [[ -z $value ]]; then
+ return
+ fi
+
+ # Test value specified wrongly. Must fail.
+ set_sysfs_policy_must_fail $dev $attr "'$policy: $value'"
+ set_sysfs_policy_must_fail $dev $attr "'$policy:$value '"
+ set_sysfs_policy_must_fail $dev $attr "'$policy:$value typo'"
+ set_sysfs_policy_must_fail $dev $attr "'$policy:${value}typo'"
+ set_sysfs_policy_must_fail $dev $attr "'$policy :$value'"
+
+ # Test policy and value all specified correctly. Must pass.
+ set_sysfs_policy $dev $attr $policy:$value
+}
+
--
2.47.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] fstests: common/rc: add sysfs argument verification helpers
2025-02-28 5:55 ` [PATCH v4 3/5] fstests: common/rc: add sysfs argument verification helpers Anand Jain
@ 2025-03-31 13:15 ` Zorro Lang
2025-03-31 13:58 ` Zorro Lang
1 sibling, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-03-31 13:15 UTC (permalink / raw)
To: Anand Jain; +Cc: fstests, linux-btrfs
On Fri, Feb 28, 2025 at 01:55:21PM +0800, Anand Jain wrote:
> Introduce `verify_sysfs_syntax()` and `_require_fs_sysfs_attr_policy()` to verify
> whether a sysfs attribute rejects invalid input arguments during writes.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
The subject is "common/rc: ...", but this patch isn't about common/rc.
So feel free to remove the "/rc", or use "/sysfs".
> common/sysfs | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 142 insertions(+)
> create mode 100644 common/sysfs
>
> diff --git a/common/sysfs b/common/sysfs
> new file mode 100644
> index 000000000000..1362a1261dfc
> --- /dev/null
> +++ b/common/sysfs
> @@ -0,0 +1,142 @@
> +##/bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2025 Oracle. All Rights Reserved.
> +#
> +# Common/sysfs file for the sysfs related helper functions.
> +
> +# Test for the existence of a policy 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
> +# - policy: policy within /sys/fs/$FSTYP/$dev
> +#
> +# Usage example:
> +# _has_fs_sysfs_attr_policy /dev/mapper/scratch-dev read_policy round-robin
> +_has_fs_sysfs_attr_policy()
> +{
> + local dev=$1
> + local attr=$2
> + local policy=$3
> +
> + if [ ! -b "$dev" -o -z "$attr" -o -z "$policy" ]; then
> + _fail \
> + "Usage: _has_fs_sysfs_attr_policy <mounted_device> <attr> <policy>"
> + fi
> +
> + local dname=$(_fs_sysfs_dname $dev)
> + test -e /sys/fs/${FSTYP}/${dname}/${attr}
> +
> + cat /sys/fs/${FSTYP}/${dname}/${attr} | grep -q ${policy}
> +}
> +
> +# Require the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ATTR
> +# and value in it contains $policy
> +# All arguments are necessary, and in this order:
> +# - dev: device name, e.g. $SCRATCH_DEV
> +# - attr: path name under /sys/fs/$FSTYP/$dev
> +# - policy: mentioned in /sys/fs/$FSTYP/$dev/$attr
> +#
> +# Usage example:
> +# _require_fs_sysfs_attr_policy /dev/mapper/scratch-dev read_policy round-robin
> +_require_fs_sysfs_attr_policy()
> +{
> + _has_fs_sysfs_attr_policy "$@" && return
> +
> + local dev=$1
> + local attr=$2
> + local policy=$3
> + local dname=$(_fs_sysfs_dname $dev)
> +
> + _notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr} ${policy}"
> +}
> +
> +set_sysfs_policy()
> +{
> + local dev=$1
> + local attr=$2
> + shift
> + shift
> + local policy=$@
> +
> + _set_fs_sysfs_attr $dev $attr ${policy}
> +
> + case "$FSTYP" in
> + btrfs)
> + _get_fs_sysfs_attr $dev $attr | grep -q "[${policy}]"
> + if [[ $? != 0 ]]; then
> + echo "Setting sysfs $attr $policy failed"
> + fi
> + ;;
> + *)
> + _fail \
> +"sysfs syntax verification for '${attr}' '${policy}' for '${FSTYP}' is not implemented"
> + ;;
> + esac
> +}
> +
> +set_sysfs_policy_must_fail()
> +{
> + local dev=$1
> + local attr=$2
> + shift
> + shift
> + local policy=$@
> +
> + _set_fs_sysfs_attr $dev $attr ${policy} | _filter_sysfs_error \
> + | tee -a $seqres.full
> +}
> +
> +# Verify sysfs attribute rejects invalid input.
> +# Usage syntax:
> +# verify_sysfs_syntax <$dev> <$attr> <$policy> [$value]
> +# Examples:
> +# verify_sysfs_syntax $TEST_DEV read_policy pid
> +# verify_sysfs_syntax $TEST_DEV read_policy round-robin 4k
> +# Note:
> +# Process must call . ./common/filter
> +verify_sysfs_syntax()
> +{
> + local dev=$1
> + local attr=$2
> + local policy=$3
> + local value=$4
> +
> + # Do this in the test case so that we know its prerequisites.
> + # '_require_fs_sysfs_attr_policy $TEST_DEV $attr $policy'
> +
> + # Test policy specified wrongly. Must fail.
> + set_sysfs_policy_must_fail $dev $attr "'$policy $policy'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy t'"
> + set_sysfs_policy_must_fail $dev $attr "' '"
> + set_sysfs_policy_must_fail $dev $attr "'${policy} n'"
> + set_sysfs_policy_must_fail $dev $attr "'n ${policy}'"
> + set_sysfs_policy_must_fail $dev $attr "' ${policy}'"
> + set_sysfs_policy_must_fail $dev $attr "' ${policy} '"
> + set_sysfs_policy_must_fail $dev $attr "'${policy} '"
> + set_sysfs_policy_must_fail $dev $attr _${policy}
> + set_sysfs_policy_must_fail $dev $attr ${policy}_
> + set_sysfs_policy_must_fail $dev $attr _${policy}_
> + set_sysfs_policy_must_fail $dev $attr ${policy}:
> + # Test policy longer than 32 chars fails stable.
> + set_sysfs_policy_must_fail $dev $attr 'jfdkkkkjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjffjfjfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff'
> +
> + # Test policy specified correctly. Must pass.
> + set_sysfs_policy $dev $attr $policy
> +
> + # If the policy has no value return
> + if [[ -z $value ]]; then
> + return
> + fi
> +
> + # Test value specified wrongly. Must fail.
> + set_sysfs_policy_must_fail $dev $attr "'$policy: $value'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy:$value '"
> + set_sysfs_policy_must_fail $dev $attr "'$policy:$value typo'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy:${value}typo'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy :$value'"
> +
> + # Test policy and value all specified correctly. Must pass.
> + set_sysfs_policy $dev $attr $policy:$value
> +}
> +
I got this warning:
Applying: fstests: common/rc: add sysfs argument verification helpers
.git/rebase-apply/patch:153: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Other looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>
Thanks,
Zorro
> --
> 2.47.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] fstests: common/rc: add sysfs argument verification helpers
2025-02-28 5:55 ` [PATCH v4 3/5] fstests: common/rc: add sysfs argument verification helpers Anand Jain
2025-03-31 13:15 ` Zorro Lang
@ 2025-03-31 13:58 ` Zorro Lang
1 sibling, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-03-31 13:58 UTC (permalink / raw)
To: Anand Jain; +Cc: fstests, linux-btrfs, david, djwong
On Fri, Feb 28, 2025 at 01:55:21PM +0800, Anand Jain wrote:
> Introduce `verify_sysfs_syntax()` and `_require_fs_sysfs_attr_policy()` to verify
> whether a sysfs attribute rejects invalid input arguments during writes.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> common/sysfs | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 142 insertions(+)
> create mode 100644 common/sysfs
>
> diff --git a/common/sysfs b/common/sysfs
> new file mode 100644
> index 000000000000..1362a1261dfc
> --- /dev/null
> +++ b/common/sysfs
> @@ -0,0 +1,142 @@
> +##/bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2025 Oracle. All Rights Reserved.
> +#
> +# Common/sysfs file for the sysfs related helper functions.
> +
> +# Test for the existence of a policy 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
> +# - policy: policy within /sys/fs/$FSTYP/$dev
> +#
> +# Usage example:
> +# _has_fs_sysfs_attr_policy /dev/mapper/scratch-dev read_policy round-robin
> +_has_fs_sysfs_attr_policy()
> +{
> + local dev=$1
> + local attr=$2
> + local policy=$3
> +
> + if [ ! -b "$dev" -o -z "$attr" -o -z "$policy" ]; then
> + _fail \
> + "Usage: _has_fs_sysfs_attr_policy <mounted_device> <attr> <policy>"
> + fi
> +
> + local dname=$(_fs_sysfs_dname $dev)
> + test -e /sys/fs/${FSTYP}/${dname}/${attr}
> +
> + cat /sys/fs/${FSTYP}/${dname}/${attr} | grep -q ${policy}
> +}
> +
> +# Require the existence of a sysfs entry at /sys/fs/$FSTYP/$DEV/$ATTR
> +# and value in it contains $policy
> +# All arguments are necessary, and in this order:
> +# - dev: device name, e.g. $SCRATCH_DEV
> +# - attr: path name under /sys/fs/$FSTYP/$dev
> +# - policy: mentioned in /sys/fs/$FSTYP/$dev/$attr
> +#
> +# Usage example:
> +# _require_fs_sysfs_attr_policy /dev/mapper/scratch-dev read_policy round-robin
> +_require_fs_sysfs_attr_policy()
> +{
> + _has_fs_sysfs_attr_policy "$@" && return
> +
> + local dev=$1
> + local attr=$2
> + local policy=$3
> + local dname=$(_fs_sysfs_dname $dev)
> +
> + _notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr} ${policy}"
> +}
> +
> +set_sysfs_policy()
> +{
> + local dev=$1
> + local attr=$2
> + shift
> + shift
> + local policy=$@
> +
> + _set_fs_sysfs_attr $dev $attr ${policy}
> +
> + case "$FSTYP" in
> + btrfs)
> + _get_fs_sysfs_attr $dev $attr | grep -q "[${policy}]"
> + if [[ $? != 0 ]]; then
> + echo "Setting sysfs $attr $policy failed"
> + fi
> + ;;
> + *)
> + _fail \
> +"sysfs syntax verification for '${attr}' '${policy}' for '${FSTYP}' is not implemented"
> + ;;
> + esac
> +}
> +
> +set_sysfs_policy_must_fail()
> +{
> + local dev=$1
> + local attr=$2
> + shift
> + shift
> + local policy=$@
> +
> + _set_fs_sysfs_attr $dev $attr ${policy} | _filter_sysfs_error \
> + | tee -a $seqres.full
> +}
> +
> +# Verify sysfs attribute rejects invalid input.
> +# Usage syntax:
> +# verify_sysfs_syntax <$dev> <$attr> <$policy> [$value]
> +# Examples:
> +# verify_sysfs_syntax $TEST_DEV read_policy pid
> +# verify_sysfs_syntax $TEST_DEV read_policy round-robin 4k
> +# Note:
> +# Process must call . ./common/filter
> +verify_sysfs_syntax()
Wait a moment, we generally has "_" for common helpers (is there any exception you
can find in common/ ?).
So better to add rename this function as _verify_sysfs_syntax.
Same to above two functions, please rename them to:
_set_sysfs_policy, _set_sysfs_policy_must_fail,
> +{
> + local dev=$1
> + local attr=$2
> + local policy=$3
> + local value=$4
> +
> + # Do this in the test case so that we know its prerequisites.
> + # '_require_fs_sysfs_attr_policy $TEST_DEV $attr $policy'
> +
> + # Test policy specified wrongly. Must fail.
> + set_sysfs_policy_must_fail $dev $attr "'$policy $policy'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy t'"
> + set_sysfs_policy_must_fail $dev $attr "' '"
> + set_sysfs_policy_must_fail $dev $attr "'${policy} n'"
> + set_sysfs_policy_must_fail $dev $attr "'n ${policy}'"
> + set_sysfs_policy_must_fail $dev $attr "' ${policy}'"
> + set_sysfs_policy_must_fail $dev $attr "' ${policy} '"
> + set_sysfs_policy_must_fail $dev $attr "'${policy} '"
> + set_sysfs_policy_must_fail $dev $attr _${policy}
> + set_sysfs_policy_must_fail $dev $attr ${policy}_
> + set_sysfs_policy_must_fail $dev $attr _${policy}_
> + set_sysfs_policy_must_fail $dev $attr ${policy}:
> + # Test policy longer than 32 chars fails stable.
> + set_sysfs_policy_must_fail $dev $attr 'jfdkkkkjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjffjfjfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff'
> +
> + # Test policy specified correctly. Must pass.
> + set_sysfs_policy $dev $attr $policy
> +
> + # If the policy has no value return
> + if [[ -z $value ]]; then
> + return
> + fi
> +
> + # Test value specified wrongly. Must fail.
> + set_sysfs_policy_must_fail $dev $attr "'$policy: $value'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy:$value '"
> + set_sysfs_policy_must_fail $dev $attr "'$policy:$value typo'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy:${value}typo'"
> + set_sysfs_policy_must_fail $dev $attr "'$policy :$value'"
> +
> + # Test policy and value all specified correctly. Must pass.
> + set_sysfs_policy $dev $attr $policy:$value
> +}
> +
> --
> 2.47.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 4/5] fstests: btrfs: testcase for sysfs policy syntax verification
2025-02-28 5:55 [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
` (2 preceding siblings ...)
2025-02-28 5:55 ` [PATCH v4 3/5] fstests: common/rc: add sysfs argument verification helpers Anand Jain
@ 2025-02-28 5:55 ` Anand Jain
2025-03-31 14:04 ` Zorro Lang
2025-02-28 5:55 ` [PATCH v4 5/5] fstests: btrfs: testcase for sysfs chunk_size attribute validation Anand Jain
2025-03-28 0:40 ` [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-02-28 5:55 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, david, djwong
Checks if the sysfs attribute sanitizes arguments and verifies
input syntax.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
tests/btrfs/329 | 19 +++++++++++++++++++
tests/btrfs/329.out | 19 +++++++++++++++++++
2 files changed, 38 insertions(+)
create mode 100755 tests/btrfs/329
create mode 100644 tests/btrfs/329.out
diff --git a/tests/btrfs/329 b/tests/btrfs/329
new file mode 100755
index 000000000000..48849ac82706
--- /dev/null
+++ b/tests/btrfs/329
@@ -0,0 +1,19 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 Oracle. All Rights Reserved.
+#
+# FS QA Test 329
+#
+# Verify sysfs knob input syntax for read_policy round-robin
+#
+. ./common/preamble
+_begin_fstest auto quick
+
+. ./common/sysfs
+. ./common/filter
+
+_require_fs_sysfs_attr_policy $TEST_DEV read_policy round-robin
+verify_sysfs_syntax $TEST_DEV read_policy round-robin 4096
+
+status=0
+exit
diff --git a/tests/btrfs/329.out b/tests/btrfs/329.out
new file mode 100644
index 000000000000..eff7573adb6a
--- /dev/null
+++ b/tests/btrfs/329.out
@@ -0,0 +1,19 @@
+QA output created by 329
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
--
2.47.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 4/5] fstests: btrfs: testcase for sysfs policy syntax verification
2025-02-28 5:55 ` [PATCH v4 4/5] fstests: btrfs: testcase for sysfs policy syntax verification Anand Jain
@ 2025-03-31 14:04 ` Zorro Lang
0 siblings, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-03-31 14:04 UTC (permalink / raw)
To: Anand Jain; +Cc: fstests, linux-btrfs, david, djwong
On Fri, Feb 28, 2025 at 01:55:22PM +0800, Anand Jain wrote:
> Checks if the sysfs attribute sanitizes arguments and verifies
> input syntax.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> tests/btrfs/329 | 19 +++++++++++++++++++
> tests/btrfs/329.out | 19 +++++++++++++++++++
> 2 files changed, 38 insertions(+)
> create mode 100755 tests/btrfs/329
> create mode 100644 tests/btrfs/329.out
>
> diff --git a/tests/btrfs/329 b/tests/btrfs/329
> new file mode 100755
> index 000000000000..48849ac82706
> --- /dev/null
> +++ b/tests/btrfs/329
> @@ -0,0 +1,19 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 329
> +#
> +# Verify sysfs knob input syntax for read_policy round-robin
> +#
> +. ./common/preamble
> +_begin_fstest auto quick
> +
> +. ./common/sysfs
> +. ./common/filter
> +
> +_require_fs_sysfs_attr_policy $TEST_DEV read_policy round-robin
> +verify_sysfs_syntax $TEST_DEV read_policy round-robin 4096
Please change the verify_sysfs_syntax to _verify_sysfs_syntax after you
update the patch 3/5.
And just be curious, do we need to make sure the $TEST_DEV is mounted
before testing the sysfs' read_policy of $TEST_DEV? For example, calls
_require_test
Thanks,
Zorro
> +
> +status=0
> +exit
> diff --git a/tests/btrfs/329.out b/tests/btrfs/329.out
> new file mode 100644
> index 000000000000..eff7573adb6a
> --- /dev/null
> +++ b/tests/btrfs/329.out
> @@ -0,0 +1,19 @@
> +QA output created by 329
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> --
> 2.47.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 5/5] fstests: btrfs: testcase for sysfs chunk_size attribute validation
2025-02-28 5:55 [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
` (3 preceding siblings ...)
2025-02-28 5:55 ` [PATCH v4 4/5] fstests: btrfs: testcase for sysfs policy syntax verification Anand Jain
@ 2025-02-28 5:55 ` Anand Jain
2025-03-31 14:05 ` Zorro Lang
2025-03-28 0:40 ` [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-02-28 5:55 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, david, djwong
Checks if the sysfs attribute sanitizes arguments and verifies
input syntax allocation/data/chunk_size.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
tests/btrfs/334 | 19 +++++++++++++++++++
tests/btrfs/334.out | 14 ++++++++++++++
2 files changed, 33 insertions(+)
create mode 100755 tests/btrfs/334
create mode 100644 tests/btrfs/334.out
diff --git a/tests/btrfs/334 b/tests/btrfs/334
new file mode 100755
index 000000000000..532fe37a0489
--- /dev/null
+++ b/tests/btrfs/334
@@ -0,0 +1,19 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 Oracle. All Rights Reserved.
+#
+# FS QA Test 334
+#
+# Verify sysfs knob input syntax for allocation/data/chunk_size
+#
+. ./common/preamble
+_begin_fstest auto quick
+
+. ./common/sysfs
+. ./common/filter
+
+_require_fs_sysfs_attr $TEST_DEV allocation/data/chunk_size
+verify_sysfs_syntax $TEST_DEV allocation/data/chunk_size 256m
+
+status=0
+exit
diff --git a/tests/btrfs/334.out b/tests/btrfs/334.out
new file mode 100644
index 000000000000..f64f9ac09499
--- /dev/null
+++ b/tests/btrfs/334.out
@@ -0,0 +1,14 @@
+QA output created by 334
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
+Invalid argument
--
2.47.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 5/5] fstests: btrfs: testcase for sysfs chunk_size attribute validation
2025-02-28 5:55 ` [PATCH v4 5/5] fstests: btrfs: testcase for sysfs chunk_size attribute validation Anand Jain
@ 2025-03-31 14:05 ` Zorro Lang
0 siblings, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-03-31 14:05 UTC (permalink / raw)
To: Anand Jain; +Cc: fstests, linux-btrfs, david, djwong
On Fri, Feb 28, 2025 at 01:55:23PM +0800, Anand Jain wrote:
> Checks if the sysfs attribute sanitizes arguments and verifies
> input syntax allocation/data/chunk_size.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> tests/btrfs/334 | 19 +++++++++++++++++++
> tests/btrfs/334.out | 14 ++++++++++++++
> 2 files changed, 33 insertions(+)
> create mode 100755 tests/btrfs/334
> create mode 100644 tests/btrfs/334.out
>
> diff --git a/tests/btrfs/334 b/tests/btrfs/334
> new file mode 100755
> index 000000000000..532fe37a0489
> --- /dev/null
> +++ b/tests/btrfs/334
> @@ -0,0 +1,19 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 334
> +#
> +# Verify sysfs knob input syntax for allocation/data/chunk_size
> +#
> +. ./common/preamble
> +_begin_fstest auto quick
> +
> +. ./common/sysfs
> +. ./common/filter
> +
> +_require_fs_sysfs_attr $TEST_DEV allocation/data/chunk_size
> +verify_sysfs_syntax $TEST_DEV allocation/data/chunk_size 256m
Same review points with patch 4/5.
> +
> +status=0
> +exit
> diff --git a/tests/btrfs/334.out b/tests/btrfs/334.out
> new file mode 100644
> index 000000000000..f64f9ac09499
> --- /dev/null
> +++ b/tests/btrfs/334.out
> @@ -0,0 +1,14 @@
> +QA output created by 334
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> +Invalid argument
> --
> 2.47.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments
2025-02-28 5:55 [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
` (4 preceding siblings ...)
2025-02-28 5:55 ` [PATCH v4 5/5] fstests: btrfs: testcase for sysfs chunk_size attribute validation Anand Jain
@ 2025-03-28 0:40 ` Anand Jain
2025-03-28 2:55 ` Anand Jain
5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-03-28 0:40 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, david, djwong
On 28/2/25 13:55, Anand Jain wrote:
> v4:
> Fixed the double quotes in 1/5.
> (Thanks for the review! If no other comments, I'll push this set on the
> weekend.)
>
> v3:
> https://lore.kernel.org/fstests/b297a34f-4c09-48bb-86a3-fea50c364ba8@oracle.com/
>
> v2:
> https://lore.kernel.org/fstests/cover.1738752716.git.anand.jain@oracle.com/
>
> v1:
> https://lwn.net/ml/all/cover.1738161075.git.anand.jain@oracle.com/
>
> Anand Jain (5):
> fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout
> fstests: filter: helper for sysfs error filtering
> fstests: common/rc: add sysfs argument verification helpers
> fstests: btrfs: testcase for sysfs policy syntax verification
> fstests: btrfs: testcase for sysfs chunk_size attribute validation
Applied to the upcoming for-next.
Thanks, Anand
>
> common/filter | 9 +++
> common/rc | 3 +-
> common/sysfs | 142 ++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/329 | 19 ++++++
> tests/btrfs/329.out | 19 ++++++
> tests/btrfs/334 | 19 ++++++
> tests/btrfs/334.out | 14 +++++
> 7 files changed, 224 insertions(+), 1 deletion(-)
> create mode 100644 common/sysfs
> create mode 100755 tests/btrfs/329
> create mode 100644 tests/btrfs/329.out
> create mode 100755 tests/btrfs/334
> create mode 100644 tests/btrfs/334.out
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments
2025-03-28 0:40 ` [PATCH v4 0/5] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
@ 2025-03-28 2:55 ` Anand Jain
0 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2025-03-28 2:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: david, djwong, fstests
On 28/3/25 08:40, Anand Jain wrote:
>
>
> On 28/2/25 13:55, Anand Jain wrote:
>> v4:
>> Fixed the double quotes in 1/5.
>> (Thanks for the review! If no other comments, I'll push this set on the
>> weekend.)
>>
>> v3:
>> https://lore.kernel.org/fstests/b297a34f-4c09-48bb-86a3-
>> fea50c364ba8@oracle.com/
>>
>> v2:
>> https://lore.kernel.org/fstests/
>> cover.1738752716.git.anand.jain@oracle.com/
>>
>> v1:
>> https://lwn.net/ml/all/cover.1738161075.git.anand.jain@oracle.com/
>>
>> Anand Jain (5):
>> fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout
>> fstests: filter: helper for sysfs error filtering
>> fstests: common/rc: add sysfs argument verification helpers
>> fstests: btrfs: testcase for sysfs policy syntax verification
>> fstests: btrfs: testcase for sysfs chunk_size attribute validation
>
> Applied to the upcoming for-next.
Postponed for now. Waiting on RVB. Thanks!
>
> Thanks, Anand
>
>
>>
>> common/filter | 9 +++
>> common/rc | 3 +-
>> common/sysfs | 142 ++++++++++++++++++++++++++++++++++++++++++++
>> tests/btrfs/329 | 19 ++++++
>> tests/btrfs/329.out | 19 ++++++
>> tests/btrfs/334 | 19 ++++++
>> tests/btrfs/334.out | 14 +++++
>> 7 files changed, 224 insertions(+), 1 deletion(-)
>> create mode 100644 common/sysfs
>> create mode 100755 tests/btrfs/329
>> create mode 100644 tests/btrfs/329.out
>> create mode 100755 tests/btrfs/334
>> create mode 100644 tests/btrfs/334.out
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread