[libvirt] [PATCH 02/13] conf: Introduce helper to help getting correct def for getter functions

[John Ferlan]

On 06/15/2015 03:47 PM, Peter Krempa wrote:
> virDomainObjGetOneDef will help to retrieve the correct definition
> pointer from @vm in cases where VIR_DOMAIN_AFFECT_LIVE and
> VIR_DOMAIN_AFFECT_CONFIG are mutually exclusive. The function simply
> returns the correct pointer. This similarly to virDomainObjGetDefs will
> greatly simplify the code.
> ---
>  src/conf/domain_conf.c   | 36 ++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h   |  1 +
>  src/libvirt_private.syms |  1 +
>  3 files changed, 38 insertions(+)
> 

I dunno - I just have this little voice asking is there some corner case
from the prior virDomainLiveConfigHelperMethod thruough possibly
creating 'newDef' in virDomainObjSetDefTransient that this new code will
miss...  IOW - is there a condition where CONFIG is wanted, domain is
running, but newDef is NULL.

OTOH - one wonders if the previous code went through the patch into
virDomainObjSetDefTransient

All the callers check for NULL, so that's not an issue - just
considering some strange corner case for a transient domain.

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3cc182b..35e1cb4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2948,6 +2948,42 @@ virDomainObjGetDefs(virDomainObjPtr vm,
>  }
> 
> 
> +/**
> + * virDomainObjGetOneDef:
> + *
> + * @vm: Domain object
> + * @flags: for virDomainModificationImpact
> + *
> + * Helper function to resolve @flags and return the correct domain pointer
> + * object. This function returns one of @vm->def or @vm->persistentDef
> + * according to @flags. This helper should be used only in APIs that guarantee
> + * that @flags contains exactly one of VIR_DOMAIN_AFFECT_LIVE,

s/,/ or/

> + * VIR_DOMAIN_AFFECT_CONFIG.

and to be really redundant ;-) - couldn't resist.

s/./ and not both./


ACK - although "GetOneDef" isn't my favorite, I don't have a better
suggestion either.

John
> + *
> + * Returns the correct definition pointer or NULL on error.
> + */
> +virDomainDefPtr
> +virDomainObjGetOneDef(virDomainObjPtr vm,
> +                      unsigned int flags)
> +{
> +    if (flags & VIR_DOMAIN_AFFECT_LIVE && flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +            virReportInvalidArg(ctl, "%s",
> +                                _("Flags 'VIR_DOMAIN_AFFECT_LIVE' and "
> +                                  "'VIR_DOMAIN_AFFECT_CONFIG' are mutually "
> +                                  "exclusive"));
> +            return NULL;
> +    }
> +
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        return NULL;
> +
> +    if (virDomainObjIsActive(vm) && flags & VIR_DOMAIN_AFFECT_CONFIG)
> +        return vm->newDef;
> +    else
> +        return vm->def;
> +}
> +
> +
>  /*
>   * The caller must hold a lock on the driver owning 'doms',
>   * and must also have locked 'dom', to ensure no one else
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ba17a8d..db49d46 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2553,6 +2553,7 @@ int virDomainObjGetDefs(virDomainObjPtr vm,
>                          unsigned int flags,
>                          virDomainDefPtr *liveDef,
>                          virDomainDefPtr *persDef);
> +virDomainDefPtr virDomainObjGetOneDef(virDomainObjPtr vm, unsigned int flags);
> 
>  int
>  virDomainLiveConfigHelperMethod(virCapsPtr caps,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index dc8a52d..858c00f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -385,6 +385,7 @@ virDomainObjEndAPI;
>  virDomainObjFormat;
>  virDomainObjGetDefs;
>  virDomainObjGetMetadata;
> +virDomainObjGetOneDef;
>  virDomainObjGetPersistentDef;
>  virDomainObjGetState;
>  virDomainObjListAdd;
>