Re: [PATCH V3 2/2] enhance processWatchdogEvent()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]