[libvirt] [PATCH 3/3] hold an extra reference while handling watchdog event

[Wen Congyang]

At 03/30/2011 02:05 PM, Hu Tao Write:
> On Wed, Mar 30, 2011 at 12:34:49PM +0800, Wen Congyang wrote:
>> If the domain is not persistent, and qemu quited unexpectedly before
>> calling processWatchdogEvent(), vm will be freed and the function
>> processWatchdogEvent() will be dangerous.
>>
>> ---
>>  src/qemu/qemu_driver.c  |   10 ++++++----
>>  src/qemu/qemu_process.c |    4 ++++
>>  2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index d79d61b..c9c681f 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -2443,15 +2443,17 @@ static void processWatchdogEvent(void *data, void *opaque)
>>      }
>>  
>>  endjob:
>> -    if (qemuDomainObjEndJob(wdEvent->vm) == 0)
>> -        wdEvent->vm = NULL;
>> +    /* Safe to ignore value since ref count was incremented in
>> +     * qemuProcessHandleWatchdog().
>> +     */
>> +    ignore_value(qemuDomainObjEndJob(wdEvent->vm));
>>  
>>  unlock:
>> -    if (wdEvent->vm)
>> -        virDomainObjUnlock(wdEvent->vm);
>>      qemuDriverUnlock(driver);
>>  
>>  cleanup:
>> +    if (virDomainObjUnref(wdEvent->vm) > 0)
>> +        virDomainObjUnlock(wdEvent->vm);
> 
> These two lines should be protected by qemu driver lock.

We should not unlock domain here as we reached here without locking
domain.

Will update this patch.

> 
>>      VIR_FREE(wdEvent);
>>  }
>>  
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index e31e1b4..cd8c726 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -426,6 +426,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));
>>          } else
>>              virReportOOMError();
>> -- 
>> 1.7.1
>