On Fri, Feb 01, 2013 at 09:20:25PM +0100, Jiri Denemark wrote:
On Fri, Feb 01, 2013 at 11:18:23 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently the virQEMUDriverPtr struct contains an wide variety of data with varying access needs. Move all the static config data into a dedicated virQEMUDriverConfigPtr object. The only locking requirement is to hold the driver lock, while obtaining an instance of virQEMUDriverConfigPtr. Once a reference is held on the config object, it can be used completely lockless since it is immutable.
NB, not all APIs correctly hold the driver lock while getting a reference to the config object in this patch. This is safe for now since the config is never updated on the fly. Later patches will address this fully.
...
15 files changed, 1099 insertions(+), 755 deletions(-)
Uff. I don't want to go through this patch more than once so please don't send v2, just send a diff between v1 and v2, which should be much easier to review.
Here are the changes I propose - should adress every point you mention. Daniel diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 482989f..376a21d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -50,6 +50,7 @@ bool qemuCgroupControllerActive(virQEMUDriverPtr driver, { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); bool ret = false; + if (driver->cgroup == NULL) goto cleanup; if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ef04634..fbc5481 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3294,6 +3294,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("scripts are not supported on interfaces of type %s"), virDomainNetTypeToString(netType)); + virObjectUnref(cfg); return NULL; } diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 46c1892..454aa35 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -104,16 +104,13 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (privileged) { if (virGetUserID(QEMU_USER, &cfg->user) < 0) goto error; - } else { - cfg->user = 0; - } - if (privileged) { if (virGetGroupID(QEMU_GROUP, &cfg->group) < 0) goto error; } else { + cfg->user = 0; cfg->group = 0; } - cfg->dynamicOwnership = privileged ? true : false; + cfg->dynamicOwnership = privileged; cfg->cgroupControllers = (1 << VIR_CGROUP_CONTROLLER_CPU) | diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6c328d6..d2b717b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1873,7 +1873,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, &dev->data.vnc.auth, cfg->vncPassword); if (ret < 0) - return ret; + goto cleanup; /* Steal the new dev's char * reference */ VIR_FREE(olddev->data.vnc.auth.passwd); @@ -1934,7 +1934,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, cfg->spicePassword); if (ret < 0) - return ret; + goto cleanup; /* Steal the new dev's char * reference */ VIR_FREE(olddev->data.spice.auth.passwd); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d1d3d95..bf97edd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3550,7 +3550,7 @@ int qemuProcessStart(virConnectPtr conn, char *nodeset = NULL; virBitmapPtr nodemask = NULL; unsigned int stop_flags; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virQEMUDriverConfigPtr cfg; /* Okay, these are just internal flags, * but doesn't hurt to check */ @@ -3558,6 +3558,8 @@ int qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESROY, -1); + cfg = virQEMUDriverGetConfig(driver); + /* From now on until domain security labeling is done: * if any operation fails and we goto cleanup, we must not * restore any security label as we would overwrite labels -- |: 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 :|