public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments
@ 2025-04-09  7:43 Anand Jain
  2025-04-09  7:43 ` [PATCH v6 1/6] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout Anand Jain
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Anand Jain @ 2025-04-09  7:43 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, zlang, djwong

v6:
 - Add missing if statement for the test in `_has_fs_sysfs_attr_policy()`.
 - Add a comment to `_verify_sysfs_syntax()`.
 - Set seqres early in `check::run_section()`.
 - Add _require_test to `btrfs/334` and `btrfs/329`.

v5:
https://lore.kernel.org/fstests/cover.1743996408.git.anand.jain@oracle.com/

v4:
https://lore.kernel.org/fstests/cover.1740721626.git.anand.jain@oracle.com/

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 (6):
  fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout
  fstests: common/rc: fix unset seqres in _set_fs_sysfs_attr
  fstests: filter: helper for sysfs error filtering
  fstests: common/sysfs: add new file sysfs and helpers
  fstests: btrfs: testcase for sysfs policy syntax verification
  fstests: btrfs: testcase for sysfs chunk_size attribute validation

 check               |   2 +-
 common/filter       |   9 +++
 common/rc           |   3 +-
 common/sysfs        | 145 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/329     |  21 +++++++
 tests/btrfs/329.out |  19 ++++++
 tests/btrfs/334     |  21 +++++++
 tests/btrfs/334.out |  14 +++++
 8 files changed, 232 insertions(+), 2 deletions(-)
 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

-- 
2.47.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 1/6] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout
  2025-04-09  7:43 [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
@ 2025-04-09  7:43 ` Anand Jain
  2025-04-09  7:43 ` [PATCH v6 2/6] fstests: check: fix unset seqres in run_section() Anand Jain
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2025-04-09  7:43 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, zlang, 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>
Reviewed-by: Zorro Lang <zlang@redhat.com>
---
 common/rc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index 16d627e1bdd4..e89eee5de840 100644
--- a/common/rc
+++ b/common/rc
@@ -5208,7 +5208,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

* [PATCH v6 2/6] fstests: check: fix unset seqres in run_section()
  2025-04-09  7:43 [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
  2025-04-09  7:43 ` [PATCH v6 1/6] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout Anand Jain
