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

[Markus Armbruster]

Philippe Mathieu-Daudé <address@hidden> writes:

> On 08/15/2018 11: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)
>> 
>> 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() 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
>
> "abort handler"?

The variable is called @abrt_hooks, like SIGABRT[*].  But you're right,
I should either use the English word spelled correctly, or use the
identifier spelled correctly.  I think I'll use the identifier.

>> atexit() or something.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Thanks!


[*] The ancients sacrificed the occasional vowel to the God of Spartan
Linkers and ASR-33 Teletypes.