Re: [PATCH 1/2] livepatch: Move tests from lib/livepatch to selftests/livepatch

[Shuah Khan <skhan@linuxfoundation.org>]

On 6/9/22 2:00 PM, Shuah Khan wrote:
> On 6/3/22 8:32 AM, Marcos Paulo de Souza wrote:
>> It allows writing more complex tests, for example, an userspace C code
>> that would use the livepatched kernel code. As a bonus it allows to use
>> "gen_tar" to export the livepatch selftests, rebuild the modules by
>> running make in selftests/livepatch directory and simplifies the process
>> of creating and debugging new selftests.
>>
>> It keeps the ability to execute the tests by running the shell scripts,
>> like "test-livepatch.sh", but beware that the kernel modules
>> might not be up-to-date.
>>
>> Remove 'modprobe --dry-run' call as the modules will be built before
>> running the tests. Also remove the TEST_LIVEPATCH Kconfig since the
>> modules won't be build based on a Kconfig.
>>
>> Adjust functions.sh to call insmod and fix the check_result strings to
>> reflect the change.
>>
>> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
>> ---
>>   arch/s390/configs/debug_defconfig             |  1 -
>>   arch/s390/configs/defconfig                   |  1 -
>>   lib/Kconfig.debug                             | 22 --------
>>   lib/Makefile                                  |  2 -
>>   lib/livepatch/Makefile                        | 14 ------
>>   tools/testing/selftests/livepatch/Makefile    | 27 +++++++++-
>>   tools/testing/selftests/livepatch/README      |  5 +-
>>   tools/testing/selftests/livepatch/config      |  1 -
>>   .../testing/selftests/livepatch/functions.sh  | 34 +++++--------
>>   .../selftests/livepatch/test-callbacks.sh     | 50 +++++++++----------
>>   .../selftests/livepatch/test-ftrace.sh        |  6 +--
>>   .../selftests/livepatch/test-livepatch.sh     | 10 ++--
>>   .../selftests/livepatch/test-shadow-vars.sh   |  2 +-
>>   .../testing/selftests/livepatch/test-state.sh | 18 +++----
>>   .../selftests/livepatch/test_modules/Makefile | 24 +++++++++
>>   .../test_modules}/test_klp_atomic_replace.c   |  0
>>   .../test_modules}/test_klp_callbacks_busy.c   |  0
>>   .../test_modules}/test_klp_callbacks_demo.c   |  0
>>   .../test_modules}/test_klp_callbacks_demo2.c  |  0
>>   .../test_modules}/test_klp_callbacks_mod.c    |  0
>>   .../test_modules}/test_klp_livepatch.c        |  0
>>   .../test_modules}/test_klp_shadow_vars.c      |  0
>>   .../livepatch/test_modules}/test_klp_state.c  |  0
>>   .../livepatch/test_modules}/test_klp_state2.c |  0
>>   .../livepatch/test_modules}/test_klp_state3.c |  0
>>   25 files changed, 107 insertions(+), 110 deletions(-)
>>   delete mode 100644 lib/livepatch/Makefile
>>   create mode 100644 tools/testing/selftests/livepatch/test_modules/Makefile
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_atomic_replace.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_busy.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_demo.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_demo2.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_mod.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_livepatch.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_shadow_vars.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_state.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_state2.c (100%)
>>   rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_state3.c (100%)
>>
> 
> Looks good to me.
> 
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> 

Looks like this patch is moving modules under selftests. We keep modules
under lib.

I have to look at this more closely - so for now I am not okay with this
change.

thanks,
-- Shuah