@ 2025-04-09  7:43 ` Anand Jain
  2025-04-09  9:57   ` Zorro Lang
  2025-04-09  7:43 ` [PATCH v6 3/6] fstests: filter: helper for sysfs error filtering Anand Jain
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-04-09  7:43 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, zlang, djwong

Ensure seqres is set early in run_section().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 check | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/check b/check
index 32890470a020..16f695e9d75c 100755
--- a/check
+++ b/check
@@ -804,6 +804,7 @@ function run_section()
 
 	seq="check.$$"
 	check="$RESULT_BASE/check"
+	seqres="$check"
 
 	# don't leave old full output behind on a clean run
 	rm -f $check.full
@@ -849,7 +850,6 @@ function run_section()
 	  fi
 	fi
 
-	seqres="$check"
 	_check_test_fs
 
 	loop_status=()	# track rerun-on-failure state
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v6 3/6] fstests: filter: helper for sysfs error filtering
  2025-04-09  7:43 [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
  2025-04-09  7:43 ` [PATCH v6 1/6] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout Anand Jain
  2025-04-09  7:43 ` [PATCH v6 2/6] fstests: check: fix unset seqres in run_section() Anand Jain
@ 2025-04-09  7:43 ` Anand Jain
  2025-04-09  7:43 ` [PATCH v6 4/6] fstests: common/sysfs: add new file sysfs and helpers Anand Jain
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2025-04-09  7:43 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, zlang, djwong

Added filter helper to filter sysfs write errors, retain only the
error part.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Zorro Lang <zlang@redhat.com>
---
 common/filter | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/filter b/common/filter
index 1ebfd27e898e..bbe13f4c8a8d 100644
--- a/common/filter
+++ b/common/filter
@@ -674,5 +674,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

* [PATCH v6 4/6] fstests: common/sysfs: add new file sysfs and helpers
  2025-04-09  7:43 [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
                   ` (2 preceding siblings ...)
  2025-04-09  7:43 ` [PATCH v6 3/6] fstests: filter: helper for sysfs error filtering Anand Jain
@ 2025-04-09  7:43 ` Anand Jain
  2025-04-09 10:21   ` Zorro Lang
  2025-04-09  7:43 ` [PATCH v6 5/6] fstests: btrfs: testcase for sysfs policy syntax verification Anand Jain
  2025-04-09  7:43 ` [PATCH v6 6/6] fstests: btrfs: testcase for sysfs chunk_size attribute validation Anand Jain
  5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-04-09  7:43 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, zlang, 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 | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)
 create mode 100644 common/sysfs

diff --git a/common/sysfs b/common/sysfs
new file mode 100644
index 000000000000..16d4b482f9e9
--- /dev/null
+++ b/common/sysfs
@@ -0,0 +1,145 @@
+##/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)
+	if ! test -e /sys/fs/${FSTYP}/${dname}/${attr}; then
+		return 1
+	fi
+
+	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:
+#  Testcase must include
+#      . ./common/filter
+# Prerequisite checks are kept outside this function
+# to make them clear to the test case, rather than hiding
+# them deep inside another function.
+#      _require_fs_sysfs_attr_policy $TEST_DEV $attr $policy
+_verify_sysfs_syntax()
+{
+	local dev=$1
+	local attr=$2
+	local policy=$3
+	local value=$4
+
+	# 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

* [PATCH v6 5/6] fstests: btrfs: testcase for sysfs policy syntax verification
  2025-04-09  7:43 [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
                   ` (3 preceding siblings ...)
  2025-04-09  7:43 ` [PATCH v6 4/6] fstests: common/sysfs: add new file sysfs and helpers Anand Jain
@ 2025-04-09  7:43 ` Anand Jain
  2025-04-09 10:22   ` Zorro Lang
  2025-04-09  7:43 ` [PATCH v6 6/6] fstests: btrfs: testcase for sysfs chunk_size attribute validation Anand Jain
  5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-04-09  7:43 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, zlang, djwong

Checks if the sysfs attribute sanitizes arguments and verifies
input syntax.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/btrfs/329     | 21 +++++++++++++++++++++
 tests/btrfs/329.out | 19 +++++++++++++++++++
 2 files changed, 40 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..f4faddedf076
--- /dev/null
+++ b/tests/btrfs/329
@@ -0,0 +1,21 @@
+#! /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_test
+_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

* [PATCH v6 6/6] fstests: btrfs: testcase for sysfs chunk_size attribute validation
  2025-04-09  7:43 [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
                   ` (4 preceding siblings ...)
  2025-04-09  7:43 ` [PATCH v6 5/6] fstests: btrfs: testcase for sysfs policy syntax verification Anand Jain
@ 2025-04-09  7:43 ` Anand Jain
  2025-04-09 10:23   ` Zorro Lang
  5 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2025-04-09  7:43 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, zlang, 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     | 21 +++++++++++++++++++++
 tests/btrfs/334.out | 14 ++++++++++++++
 2 files changed, 35 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..d81ec921f1f2
--- /dev/null
+++ b/tests/btrfs/334
@@ -0,0 +1,21 @@
+#! /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_test
+_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 v6 2/6] fstests: check: fix unset seqres in run_section()
  2025-04-09  7:43 ` [PATCH v6 2/6] fstests: check: fix unset seqres in run_section() Anand Jain
@ 2025-04-09  9:57   ` Zorro Lang
  2025-04-09 21:31     ` Anand Jain
  0 siblings, 1 reply; 14+ messages in thread
From: Zorro Lang @ 2025-04-09  9:57 UTC (permalink / raw)
  To: Anand Jain; +Cc: fstests, linux-btrfs, djwong

On Wed, Apr 09, 2025 at 03:43:14PM +0800, Anand Jain wrote:
> Ensure seqres is set early in run_section().
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  check | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/check b/check
> index 32890470a020..16f695e9d75c 100755
> --- a/check
> +++ b/check
> @@ -804,6 +804,7 @@ function run_section()
>  
>  	seq="check.$$"
>  	check="$RESULT_BASE/check"
> +	seqres="$check"

The "seqres" even might be used earlier than that. If your rootfs is readonly,
you'll see that.

Thanks,
Zorro

>  
>  	# don't leave old full output behind on a clean run
>  	rm -f $check.full
> @@ -849,7 +850,6 @@ function run_section()
>  	  fi
>  	fi
>  
> -	seqres="$check"
>  	_check_test_fs
>  
>  	loop_status=()	# track rerun-on-failure state
> -- 
> 2.47.0
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 4/6] fstests: common/sysfs: add new file sysfs and helpers
  2025-04-09  7:43 ` [PATCH v6 4/6] fstests: common/sysfs: add new file sysfs and helpers Anand Jain
