On 25.05.2018 14:15, Michael S. Tsirkin wrote: > On Fri, May 25, 2018 at 08:10:48AM +0200, Thomas Huth wrote: >> 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 <m...@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(). > > But only the first one will cause a coredump. > >> 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 > > I think I'll drop the second patch for now. failing test on coredump > is clearly correct. The rest can wait until someone has the energy > to look into all the intricacies of signal handling.
Ok, sounds like a plan. Acked-by: Thomas Huth <th...@redhat.com>