Re: [Qemu-devel] [PATCH v2 2/1] libqtest: add more exit status checks

[Michael S. Tsirkin]

On Thu, May 24, 2018 at 10:52:26AM -0500, Eric Blake wrote:
> On 05/24/2018 10:38 AM, 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
> > 
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > ---
> > 
> > Changes from v1:
> > - drop SIGTERM as suggested by Eric
> > 
> 
> > +++ b/tests/libqtest.c
> > @@ -110,7 +110,12 @@ static void kill_qemu(QTestState *s)
> >           pid = waitpid(s->qemu_pid, &wstatus, 0);
> >           if (pid == s->qemu_pid && WIFSIGNALED(wstatus)) {
> 
> Wait a moment. If WIFSIGNALED() is true...
> 
> > +            /* Core dump is never OK */
> >               assert(!WCOREDUMP(wstatus));
> > +            /* Must exit normally */
> > +            assert(WIFEXITED(wstatus));
> 
> ...then WIFEXITED() is false.  This is bogus.
> 
> > +            /* If exited normally - check exit status */
> > +            assert(!WIFEXITED(wstatus) || !WEXITSTATUS(wstatus));
> 
> And you have some redundancy - !WIFEXITED() is not possible if you just
> asserted WIFEXITED().
> 
> Better would be:
> 
> if (pid == s->qemu_pid) {
>     /*
>      * Since sending SIGTERM turns into a normal exit, we want to flag
>      * any non-normal exit, whether or not it dumped core, as a test
>      * failure (even if it was a SIGKILL from someone desperate to stop
>      * the testsuite).
>      */
>     assert(WIFEXITED(wstatus) && !WEXITSTATUS(wstatus));
> }
> 
> Also, since waitpid() can only return either s->qemu_pid or -1 as we aren't
> using WNOHANG,


> it may also be worth asserting that if pid == -1, we either
> have EAGAIN (but why aren't we looping in that case?)

I don't think waitpid can return EAGAIN

> or ECHILD.

Right but checking for known failures explicitly is helpful for debugging.


> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org