@ 2025-04-09 10:21   ` Zorro Lang
  0 siblings, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-04-09 10:21 UTC (permalink / raw)
  To: Anand Jain; +Cc: fstests, linux-btrfs, djwong

On Wed, Apr 09, 2025 at 03:43:16PM +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>
> ---

This version is good to me,

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/sysfs | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 145 insertions(+)
>  create mode 100644 common/sysfs
> 
> diff --git a/common/sysfs b/common/sysfs
> new file mode 100644
> index 000000000000..16d4b482f9e9
> --- /dev/null
> +++ b/common/sysfs
> @@ -0,0 +1,145 @@
> +##/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)
> +	if ! test -e /sys/fs/${FSTYP}/${dname}/${attr}; then
> +		return 1
> +	fi
> +
> +	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:
> +#  Testcase must include
> +#      . ./common/filter
> +# Prerequisite checks are kept outside this function
> +# to make them clear to the test case, rather than hiding
> +# them deep inside another function.
> +#      _require_fs_sysfs_attr_policy $TEST_DEV $attr $policy
> +_verify_sysfs_syntax()
> +{
> +	local dev=$1
> +	local attr=$2
> +	local policy=$3
> +	local value=$4
> +
> +	# 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

* Re: [PATCH v6 5/6] fstests: btrfs: testcase for sysfs policy syntax verification
  2025-04-09  7:43 ` [PATCH v6 5/6] fstests: btrfs: testcase for sysfs policy syntax verification Anand Jain
@ 2025-04-09 10:22   ` Zorro Lang
  0 siblings, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-04-09 10:22 UTC (permalink / raw)
  To: Anand Jain; +Cc: fstests, linux-btrfs, djwong

On Wed, Apr 09, 2025 at 03:43:17PM +0800, Anand Jain wrote:
> Checks if the sysfs attribute sanitizes arguments and verifies
> input syntax.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---

This version is good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/btrfs/329     | 21 +++++++++++++++++++++
>  tests/btrfs/329.out | 19 +++++++++++++++++++
>  2 files changed, 40 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..f4faddedf076
> --- /dev/null
> +++ b/tests/btrfs/329
> @@ -0,0 +1,21 @@
> +#! /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_test
> +_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	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 6/6] fstests: btrfs: testcase for sysfs chunk_size attribute validation
  2025-04-09  7:43 ` [PATCH v6 6/6] fstests: btrfs: testcase for sysfs chunk_size attribute validation Anand Jain
@ 2025-04-09 10:23   ` Zorro Lang
  0 siblings, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-04-09 10:23 UTC (permalink / raw)
  To: Anand Jain; +Cc: fstests, linux-btrfs, djwong

On Wed, Apr 09, 2025 at 03:43:18PM +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>
> ---

This version is good to me,

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/btrfs/334     | 21 +++++++++++++++++++++
>  tests/btrfs/334.out | 14 ++++++++++++++
>  2 files changed, 35 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..d81ec921f1f2
> --- /dev/null
> +++ b/tests/btrfs/334
> @@ -0,0 +1,21 @@
> +#! /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_test
> +_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	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 2/6] fstests: check: fix unset seqres in run_section()
  2025-04-09  9:57   ` Zorro Lang
