[libvirt] [PATCH 3/7] conf: Initialize the pinning policy for vcpus

[Martin Kletzander]

On 10/12/2012 11:50 AM, Osier Yang wrote:
> Document for <vcpu>'s "cpuset" says:
> 
> Since 0.4.4, this element can contain an optional cpuset attribute,
> which is a comma-separated list of physical CPU numbers that virtual
> CPUs can be pinned to.
> 
> However, it's not the truth, libvirt actually pins the domain
> process to the specified pCPUs by "cpuset" of <vcpu>. And the
> vcpu thread are pinned to all available pCPUs if no <vcpupin>
> is specified for it.
> 
> PATCH 1/7 provides more details on how this set resolve the
> confused relationship between <vcpu> and <vcpupin>. This patch
> is to implement the codes to inherit <vcpu>'s "cpuset" for
> vcpu that doesn't have <vcpupin> specified, and <vcpupin>
> for these vcpu will be ignored when formating. Underlying
> driver implementation will make sure the vcpu thread pinned
> to correct pCPUs.
> ---
>  src/conf/domain_conf.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 70 insertions(+), 1 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d8ea8ce..e404a68 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8810,6 +8810,41 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>      }
>      VIR_FREE(nodes);
>  
> +    /* Initialize the pinning policy for vcpus which doesn't has
> +     * the policy specified explicitly as def->cpuset.
> +     */
> +    if (def->cpumask) {
> +        if (!def->cputune.vcpupin) {
> +            if (VIR_ALLOC_N(def->cputune.vcpupin, def->vcpus) < 0) {
> +                virReportOOMError();
> +                goto error;
> +            }
> +        } else {
> +            if (VIR_REALLOC_N(def->cputune.vcpupin, def->vcpus) < 0) {
> +                virReportOOMError();
> +                goto error;
> +            }
> +        }

You can always use VIR_REALLOC_N, as I mentioned with v1, no need for
the condition.

> +
> +        for (i = 0; i < def->vcpus; i++) {
> +            if (!virDomainVcpuPinIsDuplicate(def->cputune.vcpupin,
> +                                             def->cputune.nvcpupin,
> +                                             i)) {
> +                virDomainVcpuPinDefPtr vcpupin = NULL;
> +
> +                if (VIR_ALLOC(vcpupin) < 0) {
> +                    virReportOOMError();
> +                    goto error;
> +                }
> +
> +                vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN);
> +                virBitmapCopy(vcpupin->cpumask, def->cpumask);
> +                vcpupin->vcpuid = i;
> +                def->cputune.vcpupin[def->cputune.nvcpupin++] = vcpupin;
> +            }
> +        }
> +    }
> +
>      if ((n = virXPathNodeSet("./cputune/emulatorpin", ctxt, &nodes)) < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("cannot extract emulatorpin nodes"));
[...]

Not exactly what I had in mind, but it fixes all the things I was
worried about, so ACK (with the REALLOC fixed).

Martin