On 08.07.2014 13:50, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in | 15 ++
> docs/schemas/domaincommon.rng | 17 ++
> src/conf/numatune_conf.c | 183 +++++++++++++++++++--
> .../qemuxml2argv-numatune-memnode-no-memory.xml | 30 ++++
> .../qemuxml2argv-numatune-memnode-nocpu.xml | 25 +++
> .../qemuxml2argv-numatune-memnode.xml | 31 ++++
> .../qemuxml2argv-numatune-memnodes-problematic.xml | 31 ++++
> tests/qemuxml2argvtest.c | 2 +
> .../qemuxml2xmlout-numatune-memnode.xml | 33 ++++
> tests/qemuxml2xmltest.c | 2 +
> 10 files changed, 356 insertions(+), 13 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index ad87b7c..d845c1b 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -709,6 +709,8 @@
> ...
> <numatune>
> <memory mode="strict" nodeset="1-4,^3"/>
> + <memnode cellid="0" mode="strict" nodeset="1"/>
> + <memnode cellid="2" mode="preferred" nodeset="2"/>
> </numatune>
> ...
> </domain>
> @@ -745,6 +747,19 @@
>
> <span class='since'>Since 0.9.3</span>
> </dd>
> + <dt><code>memnode</code></dt>
> + <dd>
> + Optional <code>memnode</code> elements can specify memory allocation
> + policies per each guest NUMA node. For those nodes having no
> + corresponding <code>memnode</code> element, the default from
> + element <code>memory</code> will be used. Attribute <code>cellid</code>
> + addresses guest NUMA node for which the settings are applied.
> + Attributes <code>mode</code> and <code>nodeset</code> have the same
> + meaning and syntax as in <code>memory</code> element.
> +
> + This setting is not compatible with automatic placement.
> + <span class='since'>QEMU Since 1.2.6</span>
1.2.8 actually
> + </dd>
> </dl>
>
>
>
> +static int
> +virDomainNumatuneNodeParseXML(virDomainDefPtr def,
> + xmlXPathContextPtr ctxt)
> +{
> + int n = 0;;
> + int ret = -1;
> + size_t i = 0;
> + xmlNodePtr *nodes = NULL;
> +
> + if ((n = virXPathNodeSet("./numatune/memnode", ctxt, &nodes)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Cannot extract memnode nodes"));
> + goto cleanup;
> + }
> +
> + if (!n)
> + return 0;
> +
> + if (def->numatune && def->numatune->memory.specified &&
> + def->numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Per-node binding is not compatible with "
> + "automatic NUMA placement."));
> + goto cleanup;
> + }
> +
> + if (!def->cpu || !def->cpu->ncells) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Element 'memnode' is invalid without "
> + "any guest NUMA cells"));
> + goto cleanup;
> + }
> +
> + if (!def->numatune && VIR_ALLOC(def->numatune) < 0)
> + goto cleanup;
Here you allow def->numatune to be allocated already.
> +
> + if (VIR_ALLOC_N(def->numatune->mem_nodes, def->cpu->ncells) < 0)
> + goto cleanup;
Which means, this can exists too. VIR_REALLOC_N() is safer IMO.
> +
> + def->numatune->nmem_nodes = def->cpu->ncells;
> +
> + for (i = 0; i < n; i++) {
> + const char *tmp = NULL;
No. s/const//.
> + int mode = 0;
> + unsigned int cellid = 0;
> + virDomainNumatuneNodePtr mem_node = NULL;
> + xmlNodePtr cur_node = nodes[i];
> +
> + tmp = virXMLPropString(cur_node, "cellid");
> + if (!tmp) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing required cellid attribute "
> + "in memnode element"));
> + goto cleanup;
> + }
> + if (virStrToLong_uip(tmp, NULL, 10, &cellid) < 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Invalid cellid attribute in memnode element"));
Moreover, @tmp is leaked here. And it would be nice to tell users in the error message @tmp somehow.
> + goto cleanup;
> + }
> + VIR_FREE(tmp);
> +
> + if (cellid >= def->numatune->nmem_nodes) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Argument 'cellid' in memnode element must "
> + "correspond to existing guest's NUMA cell"));
> + goto cleanup;
> + }
> +
> + mem_node = &def->numatune->mem_nodes[cellid];
> +
> + if (mem_node->nodeset) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Multiple memnode elements with cellid %u"),
> + cellid);
> + goto cleanup;
> + }
> +
> + tmp = virXMLPropString(cur_node, "mode");
> + if (!tmp) {
> + mem_node->mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
> + } else {
> + if ((mode = virDomainNumatuneMemModeTypeFromString(tmp)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Invalid mode attribute in memnode element"));
> + goto cleanup;
> + }
> + VIR_FREE(tmp);
> + mem_node->mode = mode;
> + }
> +
> + tmp = virXMLPropString(cur_node, "nodeset");
> + if (!tmp) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing required nodeset attribute "
> + "in memnode element"));
> + goto cleanup;
> + }
> + if (virBitmapParse(tmp, 0, &mem_node->nodeset,
> + VIR_DOMAIN_CPUMASK_LEN) < 0)
> + goto cleanup;
> + VIR_FREE(tmp);
> + }
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(nodes);
> + return ret;
> +}
> +
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
> new file mode 100644
> index 0000000..82b5f61
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
Are you sure this is the correct content?
> @@ -0,0 +1,33 @@
> +<domain type='qemu'>
> + <name>QEMUGuest</name>
> + <uuid>9f4b6512-e73a-4a25-93e8-5307802821ce</uuid>
> + <memory unit='KiB'>24682468</memory>
In the qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml you have 65MB of RAM while here ~23.5GB.
> + <currentMemory unit='KiB'>24682468</currentMemory>
> + <vcpu placement='static'>32</vcpu>
and only 2 vcpus, while here is 32.
> + <numatune>
> + <memory mode='strict' nodeset='0-7'/>
> + <memnode cellid='0' mode='preferred' nodeset='3'/>
> + <memnode cellid='2' mode='strict' nodeset='1-2,5,7'/>
> + </numatune>
> + <os>
> + <type arch='x86_64' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <cpu>
> + <numa>
> + <cell id='0' cpus='0' memory='20002'/>
> + <cell id='1' cpus='1-27,29' memory='660066'/>
> + <cell id='2' cpus='28-31,^29' memory='24002400'/>
And this is broken too.
> + </numa>
> + </cpu>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/kvm</emulator>
> + <controller type='usb' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <memballoon model='virtio'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 3bba565..4beb799 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -372,6 +372,8 @@ mymain(void)
> DO_TEST_DIFFERENT("cpu-numa2");
>
> DO_TEST_DIFFERENT("numatune-auto-prefer");
> + DO_TEST_DIFFERENT("numatune-memnode");
> + DO_TEST("numatune-memnode-no-memory");
>
> virObjectUnref(driver.caps);
> virObjectUnref(driver.xmlopt);
>
I can't ACK this one, until the issue is resolved. If I squash this in, the test passes again:
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
index 82b5f61..18b00d8 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
@@ -1,13 +1,12 @@
<domain type='qemu'>
<name>QEMUGuest</name>
<uuid>9f4b6512-e73a-4a25-93e8-5307802821ce</uuid>
- <memory unit='KiB'>24682468</memory>
- <currentMemory unit='KiB'>24682468</currentMemory>
- <vcpu placement='static'>32</vcpu>
+ <memory unit='KiB'>65536</memory>
+ <currentMemory unit='KiB'>65536</currentMemory>
+ <vcpu placement='static'>2</vcpu>
<numatune>
- <memory mode='strict' nodeset='0-7'/>
+ <memory mode='strict' nodeset='0-3'/>
<memnode cellid='0' mode='preferred' nodeset='3'/>
- <memnode cellid='2' mode='strict' nodeset='1-2,5,7'/>
</numatune>
<os>
<type arch='x86_64' machine='pc'>hvm</type>
@@ -15,9 +14,8 @@
</os>
<cpu>
<numa>
- <cell id='0' cpus='0' memory='20002'/>
- <cell id='1' cpus='1-27,29' memory='660066'/>
- <cell id='2' cpus='28-31,^29' memory='24002400'/>
+ <cell id='0' cpus='0' memory='32768'/>
+ <cell id='1' cpus='1' memory='32768'/>
</numa>
</cpu>
<clock offset='utc'/>
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list