[libvirt] [PATCH 3/3] common-impl: make virDomainObjGetPersistentDef return only persistent config

[Nikolay Shirokovskiy]

ping

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.
>>
>> John
>>> 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
>>>   */
>>>  virDomainDefPtr
>>>  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
> https://www.redhat.com/mailman/listinfo/libvir-list
>