On 08/03/2018 10:51 AM, Markus Armbruster wrote:
Eric Blake <[email protected]> writes:In kill_qemu() we have an assert that checks that the QEMU process didn't dump core: assert(!WCOREDUMP(wstatus)); Unfortunately the WCOREDUMP macro here means the resulting message is not very easy to comprehend on at least some systems: ahci-test: tests/libqtest.c:113: kill_qemu: Assertion `!(((__extension__ (((union { __typeof(wstatus) __in; int __i; }) { .__in = (wstatus) }).__i))) & 0x80)' failed. and it doesn't identify what signal the process took. - if (pid == s->qemu_pid && WIFSIGNALED(wstatus)) { - assert(!WCOREDUMP(wstatus)); + assert(pid == s->qemu_pid); + /* + * We expect qemu to exit with status 0; anything else is + * fishy and should be logged. Abort except when death by + * signal is not accompanied by a coredump (as that's the only + * time it was likely that the user is trying to kill the + * testsuite early). + */ + if (wstatus) { + die = true; + if (WIFEXITED(wstatus)) { + fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU " + "process but encountered exit status %d\n", + __FILE__, __LINE__, WEXITSTATUS(wstatus)); + } else if (WIFSIGNALED(wstatus)) { + int sig = WTERMSIG(wstatus);
In review of v2, we found that WCOREDUMP() depends on the user's environment. I doubt we should use it this way. Why is suppressing the abort() when "the user is trying to kill the testsuite early" important?
Only because the old code was special-casing based on whether WCOREDUMP. I'm not particularly attached to it, and am just fine submitting a v4 that just quits on ALL death-by-signal, rather than caring whether a core dump was present.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
