Re: [libvirt] [PATCH 3/3] Stop accessing driver->caps directly in QEMU driver
On Wed, Feb 06, 2013 at 11:54:51 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'driver->caps' pointer can be changed on the fly. Accessing
it currently requires the global driver lock. Isolate this
access in a single helper, so a future patch can relax the
locking constraints.
...
I checked that no direct access to driver->caps remained.
There are quite a few places where virQEMUDriverGetCapabilities is
called on an unlocked driver but that's not your fault. The bug has been
there even for years in some cases. Anyway, I guess one of your next
series will fix all these issues by changing the way we use qemu driver
lock.
...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 18c4d3c..7ada7c3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
...
@@ -956,20 +888,25 @@ static void
qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
static int
qemuReload(void) {
virQEMUDriverConfigPtr cfg;
+ virCapsPtr caps;
if (!qemu_driver)
return 0;
+ if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false)))
+ return -1;
+
I know you're going to revisit qemu_driver locking in a future series
but you can still add virQEMUDriverGetCapabilities call after
qemu_driver is actually locked here.
qemuDriverLock(qemu_driver);
cfg = virQEMUDriverGetConfig(qemu_driver);
virDomainObjListLoadAllConfigs(qemu_driver->domains,
- qemu_driver->caps,
+ caps,
cfg->configDir,
cfg->autostartDir,
0, QEMU_EXPECTED_VIRT_TYPES,
qemuNotifyLoadDomain, qemu_driver);
qemuDriverUnlock(qemu_driver);
virObjectUnref(cfg);
+ virObjectUnref(caps);
return 0;
}
...
ACK with this small change.
Jirka