[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260205221122.2415333-1-jordanrichards@google.com>
Date: Thu, 5 Feb 2026 22:11:22 +0000
From: Jordan Richards <jordanrichards@...gle.com>
To: rppt@...nel.org
Cc: dmatlack@...gle.com, jasonmiu@...gle.com, jordanrichards@...gle.com,
linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
linux-mm@...ck.org, pasha.tatashin@...een.com, pratyush@...nel.org,
shuah@...nel.org
Subject: Re: [PATCH] selftests/liveupdate: add end to end test infrastructure
and scripts
Sending v2 addressing this feedback soon:
>> + err = kexec_file_load(kernel_fd, initrd_fd, strlen(cmdline) + 1,
>> + cmdline, 0);
>> +
>> + close(initrd_fd);
>> + close(kernel_fd);
>> +
>> + return err ? : 0;
>
> Just return err?
Good point, fixed.
>> diff --git a/tools/testing/selftests/liveupdate/luo_test.sh b/tools/testing/selftests/liveupdate/luo_test.sh
>> new file mode 100755
>> index 000000000000..90ecb16e87bb
>> --- /dev/null
>> +++ b/tools/testing/selftests/liveupdate/luo_test.sh
>
>...
>
>> +function detect_cross_compile() {
>> + local target=$1
>> + local host=$(uname -m)
>> +
>
> This function works fine if you run luo_test.sh directly or have cross
> compilers named the way it expects in $PATH.
>
> But if I run
> CROSS_COMPILE=~/cross/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux- ./run.sh
> on x86, x86 tests fail
>
>> + if [ -n "$CROSS_COMPILE" ]; then
>> + return
>> + fi
>> +
>> + [[ "$host" == "arm64" ]] && host="aarch64"
>> + [[ "$target" == "arm64" ]] && target="aarch64"
>> +
>> + if [[ "$host" == "$target" ]]; then
>> + CROSS_COMPILE=""
>> + return
>> + fi
The host==target test needed to be first, fixed.
>> + local fragments=()
>> +
>> + if [[ -f "$common_conf" ]]; then
>> + fragments+=("$common_conf")
>> + fi
>> +
>> + if [[ -f "$arch_conf" ]]; then
>> + fragments+=("$arch_conf")
>> + fi
>
> I think the common and arch config fragments are required and we can just
> assign fragments directly and run merge_config.sh.
Done.
I wasn't sure if arch_conf would be necessary for all architectures, but it
is kind of needless complexity.
> + if [[ "$qemu_cmd" == *"$host_machine"* ]]; then
> + if [ -w /dev/kvm ]; then
> + accel="-accel kvm"
> + fi
> + fi
>
> Do we care that much about qemu warnings about invalid accelerator to have
> this logic here?
>
> -accel kvm -accel hvf -accel tcg
>
> seems to cover all bases.
The warning doesn't break the tests, so it should be fine.
>> +#include <sys.h>
>
> This breaks running normal make:
>
> luo_test_utils.c:16:10: fatal error: sys.h: No such file or directory
> 16 | #include <sys.h>
> | ^~~~~~~
>
> NOLIBC specific includes and calls should be guarded with #ifdef NOLIBC
Done.
> diff --git a/tools/testing/selftests/liveupdate/run.sh b/tools/testing/selftests/liveupdate/run.sh
> new file mode 100755
> index 000000000000..3f6b29a26648
> --- /dev/null
> +++ b/tools/testing/selftests/liveupdate/run.sh
> @@ -0,0 +1,68 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
>> +OUTPUT_DIR="results_$(date +%Y%m%d_%H%M%S)"
>
> I don't think that putting the results in the current directory rather than
> in SCRIPT_DIR or in an explicitly named directory is a good idea.
Changed to "$SCRIPT_DIR/results_*".
>> +TEST_NAMES=()
>> +while IFS= read -r file; do
>> + TEST_NAMES+=("$(basename "$file" .c)")
>> +done < <(find "$SCRIPT_DIR" -maxdepth 1 -name "luo_*.c" ! -name "luo_test_utils.c")
>
> I don't like name based detection of tests. Listing them explicitly seems a
viable option.
Agreed, changed to a fixed array.
Powered by blists - more mailing lists