On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Tang Chen <tangchen@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@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org