Re: [Qemu-devel] [PATCH for-2.12] monitor: bind dispatch bh to iohandler context

[Eric Blake]

On 04/03/2018 12:01 AM, Peter Xu wrote:
> Eric Auger reported the problem days ago that OOB broke ARM when running
> with libvirt:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06231.html
> 
> This patch fixes the problem.
> 
> It's not really needed now since we have turned OOB off now, but it's
> still a bug fix, and it'll start to work when we turn OOB on for ARM.

It may be more than just ARM affected, and just that ARM happened to
trigger the problem easier.

I'm queuing this on my qapi tree, and will send a pull request,
hopefully for -rc2, but even if I miss that, this qualifies as -rc3
material.

> 
> The problem was that the monitor dispatcher bottom half was bound to
> qemu_aio_context, but that context seems to be for block only.  For the
> rest of the QEMU world we should be using iohandler context.  So
> assigning monitor dispatcher bottom half to that context.
> 

> 
> A quick test of mine shows that after this patch applied we can pass all
> raw iotests even with OOB on by default.
> 
> CC: Eric Blake <address@hidden>
> CC: Markus Armbruster <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> CC: Fam Zheng <address@hidden>
> Reported-by: Eric Auger <address@hidden>
> Tested-by: Eric Auger <address@hidden>
> Signed-off-by: Peter Xu <address@hidden>
> ---
> 
> This patch will fix all known OOB breakages I know so far, but I think
> for better safety I'll still keep OOB off, and I'll send another patch
> to turn default OOB on after 2.12 release.

Yes, leaving OOB defaulting to off unless you use x-oob=on for 2.12 is
the most conservative action, although we still want to fix all known
bugs that you can trigger by using that.

How did you run iotests with OOB on by default, by recompiling the
binary to undo commit be933ffc2, or by tweaking the iotests scripts to
pass x-oob=on by default?

> ---
>  monitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 51f4cf480f..39f8ee17ba 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4467,7 +4467,7 @@ static void monitor_iothread_init(void)
>       * have assumption to be run on main loop thread.  It would be
>       * nice that one day we can remove this assumption in the future.
>       */
> -    mon_global.qmp_dispatcher_bh = aio_bh_new(qemu_get_aio_context(),
> +    mon_global.qmp_dispatcher_bh = aio_bh_new(iohandler_get_aio_context(),
>                                                monitor_qmp_bh_dispatcher,
>                                                NULL);
>  
> 

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

signature.asc
Description: OpenPGP digital signature