[libvirt] [PATCH v3 11/16] Add XML config for resource partitions

[Michal Privoznik]

On 10.04.2013 12:08, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  docs/formatdomain.html.in                    | 26 ++++++++++
>  docs/schemas/domaincommon.rng                | 12 +++++
>  src/conf/domain_conf.c                       | 78 ++++++++++++++++++++++++++++
>  src/conf/domain_conf.h                       |  7 +++
>  tests/domainschemadata/domain-lxc-simple.xml |  3 ++
>  5 files changed, 126 insertions(+)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d400e35..5551187 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -716,6 +716,32 @@
>      </dl>
>  
>  
> +    <h3><a name="resPartition">Resource partitioning</a></h3>
> +
> +    <p>
> +      Hypervisors may allow for virtual machines to be placed into
> +      resource partitions, potentially with nesting of said partitions.
> +      The <code>resource</code> element groups together configuration
> +      related to resource partitioning. It currently supports a child
> +      element <code>partition</code> whose content defines the path
> +      of the resource partition in which to place the domain. If no
> +      partition is listed, then the domain will be placed in a default
> +      partition.
> +    </p>

We should mention here the fact you are stating in the next patch:
The partition path has to exists and it's admin responsibility to
pre-create it.

> +<pre>
> +  ...
> +  <resource>
> +    <partition>/virtualmachines/production</partition>
> +  </resource>
> +  ...
> +</pre>
> +
> +    <p>
> +      Resource partitions are currently supported by the QEMU and
> +      LXC drivers, which map partition paths onto cgroups directories,
> +      in all mounted controllers. <span class="since">Since 1.0.5</pan>
> +    </p>
> +
>      <h3><a name="elementsCPU">CPU model and topology</a></h3>
>  
>      <p>

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e00a532..ae1dfd3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10255,6 +10300,25 @@ virDomainDefParseXML(xmlDocPtr xml,
>      }
>      VIR_FREE(nodes);
>  
> +    /* Extract numatune if exists. */
> +    if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("cannot extract resource nodes"));
> +        goto error;
> +    }
> +
> +    if (n > 1) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("only one resource element is supported"));
> +        VIR_FREE(nodes);
> +        goto error;
> +    }
> +
> +    if (n &&
> +        !(def->resource = virDomainResourceDefParse(nodes[0], ctxt)))
> +        goto error;
> +    VIR_FREE(nodes);
> +

Even though there is no real leak here, it seems a bit odd to
VIR_FREE(nodes) in the 2nd 'if' statement, but not this in one. For
consistency we should drop the fly-away VIR_FREE().

>      if ((n = virXPathNodeSet("./features/*", ctxt, &nodes)) < 0)
>          goto error;
>  
> @@ -14870,6 +14934,17 @@ virDomainIsAllVcpupinInherited(virDomainDefPtr def)
>     }
>  }
>  

ACK

Michal