| Message ID | 3aecf19197d07ff18ed1c0dda9e63fcaa49b69d1.1738161075.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fstests: btrfs: add test case to validate sysfs input arguments | expand |
On Wed, Jan 29, 2025 at 11:19:54PM +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 | 92 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/329.out | 2 + > 2 files changed, 94 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..9f63ab951eac > --- /dev/null > +++ b/tests/btrfs/329 > @@ -0,0 +1,92 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2025 Oracle. All Rights Reserved. > +# > +# FS QA Test 329 > +# > +# Verify sysfs knob input syntax. > +# > +. ./common/preamble > +_begin_fstest auto quick > + > +. ./common/filter > + > +# Modify as appropriate. > +_require_scratch > +_require_fs_sysfs read_policy > + > +_scratch_mkfs > /dev/null 2>&1 || _fail "mkfs failed" > +_scratch_mount > + > +set_sysfs_policy() > +{ > + local attr=$1 > + shift > + local policy=$@ > + > + _set_fs_sysfs_attr $SCRATCH_DEV $attr ${policy} > + _get_fs_sysfs_attr $SCRATCH_DEV $attr | grep -q "[${policy}]" > + if [[ $? != 0 ]]; then > + echo "Setting sysfs $attr $policy failed" > + fi > +} > + > +set_sysfs_policy_must_fail() > +{ > + local attr=$1 > + shift > + local policy=$@ > + > + _set_fs_sysfs_attr $SCRATCH_DEV $attr ${policy} | _filter_sysfs_error \ > + | _expect_error_invalid_argument | tee -a $seqres.full This "catch an exact error or output a different error then use golden image match failure on secondary error to mark the test as failed" semantic is .... overly complex. The output on failure of _filter_sysfs_error will be "Invalid input". If there's some other failure or it succeeds, the output will indicate the failure that occurred (i.e. missing line means no error, different error will output directly by the filter). The golden image matching will still fail the test. IOWs, _expect_error_invalid_argument and the output to seqres.full can go away if the test.out file has a matching error for each call to set_sysfs_policy_must_fail(). i.e it looks like: QA output created by 329 Invalid input Invalid input Invalid input Invalid input Invalid input Invalid input ..... Invalid input -Dave.
>> +set_sysfs_policy_must_fail() >> +{ >> + local attr=$1 >> + shift >> + local policy=$@ >> + >> + _set_fs_sysfs_attr $SCRATCH_DEV $attr ${policy} | _filter_sysfs_error \ >> + | _expect_error_invalid_argument | tee -a $seqres.full > > This "catch an exact error or output a different error then use > golden image match failure on secondary error to mark the test as > failed" semantic is .... overly complex. > > The output on failure of _filter_sysfs_error will be "Invalid > input". If there's some other failure or it succeeds, the output > will indicate the failure that occurred (i.e. missing line means no > error, different error will output directly by the filter). The > golden image matching will still fail the test. > > IOWs, _expect_error_invalid_argument and the output to seqres.full > can go away if the test.out file has a matching error for each > call to set_sysfs_policy_must_fail(). i.e it looks like: > > QA output created by 329 > Invalid input > Invalid input > Invalid input > Invalid input > Invalid input > Invalid input > ..... > Invalid input Thanks for the review. This test case verifies the sysfs interface syntax in general. Relying on golden output can cause false negatives on older kernels lacking support for newer sysfs policies. Creating individual test cases for each sysfs interface is unnecessary overhead. With this approach, when needed, we use: if _has_fs_sysfs_attr $dev <sysfs-interface>; then verify_sysfs_syntax <sysfs-interface> <value> fi - Anand
diff --git a/tests/btrfs/329 b/tests/btrfs/329 new file mode 100755 index 000000000000..9f63ab951eac --- /dev/null +++ b/tests/btrfs/329 @@ -0,0 +1,92 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2025 Oracle. All Rights Reserved. +# +# FS QA Test 329 +# +# Verify sysfs knob input syntax. +# +. ./common/preamble +_begin_fstest auto quick + +. ./common/filter + +# Modify as appropriate. +_require_scratch +_require_fs_sysfs read_policy + +_scratch_mkfs > /dev/null 2>&1 || _fail "mkfs failed" +_scratch_mount + +set_sysfs_policy() +{ + local attr=$1 + shift + local policy=$@ + + _set_fs_sysfs_attr $SCRATCH_DEV $attr ${policy} + _get_fs_sysfs_attr $SCRATCH_DEV $attr | grep -q "[${policy}]" + if [[ $? != 0 ]]; then + echo "Setting sysfs $attr $policy failed" + fi +} + +set_sysfs_policy_must_fail() +{ + local attr=$1 + shift + local policy=$@ + + _set_fs_sysfs_attr $SCRATCH_DEV $attr ${policy} | _filter_sysfs_error \ + | _expect_error_invalid_argument | tee -a $seqres.full +} + +verify_sysfs_syntax() +{ + local attr=$1 + local policy=$2 + local value=$3 + + # Test policy specified wrongly. Must fail. + set_sysfs_policy_must_fail $attr "'$policy $policy'" + set_sysfs_policy_must_fail $attr "'$policy t'" + set_sysfs_policy_must_fail $attr "' '" + set_sysfs_policy_must_fail $attr "'${policy} n'" + set_sysfs_policy_must_fail $attr "'n ${policy}'" + set_sysfs_policy_must_fail $attr "' ${policy}'" + set_sysfs_policy_must_fail $attr "' ${policy} '" + set_sysfs_policy_must_fail $attr "'${policy} '" + set_sysfs_policy_must_fail $attr _${policy} + set_sysfs_policy_must_fail $attr ${policy}_ + set_sysfs_policy_must_fail $attr _${policy}_ + set_sysfs_policy_must_fail $attr ${policy}: + # Test policy longer than 32 chars fails stable. + set_sysfs_policy_must_fail $attr 'jfdkkkkjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjffjfjfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' + + # Test policy specified correctly. Must pass. + set_sysfs_policy $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 $attr "'$policy: $value'" + set_sysfs_policy_must_fail $attr "'$policy:$value '" + set_sysfs_policy_must_fail $attr "'$policy:$value '" + set_sysfs_policy_must_fail $attr "'$policy: $value'" + set_sysfs_policy_must_fail $attr "'$policy :$value'" + + # Test policy and value all specified correctly. Must pass. + set_sysfs_policy $attr $policy:$value +} + +verify_sysfs_syntax read_policy pid +verify_sysfs_syntax read_policy round-robin 4k +verify_sysfs_syntax allocation/data/chunk_size 10g + +echo Silence is golden + +status=0 +exit diff --git a/tests/btrfs/329.out b/tests/btrfs/329.out new file mode 100644 index 000000000000..9794dc15960d --- /dev/null +++ b/tests/btrfs/329.out @@ -0,0 +1,2 @@ +QA output created by 329 +Silence is golden
Checks if the sysfs attribute sanitizes arguments and verifies input syntax. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- tests/btrfs/329 | 92 +++++++++++++++++++++++++++++++++++++++++++++ tests/btrfs/329.out | 2 + 2 files changed, 94 insertions(+) create mode 100755 tests/btrfs/329 create mode 100644 tests/btrfs/329.out