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

Osier Yang jyang at redhat.com
Tue Dec 18 03:01:19 UTC 2012


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






More information about the libvir-list mailing list