Re: [PATCH v7 19/22] selftests/liveupdate: add test infrastructure and scripts

[Pasha Tatashin]

On Mon, Nov 24, 2025 at 2:54 AM Mike Rapoport <rppt@xxxxxxxxxx> wrote:
>
> On Sat, Nov 22, 2025 at 05:23:46PM -0500, Pasha Tatashin wrote:
> > Subject: [PATCH v7 19/22] selftests/liveupdate: add test infrastructure and scripts
>
> Maybe                                                ^ end to end

Done.

>
> > Add the testing infrastructure required to verify the liveupdate
> > feature. This includes a custom init process, a test orchestration
> > script, and a batch runner.
>
> And say here that it's end to end test.

Done

> > +static int is_stage_2(void)
> > +{
> > +     char cmdline[COMMAND_LINE_SIZE];
> > +     ssize_t len;
> > +     int fd;
> > +
> > +     fd = open("/proc/cmdline", O_RDONLY);
> > +     if (fd < 0)
> > +             return 0;
> > +
> > +     len = read(fd, cmdline, sizeof(cmdline) - 1);
> > +     close(fd);
> > +
> > +     if (len < 0)
> > +             return 0;
>
> Shouldn't we bail out of the test if read of command line failed?

Sure, done.

> > +function cleanup() {
> > +     local exit_code=$?
> > +
> > +     if [ -z "$workspace_dir" ]; then
> > +             ktap_finished
> > +             return
> > +     fi
> > +
> > +     if [ $exit_code -ne 0 ]; then
> > +             echo "# Test failed (exit code $exit_code)."
> > +             echo "# Workspace preserved at: $workspace_dir"
> > +     elif [ "$KEEP_WORKSPACE" -eq 1 ]; then
> > +             echo "# Workspace preserved (user request) at: $workspace_dir"
> > +     else
> > +             rm -fr "$workspace_dir"
> > +     fi
> > +     ktap_finished
>
>         exit $exit_code

Done

> > +function build_kernel() {
> > +     local build_dir=$1
> > +     local make_cmd=$2
> > +     local kimage=$3
> > +     local target_arch=$4
> > +
> > +     local kconfig="$build_dir/.config"
> > +     local common_conf="$test_dir/config"
> > +     local arch_conf="$test_dir/config.$target_arch"
> > +
> > +     echo "# Building kernel in: $build_dir"
> > +     $make_cmd defconfig
> > +
> > +     local fragments=""
> > +     if [[ -f "$common_conf" ]]; then
> > +             fragments="$fragments $common_conf"
> > +     fi
>
> Without this CONFIG_LIVEUPDATE won't be set
> > +
> > +     if [[ -f "$arch_conf" ]]; then
> > +             fragments="$fragments $arch_conf"
> > +     fi
> > +
> > +     if [[ -n "$fragments" ]]; then
> > +             "$kernel_dir/scripts/kconfig/merge_config.sh" \
> > +                     -Q -m -O "$build_dir" "$kconfig" $fragments >> /dev/null
> > +     fi
>
> I believe you can just
>
>         cat $common_conf $fragments >  $build_dir/.config
>         make olddefconfig
>
> without running defconfig at the beginning
> It will build faster, just make sure to add CONFIG_SERIAL_ to $arch_conf

I will look into that, so how performance really changes,  I liked
using merge_config.sh as it does not print warnings.

>
> > +     $make_cmd olddefconfig
> > +     $make_cmd "$kimage"
> > +     $make_cmd headers_install INSTALL_HDR_PATH="$headers_dir"
> > +}
> > +
> > +function mkinitrd() {
> > +     local build_dir=$1
> > +     local kernel_path=$2
> > +     local test_name=$3
> > +
> > +     # 1. Compile the test binary and the init process
>
> Didn't find 2. ;-)
> Don't think we want the numbering here, plain comments are fine

Updated comment.


>
> > +     "$CROSS_COMPILE"gcc -static -O2 \
> > +             -I "$headers_dir/include" \
> > +             -I "$test_dir" \
> > +             -o "$workspace_dir/test_binary" \
> > +             "$test_dir/$test_name.c" "$test_dir/luo_test_utils.c"
>
> This will have hard time cross-compiling with -nolibc toolchains

Hm, it works for me, I am not sure with nolibc cross compiler, am I
missing something?

>
> > +
> > +     "$CROSS_COMPILE"gcc -s -static -Os -nostdinc -nostdlib          \
> > +                     -fno-asynchronous-unwind-tables -fno-ident      \
> > +                     -fno-stack-protector                            \
> > +                     -I "$headers_dir/include"                       \
> > +                     -I "$kernel_dir/tools/include/nolibc"           \
> > +                     -o "$workspace_dir/init" "$test_dir/init.c"
>
> This failed for me with gcc 14.2.0 (Debian 14.2.0-19):


Updated, removed the extra const, and static.

>
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c: In function ‘run_test’:
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:111:65: error: initializer element is not constant
>   111 |             static const char *const argv[] = {TEST_BINARY, stage_arg, NULL};
>       |                                                             ^~~~~~~~~
>
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:111:65: note: (near initialization for ‘argv[1]’)
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:113:37: error: passing argument 2 of ‘execve’ from incompatible pointer type [-Wincompatible-pointer-types]
>   113 |                 execve(TEST_BINARY, argv, NULL);
>       |                                     ^~~~
>       |                                     |
>       |                                     const char * const*
> In file included from /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:16:
> /usr/include/unistd.h:572:52: note: expected ‘char * const*’ but argument is of type ‘const char * const*’
>   572 | extern int execve (const char *__path, char *const __argv[],
>       |                                        ~~~~~~~~~~~~^~~~~~~~
>
> > +
> > +     cat > "$workspace_dir/cpio_list_inner" <<EOF
> > +dir /dev 0755 0 0
> > +dir /proc 0755 0 0
> > +dir /debugfs 0755 0 0
> > +nod /dev/console 0600 0 0 c 5 1
>
> Don't you need /dev/liveupdate node?

That should be created by the kernel itself.

>
> > +file /init $workspace_dir/init 0755 0 0
> > +file /test_binary $workspace_dir/test_binary 0755 0 0
> > +EOF
> > +
> > +     # Generate inner_initrd.cpio
> > +     "$build_dir/usr/gen_init_cpio" "$workspace_dir/cpio_list_inner" > "$workspace_dir/inner_initrd.cpio"
> > +
> > +     cat > "$workspace_dir/cpio_list" <<EOF
> > +dir /dev 0755 0 0
> > +dir /proc 0755 0 0
> > +dir /debugfs 0755 0 0
> > +nod /dev/console 0600 0 0 c 5 1
>
> And here as well.

Not needed.

>
> > +file /init $workspace_dir/init 0755 0 0
> > +file /kernel $kernel_path 0644 0 0
> > +file /test_binary $workspace_dir/test_binary 0755 0 0
> > +file /initrd.img $workspace_dir/inner_initrd.cpio 0644 0 0
> > +EOF
> > +
> > +     # Generate the final initrd
> > +     "$build_dir/usr/gen_init_cpio" "$workspace_dir/cpio_list" > "$initrd"
> > +     local size=$(du -h "$initrd" | cut -f1)
> > +}
> > +
> > +function run_qemu() {
> > +     local qemu_cmd=$1
> > +     local cmdline=$2
> > +     local kernel_path=$3
> > +     local serial="$workspace_dir/qemu.serial"
> > +
> > +     local accel="-accel tcg"
> > +     local host_machine=$(uname -m)
> > +
> > +     [[ "$host_machine" == "arm64" ]] && host_machine="aarch64"
> > +     [[ "$host_machine" == "x86_64" ]] && host_machine="x86_64"
> > +
> > +     if [[ "$qemu_cmd" == *"$host_machine"* ]]; then
> > +             if [ -w /dev/kvm ]; then
> > +                     accel="-accel kvm"
>
> Just pass both kvm and tcg and let qemu complain.

I hated those warnings, this is why I added this "if" in the first place :-)

Thank you for your reviews, I am going to send this patch separately
from this series, so let's continue the discussion there.

Pasha