Re: [libvirt] [PATCH 0/9] Resolve libvirtd hang on termination with connected long running client

On 01/23/2018 04:21 AM, Marc Hartmayer wrote: > On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com&gt; wrote: >> RFC: >> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html >> >> Adjustments since RFC... >> >> Patches 1&2: No change, were already R-B'd >> Patch 3: Removed code as noted in code review, update commit message >> Patch 4: From old series removed, see below for more details >> Patch 9: no change >> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; >> are removed as they seemed to not be necessary >> >> Replaced the former patch 4 with series of patches to (slowly) provide >> support to disable new connections, handle removing waiting jobs, causing >> the waiting workers to quit, and allow any running jobs to complete. >> >> As it turns out, waiting for running jobs to complete cannot be done >> from the virNetServerClose callbacks because that means the event loop >> processing done during virNetServerRun will not allow any currently >> long running worker job thread a means to complete. >> >> So when virNetDaemonQuit is called as a result of the libvirtd signal >> handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun >> to quit immediately, alter to using a quitRequested flag and then use >> that quitRequested flag to check for long running worker threads before >> causing the event loop to quit resulting in libvirtd being able to run >> through the virNetDaemonClose processing. > > Gave a quick test: > + Didn't get a segmentation fault at the end of libvirtd (at least for > my quick test) > - a single SIGTERM doesn’t always lead to the termination of libvirtd > now (debugged it: main thread is waiting for poll()). This behavior > can be easily reproduced: Start libvirtd on the CLI, wait some > seconds for the first initialization -> CTRL + C -> libvirtd doesn’t > terminate, but also doesn’t accept new connections. > Does this include the "update" in my response to patch 7? It should be extract-able and apply-able.

John >> >> John Ferlan (9): >> libvirtd: Alter refcnt processing for domain server objects >> libvirtd: Alter refcnt processing for server program objects >> netserver: Remove ServiceToggle during ServerDispose >> util: Introduce virThreadPoolDrain >> rpc: Introduce virNetServerQuitRequested >> rpc: Introduce virNetServerWorkerCount >> rpc: Alter virNetDaemonQuit processing >> docs: Add news article for libvirtd issue >> APPLY ONLY FOR TESTING PURPOSES >> >> daemon/libvirtd.c | 43 +++++++++++++++++++++++--------- >> docs/news.xml | 12 +++++++++ >> src/libvirt_private.syms | 1 + >> src/libvirt_remote.syms | 2 ++ >> src/qemu/qemu_driver.c | 5 ++++ >> src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++- >> src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++--- >> src/rpc/virnetserver.h | 4 +++ >> src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++-------- >> src/util/virthreadpool.h | 2 ++ >> 10 files changed, 204 insertions(+), 26 deletions(-) >> >> -- >> 2.13.6 >> >> -- >> libvir-list mailing list >> libvir-list(a)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 >