On 04/18/2011 05:41 AM, Daniel P. Berrange wrote:
Looks like your v2 caught my review comments correctly. But I found one more issue:
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
Oh well - that's what we get for incorporating something that I just typed, rather than testing...
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(); }
While we're at it, let's fix this else branch to have proper {} usage, per HACKING style guidelines.
- virDomainObjUnlock(vm); + if (vm) + virDomainObjUnlock(vm);
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org