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

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),

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