Re: [RFC PATCH 1/2] add basic user-space tests with qemu

[Samuel Thibault]

Hello,

Nice! :D

Luca Dariz, le jeu. 19 oct. 2023 20:57:46 +0200, a ecrit:
> * tests/configfrag.ac: add MIGUSER, required for user-space tests as
>   it might be different from the one used to ubild the kernel.

Can't we just automatically select the proper mig? depending on
user-land bitness we can use i686-gnu-mig or x86_64-gnu-mig.

> * tests/include/kern/printf.h: reuse kern/printf.h
> * tests/include/util/atoi.h: reuse util/atoi.h

Rather use a symlink for such files?

> diff --git a/tests/Makefrag.am b/tests/Makefrag.am
> index 2723f64a..c16326e8 100644
> --- a/tests/Makefrag.am
> +++ b/tests/Makefrag.am
> @@ -23,4 +23,129 @@

> +if HOST_ix86
> +QEMU=qemu-system-i386

Using qemu-system-i386 fails on my system: grub starts, loads the
kernel, but booting it fails. It doesn't seem related to this patch
series, as qemu-system-i386 -kernel gnumach seems to fail the same way.
But we'd rather fix it before adding the test support :)

> +QEMU_GDB_PORT ?= 11234

Typo?

> +     cat $(srcdir)/tests/grub.cfg.single.template | \
> +             sed -e s/BOOTMODULE/$</g | \
> +             sed -e "s/GNUMACHARGS/$(GNUMACH_ARGS)/g" | \
> +             cat >$(builddir)/tests/isofiles/boot/grub/grub.cfg

rather
        < $(srcdir)/tests/grub.cfg.single.template \
                sed -e s/BOOTMODULE/$</g \
                    -e "s/GNUMACHARGS/$(GNUMACH_ARGS)/g" \
                >$(builddir)/tests/isofiles/boot/grub/grub.cfg


> diff --git a/tests/run-qemu.sh.template b/tests/run-qemu.sh.template
> new file mode 100644
> index 00000000..1f116354
> --- /dev/null
> +++ b/tests/run-qemu.sh.template
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +set -e
> +
> +cmd="QEMU QEMU_OPTS -cdrom test-TESTNAME.iso"
> +out=out-TESTNAME
> +if which QEMU >/dev/null ; then
> +    if ! timeout -v --foreground --kill-after=3 15s $cmd > $out; then
> +        tail -n +18 $out  # skip terminal reconfiguration

Better use sed -n '/start module-/,$p' or such to properly skip all
kernel startup prints. You could also make the grub script print
something, that you can catch in the sed script. And you can put the sed
script instead of "> $out", so the output file is already clean and you
don't have to duplicate the one-liner to print it.

Also, please think about cleaning files :)

> diff --git a/tests/testlib.c b/tests/testlib.c
> new file mode 100644
> index 00000000..13aa2614
> --- /dev/null
> +++ b/tests/testlib.c
> @@ -0,0 +1,125 @@
> +// from glibc's sysdeps/mach/usleep.c

No need to keep the reference, it's small and trivial enough that it's
not copyrightable :)

> +int msleep(uint32_t timeout)
> +{
> +  mach_port_t recv = mach_reply_port();
> +  return mach_msg(NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,
> +                  0, 0, recv, timeout, MACH_PORT_NULL);
> +}
> +
> +const char* e2s(int err)
> +{
> +    switch (err)
> +    {
> +    case MACH_SEND_INVALID_DATA: return "MACH_SEND_INVALID_DATA";

You could use a macro

#define E2S(s) \
    case s: return #s;

        E2S(MACH_SEND_INVALID_DATA)

Samuel