On Fri, Mar 06, 2020 at 02:42:45PM +0100, Michal Prívozník wrote: > 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@xxxxxxxxxx> > > Should we also delete the event loop from the qemu_shim.c then? I've got patches for that, but its not possible until we can make the virObjectEventState struct use a thread for dispatch, instead of using a event loop zero-second timer. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
