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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxxxx>
>> ---
>>  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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]