On Tue, Apr 03, 2018 at 01:01:15PM +0800, Peter Xu wrote: I have changed my mind about this: this patch is necessary. It is needed in QEMU 2.12.
> 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. The general problem is that the monitor should only dispatch commands from main_loop_wait(). That's how the code worked before OOB. After OOB commands are also dispatched from aio_poll(). This causes unexpected behavior. Here is another scenario that this patch fixes: If the monitor IOThread parses a command while a monitor command is in aio_poll() then qmp_dispatch() is re-entered. Monitor command code is not designed to handle this! > 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. No, it is needed even when OOB is disabled. Consider the case where the chardev handler is invoked by main_loop_wait() and command parsing completes. qmp_dispatcher_bh will be marked scheduled=1. Before qmp_dispatcher_bh executes another fd handler runs and invokes aio_poll(). Now qmp_dispatcher_bh runs from aio_poll() instead of from main_loop_wait(). Before OOB this was impossible and it's likely to hang or crash. > The problem was that the monitor dispatcher bottom half was bound to > qemu_aio_context, but that context seems to be for block only. s/but that context seems to be for block only/which is used for nested event loops/ It's not true that qemu_aio_context is used for block only. All qemu_bh_new() users (many emulated devices, for example) use qemu_aio_context, as well as TPM and 9p. > For the > rest of the QEMU world we should be using iohandler context. So > assigning monitor dispatcher bottom half to that context. TPM and 9p also use nested event loops, they need to use qemu_aio_context. The choice depends on whether or not nested event loops are needed. Code that isn't designed for nested event loops has to use iohandler_ctx (usually via qemu_set_fd_handler()). Code that is designed for nested event loops has to use qemu_aio_context.
signature.asc
Description: PGP signature
