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