On 12/20/2017 11:47 AM, Daniel P. Berrange wrote:
> Currently drivers can only do a bulk load of config / status files for
> their guests. This exposes some helper methods to allow individual
> guests to be loaded.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> src/conf/virdomainobjlist.c | 98 ++++++++++++++++++++++++++++++++-------------
> src/conf/virdomainobjlist.h | 17 ++++++++
> src/libvirt_private.syms | 2 +
> 3 files changed, 89 insertions(+), 28 deletions(-)
>
> diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
> index 87a742b1ea..37f795fe6d 100644
> --- a/src/conf/virdomainobjlist.c
> +++ b/src/conf/virdomainobjlist.c
> @@ -443,16 +443,15 @@ void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
> virObjectUnlock(dom);
> }
>
> -
> static virDomainObjPtr
> -virDomainObjListLoadConfig(virDomainObjListPtr doms,
> - virCapsPtr caps,
> - virDomainXMLOptionPtr xmlopt,
> - const char *configDir,
> - const char *autostartDir,
> - const char *name,
> - virDomainLoadConfigNotify notify,
> - void *opaque)
> +virDomainObjListLoadConfigLocked(virDomainObjListPtr doms,
> + virCapsPtr caps,
> + virDomainXMLOptionPtr xmlopt,
> + const char *configDir,
> + const char *autostartDir,
> + const char *name,
> + virDomainLoadConfigNotify notify,
> + void *opaque)
> {
> char *configFile = NULL, *autostartLink = NULL;
> virDomainDefPtr def = NULL;
> @@ -496,14 +495,36 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
> }
>
>
> -static virDomainObjPtr
> -virDomainObjListLoadStatus(virDomainObjListPtr doms,
> - const char *statusDir,
> - const char *name,
> +virDomainObjPtr
> +virDomainObjListLoadConfig(virDomainObjListPtr doms,
> virCapsPtr caps,
> virDomainXMLOptionPtr xmlopt,
> + const char *configDir,
> + const char *autostartDir,
> + const char *name,
> virDomainLoadConfigNotify notify,
> void *opaque)
> +{
> + virDomainObjPtr vm = NULL;
> + virObjectRWLockWrite(doms);
> +
> + vm = virDomainObjListLoadConfigLocked(doms, caps, xmlopt, configDir,
> + autostartDir, name, notify, opaque);
> + virObjectRef(vm);
<sigh> IIRC this Ref is done because the virDomainObjListAddLocked
doesn't add enough refs. One of those things that's been on my list to
get to after finishing up the storage/nwfilter "common object"
adjustments. IOW: AddLocked is broken because it only adds one ref even
though the obj is in both the UUID and Name tables - so callers are
forced to "adjust". When we leave AddLocked there should be 3 refs
and 1 lock, but there are 2 refs and 1 lock.
The other concerns here would be when virDomainObjListLoadAllConfigs
returns a valid @dom will:
1. Set "dom->persistent" based upon the @liveStatus pool passed in
2. Call virObjectUnlock(dom)
Neither happens in these new API's. So all new consumers would need to
know/remember to unlock (or EndAPI) and the LoadConfig caller would need
to set persistent (unless we did so in "this" function").
Probably something that needs to be documented in the two new functions.
John
> +
> + virObjectRWUnlock(doms);
> + return vm;
> +}
> +
> +
> +static virDomainObjPtr
> +virDomainObjListLoadStatusLocked(virDomainObjListPtr doms,
> + const char *statusDir,
> + const char *name,
> + virCapsPtr caps,
> + virDomainXMLOptionPtr xmlopt,
> + virDomainLoadConfigNotify notify,
> + void *opaque)
> {
> char *statusFile = NULL;
> virDomainObjPtr obj = NULL;
> @@ -555,6 +576,27 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms,
> }
>
>
> +virDomainObjPtr
> +virDomainObjListLoadStatus(virDomainObjListPtr doms,
> + const char *statusDir,
> + const char *name,
> + virCapsPtr caps,
> + virDomainXMLOptionPtr xmlopt,
> + virDomainLoadConfigNotify notify,
> + void *opaque)
> +{
> + virDomainObjPtr vm = NULL;
> + virObjectRWLockWrite(doms);
> +
> + vm = virDomainObjListLoadStatusLocked(doms, statusDir, name, caps, xmlopt,
> + notify, opaque);
> + virObjectRef(vm);
> +
> + virObjectRWUnlock(doms);
> + return vm;
> +}
> +
> +
> int
> virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
> const char *configDir,
> @@ -587,22 +629,22 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
> kill the whole process */
> VIR_INFO("Loading config file '%s.xml'", entry->d_name);
> if (liveStatus)
> - dom = virDomainObjListLoadStatus(doms,
> - configDir,
> - entry->d_name,
> - caps,
> - xmlopt,
> - notify,
> - opaque);
> + dom = virDomainObjListLoadStatusLocked(doms,
> + configDir,
> + entry->d_name,
> + caps,
> + xmlopt,
> + notify,
> + opaque);
> else
> - dom = virDomainObjListLoadConfig(doms,
> - caps,
> - xmlopt,
> - configDir,
> - autostartDir,
> - entry->d_name,
> - notify,
> - opaque);
> + dom = virDomainObjListLoadConfigLocked(doms,
> + caps,
> + xmlopt,
> + configDir,
> + autostartDir,
> + entry->d_name,
> + notify,
> + opaque);
> if (dom) {
> if (!liveStatus)
> dom->persistent = 1;
> diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
> index bb186bde30..4aee8fae13 100644
> --- a/src/conf/virdomainobjlist.h
> +++ b/src/conf/virdomainobjlist.h
> @@ -77,6 +77,23 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
> virDomainXMLOptionPtr xmlopt,
> virDomainLoadConfigNotify notify,
> void *opaque);
> +virDomainObjPtr
> +virDomainObjListLoadConfig(virDomainObjListPtr doms,
> + virCapsPtr caps,
> + virDomainXMLOptionPtr xmlopt,
> + const char *configDir,
> + const char *autostartDir,
> + const char *name,
> + virDomainLoadConfigNotify notify,
> + void *opaque);
> +virDomainObjPtr
> +virDomainObjListLoadStatus(virDomainObjListPtr doms,
> + const char *statusDir,
> + const char *name,
> + virCapsPtr caps,
> + virDomainXMLOptionPtr xmlopt,
> + virDomainLoadConfigNotify notify,
> + void *opaque);
>
> int virDomainObjListNumOfDomains(virDomainObjListPtr doms,
> bool active,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a0fde65dba..be631e3dd7 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -938,6 +938,8 @@ virDomainObjListForEach;
> virDomainObjListGetActiveIDs;
> virDomainObjListGetInactiveNames;
> virDomainObjListLoadAllConfigs;
> +virDomainObjListLoadConfig;
> +virDomainObjListLoadStatus;
> virDomainObjListNew;
> virDomainObjListNumOfDomains;
> virDomainObjListRemove;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
