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

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

[Anand Jain]

v3:
Move sysfs functions from common/rc to common/sysfs
btrfs/329: use 4096 instead of 4k

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

 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

-- 
2.43.5

[PATCH v3 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 cf6316a224ff..942e201649dd 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.43.5

[PATCH v3 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.43.5

[PATCH v3 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/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.43.5

[PATCH v3 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     | 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.43.5

[PATCH v3 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     | 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.43.5

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

[Darrick J. Wong]

On Mon, Feb 24, 2025 at 08:15:04PM +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 cf6316a224ff..942e201649dd 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

Did you mean to    ^ escape ^ these double-quotes?  Without it,
whitespace in $content might not be logged correctly.

--D

> +	echo "$content" 2>&1 > /sys/fs/${FSTYP}/${dname}/${attr} | tee -a $seqres.full
>  }
>  
>  # Print the content of /sys/fs/$FSTYP/$DEV/$ATTR
> -- 
> 2.43.5
> 
> 

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

[Anand Jain]

On 2/25/25 02:18, Darrick J. Wong wrote:
> On Mon, Feb 24, 2025 at 08:15:04PM +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 cf6316a224ff..942e201649dd 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
> 
> Did you mean to    ^ escape ^ these double-quotes?  Without it,
> whitespace in $content might not be logged correctly.
> 

Oh no! I remember fixing this. Looks like I messed it up during the rebase.

I’ve fixed it locally and will wait before sending.

Thx.


> --D
> 
>> +	echo "$content" 2>&1 > /sys/fs/${FSTYP}/${dname}/${attr} | tee -a $seqres.full
>>   }
>>   
>>   # Print the content of /sys/fs/$FSTYP/$DEV/$ATTR
>> -- 
>> 2.43.5
>>
>>