On 02/11/2013 09:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
With the majority of fields in the virQEMUDriverPtr struct now immutable or self-locking, there is no need for practically any methods to be using the QEMU driver lock. Only a handful of helper APIs in qemu_conf.c now need it
I agree with squashing in Martin's cleanups. Beyond that, the locking changes looked sane. ACK. Let's get this in and stress-tested before the 1.0.3 release.
+++ b/src/qemu/qemu_driver.c @@ -153,16 +153,9 @@ virQEMUDriverPtr qemu_driver = NULL;
static void -qemuVMDriverLock(void) { - qemuDriverLock(qemu_driver); -}; - - +qemuVMDriverLock(void) {} static void -qemuVMDriverUnlock(void) { - qemuDriverUnlock(qemu_driver); -}; - +qemuVMDriverUnlock(void) {}
Do we need to keep these functions any more, or can we get rid of them in a followup patch?
@@ -3316,7 +3234,7 @@ static int qemuDomainCoreDump(virDomainPtr dom, goto cleanup; }
- if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, + if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_DUMP) < 0)
Indentation looks off.
@@ -929,14 +900,10 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virObjectUnlock(vm);
cleanup: - if (watchdogEvent || lifecycleEvent) { - qemuDriverLock(driver); - if (watchdogEvent) - qemuDomainEventQueue(driver, watchdogEvent); - if (lifecycleEvent) - qemuDomainEventQueue(driver, lifecycleEvent); - qemuDriverUnlock(driver); - } + if (watchdogEvent) + qemuDomainEventQueue(driver, watchdogEvent); + if (lifecycleEvent) + qemuDomainEventQueue(driver, lifecycleEvent);
Is it worth a followup patch to qemuDomainEventQueue that becomes a no-op when passed a NULL event, so we can let the callers avoid the 'if'? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org