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

[Eric Blake]

On 04/06/2011 01:58 AM, Wen Congyang wrote:
> If the domain is not persistent, and qemu quited unexpectedly before

s/quited/quits/

> calling processWatchdogEvent(), vm will be freed and the function
> processWatchdogEvent() will be dangerous.
> 
> ---
>  src/qemu/qemu_driver.c  |   12 ++++++++----
>  src/qemu/qemu_process.c |    4 ++++
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 628cfe3..d02891c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2436,15 +2436,19 @@ static void processWatchdogEvent(void *data, void *opaque)
>          }
>          break;
>      default:
> -        goto cleanup;
> +        qemuDriverLock(driver);
> +        virDomainObjLock(wdEvent->vm);
> +        goto unlock;
>      }
>  
>  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)
> +    if (virDomainObjUnref(wdEvent->vm) > 0)
>          virDomainObjUnlock(wdEvent->vm);
>      qemuDriverUnlock(driver);
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 48ecd5c..321374c 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));

ACK to the need to do this.  Hmm, maybe we should just squash 2 and 3
together into one patch.  Also, I found a flaw in 2 while reviewing this
one.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110414/563deb7e/attachment-0001.sig>