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@xxxxxxxxxx>
> >
> > 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list