On Tue, Jan 23, 2018 at 04:01 PM +0100, John Ferlan <jferlan@xxxxxxxxxx> wrote: > On 01/23/2018 04:21 AM, Marc Hartmayer wrote: >> On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan@xxxxxxxxxx> 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@xxxxxxxxxxxxx> >>> 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. No, I forgot this one :( I’ll try it tomorrow. > > 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@xxxxxxxxxx >>> 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 >> > -- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list