On Tue, Oct 24, 2017 at 07:34 PM +0200, Prerna Saxena <saxenap.ltc@gmail.com> wrote:
> As noted in
> https://www.redhat.com/archives/libvir-list/2017-May/msg00016.html
> libvirt-QEMU driver handles all async events from the main loop.
> Each event handling needs the per-VM lock to make forward progress. In
> the case where an async event is received for the same VM which has an
> RPC running, the main loop is held up contending for the same lock.

What's about the remaining qemuMonitorCallbacks? The main event loop can
still be 'blocked' by e.g. qemuProcessHandleMonitorEOF if the VM is
already locked by a worker thread. In fact, we currently have a problem
with D-Bus which causes a D-Bus call (of a worker thread) to run into
the timeout of 30 seconds. During these 30 seconds the main event loop
is stuck.

I tried the patch series and got a segmentation fault:

Thread 1 "libvirtd" received signal SIGSEGV, Segmentation fault.
0x000003ff98faa452 in virEnqueueVMEvent (qlist=0x3ff908ce760,
ev=<optimized out>) at ../../src/qemu/qemu_event.c:153 153
vmq_entry->ev->ev_id = vmq->last->ev->ev_id + 1;
(gdb) bt
#0 0x000003ff98faa452 in virEnqueueVMEvent (qlist=0x3ff908ce760,
 ev=<optimized out>) at ../../src/qemu/qemu_event.c:153
#1 0x000003ff98fc3564 in qemuProcessEnqueueEvent (mon=<optimized out>,
 vm=<optimized out>, ev=<optimized out>, opaque=0x3ff90548ec0) at
 ../../src/qemu/qemu_process.c:1864
#2 0x000003ff98fe4804 in qemuMonitorEnqueueEvent
 (mon=mon@entry=0x3ff4c007440, ev=0x2aa1e0104c0) at
 ../../src/qemu/qemu_monitor.c:1325
#3 0x000003ff98fe7102 in qemuMonitorEmitShutdown
 (mon=mon@entry=0x3ff4c007440, guest=<optimized out>,
 seconds=seconds@entry=1510683878, micros=micros@entry=703956) at
 ../../src/qemu/qemu_monitor.c:1365
#4 0x000003ff98ffc19a in qemuMonitorJSONHandleShutdown
 (mon=0x3ff4c007440, data=<optimized out>, seconds=1510683878,
 micros=<optimized out>) at ../../src/qemu/qemu_monitor_json.c:552
#5 0x000003ff98ffbb8a in qemuMonitorJSONIOProcessEvent
 (mon=mon@entry=0x3ff4c007440, obj=obj@entry=0x2aa1e012030) at
 ../../src/qemu/qemu_monitor_json.c:208
#6 0x000003ff99002138 in qemuMonitorJSONIOProcessLine
 (mon=mon@entry=0x3ff4c007440, line=0x2aa1e010460 "{\"timestamp\":
 {\"seconds\": 1510683878, \"microseconds\": 703956}, \"event\":
 \"SHUTDOWN\"}", msg=msg@entry=0x0) at
 ../../src/qemu/qemu_monitor_json.c:237
#7 0x000003ff990022b4 in qemuMonitorJSONIOProcess
 (mon=mon@entry=0x3ff4c007440, data=0x2aa1e014bc0 "{\"timestamp\":
 {\"seconds\": 1510683878, \"microseconds\": 703956}, \"event\":
 \"SHUTDOWN\"}\r\n", len=85, msg=msg@entry=0x0) at
 ../../src/qemu/qemu_monitor_json.c:279
#8 0x000003ff98fe4b44 in qemuMonitorIOProcess
 (mon=mon@entry=0x3ff4c007440) at ../../src/qemu/qemu_monitor.c:443
#9 0x000003ff98fe5d00 in qemuMonitorIO (watch=<optimized out>,
 fd=<optimized out>, events=0, opaque=0x3ff4c007440) at
 ../../src/qemu/qemu_monitor.c:697
#10 0x000003ffa68d6442 in virEventPollDispatchHandles (nfds=<optimized
 out>, fds=0x2aa1e013990) at ../../src/util/vireventpoll.c:508
#11 0x000003ffa68d66c8 in virEventPollRunOnce () at
 ../../src/util/vireventpoll.c:657
