On Wed, Jul 17, 2013 at 03:04:19PM +0200, Michal Privoznik wrote:
> Currently the virLXCDriverPtr struct contains an wide variety
> of data with varying access needs. Move all the static config
> data into a dedicated virLXCDriverConfigPtr object. The only
> locking requirement is to hold the driver lock, while obtaining
> an instance of virLXCDriverConfigPtr. 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.
> ---
> src/lxc/lxc_conf.c | 81 +++++++++++++++++++++-------
> src/lxc/lxc_conf.h | 41 +++++++++-----
> src/lxc/lxc_driver.c | 145 ++++++++++++++++++++++++++++++++++----------------
> src/lxc/lxc_process.c | 39 +++++++++-----
> 4 files changed, 214 insertions(+), 92 deletions(-)
> diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
> index 5a5b9aa..6ca6198 100644
> --- a/src/lxc/lxc_conf.h
> +++ b/src/lxc/lxc_conf.h
> @@ -46,44 +46,57 @@
> typedef struct _virLXCDriver virLXCDriver;
> typedef virLXCDriver *virLXCDriverPtr;
>
> +typedef struct _virLXCDriverConfig virLXCDriverConfig;
> +typedef virLXCDriverConfig *virLXCDriverConfigPtr;
> +
> +struct _virLXCDriverConfig {
> + virObject parent;
> +
> + char *configDir;
> + char *autostartDir;
> + char *stateDir;
> + char *logDir;
> + int log_libvirtd;
> + int have_netns;
> +
> + char *securityDriverName;
> + bool securityDefaultConfined;
> + bool securityRequireConfined;
> +};
> +
> struct _virLXCDriver {
> virMutex lock;
>
> + virLXCDriverConfigPtr config;
> +
> virCapsPtr caps;
> - virDomainXMLOptionPtr xmlopt;
>
> virCgroupPtr cgroup;
>
> + virDomainXMLOptionPtr xmlopt;
> +
If I'm being fussy I'd say you shouldn't be moving the 'xmlopt'
field in this patch.
> virSysinfoDefPtr hostsysinfo;
>
> size_t nactive;
> +
> virStateInhibitCallback inhibitCallback;
> void *inhibitOpaque;
>
> virDomainObjListPtr domains;
> - char *configDir;
> - char *autostartDir;
> - char *stateDir;
> - char *logDir;
> - int log_libvirtd;
> - int have_netns;
>
> virUSBDeviceListPtr activeUsbHostdevs;
>
> virDomainEventStatePtr domainEventState;
>
> - char *securityDriverName;
> - bool securityDefaultConfined;
> - bool securityRequireConfined;
> virSecurityManagerPtr securityManager;
>
> - /* Mapping of 'char *uuidstr' -> virConnectPtr
> - * of guests which will be automatically killed
> - * when the virConnectPtr is closed*/
Nor removing this comment, until the later patch which
changes this field to use the generic callbacks.
> virHashTablePtr autodestroy;
> };
ACK anyway. This code pattern matches what was done in the QEMU driver
which has proved itself succesful.
Daniel
--
|: 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