On Wed, Jun 17, 2015 at 16:53:14 -0400, John Ferlan wrote:
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.
Well, up until now I've inspected the qemu driver and the test driver and both set the transient def upon start of a VM, so we should be fine using this in those. Honestly I think that all drivers should be fixed and virDomainLiveConfigHelperMethod should be killed with fire forever.
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.
Me neither but that can be changed in the future.
John
I've pushed this series with the suggested changes and I'll follow up with fixes to the endjob label and the double space. Thanks. Peter