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

[Peter Xu <peterx@redhat.com>]

On Thu, Apr 05, 2018 at 01:34:31PM +0100, Stefan Hajnoczi wrote:
> On Wed, Apr 04, 2018 at 10:07:34AM -0500, Eric Blake wrote:
> > On 04/04/2018 09:22 AM, Stefan Hajnoczi wrote:
> > > On Tue, Apr 03, 2018 at 01:01:15PM +0800, 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.
> > >>
> > >> The problem was that the monitor dispatcher bottom half was bound to
> > >> qemu_aio_context, but that context seems to be for block only.
> > > 
> > > No, it is not block-only.  iohandler_ctx is for the legacy
> > > qemu_set_fd_handler() API only and modern code should use
> > > qemu_aio_context.
> > > 
> > > The difference between qemu_aio_context and iohandler_ctx is that
> > > aio_poll(qemu_aio_context) does not process iohandler_ctx (since it's a
> > > difference context).  That is the legacy behavior that
> > > qemu_set_fd_handler() expects and it's implemented by keeping a separate
> > > iohandler_ctx.
> > 
> > Do I need to put a hold on my pull request while we come to a better
> > understanding of root cause, or is this patch still okay to include?
> 
> The cover letter says this patch is not critical for 2.12:
> 
> "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."
> 
> Please hold off until the nature of the bug is understood.

But frankly speaking I would still prefer the fix be ready for 2.12
since that's still a bugfix and after all we have x-oob=on parameter
to enable that (that's why I mentioned for-2.12 in subject).  The fix
can be something better than this patch, but still it'll be good 2.12
has that.

Let's first discuss which's the best way to fix this up, then we'll
see whether this can be fixed for 2.12, or later.

Thanks,

-- 
Peter Xu