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

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