[PATCH v2 0/5] fstests: btrfs: add test case to validate sysfs input arguments

[PATCH v2 0/5] fstests: btrfs: add test case to validate sysfs input arguments

[Anand Jain]

v2:
Mainly adds a generic function to check if a sysfs attribute reports
invalid input.

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

 common/filter       |   9 +++
 common/rc           | 139 +++++++++++++++++++++++++++++++++++++++++++-
 tests/btrfs/329     |  18 ++++++
 tests/btrfs/329.out |  19 ++++++
 tests/btrfs/334     |  18 ++++++
 tests/btrfs/334.out |  14 +++++
 6 files changed, 216 insertions(+), 1 deletion(-)
 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

[PATCH v2 1/5] fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout

[Anand Jain]

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 c73007fef762..def55ac68771 100644
--- a/common/rc
+++ b/common/rc
@@ -5081,7 +5081,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

[PATCH v2 2/5] fstests: filter: helper for sysfs error filtering

[Anand Jain]

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

[PATCH v2 3/5] fstests: common/rc: add sysfs argument verification helpers

[Anand Jain]

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/rc | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)

diff --git a/common/rc b/common/rc
index def55ac68771..cc5680581608 100644
--- a/common/rc
+++ b/common/rc
@@ -5147,6 +5147,142 @@ _require_fs_sysfs_attr()
 	_notrun "This test requires /sys/fs/${FSTYP}/${dname}/${attr}"
 }
 
+# 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
+}
+
 # Test for the existence of a sysfs entry at /sys/fs/$FSTYP/DEV/$ATTR
 #
 # Only one argument is needed:
-- 
2.47.0

[PATCH v2 4/5] fstests: btrfs: testcase for sysfs policy syntax verification

[Anand Jain]

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

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/btrfs/329     | 18 ++++++++++++++++++
 tests/btrfs/329.out | 19 +++++++++++++++++++
 2 files changed, 37 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..3b52d51bb477
--- /dev/null
+++ b/tests/btrfs/329
@@ -0,0 +1,18 @@
+#! /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/filter
+
+_require_fs_sysfs_attr_policy $TEST_DEV read_policy round-robin
+verify_sysfs_syntax $TEST_DEV read_policy round-robin 4k
+
+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

[PATCH v2 5/5] fstests: btrfs: testcase for sysfs chunk_size attribute validation

[Anand Jain]

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     | 18 ++++++++++++++++++
 tests/btrfs/334.out | 14 ++++++++++++++
 2 files changed, 32 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..b662fe2aca63
--- /dev/null
+++ b/tests/btrfs/334
@@ -0,0 +1,18 @@
+#! /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/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

Re: [PATCH v2 3/5] fstests: common/rc: add sysfs argument verification helpers

[Dave Chinner]

On Wed, Feb 05, 2025 at 07:06:38PM +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/rc | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 136 insertions(+)

Can we move all this sysfs specific stuff to a new common/sysfs
file? common/rc is already too large and there's more than enough
sysfs specific code here to justify it's own file...

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2 0/5] fstests: btrfs: add test case to validate sysfs input arguments

[Dave Chinner]

On Wed, Feb 05, 2025 at 07:06:35PM +0800, Anand Jain wrote:
> v2:
> Mainly adds a generic function to check if a sysfs attribute reports
> invalid input.
> 
> 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

All looks ok, except for my comment about creating common/sysfs for
all the sysfs stuff. The individual tests look much simpler and
nicer, too.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2 0/5] fstests: btrfs: add test case to validate sysfs input arguments

[Anand Jain]

On 6/2/25 05:21, Dave Chinner wrote:
> On Wed, Feb 05, 2025 at 07:06:35PM +0800, Anand Jain wrote:
>> v2:
>> Mainly adds a generic function to check if a sysfs attribute reports
>> invalid input.
>>
>> 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
> 
> All looks ok, except for my comment about creating common/sysfs for
> all the sysfs stuff. The individual tests look much simpler and
> nicer, too.
> 

Yeah, the sysfs functions need a new file. I'll send the reorg cleanups.

Thx, Anand

> -Dave.