qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 31/51] tests/qtest: Support libqtest to build and run on Wind


From: Bin Meng
Subject: Re: [PATCH 31/51] tests/qtest: Support libqtest to build and run on Windows
Date: Mon, 19 Sep 2022 18:00:37 +0800

Hi,

On Thu, Sep 1, 2022 at 12:29 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Wed, Aug 24, 2022 at 2:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> At present the libqtest codes were written to depend on several
>> POSIX APIs, including fork(), kill() and waitpid(). Unfortunately
>> these APIs are not available on Windows.
>>
>> This commit implements the corresponding functionalities using
>> win32 native APIs. With this change, all qtest cases can build
>> successfully on a Windows host, and we can start qtest testing
>> on Windows now.
>>
>> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> ---
>>
>>  tests/qtest/libqtest.c  | 101 +++++++++++++++++++++++++++++++++++++++-
>>  tests/qtest/meson.build |   5 +-
>>  2 files changed, 101 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index 70d7578740..99e52ff571 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -16,9 +16,11 @@
>>
>>  #include "qemu/osdep.h"
>>
>> +#ifndef _WIN32
>>  #include <sys/socket.h>
>>  #include <sys/wait.h>
>>  #include <sys/un.h>
>> +#endif /* _WIN32 */
>>  #ifdef __linux__
>>  #include <sys/prctl.h>
>>  #endif /* __linux__ */
>> @@ -27,6 +29,7 @@
>>  #include "libqmp.h"
>>  #include "qemu/ctype.h"
>>  #include "qemu/cutils.h"
>> +#include "qemu/sockets.h"
>>  #include "qapi/qmp/qdict.h"
>>  #include "qapi/qmp/qjson.h"
>>  #include "qapi/qmp/qlist.h"
>> @@ -35,6 +38,16 @@
>>  #define MAX_IRQ 256
>>  #define SOCKET_TIMEOUT 50
>>
>> +#ifndef _WIN32
>> +# define CMD_EXEC   "exec "
>> +# define DEV_STDERR "/dev/fd/2"
>> +# define DEV_NULL   "/dev/null"
>> +#else
>> +# define CMD_EXEC   ""
>> +# define DEV_STDERR "2"
>> +# define DEV_NULL   "nul"
>> +#endif
>> +
>>  typedef void (*QTestSendFn)(QTestState *s, const char *buf);
>>  typedef void (*ExternalSendFn)(void *s, const char *buf);
>>  typedef GString* (*QTestRecvFn)(QTestState *);
>> @@ -68,6 +81,9 @@ struct QTestState
>>  QTestState *global_qtest;
>>
>>  static GHookList abrt_hooks;
>> +#ifdef _WIN32
>> +typedef void (*sighandler_t)(int);
>> +#endif
>>  static sighandler_t sighandler_old;
>>
>>  static int qtest_query_target_endianness(QTestState *s);
>> @@ -120,10 +136,18 @@ bool qtest_probe_child(QTestState *s)
>>      pid_t pid = s->qemu_pid;
>>
>>      if (pid != -1) {
>> +#ifndef _WIN32
>>          pid = waitpid(pid, &s->wstatus, WNOHANG);
>>          if (pid == 0) {
>>              return true;
>>          }
>> +#else
>> +        DWORD exit_code;
>> +        GetExitCodeProcess((HANDLE)pid, &exit_code);
>> +        if (exit_code == STILL_ACTIVE) {
>> +            return true;
>> +        }
>> +#endif
>>          s->qemu_pid = -1;
>>      }
>>      return false;
>> @@ -137,13 +161,23 @@ void qtest_set_expected_status(QTestState *s, int 
>> status)
>>  void qtest_kill_qemu(QTestState *s)
>>  {
>>      pid_t pid = s->qemu_pid;
>> +#ifndef _WIN32
>>      int wstatus;
>> +#else
>> +    DWORD ret, exit_code;
>> +#endif
>>
>>      /* Skip wait if qtest_probe_child already reaped.  */
>>      if (pid != -1) {
>> +#ifndef _WIN32
>>          kill(pid, SIGTERM);
>>          TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
>>          assert(pid == s->qemu_pid);
>> +#else
>> +        TerminateProcess((HANDLE)pid, s->expected_status);
>> +        ret = WaitForSingleObject((HANDLE)pid, INFINITE);
>> +        assert(ret == WAIT_OBJECT_0);
>> +#endif
>>          s->qemu_pid = -1;
>>      }
>>
>> @@ -151,6 +185,7 @@ void qtest_kill_qemu(QTestState *s)
>>       * Check whether qemu exited with expected exit status; anything else is
>>       * fishy and should be logged with as much detail as possible.
>>       */
>> +#ifndef _WIN32
>>      wstatus = s->wstatus;
>>      if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) {
>>          fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
>> @@ -167,6 +202,16 @@ void qtest_kill_qemu(QTestState *s)
>>                  __FILE__, __LINE__, sig, signame, dump);
>>          abort();
>>      }
>> +#else
>> +    GetExitCodeProcess((HANDLE)pid, &exit_code);
>> +    CloseHandle((HANDLE)pid);
>> +    if (exit_code != s->expected_status) {
>> +        fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
>> +                "process but encountered exit status %ld (expected %d)\n",
>> +                __FILE__, __LINE__, exit_code, s->expected_status);
>> +        abort();
>> +    }
>> +#endif
>>  }
>>
>>  static void kill_qemu_hook_func(void *s)
>> @@ -245,6 +290,38 @@ static const char *qtest_qemu_binary(void)
>>      return qemu_bin;
>>  }
>>
>> +#ifdef _WIN32
>> +static pid_t qtest_create_process(char *cmd)
>> +{
>> +    STARTUPINFO si;
>> +    PROCESS_INFORMATION pi;
>> +    BOOL ret;
>> +
>> +    ZeroMemory(&si, sizeof(si));
>> +    si.cb = sizeof(si);
>> +    ZeroMemory(&pi, sizeof(pi));
>> +
>> +    ret = CreateProcess(NULL,   /* module name */
>> +                        cmd,    /* command line */
>> +                        NULL,   /* process handle not inheritable */
>> +                        NULL,   /* thread handle not inheritable */
>> +                        FALSE,  /* set handle inheritance to FALSE */
>> +                        0,      /* No creation flags */
>> +                        NULL,   /* use parent's environment block */
>> +                        NULL,   /* use parent's starting directory */
>> +                        &si,    /* pointer to STARTUPINFO structure */
>> +                        &pi     /* pointer to PROCESS_INFORMATION structure 
>> */
>> +                        );
>> +    if (ret == 0) {
>> +        fprintf(stderr, "%s:%d: unable to create a new process (%s)\n",
>> +                __FILE__, __LINE__, strerror(GetLastError()));
>> +        abort();
>> +    }
>> +
>> +    return (pid_t)pi.hProcess;
>> +}
>> +#endif /* _WIN32 */
>> +
>>  QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>>  {
>>      QTestState *s;
>> @@ -272,6 +349,9 @@ QTestState *qtest_init_without_qmp_handshake(const char 
>> *extra_args)
>>      unlink(socket_path);
>>      unlink(qmp_socket_path);
>>
>> +#ifdef _WIN32
>> +    socket_init();
>> +#endif
>
>
> You can call this unconditionally, afaict
>

Will fix in v2.

>>
>>      sock = init_socket(socket_path);
>>      qmpsock = init_socket(qmp_socket_path);
>>
>> @@ -280,7 +360,7 @@ QTestState *qtest_init_without_qmp_handshake(const char 
>> *extra_args)
>>
>>      qtest_add_abrt_handler(kill_qemu_hook_func, s);
>>
>> -    command = g_strdup_printf("exec %s %s"
>> +    command = g_strdup_printf(CMD_EXEC "%s %s"
>>                                "-qtest unix:%s "
>>                                "-qtest-log %s "
>>                                "-chardev socket,path=%s,id=char0 "
>> @@ -289,7 +369,7 @@ QTestState *qtest_init_without_qmp_handshake(const char 
>> *extra_args)
>>                                "%s"
>>                                " -accel qtest",
>>                                qemu_binary, tracearg, socket_path,
>> -                              getenv("QTEST_LOG") ? "/dev/fd/2" : 
>> "/dev/null",
>> +                              getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL,
>>                                qmp_socket_path,
>>                                extra_args ?: "");
>>
>> @@ -298,6 +378,7 @@ QTestState *qtest_init_without_qmp_handshake(const char 
>> *extra_args)
>>      s->pending_events = NULL;
>>      s->wstatus = 0;
>>      s->expected_status = 0;
>> +#ifndef _WIN32
>>      s->qemu_pid = fork();
>>      if (s->qemu_pid == 0) {
>>  #ifdef __linux__
>> @@ -320,6 +401,9 @@ QTestState *qtest_init_without_qmp_handshake(const char 
>> *extra_args)
>>          execlp("/bin/sh", "sh", "-c", command, NULL);
>>          exit(1);
>>      }
>> +#else
>> +    s->qemu_pid = qtest_create_process(command);
>
>
> Why not replace the fork/exec with g_spawn_async() ?
>

I tried g_spawn_async(), but I am getting in trouble creating the
argument vector of the QEMU command lines, to pass in g_spawn_async().

g_strsplit() with SPACE as the delimiter does not work, as there might
be space in the command line surrounded by quotes.

g_shell_parse_argv() does not work either, as it just ate all Windows
path separator "\", so a patch name like
"D:\msys64\tmp/qtest-4220.qmp" becomes "D:msys64tmp/qtest-4220.qmp".

Do you have any suggestions?

>>
>> +#endif /* _WIN32 */
>>
>>      g_free(command);
>>      s->fd = socket_accept(sock);
>> @@ -338,9 +422,19 @@ QTestState *qtest_init_without_qmp_handshake(const char 
>> *extra_args)
>>          s->irq_level[i] = false;
>>      }
>>
>> +    /*
>> +     * Stopping QEMU for debugging is not supported on Windows.
>> +     *
>> +     * Using DebugActiveProcess() API can suspend the QEMU process,
>> +     * but gdb cannot attach to the process. Using the undocumented
>> +     * NtSuspendProcess() can suspend the QEMU process and gdb can
>> +     * attach to the process, but gdb cannot resume it.
>> +     */
>> +#ifndef _WIN32
>>      if (getenv("QTEST_STOP")) {
>>          kill(s->qemu_pid, SIGSTOP);
>>      }
>> +#endif
>>
>>      /* ask endianness of the target */
>>
>> @@ -393,6 +487,9 @@ QTestState *qtest_init_with_serial(const char 
>> *extra_args, int *sock_fd)
>>      g_assert_true(g_mkdtemp(sock_dir) != NULL);
>>      sock_path = g_strdup_printf("%s/sock", sock_dir);
>>
>> +#ifdef _WIN32
>> +    socket_init();
>> +#endif
>
>
> same
>
>>
>>      sock_fd_init = init_socket(sock_path);
>>
>>      qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>> index 0291b3966c..6d469a1822 100644
>> --- a/tests/qtest/meson.build
>> +++ b/tests/qtest/meson.build
>> @@ -1,6 +1,5 @@
>> -# All QTests for now are POSIX-only, but the dependencies are
>> -# really in libqtest, not in the testcases themselves.
>> -if not config_host.has_key('CONFIG_POSIX')
>> +# Build all QTests for POSIX and Windows
>> +if not config_host.has_key('CONFIG_POSIX') and not 
>> config_host.has_key('CONFIG_WIN32')
>>    subdir_done()
>>  endif
>>

Regards,
Bin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]