Re: [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08.07.2014 13:50, Martin Kletzander wrote:
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways.  On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).

In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.

Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
---
  po/POTFILES.in                                     |   1 +
  src/conf/domain_conf.c                             | 159 ++---------
  src/conf/domain_conf.h                             |   8 +-
  src/conf/numatune_conf.c                           | 316 +++++++++++++++++++++
  src/conf/numatune_conf.h                           |  72 ++++-
  src/libvirt_private.syms                           |  11 +
  src/lxc/lxc_cgroup.c                               |  19 +-
  src/lxc/lxc_controller.c                           |   5 +-
  src/lxc/lxc_native.c                               |  15 +-
  src/parallels/parallels_driver.c                   |   7 +-
  src/qemu/qemu_cgroup.c                             |  23 +-
  src/qemu/qemu_driver.c                             |  84 +++---
  src/qemu/qemu_process.c                            |   8 +-
  src/util/virnuma.c                                 |  48 ++--
  src/util/virnuma.h                                 |   2 +-
  .../qemuxml2argv-numatune-auto-prefer.xml          |  29 ++
  .../qemuxml2xmlout-numatune-auto-prefer.xml        |  29 ++
  tests/qemuxml2xmltest.c                            |   2 +
  18 files changed, 553 insertions(+), 285 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml

Nice.

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 14300f0..8b66558 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c

+int
+virDomainNumatuneParseXML(virDomainDefPtr def,
+                          xmlXPathContextPtr ctxt)
+{
+    char *tmp = NULL;
+    int mode = -1;
+    int n = 0;
+    int placement = -1;
+    int ret = -1;
+    virBitmapPtr nodeset = NULL;
+    xmlNodePtr node = NULL;
+
+    if (virXPathInt("count(./numatune)", ctxt, &n) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("cannot extract numatune nodes"));
+        goto cleanup;
+    } else if (n > 1) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("only one numatune is supported"));
+        goto cleanup;
+    }
+
+    node = virXPathNode("./numatune/memory[1]", ctxt);
+
+    if (def->numatune) {
+        virDomainNumatuneFree(def->numatune);
+        def->numatune = NULL;
+    }
+
+    if (!node && def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
+        return 0;
+
+    if (!node) {
+        /* We know that def->placement_mode is "auto" if we're here */
+        if (virDomainNumatuneSet(def, VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO,
+                                 -1, NULL) < 0)
+            goto cleanup;
+        return 0;
+    }
+
+    tmp = virXMLPropString(node, "mode");
+    if (tmp) {
+        mode = virDomainNumatuneMemModeTypeFromString(tmp);
+        if (mode < 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Unsupported NUMA memory tuning mode '%s'"),
+                           tmp);
+            goto cleanup;
+        }
+    }
+    VIR_FREE(tmp);
+
+    tmp = virXMLPropString(node, "placement");
+    if (tmp) {
+        placement = virDomainNumatunePlacementTypeFromString(tmp);
+        if (placement < 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Unsupported NUMA memory placement mode '%s'"),
+                           tmp);
+            goto cleanup;
+        }
+    }
+    VIR_FREE(tmp);
+
+    tmp = virXMLPropString(node, "nodeset");
+    if (tmp && virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
+        goto cleanup;
+    VIR_FREE(tmp);
+
+    if (virDomainNumatuneSet(def, placement, mode, nodeset) < 0)

The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call virBitmaskFree(nodeset); at the cleanup label.

+        goto cleanup;
+
+    if (!n) {
+        ret = 0;
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(tmp);
+    return ret;
+}
+
+int
+virDomainNumatuneFormatXML(virBufferPtr buf,
+                           virDomainNumatunePtr numatune)
+{
+    const char *tmp = NULL;

s /const// ..

+
+    if (!numatune)
+        return 0;
+
+    virBufferAddLit(buf, "<numatune>\n");
+    virBufferAdjustIndent(buf, 2);
+
+    tmp = virDomainNumatuneMemModeTypeToString(numatune->memory.mode);
+    virBufferAsprintf(buf, "<memory mode='%s' ", tmp);
+
+    if (numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
+        if (!(tmp = virBitmapFormat(numatune->memory.nodeset)))
+            return -1;
+        virBufferAsprintf(buf, "nodeset='%s'/>\n", tmp);
+        VIR_FREE(tmp);

.. because free()-ing a const char * is not nice. If you, however, do this I bet you'll get error in TypeToString(). So just leave tmp as const char * and introduce char *nodeset;

+    } else if (numatune->memory.placement) {
+        tmp = virDomainNumatunePlacementTypeToString(numatune->memory.placement);
+        virBufferAsprintf(buf, "placement='%s'/>\n", tmp);
+    }
+
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</numatune>\n");
+    return 0;
+}
+

Michal

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]