@ 2025-04-09 21:31     ` Anand Jain
  2025-04-10  7:12       ` Zorro Lang
  2025-05-24  1:39       ` Anand Jain
  0 siblings, 2 replies; 14+ messages in thread
From: Anand Jain @ 2025-04-09 21:31 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, linux-btrfs, djwong



On 9/4/25 17:57, Zorro Lang wrote:
> On Wed, Apr 09, 2025 at 03:43:14PM +0800, Anand Jain wrote:
>> Ensure seqres is set early in run_section().
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   check | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/check b/check
>> index 32890470a020..16f695e9d75c 100755
>> --- a/check
>> +++ b/check
>> @@ -804,6 +804,7 @@ function run_section()
>>   
>>   	seq="check.$$"
>>   	check="$RESULT_BASE/check"
>> +	seqres="$check"
> 
> The "seqres" even might be used earlier than that. If your rootfs is readonly,
> you'll see that.
> 

Zorro,

Thanks a lot for the review and RVB!

I’ll take care of this patch 2/6 in a separate patchset.
Meanwhile, could you help merge the rest of the sysfs patches,
except for patch 2/6? I don't want the seqres issue to block
the rest of the sysfs patches.

Thanks, Anand

> Thanks,
> Zorro
> 
>>   
>>   	# don't leave old full output behind on a clean run
>>   	rm -f $check.full
>> @@ -849,7 +850,6 @@ function run_section()
>>   	  fi
>>   	fi
>>   
>> -	seqres="$check"
>>   	_check_test_fs
>>   
>>   	loop_status=()	# track rerun-on-failure state
>> -- 
>> 2.47.0
>>
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 2/6] fstests: check: fix unset seqres in run_section()
  2025-04-09 21:31     ` Anand Jain
@ 2025-04-10  7:12       ` Zorro Lang
  2025-05-24  1:39       ` Anand Jain
  1 sibling, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2025-04-10  7:12 UTC (permalink / raw)
  To: Anand Jain; +Cc: fstests, linux-btrfs, djwong

On Thu, Apr 10, 2025 at 05:31:39AM +0800, Anand Jain wrote:
> 
> 
> On 9/4/25 17:57, Zorro Lang wrote:
> > On Wed, Apr 09, 2025 at 03:43:14PM +0800, Anand Jain wrote:
> > > Ensure seqres is set early in run_section().
> > > 
> > > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > > ---
> > >   check | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/check b/check
> > > index 32890470a020..16f695e9d75c 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -804,6 +804,7 @@ function run_section()
> > >   	seq="check.$$"
> > >   	check="$RESULT_BASE/check"
> > > +	seqres="$check"
> > 
> > The "seqres" even might be used earlier than that. If your rootfs is readonly,
> > you'll see that.
> > 
> 
> Zorro,
> 
> Thanks a lot for the review and RVB!
> 
> I’ll take care of this patch 2/6 in a separate patchset.
> Meanwhile, could you help merge the rest of the sysfs patches,
> except for patch 2/6? I don't want the seqres issue to block
> the rest of the sysfs patches.

OK, let's have the test coverage at first. You can merge this patchset without
the 2/6 in your branch, (or use the way you did in
https://lore.kernel.org/fstests/5e081252abdcf7253ad83d2b5eda49a8818305ad.1743996408.git.anand.jain@oracle.com/
temporarily). Then send PR to me, I'll try to push this patchset in this week :)

Thanks,
Zorro

