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@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