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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list