[PATCH] conf: Format numatune XML correctly while placement is none

[Osier Yang <jyang@xxxxxxxxxx>]

setNumaParameters tunes the numa setting using cgroup, it's another
entry except libnuma/numad for numa tuning. And it doesn't set the
placement, and further more, the formating codes doesn't take this
into consideration.

How to reproduce:

conn = libvirt.open(None)
dom = conn.lookupByName('linux')
param = {'numa_nodeset': '0', 'numa_mode': 1}
dom.setNumaParameters(param, 2)

% virsh start linux
error: Failed to start domain rhel6.3rc
error: (domain_definition):8: error parsing attribute name
    <memory mode='preferred'   </numatune>
-------------------------------^

---
By the way, I see problems of setNumaParameters too.

conn = libvirt.open(None)
dom = conn.lookupByName('linux')
param = {'numa_mode': 1}
dom.setNumaParameters(param, 2)

The numa 'mode' will be just ignored, and no 'numatune' XML is formated,
as neither 'nodeset' nor 'placement' is specified. I'd think it's
right to ignore it when formating, it's meaningless to only specify
the 'mode'. However, we might have to fix setNumaParameters to prevent
setting the numa mode without nodeset, and error out, as it's really a
bad user experience to see the API call succeeded, but the expected
XML doesn't show up in the end.
---
 src/conf/domain_conf.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 81c6308..c44d89d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12795,23 +12795,26 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         const char *placement;
 
         mode = virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode);
-        virBufferAsprintf(buf, "    <memory mode='%s' ", mode);
+        virBufferAsprintf(buf, "    <memory mode='%s'", mode);
 
-        if (def->numatune.memory.placement_mode ==
-            VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) {
+        if (def->numatune.memory.nodemask) {
             nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask,
-                                         VIR_DOMAIN_CPUMASK_LEN);
+                                             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);
+            virBufferAsprintf(buf, " nodeset='%s'/>\n", nodemask);
             VIR_FREE(nodemask);
-        } else if (def->numatune.memory.placement_mode) {
+        } else if (def->numatune.memory.placement_mode ==
+            VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) {
             placement = virDomainNumatuneMemPlacementModeTypeToString(def->numatune.memory.placement_mode);
-            virBufferAsprintf(buf, "placement='%s'/>\n", placement);
+            virBufferAsprintf(buf, " placement='%s'/>\n", placement);
+        } else {
+            /* Should not hit here. */
+            virBufferAddLit(buf, "/>\n");
         }
         virBufferAddLit(buf, "  </numatune>\n");
     }
-- 
1.7.7.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list