Re: [libvirt PATCH v2 0/7] qemu: introduce a per-VM event loop thread

On 5. 3. 2020 13:51, Daniel P. Berrangé wrote: > This series changes the way we manage the QEMU monitor and > QEMU agent, such that all I/O is processed by a dedicated > event loop thread. > > Many times in the past years people are reported issues > where long running monitor event callbacks block the main > libvirtd event loop for an unacceptably long period of > time. In the best case, this delays other work being > completed, but in bad cases it leads to mgmt app failures > when keepalive times trigger a client disconnect. > > With this series, when we spawn QEMU, we also spawn a > dedicated thread running a GMainLoop instance. Then QEMU > monitor and QEMU agent UNIX sockets are switched to use > GMainContext for events instead of the traditional libvirt > event loop APIs. We kill off the event thread when we see > EOF on the QEMU monitor during shutdown. > > The cost of this approach is one extra thread per VM, > which incurs a new OS process and a new stack allocation. > > The QEMU driver already delegates some QMP event handling > to a thread pool for certain types of event. This was a > previous hack to mitigate the impact on the main event > loop. It is likely that we can remove this thread pool > from the QEMU driver & rely on the per-VM event threads > to do all the work. This will, however, require careful > analysis of each handler we pushed into the thread pool > to make sure its work doesn't have a dependency on the > event loop running in parallel. > > This is one step towards eliminating the need to have the > libvirt event loop registered when using the embedded QEMU > driver. A further step is using a thread to dispatch the > lifecycle events, since that currently relies on a zero > second timer being registered with the event loop. > > Changed in v2: > > - Fixed race accessing free'd memory causing crash > - Fixed unused variables > - Merged first acked patches > > Daniel P. Berrangé (7): > src: introduce an abstraction for running event loops > qemu: start/stop an event loop thread for domains > qemu: start/stop an event thread for QMP probing > tests: start/stop an event thread for QEMU monitor/agent tests > qemu: convert monitor to use the per-VM event loop > qemu: fix variable naming in agent code > qemu: convert agent to use the per-VM event loop > > po/POTFILES.in | 1 + > src/libvirt_private.syms | 5 + > src/qemu/qemu_agent.c | 600 ++++++++++++++++++----------------- > src/qemu/qemu_agent.h | 1 + > src/qemu/qemu_domain.c | 33 ++ > src/qemu/qemu_domain.h | 6 + > src/qemu/qemu_monitor.c | 145 ++++----- > src/qemu/qemu_monitor.h | 3 +- > src/qemu/qemu_process.c | 43 ++- > src/qemu/qemu_process.h | 2 + > src/util/Makefile.inc.am | 2 + > src/util/vireventthread.c | 190 +++++++++++ > src/util/vireventthread.h | 31 ++ > tests/qemumonitortestutils.c | 14 + > 14 files changed, 700 insertions(+), 376 deletions(-) > create mode 100644 src/util/vireventthread.c > create mode 100644 src/util/vireventthread.h > Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Should we also delete the event loop from the qemu_shim.c then?