[libvirt] [PATCH 3/5] conf: Don't format cputune element when not needed

[Osier Yang]

On 2012年12月18日 19:20, Martin Kletzander wrote:
> On 12/18/2012 04:01 AM, Osier Yang wrote:
>> On 2012年12月17日 23:17, Martin Kletzander wrote:
>>> Commit 60b176c3d0f0d5037acfa5e27c7753f657833a0b introduced a bug that
>>> when editing an XML with cputune similar to this:
>>
>> Thanks for fixing this.
>>
>>>
>>> ...
>>>     <vcpu placement='static' current='1'>2</vcpu>
>>>     <cputune>
>>>       <vcpupin vcpu="1" cpuset="0"/>
>>>     </cputune>
>>> ...
>>>
>>> results in formatted XML that looks like this:
>>>
>>> ...
>>>     <vcpu placement='static' current='1'>2</vcpu>
>>>     <cputune>
>>>     </cputune>
>>> ...
>>>
>>> That is caused by a condition depending on def->cputune.vcpupin being
>>> set rather than checking def->cputune.nvcpupin.  Notice that nvcpupin
>>> can be 0 and vcpupin can still be allocated since it's a pointer to an
>>> array, so no harm done there.
>>> ---
>>>    src/conf/domain_conf.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index cba910a..329ada3 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -13896,7 +13896,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>>        virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus);
>>>
>>>        if (def->cputune.shares ||
>>> -        (def->cputune.vcpupin&&
>>> !virDomainIsAllVcpupinInherited(def)) ||
>>> +        (def->cputune.nvcpupin&&
>>> !virDomainIsAllVcpupinInherited(def)) ||
>>
>> This is a good fix, but I don't think it fixes the problem what commit
>> message describes. As both def->cputune.vcpupin and
>> def->cputune.nvcpupin should be true here for the testing case you
>> wrote in the commit message.
>>
>
> I checked this fixes it (the vcpupin is allocated, but nvcpupin is 0,
> which means no values are in vcpupin),

It sounds like a bug, as nvcpupin should not be 0, per there is one
specified in the domain config.

> but I'll check the other places
> where we format it as well.
>
>> And as long as we try to use nvcpupin here, we should use nvcpupin
>> when formating "</cputune>" too.
>>
>> Osier
>>
>>
>>
>