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