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

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

 



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.

And as long as we try to use nvcpupin here, we should use nvcpupin
when formating "</cputune>" too.

Osier



--
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]