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

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



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






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