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

[John Ferlan]

On 01/23/2018 04:21 AM, Marc Hartmayer wrote:
> On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan at redhat.com> 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 at virtuozzo.com>
>>     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 at 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
>