[libvirt] [PATCH 2/9] Introduce a virLXCDriverConfigPtr object

[Daniel P. Berrange]

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 :|