[libvirt] [PATCH 3/3] common-impl: make virDomainObjGetPersistentDef return only persistent config
nshirokovskiy at virtuozzo.com
Thu Feb 18 09:36:19 UTC 2016
On 02.02.2016 16:18, Nikolay Shirokovskiy wrote:
> On 02.02.2016 15:13, John Ferlan wrote:
>> On 01/15/2016 09:05 AM, Nikolay Shirokovskiy wrote:
>>> It's rather unexpected that function with such a name could return
>>> transient definition. This is possible if we call it for
>>> a transient active domain. But if it is called in such a conditions?
>>> No. So we would better fix this case to return NULL.
>>> Calling conditions investigation.
>>> There are 4 distinct callers:
>>> 1. migration - explicitly set 'persistent' before calling.
>>> 2. libxl_driver
>>> First it assures that (VIR_DOMAIN_VCPU_CONFIG && !persistent) is false.
>>> Then all usages of definition is under VIR_DOMAIN_VCPU_CONFIG flag,
>>> so 'persistent' is set.
>>> 3. virDomainObjCopyPersistentDef - all callers of this function
>>> has next structure:
>>> 1 call virDomainLiveConfigHelperMethod or virDomainObjUpdateModificationImpact
>>> thus we are sure that (VIR_DOMAIN_VCPU_CONFIG && !persistent) is false.
>>> 2. call function of interest under VIR_DOMAIN_VCPU_CONFIG, so
>>> 'persistent' flag is set.
>>> 4. virDomainLiveConfigHelperMethod - the same reason as in 3.
>>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
>>> src/conf/domain_conf.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>> You've based your assumptions off of the change you made in 1/3 to
>> virDomainObjUpdateModificationImpact regarding combining that if
>> statement. Also even if the following would fail, you would have need
>> to provide some sort of virReportError.
> But that change is merely a refactoring it does not change behaviour.
> I'll add an error message of course but first I'll wait for agreement on that
> this patch is correct. May be I miss something as I really don't
> see why change you mention is matter.
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index e54c097..018c77e 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -2825,18 +2825,21 @@ virDomainObjSetDefTransient(virCapsPtr caps,
>>> * Return the persistent domain configuration. If domain is transient,
>>> - * return the running config.
>>> + * return NULL.
>>> * @param caps pointer to capabilities info
>>> * @param xmlopt pointer to XML parser configuration object
>>> * @param domain domain object pointer
>>> - * @return NULL on error, virDOmainDefPtr on success
>>> + * @return NULL on error or transient domains, virDOmainDefPtr on success
>>> virDomainObjGetPersistentDef(virCapsPtr caps,
>>> virDomainXMLOptionPtr xmlopt,
>>> virDomainObjPtr domain)
>>> + if (!domain->persistent)
>>> + return NULL;
>>> if (virDomainObjSetDefTransient(caps, xmlopt, domain, false) < 0)
>>> return NULL;
> libvir-list mailing list
> libvir-list at redhat.com
More information about the libvir-list