From patchwork Fri Aug 10 13:28:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 956254 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41n5YD1QPnz9s7Q for ; Fri, 10 Aug 2018 23:28:32 +1000 (AEST) Received: from localhost ([::1]:56311 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fo7Sv-0000iN-Vw for incoming@patchwork.ozlabs.org; Fri, 10 Aug 2018 09:28:30 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57822) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fo7Sc-0000i4-PP for qemu-devel@nongnu.org; Fri, 10 Aug 2018 09:28:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fo7SY-0006OD-3w for qemu-devel@nongnu.org; Fri, 10 Aug 2018 09:28:10 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47502 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fo7SX-0006NV-U1 for qemu-devel@nongnu.org; Fri, 10 Aug 2018 09:28:06 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7151A40201C7; Fri, 10 Aug 2018 13:28:05 +0000 (UTC) Received: from red.redhat.com (ovpn-120-178.rdu2.redhat.com [10.10.120.178]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7C0B5215670D; Fri, 10 Aug 2018 13:28:04 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 10 Aug 2018 08:28:00 -0500 Message-Id: <20180810132800.38549-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 10 Aug 2018 13:28:05 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 10 Aug 2018 13:28:05 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'eblake@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v4] tests/libqtest: Improve kill_qemu() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.maydell@linaro.org, thuth@redhat.com, mst@redhat.com, f4bug@amsat.org, armbru@redhat.com, alex.bennee@linaro.org, rth@twiddle.net Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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. What's more, WCOREDUMP is not reliable - in some cases, setrlimit() coupled with kernel dump settings can result in the flag not being set. It's better to log ALL death by signal, instead of caring whether a core dump was attempted (although once we know a signal happened, also mentioning if a core dump is present can be helpful). Furthermore, we are NOT detecting EINTR (while EINTR shouldn't be happening if we didn't install signal handlers, it's still better to always be robust). Finally, even non-signal death with a non-zero status is suspicious, since qemu's SIGINT handler is supposed to result in exit(0). Instead of using a raw assert, print the information in an easier to understand way: /i386/ahci/sanity: tests/libqtest.c:119: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped) Aborted (core dumped) (Of course, the really useful information would be why the QEMU process dumped core in the first place, but we don't have that by the time the test program has picked up the exit status.) Suggested-by: Peter Maydell Signed-off-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé --- v4: don't special-case death without WCOREDUMP [Markus] v3: use TFR() instead of open-coding the retry loop [Thomas] --- tests/libqtest.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 098af6aec44..57cf89015e0 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -107,10 +107,28 @@ static void kill_qemu(QTestState *s) pid_t pid; kill(s->qemu_pid, SIGTERM); - pid = waitpid(s->qemu_pid, &wstatus, 0); + TFR(pid = waitpid(s->qemu_pid, &wstatus, 0)); - 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 with as much detail as possible. + */ + if (wstatus) { + 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); + const char *signame = strsignal(sig) ?: "unknown ???"; + const char *dump = WCOREDUMP(wstatus) ? " (dumped core)" : ""; + + fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death " + "from signal %d (%s)%s\n", + __FILE__, __LINE__, sig, signame, dump); + } + abort(); } } }