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

[Nikolay Shirokovskiy]

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);
>>  }
>>