On 08/03/2012 12:36 AM, Hu Tao wrote:
> From: Tang Chen <tangchen@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> Signed-off-by: Hu Tao <hutao@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list