Re: [Qemu-devel] [PATCH v2 3/3] qtest: kill QEMU process on g_assert() failure

[Paolo Bonzini <pbonzini@redhat.com>]

Il 18/02/2014 15:54, Stefan Hajnoczi ha scritto:
> The QEMU process stays running if the test case fails.  This patch fixes
> the leak by installing a SIGABRT signal handler which invokes
> qtest_end().
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/libqtest.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 8b2b2d7..f587d36 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -44,6 +44,7 @@ struct QTestState
>      bool irq_level[MAX_IRQ];
>      GString *rx;
>      pid_t qemu_pid;  /* our child QEMU process */
> +    struct sigaction sigact_old; /* restored on exit */
>  };
>
>  #define g_assert_no_errno(ret) do { \
> @@ -88,6 +89,19 @@ static int socket_accept(int sock)
>      return ret;
>  }
>
> +static void kill_qemu(QTestState *s)
> +{
> +    if (s->qemu_pid != -1) {
> +        kill(s->qemu_pid, SIGTERM);
> +        waitpid(s->qemu_pid, NULL, 0);
> +    }
> +}
> +
> +static void sigabrt_handler(int signo)
> +{
> +    kill_qemu(global_qtest);
> +}
> +
>  QTestState *qtest_init(const char *extra_args)
>  {
>      QTestState *s;
> @@ -96,6 +110,7 @@ QTestState *qtest_init(const char *extra_args)
>      gchar *qmp_socket_path;
>      gchar *command;
>      const char *qemu_binary;
> +    struct sigaction sigact;
>
>      qemu_binary = getenv("QTEST_QEMU_BINARY");
>      g_assert(qemu_binary != NULL);
> @@ -108,6 +123,14 @@ QTestState *qtest_init(const char *extra_args)
>      sock = init_socket(socket_path);
>      qmpsock = init_socket(qmp_socket_path);
>
> +    /* Catch SIGABRT to clean up on g_assert() failure */
> +    sigact = (struct sigaction){
> +        .sa_handler = sigabrt_handler,
> +        .sa_flags = SA_RESETHAND,
> +    };
> +    sigemptyset(&sigact.sa_mask);
> +    sigaction(SIGABRT, &sigact, &s->sigact_old);
> +
>      s->qemu_pid = fork();
>      if (s->qemu_pid == 0) {
>          command = g_strdup_printf("exec %s "
> @@ -148,13 +171,9 @@ QTestState *qtest_init(const char *extra_args)
>
>  void qtest_quit(QTestState *s)
>  {
> -    int status;
> -
> -    if (s->qemu_pid != -1) {
> -        kill(s->qemu_pid, SIGTERM);
> -        waitpid(s->qemu_pid, &status, 0);
> -    }
> +    sigaction(SIGABRT, &s->sigact_old, NULL);
>
> +    kill_qemu(s);
>      close(s->fd);
>      close(s->qmp_fd);
>      g_string_free(s->rx, true);
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>