Re: [PATCH v3 5/5] livepatch: Selftests of the API for tracking system state changes

[Joe Lawrence <joe.lawrence@redhat.com>]

On Thu, Oct 03, 2019 at 11:01:37AM +0200, Petr Mladek wrote:
> Four selftests for the new API.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Acked-by: Miroslav Benes <mbenes@suse.cz>
> ---
>  lib/livepatch/Makefile                          |   5 +-
>  lib/livepatch/test_klp_state.c                  | 161 ++++++++++++++++++++
>  lib/livepatch/test_klp_state2.c                 | 190 ++++++++++++++++++++++++
>  lib/livepatch/test_klp_state3.c                 |   5 +
>  tools/testing/selftests/livepatch/Makefile      |   3 +-
>  tools/testing/selftests/livepatch/test-state.sh | 180 ++++++++++++++++++++++
>  6 files changed, 542 insertions(+), 2 deletions(-)
>  create mode 100644 lib/livepatch/test_klp_state.c
>  create mode 100644 lib/livepatch/test_klp_state2.c
>  create mode 100644 lib/livepatch/test_klp_state3.c
>  create mode 100755 tools/testing/selftests/livepatch/test-state.sh
> 
> [ ... snip ... ]
> diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh
> new file mode 100755
> index 000000000000..1139c664c11c
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-state.sh
> @@ -0,0 +1,180 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2018 Joe Lawrence <joe.lawrence@redhat.com>
> +

nit: this should probably read as:
# Copyright (C) 2019 Petr Mladek <pmladek@suse.com>

> +. $(dirname $0)/functions.sh
> +
> +MOD_LIVEPATCH=test_klp_state
> +MOD_LIVEPATCH2=test_klp_state2
> +MOD_LIVEPATCH3=test_klp_state3
> +
> +set_dynamic_debug
> +
> +
> +# TEST: Loading and removing a module that modifies the system state
> +
> +echo -n "TEST: system state modification ... "
> +dmesg -C
> +
> +load_lp $MOD_LIVEPATCH
> +disable_lp $MOD_LIVEPATCH
> +unload_lp $MOD_LIVEPATCH
> +
> +check_result "% modprobe test_klp_state
> +livepatch: enabling patch 'test_klp_state'
> +livepatch: 'test_klp_state': initializing patching transition
> +test_klp_state: pre_patch_callback: vmlinux
> +test_klp_state: allocate_loglevel_state: allocating space to store console_loglevel
> +livepatch: 'test_klp_state': starting patching transition
> +livepatch: 'test_klp_state': completing patching transition
> +test_klp_state: post_patch_callback: vmlinux
> +test_klp_state: fix_console_loglevel: fixing console_loglevel
> +livepatch: 'test_klp_state': patching complete
> +% echo 0 > /sys/kernel/livepatch/test_klp_state/enabled
> +livepatch: 'test_klp_state': initializing unpatching transition
> +test_klp_state: pre_unpatch_callback: vmlinux
> +test_klp_state: restore_console_loglevel: restoring console_loglevel
> +livepatch: 'test_klp_state': starting unpatching transition
> +livepatch: 'test_klp_state': completing unpatching transition
> +test_klp_state: post_unpatch_callback: vmlinux
> +test_klp_state: free_loglevel_state: freeing space for the stored console_loglevel
> +livepatch: 'test_klp_state': unpatching complete
> +% rmmod test_klp_state"
> +

nit: a matter of style, and I don't mind either, but the other test
scripts used $MOD_LIVEPATCH{2,3} variable replacement in the
check_result string.  I think I originally did that when we were
reviewing the first self-test patchset and the filenames may or may not
have changed.  Those names are stable now, so I don't have a strong
opinion either way.

FWIW, if someone wants to make the copyright change on merge, I'm cool
with this version.

-- Joe