Re: [Qemu-devel] [Qemu-devel for-3.1?] [PATCH] glib-compat: work around g_test_message bug with subprocess tests

[Eric Blake]

On 11/27/18 12:44 PM, Eric Blake wrote:
> On 11/27/18 12:35 PM, Paolo Bonzini wrote:
> > Subprocesses are created by glib without leaving the file descriptors
> > open.  Therefore, g_test_message (and assertion failures, but those
> > trigger when things are going bad anyway) will think that it is writing
> > to the log file descriptor, but while actually stomping on the QMP
> > file descriptor or similar.  This causes spurious failures, which are
> > as nice to debug as the reader can imagine.  While I have opened a
> > pull request on GLib, this will probably take a while to propagate
> > to distros.
> >
> > I found this while working on qgraph, but the fix is generic.
> >
> > Signed-off-by: Paolo Bonzini <address@hidden>
> > ---
> >   include/glib-compat.h | 6 ++++++
> >   1 file changed, 6 insertions(+)
>
> Wow. I don't envy the debug session that you went through to finally 
> realize this problem.
>
> Reviewed-by: Eric Blake <address@hidden>
>
> I think this is safe for 3.1-rc3 if you want it there, as minimizing 
> spurious test failures is a good thing.

Whoops - we missed -rc3. By itself, this is not worth -rc4, but is still 
a possible candidate if more serious stuff requires a release delay.	

> > diff --git a/include/glib-compat.h b/include/glib-compat.h
> > index fdf95a2..989b9ef 100644
> > --- a/include/glib-compat.h
> > +++ b/include/glib-compat.h
> > @@ -113,4 +113,10 @@ gint g_poll_fixed(GPollFD *fds, guint nfds, gint 
> > timeout);
> >   #pragma GCC diagnostic pop
> > +/* See https://gitlab.gnome.org/GNOME/glib/merge_requests/501 */
> > +#define g_test_message(...)                            \
> > +    do {                                    \
> > +        if (!g_test_subprocess()) g_test_message(__VA_ARGS__);        \
>
> I'm surprised checkpatch.pl doesn't complain about missing {} in this 
> macro expansion, but doing it right would mean 2 more lines, so I'm fine 
> overlooking the style.

Aha - checkpatch eventually DID flag it, as well as your use of TAB 
characters, but patchew has enough lag that it took a few hours.

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