[libvirt] [PATCH V3 2/2] enhance processWatchdogEvent()
Daniel P. Berrange
berrange at redhat.com
Mon Apr 18 11:41:23 UTC 2011
On Fri, Apr 15, 2011 at 10:36:10AM -0600, Eric Blake wrote:
> 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.
This last minute addition caused a build failure
cc1: warnings being treated as errors
qemu/qemu_process.c: In function 'qemuProcessHandleWatchdog':
qemu/qemu_process.c:436:34: error: ignoring return value of 'virDomainObjUnref', declared with attribute warn_unused_result [-Wunused-result]
make[3]: *** [libvirt_driver_qemu_la-qemu_process.lo] Error 1
I think we also need this added:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d405dda..5a81265 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -433,14 +433,16 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
*/
virDomainObjRef(vm);
if (virThreadPoolSendJob(driver->workerPool, wdEvent) < 0) {
- virDomainObjUnref(vm);
+ if (virDomainObjUnref(vm) < 0)
+ vm = NULL;
VIR_FREE(wdEvent);
}
} else
virReportOOMError();
}
- virDomainObjUnlock(vm);
+ if (vm)
+ virDomainObjUnlock(vm);
if (watchdogEvent || lifecycleEvent) {
qemuDriverLock(driver);
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list