Re: [Qemu-devel] [PATCH 1/1] libqtest: Improve error reporting for bad read from QEMU

[Eric Blake]

On 08/15/2018 09:19 AM, Markus Armbruster wrote:
> When read() from the qtest socket or the QMP socket fails or EOFs, we
> report "Broken pipe" and exit(1).  This commonly happens when QEMU
> crashes.  It also happens when QEMU refuses to run because the test
> passed it bad arguments.  Sadly, we neglect to report either.
>
> Improve this by calling abort() instead of exit(1), so kill_qemu()
> runs, and reports how QEMU died.  This improves error reporting to
> something like
>
>      /x86_64/device/introspect/list: Broken pipe
>      tests/libqtest.c:129: kill_qemu() detected QEMU death from signal 6 
> (Aborted) (dumped core)

Matches your touchup to my v4 patch.

> Three exit() remain in libqtest.c:
>
> * In qmp_response(), when we can't parse a QMP reply read from the QMP
>    socket.  Change to abort() for consistency.
>
> * In qtest_qemu_binary(), when QTEST_QEMU_BINARY isn't in the
>    environment.  This can only happen before we start QEMU.  Leave
>    alone.
>
> * In qtest_init_without_qmp_handshake(), when the fork()ed child fails
>    to execlp().  Leave alone.

exit() after failed execlp() is often wrong (in particular, if you are 
fork()ed from a multi-threaded process, you REALLY want to be using 
_exit() instead).  But as you say, not the goal of this particular patch.

> exit() elsewhere are unlikely due to QEMU dying on us.  If that should
> turn out to be wrong, we can move kill_qemu() from the abrt handler to
> atexit() or something.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>   tests/libqtest.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org