[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/1] libqtest: add more exit status checks
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/1] libqtest: add more exit status checks |
Date: |
Thu, 24 May 2018 20:33:43 +0300 |
On Thu, May 24, 2018 at 07:26:16PM +0200, Thomas Huth wrote:
> On 24.05.2018 18:11, Michael S. Tsirkin wrote:
> > Add more checks on how did QEMU exit.
> >
> > Legal ways to exit right now:
> > - exit(0) or return from main
> > - kill(SIGTERM) - sent by testing infrastructure
> >
> > Anything else is illegal.
> [...]
> > - if (pid == s->qemu_pid && WIFSIGNALED(wstatus)) {
> > + /* waitpid returns child PID on success */
> > + assert(pid == s->qemu_pid);
> > +
> > + /* If exited on signal - check the reason: core dump is never OK */
> > + if (WIFSIGNALED(wstatus)) {
> > assert(!WCOREDUMP(wstatus));
> > }
> > + /* If exited normally - check exit status */
> > + if (WIFEXITED(wstatus)) {
> > + assert(!WEXITSTATUS(wstatus));
> > + }
> > + /* Valid ways to exit: right now only return from main or exit */
> > + assert(WIFEXITED(wstatus));
> > }
> > }
>
> It's strange that you always get WIFEXITED(wstatus) == true here, even
> if QEMU has been terminated by SIGTERM? I assume that's due to the fact
> that QEMU intercepts SIGTERM and terminates via exit() instead?
Right now, yes. This can of course change, so it's not
a good idea hard-coding this assumption to deep
in the code, imho.
> So I
> think you could simply replace the last three asserts with:
>
> assert(WIFEXITED(wstatus) && !WEXITSTATUS(wstatus));
>
> Thomas
I could but they would be harder to debug.
If I see
"assertion failed: !WCOREDUMP(wstatus)"
then that is very readable.
If I just see
"assertion failed: WIFEXITED(wstatus) && !WEXITSTATUS(wstatus)"
then I just know something went wrong.
--
MST
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, (continued)
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Eric Blake, 2018/05/24
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Michael S. Tsirkin, 2018/05/24
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Eric Blake, 2018/05/24
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Michael S. Tsirkin, 2018/05/24
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Eric Blake, 2018/05/24
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Michael S. Tsirkin, 2018/05/24
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Thomas Huth, 2018/05/24
- Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks, Michael S. Tsirkin, 2018/05/24
[Qemu-devel] [PATCH v3 2/1] libqtest: add more exit status checks, Michael S. Tsirkin, 2018/05/24
Re: [Qemu-devel] [PATCH] libqtest: fail if child coredumps, Peter Maydell, 2018/05/24