[libvirt] [PATCH 05/17] Support hypervisorpin xml parse.

[Eric Blake]

On 08/03/2012 12:36 AM, Hu Tao wrote:
> From: Tang Chen <tangchen at cn.fujitsu.com>
> 
> This patch adds a new xml element <hypervisorpin cpuset='1'>,

I would mention that this is a sibling to the existing <vcpupin> element
under the <cputune>.

> and also the parser functions, docs, and tests.
> hypervisorpin means pinning hypervisor threads, and cpuset = '1'
> means pinning all hypervisor threads to cpu 1.
> 
> Signed-off-by: Tang Chen <tangchen at cn.fujitsu.com>
> Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> ---
>  docs/schemas/domaincommon.rng                   |    7 ++

Missing documentation.  I can't apply this as-is unless we also update
the elementsCPUTuning section of docs/formatdomain.html.in.  That won't
stop me from reviewing the rest of the patch, though.

>  src/conf/domain_conf.c                          |   97 ++++++++++++++++++++++-
>  src/conf/domain_conf.h                          |    1 +
>  tests/qemuxml2argvdata/qemuxml2argv-cputune.xml |    1 +
>  4 files changed, 103 insertions(+), 3 deletions(-)
> 

> +++ b/src/conf/domain_conf.c
> @@ -7828,6 +7828,51 @@ error:
>      goto cleanup;
>  }
>  
> +/* Parse the XML definition for hypervisorpin */
> +static virDomainVcpuPinDefPtr
> +virDomainHypervisorPinDefParseXML(const xmlNodePtr node)
> +{
...
> +}

This is a lot of code duplication.  It might be worth refactoring things
to write a helper function that parses '@cpuset', and which can be
shared by both the existing virDomainVcpuPinDefParseXML and your new
function.

> @@ -9280,7 +9353,7 @@ no_memory:
>      virReportOOMError();
>      /* fallthrough */
>  
> - error:
> +error:
>      VIR_FREE(tmp);
>      VIR_FREE(nodes);
>      virBitmapFree(bootMap);

On its own, this whitespace cleanup has no bearing on the patch; it's
generally wise to limit cleanups to portions already affected by the patch.

But in general, the patch looked reasonable.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120806/17ff95e6/attachment-0001.sig>