Re: [libvirt] [PATCH v3 1/2] libvirtd: fix crash on termination

On 25.10.2017 11:07, John Ferlan wrote: > > > On 10/24/2017 06:47 AM, Nikolay Shirokovskiy wrote: >> The problem is incorrect order of qemu driver shutdown and shutdown >> of netserver threads that serve client requests (thru qemu driver >> particularly). >> >> Net server threads are shutdowned upon dispose which is triggered >> by last daemon object unref at the end of main function. At the same >> time qemu driver is shutdowned earlier in virStateCleanup. As a result >> netserver threads see invalid driver object in the middle of request >> processing. >> >> Let's move shutting down netserver threads earlier to virNetDaemonClose. >> >> Note: order of last daemon unref and virStateCleanup >> is introduced in 85c3a182 for a valid reason. >> --- >> >> One can use next patch to trigger crash on termination. Call domstats function >> and then send TERM to libvirtd. >> >> Patch to trigger crash >> # diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> # index cf5e4ad..39a57aa 100644 >> # --- a/src/qemu/qemu_driver.c >> # +++ b/src/qemu/qemu_driver.c >> # @@ -20144,6 +20144,8 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, >> # domflags = 0; >> # vm = vms[i]; >> # >> # + sleep(5); >> # + >> # virObjectLock(vm); >> # >> # if (HAVE_JOB(privflags) && >> >> src/rpc/virnetdaemon.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c >> index e3b9390..495bc4b 100644 >> --- a/src/rpc/virnetdaemon.c >> +++ b/src/rpc/virnetdaemon.c >> @@ -880,6 +880,8 @@ virNetDaemonClose(virNetDaemonPtr dmn) >> virObjectLock(dmn); >> >> virHashForEach(dmn->servers, daemonServerClose, NULL); >> + virHashFree(dmn->servers); > > Should this be virHashRemoveAll? Yep, it would be better. I guess no need to resend the series...

Nikolay > > Or I wonder if the virHashFree in virNetDaemonDispose. > > John > > (I'm at KVM Forum this week so responses will be delayed) > >> + dmn->servers = NULL; >> >> virObjectUnlock(dmn); >> } >>