On 9/2/22 8:37 AM, Petr Mladek wrote:
> On Mon 2022-08-01 18:08:57, Song Liu wrote:
>> Add a test for livepatch sysfs entries.
>>
>> Signed-off-by: Song Liu <song@xxxxxxxxxx>
>> ---
>> tools/testing/selftests/livepatch/Makefile | 3 +-
>> .../testing/selftests/livepatch/test-sysfs.sh | 40 +++++++++++++++++++
>> 2 files changed, 42 insertions(+), 1 deletion(-)
>> create mode 100755 tools/testing/selftests/livepatch/test-sysfs.sh
>>
>> diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
>> index 1acc9e1fa3fb..02fadc9d55e0 100644
>> --- a/tools/testing/selftests/livepatch/Makefile
>> +++ b/tools/testing/selftests/livepatch/Makefile
>> @@ -6,7 +6,8 @@ TEST_PROGS := \
>> test-callbacks.sh \
>> test-shadow-vars.sh \
>> test-state.sh \
>> - test-ftrace.sh
>> + test-ftrace.sh \
>> + test-sysfs.sh
>>
>> TEST_FILES := settings
>>
>> diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
>> new file mode 100755
>> index 000000000000..eb4a69ba1c2c
>> --- /dev/null
>> +++ b/tools/testing/selftests/livepatch/test-sysfs.sh
>> @@ -0,0 +1,40 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2022 Song Liu <song@xxxxxxxxxx>
>> +
>> +. $(dirname $0)/functions.sh
>> +
>> +MOD_LIVEPATCH=test_klp_livepatch
>> +
>> +setup_config
>> +
>> +# - load a livepatch and verifies the sysfs entries work as expected
>> +
>> +start_test "sysfs test"
>> +
>> +load_lp $MOD_LIVEPATCH
>> +
>> +grep . "/sys/kernel/livepatch/$MOD_LIVEPATCH"/* > /dev/kmsg
>> +grep . "/sys/kernel/livepatch/$MOD_LIVEPATCH"/*/* > /dev/kmsg
>> +
>
> I see the following when I run the test:
>
> host:kernel/linux/tools/testing/selftests/livepatch # ./test-sysfs.sh
> TEST: sysfs test ... grep: /sys/kernel/livepatch/test_klp_livepatch/force: Permission denied
> grep: /sys/kernel/livepatch/test_klp_livepatch/vmlinux: Is a directory
> grep: /sys/kernel/livepatch/test_klp_livepatch/vmlinux/cmdline_proc_show,1: Is a directory
> ok
>
> The errors are expected. A solution would be to redirect 2>&1 or
> 2>/dev/null. Both looks a bit ugly to me.
>
> Instead, I would suggest to create some helper scripts in functions.sh,
> for example:
>
I realize this was just a code sketch, but a few ideas before
copy/pasting the functions :)
> # check_sysfs_exists(modname, rel_path, expected_rights) - check sysfs interface
# check_sysfs_rights(modname, rel_path, expected_rights) - check sysfs
path permissions
> # modname - livepatch module creating the sysfs interface
> # rel_patch - relative patch of the sysfs interface
s/patch/path
> # expected_rights - expected access rights
> function check_sysfs_rights() {
> local mod="$1"; shift
> local rel_path="$1"; shift
> local expected_rights="$1"; shift
>
> local path="$KLP_SYSFS_DIR/$mod/$rel_path"
Checking for existence here might be cleaner than failing later. Unless
the caller is going to do it first. Or should we just have a
check_sysfs_exists() like your function comments hint at?
> local rights=`ls -l -d $path | cut -d " " -f 1`
$(/bin/stat --format '%A' "$path") easier?
>
> if test "$rights" != "$expected_rights" ; then
> die "Unexpected access rights of $path: $expected_rights vs. $rights"
> fi
> }
>
> # check_sysfs_exists(modname, rel_path, expected_value) - check sysfs interface
# check_sysfs_value(modname, rel_path, expected_value) - check sysfs value
> # modname - livepatch module creating the sysfs interface
> # rel_patch - relative patch of the sysfs interface
s/patch/path
> # expected_value - expected value read from the file
> function check_sysfs_value() {
> local mod="$1"; shift
> local rel_path="$1"; shift
> local expected_value="$1"; shift
>
> local path="$KLP_SYSFS_DIR/$mod/$rel_path"
> local value=`cat $path`
>
> if test "$value" != "$expected_value" ; then
> die "Unexpected value in $path: $expected_value vs. $value"
> fi
> }
>
> It would allow to create a fine tuned tests, for example:
>
> check_sysfs_rights "$MOD_LIVEPATCH" "" "drwxr-xr-x"
> check_sysfs_rights "$MOD_LIVEPATCH" "enabled" "-rw-r--r--"
>
> check_sysfs_value "$MOD_LIVEPATCH" "enabled" "1"
>
>
>
> Also it would be great to test that the "patched" value is "0"
> when the object is not patched. I would require to create
> a test module that might be livepatched. I mean something like:
>
>
> check_sysfs_value $MOD_LIVEPATCH "$TEST_MODULE/patched" "0"
> load_mod $TEST_MODULE
> check_sysfs_value $MOD_LIVEPATCH "$TEST_MODULE/patched" "1"
> unload_mod $TEST_MODULE
> check_sysfs_value $MOD_LIVEPATCH "$TEST_MODULE/patched" "0"
>
>
Yes, these helpers would be nice to have to better verify the sysfs
structure and values.
Regards,
--
Joe