#12 0x000003ffa68d44e4 in virEventRunDefaultImpl () at
 ../../src/util/virevent.c:327
#13 0x000003ffa6a83c5e in virNetDaemonRun (dmn=0x2aa1dfe3eb0) at
 ../../src/rpc/virnetdaemon.c:838
#14 0x000002aa1df29cc4 in main (argc=<optimized out>, argv=<optimized
 out>) at ../../daemon/libvirtd.c:1494

>
> This impacts scalability, and should be addressed on priority.
>
> Note that libvirt does have a 2-step deferred handling for a few event
> categories, but (1) That is insufficient since blockign happens before
> the handler could disambiguate which one needs to be posted to this
> other queue.
> (2) There needs to be homogeniety.
>
> The current series builds a framework for recording and handling VM
> events.
> It initializes per-VM event queue, and a global event queue pointing to
> events from all the VMs. Event handling is staggered in 2 stages:
> - When an event is received, it is enqueued in the per-VM queue as well
>   as the global queues.
> - The global queue is built into the QEMU Driver as a threadpool
>   (currently with a single thread).
> - Enqueuing of a new event triggers the global event worker thread, which
>   then attempts to take a lock for this event's VM.
>     - If the lock is available, the event worker runs the function handling
>       this event type. Once done, it dequeues this event from the global
>       as well as per-VM queues.
>     - If the lock is unavailable(ie taken by RPC thread), the event worker
>       thread leaves this as-is and picks up the next event.
> - Once the RPC thread completes, it looks for events pertaining to the
>   VM in the per-VM event queue. It then processes the events serially
>   (holding the VM lock) until there are no more events remaining for
>   this VM. At this point, the per-VM lock is relinquished.
>
> Patch Series status:
> Strictly RFC only. No compilation issues. I have not had a chance to
> (stress) test it after rebase to latest master.
> Note that documentation and test coverage is TBD, since a few open
> points remain.
>
> Known issues/ caveats:
> - RPC handling time will become non-deterministic.
> - An event will only be "notified" to a client once the RPC for same VM completes.
> - Needs careful consideration in all cases where a QMP event is used to
>   "signal" an RPC thread, else will deadlock.
>
> Will be happy to drive more discussion in the community and completely
> implement it.
>
> Prerna Saxena (8):
>   Introduce virObjectTrylock()
>   QEMU Event handling: Introduce async event helpers in qemu_event.[ch]
>   Setup global and per-VM event queues. Also initialize per-VM queues
>     when libvirt reconnects to an existing VM.
>   Events: Allow monitor to "enqueue" events to a queue. Also introduce a
>     framework of handlers for each event type, that can be called when
>     the handler is running an event.
>   Events: Plumb event handling calls before a domain's APIs complete.
>   Code refactor: Move helper functions of doCoreDump*, syncNicRxFilter*,
>     and qemuOpenFile* to qemu_process.[ch]
>   Fold back the 2-stage event implementation for a few events :
>     Watchdog, Monitor EOF, Serial changed, Guest panic, Nic RX filter
>     changed .. into single level.
>   Initialize the per-VM event queues in context of domain init.
>
>  src/Makefile.am              |    1 +
>  src/conf/domain_conf.h       |    3 +
>  src/libvirt_private.syms     |    1 +
>  src/qemu/qemu_conf.h         |    4 +
>  src/qemu/qemu_driver.c       | 1710 +++++++----------------------------
>  src/qemu/qemu_event.c        |  317 +++++++
>  src/qemu/qemu_event.h        |  231 +++++
>  src/qemu/qemu_monitor.c      |  592 ++++++++++--
>  src/qemu/qemu_monitor.h      |   80 +-
>  src/qemu/qemu_monitor_json.c |  291 +++---
>  src/qemu/qemu_process.c      | 2031 ++++++++++++++++++++++++++++++++++--------
>  src/qemu/qemu_process.h      |   88 ++
>  src/util/virobject.c         |   26 +
>  src/util/virobject.h         |    4 +
>  src/util/virthread.c         |    5 +
>  src/util/virthread.h         |    1 +
>  tests/qemumonitortestutils.c |    2 +-
>  17 files changed, 3411 insertions(+), 1976 deletions(-)
>  create mode 100644 src/qemu/qemu_event.c
>  create mode 100644 src/qemu/qemu_event.h
>
> --
> 2.9.5
>

> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294