On 05/08/2012 10:04 AM, Osier Yang wrote:
Though numad will manage the memory allocation of task dynamically, but it wants management application (libvirt) to pre-set the memory policy according to the advisory nodeset returned from querying numad, (just like pre-bind CPU nodeset for domain process), and thus the performance could benifit much more from it.
s/benifit/benefit/
This patch introduces new XML tag 'placement', value 'auto' indicates whether to set the memory policy with the advisory nodeset from numad, and its value defaults to the value of <vcpu> placement, or 'static' if 'nodeset' is specified. Example of the new XML tag's usage:
<numatune> <memory placement='auto' mode='interleave'/> </numatune>
Just like what current "numatune" does, the 'auto' numa memory policy setting uses libnuma's API too.
If <vcpu> "placement" is "auto", and <numatune> is not specified explicitly, a default <numatume> will be added with "placement" set as "auto", and "mode" set as "strict".
The following XML can now fully drive numad:
1) <vcpu> placement is 'auto', no <numatune> is specified.
<vcpu placement='auto'>10</vcpu>
2) <vcpu> placement is 'auto', no 'placement' is specified for <numatune>.
<vcpu placement='auto'>10</vcpu> <numatune> <memory mode='interleave'/> </numatune>
And it's aslo able to control the CPU placement and memory policy
s/aslo/also/
independantly. e.g.
s/independantly/independently/
1) <vcpu> placement is 'auto', and <numatune> placement is 'static'
<vcpu placement='auto'>10</vcpu> <numatune> <memory mode='strict' nodeset='0-10,^7'/> </numatune>
2) <vcpu> placement is 'static', and <numatune> placement is 'auto'
<vcpu placement='static' cpuset='0-24,^12'>10</vcpu> <numatune> <memory mode='interleave' placement='auto'/> </numatume>
A follow up patch will change the XML formating codes to always output
s/formating/formatting/
'placement' for <vcpu>, even it's 'static'.
v1 ~ v2: * Changes on <numatune> parsing so that the <numatune> "placement" could default to <vcpu> "placement". * Add member 'default' for enum virDomainNumatuneMemPlacementMode. * Output "placement" for <numatune> even it's static. * Update docs/formatdomain.html.in * Correct the changes on docs/schemas/domaincommon.rng * New tests for the combinations of <vcpu> and <numatune>. * Change on spec file is splitted off.
Patch versioning details can go after the ---, so that it isn't recorded into the actual git commit log (it is useful for review, but not worth storing permanently).
--- docs/formatdomain.html.in | 23 +++- docs/schemas/domaincommon.rng | 36 ++++-- src/conf/domain_conf.c | 128 +++++++++++++++----- src/conf/domain_conf.h | 10 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_process.c | 85 ++++++++------ .../qemuxml2argv-numad-auto-vcpu-no-numatune.xml | 29 +++++ ...muxml2argv-numad-auto-vcpu-static-numatune.args | 4 + ...emuxml2argv-numad-auto-vcpu-static-numatune.xml | 31 +++++ .../qemuxml2argv-numad-static-vcpu-no-numatune.xml | 29 +++++ tests/qemuxml2argvdata/qemuxml2argv-numad.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-numad.xml | 31 +++++ tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-numad-auto-vcpu-no-numatune.xml | 32 +++++ ...emuxml2xmlout-numad-static-vcpu-no-numatune.xml | 29 +++++ tests/qemuxml2xmltest.c | 2 + 16 files changed, 394 insertions(+), 83 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-no-numatune.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad-static-vcpu-no-numatune.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numad-auto-vcpu-no-numatune.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numad-static-vcpu-no-numatune.xml
Git (and for that matter, patch) did not like your patch; you had a hunk that was misnumbered [*]. I had a bear of a time figuring out how to get this to apply.
+++ b/src/conf/domain_conf.c ...
@@ -12491,25 +12545,33 @@ virDomainDefFormatInternal(virDomainDefPtr def,
[*] here's where the patch was corrupt - it claims 33 lines post-patch, but you only provide 32 lines...
def->cputune.period || def->cputune.quota) virBufferAddLit(buf, " </cputune>\n");
- if (def->numatune.memory.nodemask) { + if (def->numatune.memory.nodemask || + def->numatune.memory.placement_mode) { virBufferAddLit(buf, " <numatune>\n"); const char *mode; char *nodemask = NULL; - - nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); - if (nodemask == NULL) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to format nodeset for " - "NUMA memory tuning")); - goto cleanup; - } + const char *placement;
mode = virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode); - virBufferAsprintf(buf, " <memory mode='%s' nodeset='%s'/>\n", - mode, nodemask); - VIR_FREE(nodemask); + virBufferAsprintf(buf, " <memory mode='%s' ", mode);
+ if (def->numatune.memory.placement_mode == + VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) { + nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + if (nodemask == NULL) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to format nodeset for " + "NUMA memory tuning")); + goto cleanup; + } + virBufferAsprintf(buf, "nodeset='%s'/>\n", nodemask); + VIR_FREE(nodemask); + } else if (def->numatune.memory.placement_mode) { + placement = virDomainNumatuneMemPlacementModeTypeToString(def->numatune.memory.placement_mode); + virBufferAsprintf(buf, "placement='%s'/>\n", placement); + } virBufferAddLit(buf, " </numatune>\n"); }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
which mean that the patch tools tried to treat this line as part of the previous hunk instead of the start of a new file to patch. But everything else looked okay. ACK; I'll finish reviewing the rest of the series, and then probably push. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org