All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <lvivier@redhat.com>
Subject: Re: [PATCH v2 1/2] tests/qtest: fix registration of ABRT handler for QEMU cleanup
Date: Fri, 13 May 2022 18:08:47 +0200	[thread overview]
Message-ID: <af414a58-e4d7-69c0-c2b3-d1704cbf1694@redhat.com> (raw)
In-Reply-To: <20220513154906.206715-2-berrange@redhat.com>

On 13/05/2022 17.49, Daniel P. Berrangé wrote:
> qtest_init registers a hook to cleanup the running QEMU process
> should g_assert() fire before qtest_quit is called. When the first
> hook is registered, it is supposed to triggere registration of the
> SIGABRT handler. Unfortunately the logic in hook_list_is_empty is
> inverted, so the SIGABRT handler never gets registered, unless
> 2 or more QEMU processes are run concurrently. This caused qtest
> to leak QEMU processes anytime g_assert triggers.

Ouch, thanks for spotting it!

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qtest/libqtest.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 228357f1ea..4a4697c0d1 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -197,11 +197,11 @@ static bool hook_list_is_empty(GHookList *hook_list)
>       GHook *hook = g_hook_first_valid(hook_list, TRUE);
>   
>       if (!hook) {
> -        return false;
> +        return true;
>       }
>   
>       g_hook_unref(hook_list, hook);
> -    return true;
> +    return false;
>   }

Reviewed-by: Thomas Huth <thuth@redhat.com>



  reply	other threads:[~2022-05-13 16:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-13 15:49 [PATCH v2 0/2] tests/libqtest: fix cleanup of QEMU processes and add robustness Daniel P. Berrangé
2022-05-13 15:49 ` [PATCH v2 1/2] tests/qtest: fix registration of ABRT handler for QEMU cleanup Daniel P. Berrangé
2022-05-13 16:08   ` Thomas Huth [this message]
2022-05-13 15:49 ` [PATCH v2 2/2] tests/qtest: use prctl(PR_SET_PDEATHSIG) as fallback to kill QEMU Daniel P. Berrangé
2022-05-13 16:09   ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=af414a58-e4d7-69c0-c2b3-d1704cbf1694@redhat.com \
    --to=thuth@redhat.com \
    --cc=berrange@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.