<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 15, 2017 at 12:07 AM, Marc Hartmayer <span dir="ltr"><<a href="mailto:mhartmay@linux.vnet.ibm.com" target="_blank">mhartmay@linux.vnet.ibm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Tue, Oct 24, 2017 at 07:34 PM +0200, Prerna Saxena <<a href="mailto:saxenap.ltc@gmail.com">saxenap.ltc@gmail.com</a>> wrote:<br>
> As noted in<br>
> <a href="https://www.redhat.com/archives/libvir-list/2017-May/msg00016.html" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>archives/libvir-list/2017-May/<wbr>msg00016.html</a><br>
> libvirt-QEMU driver handles all async events from the main loop.<br>
> Each event handling needs the per-VM lock to make forward progress. In<br>
> the case where an async event is received for the same VM which has an<br>
> RPC running, the main loop is held up contending for the same lock.<br>
<br>
</span>What's about the remaining qemuMonitorCallbacks? The main event loop can<br>
still be 'blocked' by e.g. qemuProcessHandleMonitorEOF if the VM is<br>
already locked by a worker thread. In fact, we currently have a problem<br>
with D-Bus which causes a D-Bus call (of a worker thread) to run into<br>
the timeout of 30 seconds. During these 30 seconds the main event loop<br>
is stuck.<br>
<br></blockquote><div><br></div><div>EOF handling in the current series is still yet another event, and hence is done only once worker threads complete. </div><div>It needs to go into a priority thread pool, and should wake up RPC workers.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I tried the patch series and got a segmentation fault:<br>
<br>
Thread 1 "libvirtd" received signal SIGSEGV, Segmentation fault.<br>
0x000003ff98faa452 in virEnqueueVMEvent (qlist=0x3ff908ce760,<br>
ev=<optimized out>) at ../../src/qemu/qemu_event.c:<wbr>153 153<br>
vmq_entry->ev->ev_id = vmq->last->ev->ev_id + 1;<br>
(gdb) bt<br>
#0 0x000003ff98faa452 in virEnqueueVMEvent (qlist=0x3ff908ce760,<br>
 ev=<optimized out>) at ../../src/qemu/qemu_event.c:<wbr>153<br>
#1 0x000003ff98fc3564 in qemuProcessEnqueueEvent (mon=<optimized out>,<br>
 vm=<optimized out>, ev=<optimized out>, opaque=0x3ff90548ec0) at<br>
 ../../src/qemu/qemu_process.c:<wbr>1864<br>
#2 0x000003ff98fe4804 in qemuMonitorEnqueueEvent<br>
 (mon=mon@entry=0x3ff4c007440, ev=0x2aa1e0104c0) at<br>
 ../../src/qemu/qemu_monitor.c:<wbr>1325<br>
#3 0x000003ff98fe7102 in qemuMonitorEmitShutdown<br>
 (mon=mon@entry=0x3ff4c007440, guest=<optimized out>,<br>
 seconds=seconds@entry=<wbr>1510683878, micros=micros@entry=703956) at<br>
 ../../src/qemu/qemu_monitor.c:<wbr>1365<br>
#4 0x000003ff98ffc19a in qemuMonitorJSONHandleShutdown<br>
 (mon=0x3ff4c007440, data=<optimized out>, seconds=1510683878,<br>
 micros=<optimized out>) at ../../src/qemu/qemu_monitor_<wbr>json.c:552<br>
#5 0x000003ff98ffbb8a in qemuMonitorJSONIOProcessEvent<br>
 (mon=mon@entry=0x3ff4c007440, obj=obj@entry=0x2aa1e012030) at<br>
 ../../src/qemu/qemu_monitor_<wbr>json.c:208<br>
#6 0x000003ff99002138 in qemuMonitorJSONIOProcessLine<br>
 (mon=mon@entry=0x3ff4c007440, line=0x2aa1e010460 "{\"timestamp\":<br>
 {\"seconds\": 1510683878, \"microseconds\": 703956}, \"event\":<br>
 \"SHUTDOWN\"}", msg=msg@entry=0x0) at<br>
 ../../src/qemu/qemu_monitor_<wbr>json.c:237<br>
#7 0x000003ff990022b4 in qemuMonitorJSONIOProcess<br>
 (mon=mon@entry=0x3ff4c007440, data=0x2aa1e014bc0 "{\"timestamp\":<br>
 {\"seconds\": 1510683878, \"microseconds\": 703956}, \"event\":<br>
 \"SHUTDOWN\"}\r\n", len=85, msg=msg@entry=0x0) at<br>
 ../../src/qemu/qemu_monitor_<wbr>json.c:279<br>
#8 0x000003ff98fe4b44 in qemuMonitorIOProcess<br>
 (mon=mon@entry=0x3ff4c007440) at ../../src/qemu/qemu_monitor.c:<wbr>443<br>
#9 0x000003ff98fe5d00 in qemuMonitorIO (watch=<optimized out>,<br>
 fd=<optimized out>, events=0, opaque=0x3ff4c007440) at<br>
 ../../src/qemu/qemu_monitor.c:<wbr>697<br>
#10 0x000003ffa68d6442 in virEventPollDispatchHandles (nfds=<optimized<br>
 out>, fds=0x2aa1e013990) at ../../src/util/vireventpoll.c:<wbr>508<br>
#11 0x000003ffa68d66c8 in virEventPollRunOnce () at<br>
 ../../src/util/vireventpoll.c:<wbr>657<br>
#12 0x000003ffa68d44e4 in virEventRunDefaultImpl () at<br>
 ../../src/util/virevent.c:327<br>
