Re: [PATCH v2 2/2] selftests/livepatch: add sysfs test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux