From a05c01521ceef5e1f0715462222dcd1500d382ab Mon Sep 17 00:00:00 2001
From: Martin Kletzander <mkletzan@redhat.com>
Date: Tue, 15 Jul 2014 11:39:44 +0200
Subject: [PATCH] conf, schema: add support for memnode elements

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 docs/formatdomain.html.in                     |  15 ++
 docs/schemas/domaincommon.rng                 |  17 ++
 src/conf/numatune_conf.c                      | 187 ++++++++++++++++--
 ...emuxml2argv-numatune-memnode-no-memory.xml |  30 +++
 .../qemuxml2argv-numatune-memnode-nocpu.xml   |  25 +++
 .../qemuxml2argv-numatune-memnode.xml         |  33 ++++
 ...xml2argv-numatune-memnodes-problematic.xml |  31 +++
 tests/qemuxml2argvtest.c                      |   2 +
 .../qemuxml2xmlout-numatune-memnode.xml       |  33 ++++
 tests/qemuxml2xmltest.c                       |   2 +
 10 files changed, 362 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 eb203670d3..3c85fc591c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
   ...
   &lt;numatune&gt;
     &lt;memory mode="strict" nodeset="1-4,^3"/&gt;
+    &lt;memnode cellid="0" mode="strict" nodeset="1"/&gt;
+    &lt;memnode cellid="2" mode="preferred" nodeset="2"/&gt;
   &lt;/numatune&gt;
   ...
 &lt;/domain&gt;
@@ -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.7</span>
+      </dd>
     </dl>
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1d3b4d8544..a0ea3003a0 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -789,6 +789,23 @@
               </choice>
         </element>
       </optional>
+      <zeroOrMore>
+        <element name="memnode">
+          <attribute name="cellid">
+            <ref name="unsignedInt"/>
+          </attribute>
+          <attribute name="mode">
+            <choice>
+              <value>strict</value>
+              <value>preferred</value>
+              <value>interleave</value>
+            </choice>
+          </attribute>
+          <attribute name='nodeset'>
+            <ref name='cpuset'/>
+          </attribute>
+        </element>
+      </zeroOrMore>
     </element>
   </define>
 
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 6ce1e2d5e3..a39c028b3d 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -42,17 +42,140 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
               "static",
               "auto");
 
+typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
+typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+
 struct _virDomainNumatune {
     struct {
+        bool specified;
         virBitmapPtr nodeset;
         virDomainNumatuneMemMode mode;
         virDomainNumatunePlacement placement;
     } memory;               /* pinning for all the memory */
 
+    struct _virDomainNumatuneNode {
+        virBitmapPtr nodeset;
+        virDomainNumatuneMemMode mode;
+    } *mem_nodes;           /* fine tuning per guest node */
+    size_t nmem_nodes;
+
     /* Future NUMA tuning related stuff should go here. */
 };
 
 
+static int
+virDomainNumatuneNodeParseXML(virDomainDefPtr def,
+                              xmlXPathContextPtr ctxt)
+{
+    char *tmp = NULL;
+    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;
+
+    VIR_FREE(def->numatune->mem_nodes);
+    if (VIR_ALLOC_N(def->numatune->mem_nodes, def->cpu->ncells) < 0)
+        goto cleanup;
+
+    def->numatune->nmem_nodes = def->cpu->ncells;
+
+    for (i = 0; i < n; i++) {
+        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,
+                           _("Invalid cellid attribute in memnode element: %s"),
+                           tmp);
+            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);
+    VIR_FREE(tmp);
+    return ret;
+}
+
 int
 virDomainNumatuneParseXML(virDomainDefPtr def,
                           xmlXPathContextPtr ctxt)
@@ -82,8 +205,11 @@ virDomainNumatuneParseXML(virDomainDefPtr def,
         def->numatune = NULL;
     }
 
-    if (!node && def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
+    if (!node && def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
+        if (virDomainNumatuneNodeParseXML(def, ctxt) < 0)
+            goto cleanup;
         return 0;
+    }
 
     if (!node) {
         /* We know that def->placement_mode is "auto" if we're here */
@@ -125,10 +251,9 @@ virDomainNumatuneParseXML(virDomainDefPtr def,
     if (virDomainNumatuneSet(def, placement, mode, nodeset) < 0)
         goto cleanup;
 
-    if (!n) {
-        ret = 0;
+    if (virDomainNumatuneNodeParseXML(def, ctxt) < 0)
         goto cleanup;
-    }
+
 
     ret = 0;
  cleanup:
@@ -143,6 +268,7 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
 {
     const char *tmp = NULL;
     char *nodeset = NULL;
+    size_t i = 0;
 
     if (!numatune)
         return 0;
@@ -150,17 +276,36 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
     virBufferAddLit(buf, "<numatune>\n");
     virBufferAdjustIndent(buf, 2);
 
-    tmp = virDomainNumatuneMemModeTypeToString(numatune->memory.mode);
-    virBufferAsprintf(buf, "<memory mode='%s' ", tmp);
+    if (numatune->memory.specified) {
+        tmp = virDomainNumatuneMemModeTypeToString(numatune->memory.mode);
+        virBufferAsprintf(buf, "<memory mode='%s' ", tmp);
+
+        if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
+            if (!(nodeset = virBitmapFormat(numatune->memory.nodeset)))
+                return -1;
+            virBufferAsprintf(buf, "nodeset='%s'/>\n", nodeset);
+            VIR_FREE(nodeset);
+        } else if (numatune->memory.placement) {
+            tmp = virDomainNumatunePlacementTypeToString(numatune->memory.placement);
+            virBufferAsprintf(buf, "placement='%s'/>\n", tmp);
+        }
+    }
+
+    for (i = 0; i < numatune->nmem_nodes; i++) {
+        virDomainNumatuneNodePtr mem_node = &numatune->mem_nodes[i];
 
-    if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
-        if (!(nodeset = virBitmapFormat(numatune->memory.nodeset)))
+        if (!mem_node->nodeset)
+            continue;
+
+        if (!(nodeset = virBitmapFormat(mem_node->nodeset)))
             return -1;
-        virBufferAsprintf(buf, "nodeset='%s'/>\n", nodeset);
+
+        virBufferAsprintf(buf,
+                          "<memnode cellid='%zu' mode='%s' nodeset='%s'/>\n",
+                          i,
+                          virDomainNumatuneMemModeTypeToString(mem_node->mode),
+                          nodeset);
         VIR_FREE(nodeset);
-    } else if (numatune->memory.placement) {
-        tmp = virDomainNumatunePlacementTypeToString(numatune->memory.placement);
-        virBufferAsprintf(buf, "placement='%s'/>\n", tmp);
     }
 
     virBufferAdjustIndent(buf, -2);
@@ -171,10 +316,15 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
 void
 virDomainNumatuneFree(virDomainNumatunePtr numatune)
 {
+    size_t i = 0;
+
     if (!numatune)
         return;
 
     virBitmapFree(numatune->memory.nodeset);
+    for (i = 0; i < numatune->nmem_nodes; i++)
+        virBitmapFree(numatune->mem_nodes[i].nodeset);
+    VIR_FREE(numatune->mem_nodes);
 
     VIR_FREE(numatune);
 }
@@ -182,7 +332,7 @@ virDomainNumatuneFree(virDomainNumatunePtr numatune)
 virDomainNumatuneMemMode
 virDomainNumatuneGetMode(virDomainNumatunePtr numatune)
 {
-    return numatune ? numatune->memory.mode : 0;
+    return (numatune && numatune->memory.specified) ? numatune->memory.mode : 0;
 }
 
 virBitmapPtr
@@ -318,6 +468,8 @@ virDomainNumatuneSet(virDomainDefPtr def,
     if (placement != -1)
         numatune->memory.placement = placement;
 
+    numatune->memory.specified = true;
+
     ret = 0;
  cleanup:
     return ret;
@@ -333,6 +485,12 @@ virDomainNumatuneEquals(virDomainNumatunePtr n1,
     if (!n1 || !n2)
         return false;
 
+    if (!n1->memory.specified && !n2->memory.specified)
+        return true;
+
+    if (!n1->memory.specified || !n2->memory.specified)
+        return false;
+
     if (n1->memory.mode != n2->memory.mode)
         return false;
 
@@ -348,6 +506,9 @@ virDomainNumatuneHasPlacementAuto(virDomainNumatunePtr numatune)
     if (!numatune)
         return false;
 
+    if (!numatune->memory.specified)
+        return false;
+
     if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
         return true;
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
new file mode 100644
index 0000000000..4b2efa2105
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+  <name>QEMUGuest</name>
+  <uuid>9f4b6512-e73a-4a25-93e8-5307802821ce</uuid>
+  <memory unit='KiB'>65536</memory>
+  <currentMemory unit='KiB'>65536</currentMemory>
+  <vcpu placement='static'>2</vcpu>
+  <numatune>
+    <memnode cellid='0' mode='preferred' nodeset='3'/>
+  </numatune>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu>
+    <numa>
+      <cell id='0' cpus='0' memory='32768'/>
+      <cell id='1' cpus='1' memory='32768'/>
+    </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/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
new file mode 100644
index 0000000000..7b0e2483a5
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
@@ -0,0 +1,25 @@
+<domain type='kvm'>
+  <name>QEMUGuest</name>
+  <uuid>9f4b6512-e73a-4a25-93e8-5307802821ce</uuid>
+  <memory unit='KiB'>65536</memory>
+  <currentMemory unit='KiB'>65536</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <numatune>
+    <memory mode='strict' nodeset='0,2'/>
+    <memnode cellid='0' mode='interleave' nodeset='3'/>
+  </numatune>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <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/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
new file mode 100644
index 0000000000..49b328cb18
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
@@ -0,0 +1,33 @@
+<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>
+  <numatune>
+    <memory mode='strict' nodeset='0-7'/>
+    <memnode cellid='0' mode='preferred' nodeset='3'/>
+    <memnode cellid='2' mode='strict' nodeset='1-2,5-7,^6'/>
+  </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'/>
+    </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/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
new file mode 100644
index 0000000000..bb4e4af043
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest</name>
+  <uuid>9f4b6512-e73a-4a25-93e8-5307802821ce</uuid>
+  <memory unit='KiB'>65536</memory>
+  <currentMemory unit='KiB'>65536</currentMemory>
+  <vcpu placement='auto'>2</vcpu>
+  <numatune>
+    <memory placement='auto'/>
+    <memnode cellid='0' mode='strict' nodeset='3'/>
+  </numatune>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu>
+    <numa>
+      <cell id='0' cpus='0' memory='32768'/>
+      <cell id='1' cpus='1' memory='32768'/>
+    </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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8eb982f2c6..3a76aa6bee 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1197,6 +1197,8 @@ mymain(void)
     DO_TEST("cputune-zero-shares", QEMU_CAPS_NAME);
     DO_TEST("numatune-memory", NONE);
     DO_TEST("numatune-auto-nodeset-invalid", NONE);
+    DO_TEST_PARSE_ERROR("numatune-memnode-nocpu", NONE);
+    DO_TEST_PARSE_ERROR("numatune-memnodes-problematic", NONE);
     DO_TEST("numad", NONE);
     DO_TEST("numad-auto-vcpu-static-numatune", NONE);
     DO_TEST("numad-auto-memory-vcpu-cpuset", NONE);
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
new file mode 100644
index 0000000000..82b5f61870
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
@@ -0,0 +1,33 @@
+<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>
+  <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'/>
+    </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 8b1b11ad26..cbeabad307 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -374,6 +374,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);
-- 
GitLab