> 
> Thanks, Anand
> 
> > Thanks,
> > Zorro
> > 
> > >   	# don't leave old full output behind on a clean run
> > >   	rm -f $check.full
> > > @@ -849,7 +850,6 @@ function run_section()
> > >   	  fi
> > >   	fi
> > > -	seqres="$check"
> > >   	_check_test_fs
> > >   	loop_status=()	# track rerun-on-failure state
> > > -- 
> > > 2.47.0
> > > 
> > 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 2/6] fstests: check: fix unset seqres in run_section()
  2025-04-09 21:31     ` Anand Jain
  2025-04-10  7:12       ` Zorro Lang
@ 2025-05-24  1:39       ` Anand Jain
  1 sibling, 0 replies; 14+ messages in thread
From: Anand Jain @ 2025-05-24  1:39 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, linux-btrfs, djwong

On 10/4/25 05:31, Anand Jain wrote:
> 
> 
> On 9/4/25 17:57, Zorro Lang wrote:
>> On Wed, Apr 09, 2025 at 03:43:14PM +0800, Anand Jain wrote:
>>> Ensure seqres is set early in run_section().
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   check | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/check b/check
>>> index 32890470a020..16f695e9d75c 100755
>>> --- a/check
>>> +++ b/check
>>> @@ -804,6 +804,7 @@ function run_section()
>>>       seq="check.$$"
>>>       check="$RESULT_BASE/check"
>>> +    seqres="$check"
>>
>> The "seqres" even might be used earlier than that. If your rootfs is 
>> readonly,
>> you'll see that.

Zorro,

While testing, I saw that ".full" is created by _scratch_unmount(), 
called here:


  725 function run_section()
  ::
  815         if [ ! -z "$SCRATCH_DEV" ]; then
  816           _scratch_unmount 2> /dev/null
  817           # call the overridden mkfs - make sure the FS is built


So the fix in this patch is correct..

I also tried with a read-only rootfs—".full" wasn’t
there an error to create that file..

I’ll resend the patch soon.

Thanks. Anand


>>
> 
> Zorro,
> 
> Thanks a lot for the review and RVB!
> 
> I’ll take care of this patch 2/6 in a separate patchset.
> Meanwhile, could you help merge the rest of the sysfs patches,
> except for patch 2/6? I don't want the seqres issue to block
> the rest of the sysfs patches.
> 
> Thanks, Anand
> 
>> Thanks,
>> Zorro
>>
>>>       # don't leave old full output behind on a clean run
>>>       rm -f $check.full
>>> @@ -849,7 +850,6 @@ function run_section()
>>>         fi
>>>       fi
>>> -    seqres="$check"
>>>       _check_test_fs
>>>       loop_status=()    # track rerun-on-failure state
>>> -- 
>>> 2.47.0
>>>
>>
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-05-24  1:40 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-09  7:43 [PATCH v6 0/6] fstests: btrfs: add test case to validate sysfs input arguments Anand Jain
2025-04-09  7:43 ` [PATCH v6 1/6] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout Anand Jain
2025-04-09  7:43 ` [PATCH v6 2/6] fstests: check: fix unset seqres in run_section() Anand Jain
2025-04-09  9:57   ` Zorro Lang
2025-04-09 21:31     ` Anand Jain
2025-04-10  7:12       ` Zorro Lang
2025-05-24  1:39       ` Anand Jain
2025-04-09  7:43 ` [PATCH v6 3/6] fstests: filter: helper for sysfs error filtering Anand Jain
2025-04-09  7:43 ` [PATCH v6 4/6] fstests: common/sysfs: add new file sysfs and helpers Anand Jain
2025-04-09 10:21   ` Zorro Lang
2025-04-09  7:43 ` [PATCH v6 5/6] fstests: btrfs: testcase for sysfs policy syntax verification Anand Jain
2025-04-09 10:22   ` Zorro Lang
2025-04-09  7:43 ` [PATCH v6 6/6] fstests: btrfs: testcase for sysfs chunk_size attribute validation Anand Jain
2025-04-09 10:23   ` Zorro Lang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox