Re: [Qemu-devel] [PATCH v4 2/3] libqtest: fail if child coredumps

[Thomas Huth <thuth@redhat.com>]

On 24.05.2018 20:25, Michael S. Tsirkin wrote:
> Right now tests report OK status if QEMU crashes during cleanup.
> Let's catch that case and fail the test.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  tests/libqtest.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 43fb97e..f869854 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -103,8 +103,15 @@ static int socket_accept(int sock)
>  static void kill_qemu(QTestState *s)
>  {
>      if (s->qemu_pid != -1) {
> +        int wstatus = 0;
> +        pid_t pid;
> +
>          kill(s->qemu_pid, SIGTERM);
> -        waitpid(s->qemu_pid, NULL, 0);
> +        pid = waitpid(s->qemu_pid, &wstatus, 0);
> +
> +        if (pid == s->qemu_pid && WIFSIGNALED(wstatus)) {
> +            assert(!WCOREDUMP(wstatus));

Another ugliness that I just discovered: kill_qemu is also called from
the SIGABRT handler. So if a qtest assert() triggers an abort(), the
abort handler runs kill_qemu which now could trigger another assert()
and thus abort(). It's likely not a real problem since the abort handler
has been installed with SA_RESETHAND, but it's still quite confusing code.

Please let's clean up this ugliness properly: I think kill_qemu should
*only* be used by the abort handler, and then kill QEMU with SIGKILL for
good, to make sure that there are no stuck QEMU processes hanging around
anymore.

qtest_quit() should simply try to quit QEMU via QMP instead, and then
check for WIFEXITED(wstatus) && !WEXITSTATUS(wstatus) instead of using
the kill_qemu() function.

 Thomas