#13 0x000003ffa6a83c5e in virNetDaemonRun (dmn=0x2aa1dfe3eb0) at<br>
 ../../src/rpc/virnetdaemon.c:<wbr>838<br>
#14 0x000002aa1df29cc4 in main (argc=<optimized out>, argv=<optimized<br>
 out>) at ../../daemon/libvirtd.c:1494<br>
<div><div class="h5"><br></div></div></blockquote><div><br></div><div>Thanks for trying it out. Let me look into this.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
><br>
> This impacts scalability, and should be addressed on priority.<br>
><br>
> Note that libvirt does have a 2-step deferred handling for a few event<br>
> categories, but (1) That is insufficient since blockign happens before<br>
> the handler could disambiguate which one needs to be posted to this<br>
> other queue.<br>
> (2) There needs to be homogeniety.<br>
><br>
> The current series builds a framework for recording and handling VM<br>
> events.<br>
> It initializes per-VM event queue, and a global event queue pointing to<br>
> events from all the VMs. Event handling is staggered in 2 stages:<br>
> - When an event is received, it is enqueued in the per-VM queue as well<br>
>   as the global queues.<br>
> - The global queue is built into the QEMU Driver as a threadpool<br>
>   (currently with a single thread).<br>
> - Enqueuing of a new event triggers the global event worker thread, which<br>
>   then attempts to take a lock for this event's VM.<br>
>     - If the lock is available, the event worker runs the function handling<br>
>       this event type. Once done, it dequeues this event from the global<br>
>       as well as per-VM queues.<br>
>     - If the lock is unavailable(ie taken by RPC thread), the event worker<br>
>       thread leaves this as-is and picks up the next event.<br>
> - Once the RPC thread completes, it looks for events pertaining to the<br>
>   VM in the per-VM event queue. It then processes the events serially<br>
>   (holding the VM lock) until there are no more events remaining for<br>
>   this VM. At this point, the per-VM lock is relinquished.<br>
><br>
> Patch Series status:<br>
> Strictly RFC only. No compilation issues. I have not had a chance to<br>
> (stress) test it after rebase to latest master.<br>
> Note that documentation and test coverage is TBD, since a few open<br>
> points remain.<br>
><br>
> Known issues/ caveats:<br>
> - RPC handling time will become non-deterministic.<br>
> - An event will only be "notified" to a client once the RPC for same VM completes.<br>
> - Needs careful consideration in all cases where a QMP event is used to<br>
>   "signal" an RPC thread, else will deadlock.<br>
><br>
> Will be happy to drive more discussion in the community and completely<br>
> implement it.<br>
><br>
> Prerna Saxena (8):<br>
>   Introduce virObjectTrylock()<br>
>   QEMU Event handling: Introduce async event helpers in qemu_event.[ch]<br>
>   Setup global and per-VM event queues. Also initialize per-VM queues<br>
>     when libvirt reconnects to an existing VM.<br>
>   Events: Allow monitor to "enqueue" events to a queue. Also introduce a<br>
>     framework of handlers for each event type, that can be called when<br>
>     the handler is running an event.<br>
>   Events: Plumb event handling calls before a domain's APIs complete.<br>
>   Code refactor: Move helper functions of doCoreDump*, syncNicRxFilter*,<br>
>     and qemuOpenFile* to qemu_process.[ch]<br>
>   Fold back the 2-stage event implementation for a few events :<br>
>     Watchdog, Monitor EOF, Serial changed, Guest panic, Nic RX filter<br>
>     changed .. into single level.<br>
>   Initialize the per-VM event queues in context of domain init.<br>
><br>
>  src/Makefile.am              |    1 +<br>
>  src/conf/domain_conf.h       |    3 +<br>
>  src/libvirt_private.syms     |    1 +<br>
>  src/qemu/qemu_conf.h         |    4 +<br>
>  src/qemu/qemu_driver.c       | 1710 +++++++-----------------------<wbr>-----<br>
>  src/qemu/qemu_event.c        |  317 +++++++<br>
>  src/qemu/qemu_event.h        |  231 +++++<br>
>  src/qemu/qemu_monitor.c      |  592 ++++++++++--<br>
>  src/qemu/qemu_monitor.h      |   80 +-<br>
>  src/qemu/qemu_monitor_json.c |  291 +++---<br>
>  src/qemu/qemu_process.c      | 2031 ++++++++++++++++++++++++++++++<wbr>++++--------<br>
>  src/qemu/qemu_process.h      |   88 ++<br>
>  src/util/virobject.c         |   26 +<br>
>  src/util/virobject.h         |    4 +<br>
>  src/util/virthread.c         |    5 +<br>
>  src/util/virthread.h         |    1 +<br>
>  tests/qemumonitortestutils.c |    2 +-<br>
>  17 files changed, 3411 insertions(+), 1976 deletions(-)<br>
>  create mode 100644 src/qemu/qemu_event.c<br>
>  create mode 100644 src/qemu/qemu_event.h<br>
><br>
> --<br>
> 2.9.5<br>
><br>
</div></div>> --<br>
> libvir-list mailing list<br>
> <a href="mailto:libvir-list@redhat.com">libvir-list@redhat.com</a><br>
> <a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/libvir-list</a><br>
<br>
Beste Grüße / Kind regards<br>
   Marc Hartmayer<br>
<br>
IBM Deutschland Research & Development GmbH<br>
Vorsitzende des Aufsichtsrats: Martina Koederitz<br>
Geschäftsführung: Dirk Wittkopp<br>
Sitz der Gesellschaft: Böblingen<br>
Registergericht: Amtsgericht Stuttgart, HRB 243294<br>
<br>
</blockquote></div><br></div></div>