Re: [Qemu-devel] [PULL 00/38] QAPI patches for 2018-03-12, 2.12 softfreeze

[Eric Blake]

On 03/19/2018 04:26 AM, Peter Xu wrote:

> > > for you to fetch changes up to 75eb57e3ed3682f011a6694863044e8b143a9821:
> > >
> > >    qapi: Pass '-u' when doing non-silent diff (2018-03-16 09:00:07 -0500)

> > Hi. I get a bunch of test assertion failures with this:
> >
> > ppc64 host:
> >
> > QTEST_QEMU_BINARY=nios2-softmmu/qemu-system-nios2
> > QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((
> > ${RANDOM:-0} % 255 + 1)
> > )} gtester -k --verbose -m=quick  tests/qmp-test
> > tests/device-introspect-test tests/qom-test tests/test-hmp
> > TEST: tests/qmp-test... (pid=49431)
> >    /nios2/qmp/protocol:                                                 OK
> >    /nios2/qmp/oob:                                                      OK
> >    /nios2/qmp/query-status:                                             OK
> >    /nios2/qmp/query-block:                                              OK
> >    /nios2/qmp/query-blockstats:                                         OK
> >    /nios2/qmp/query-block-jobs:                                         OK
> >    /nios2/qmp/query-named-block-nodes:
> > qemu-system-nios2: /home/pm215/qemu/chardev/char-io.c:91: io_watc
> > h_poll_finalize: Assertion `iwp->src == ((void *)0)' failed.
> > Broken pipe
> > FAIL

I haven't been able to reproduce the testsuite failures on my Linux box, 
but if it's a race, then that doesn't make me all the more confident on 
what it takes to reproduce and/or fix the race.


> > One or two of the build hosts did pass, so that plus the varying
> > tests which failed suggests that the iwp->src assert is an
> > intermittent or timing based one. The tpm error on NetBSD
> > is probably a separate issue.
>
> I think I still need this to be squashed into "monitor: allow using IO
> thread for parsing", which I dropped during respin from v7 to v8:
>
> diff --git a/monitor.c b/monitor.c
> index f9ef3e5266..121194111f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4556,6 +4556,11 @@ void monitor_init(Chardev *chr, int flags)
>           qemu_chr_fe_set_echo(&mon->chr, true);
>           json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
>           if (mon->use_io_thr) {
> +            /*
> +             * Make sure the old iowatch be gone.  It's possible when
> +             * e.g. the chardev is in client mode, with wait=on.
> +             */
> +            remove_fd_in_watch(chr);
>               /*
>                * We can't call qemu_chr_fe_set_handlers() directly here
>                * since during the procedure the chardev will be active
>
> I thought there should be no pending task on main thread after the QIO
> and CHARDEV fixes, but I missed the most general io watch and we still
> possibly need the line.

So, should I squash in the fix and keep OOB as part of my v3 attempt, or 
are we getting close enough to rc0 that my qapi v3 pull request should 
just drop OOB, and save that as a feature for 2.13 instead?

> We should fix the assertion problem with above, but not sure about
> whether it can fix the QIO issue since I haven't seen that before (and
> I can't reproduce that too in my environment).
>
> I hope the fix can work for us.  But in all cases, please feel free to
> drop the series if needed.  Sorry for the trouble.

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