[libvirt] [PATCH V3 2/2] enhance processWatchdogEvent()

[Wen Congyang]

At 04/16/2011 12:36 AM, Eric Blake Write:
> On 04/14/2011 09:11 PM, Wen Congyang wrote:
>> This patch do the following two things:
> 
> s/do/does/
> 
>> 1. hold an extra reference while handling watchdog event
>>    If the domain is not persistent, and qemu quits unexpectedly before
>>    calling processWatchdogEvent(), vm will be freed and the function
>>    processWatchdogEvent() will be dangerous.
>>
>> 2. unlock qemu driver and vm before returning from processWatchdogEvent()
>>    When the function processWatchdogEvent() failed, we only free wdEvent,
>>    but forget to unlock qemu driver and vm, free dumpfile.
>>
>>
>> ---
>>  src/qemu/qemu_driver.c  |   34 ++++++++++++++++++++++------------
>>  src/qemu/qemu_process.c |    4 ++++
>>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> Looks like your v2 caught my review comments correctly.  But I found one
> more issue:
> 
>> +++ b/src/qemu/qemu_process.c
>> @@ -428,6 +428,10 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>>          if (VIR_ALLOC(wdEvent) == 0) {
>>              wdEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP;
>>              wdEvent->vm = vm;
>> +            /* Hold an extra reference because we can't allow 'vm' to be
>> +             * deleted before handling watchdog event is finished.
>> +             */
>> +            virDomainObjRef(vm);
>>              ignore_value(virThreadPoolSendJob(driver->workerPool, wdEvent));
> 
> Now that we have increased the ref count, we should decrease it if we
> are unable to send a job to the thread pool.  That is, replace the
> ignore_value() with:
> 
> if (virThreadPoolSendJob(...) < 0) {
>     virDomainObjUnref(vm);
>     VIR_FREE(wdEvent);
> }
> 
> ACK with that change squashed in.

I have pushed this series patch with this chaged squashed in.
Thanks for reviewing.

>