virDomainNumatuneGetMode: Report if numatune was defined

So far, we are not reporting if numatune was even defined. The
value of zero is blindly returned (which maps onto
VIR_DOMAIN_NUMATUNE_MEM_STRICT). Unfortunately, we are making
decisions based on this value. Instead, we should not only return
the correct value, but report to the caller if the value is valid
at all.

For better viewing of this patch use '-w'.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index e4dc2f88edbab24e2eb5f459cfffd9508106234f..57da215aed0a8f7562d1c3f858d9a937dea5d9ba 100644 (file)
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -351,20 +351,40 @@ virDomainNumaFree(virDomainNumaPtr numa)
     VIR_FREE(numa);
 }
 
-virDomainNumatuneMemMode
-virDomainNumatuneGetMode(virDomainNumaPtr numatune,
-                         int cellid)
+/**
+ * virDomainNumatuneGetMode:
+ * @numatune: pointer to numatune definition
+ * @cellid: cell selector
+ * @mode: where to store the result
+ *
+ * Get the defined mode for domain's memory. It's safe to pass
+ * NULL to @mode if the return value is the only info needed.
+ *
+ * Returns: 0 on success (with @mode updated)
+ *         -1 if no mode was defined in XML
+ */
+int virDomainNumatuneGetMode(virDomainNumaPtr numatune,
+                             int cellid,
+                             virDomainNumatuneMemMode *mode)
 {
+    int ret = -1;
+    virDomainNumatuneMemMode tmp_mode;
+
     if (!numatune)
-        return 0;
+        return ret;
 
     if (virDomainNumatuneNodeSpecified(numatune, cellid))
-        return numatune->mem_nodes[cellid].mode;
-
-    if (numatune->memory.specified)
-        return numatune->memory.mode;
+        tmp_mode = numatune->mem_nodes[cellid].mode;
+    else if (numatune->memory.specified)
+        tmp_mode = numatune->memory.mode;
+    else
+        goto cleanup;
 
-    return 0;
+    if (mode)
+        *mode = tmp_mode;
+    ret = 0;
+ cleanup:
+    return ret;
 }
 
 virBitmapPtr

diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index ded6e019ec2586b06923e6ae4b466d6109be028e..67390653393a7951acc7c18961ed9b96b3a215bf 100644 (file)
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -72,8 +72,9 @@ int virDomainNumatuneFormatXML(virBufferPtr buf, virDomainNumaPtr numatune)
 /*
  * Getters
  */
-virDomainNumatuneMemMode virDomainNumatuneGetMode(virDomainNumaPtr numatune,
-                                                  int cellid);
+int virDomainNumatuneGetMode(virDomainNumaPtr numatune,
+                             int cellid,
+                             virDomainNumatuneMemMode *mode);
 
 virBitmapPtr virDomainNumatuneGetNodeset(virDomainNumaPtr numatune,
                                          virBitmapPtr auto_nodeset,

diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 5e959a2ea98c15edd64a1e0681fb8fcdf8afa740..507d56759f6e6eecc57c567dd5a81041bb9fa888 100644 (file)
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -69,6 +69,7 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
 {
     int ret = -1;
     char *mask = NULL;
+    virDomainNumatuneMemMode mode;
 
     if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
         def->cpumask) {
@@ -81,8 +82,8 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
         VIR_FREE(mask);
     }
 
-    if (virDomainNumatuneGetMode(def->numa, -1) !=
-        VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+    if (virDomainNumatuneGetMode(def->numa, -1, &mode) < 0 ||
+        mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
         ret = 0;
         goto cleanup;
     }

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index ba44e098d3a02111204c05c1d4b17388ae72e272..efbe71feac1d1cffdc02e88b6eb8c6cf7b49ca48 100644 (file)
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -741,25 +741,25 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
     virBitmapPtr nodeset = NULL;
     virDomainNumatuneMemMode mode;
 
-    mode = virDomainNumatuneGetMode(ctrl->def->numa, -1);
-
-    if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
-        virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
-        /* Use virNuma* API iff necessary. Once set and child is exec()-ed,
-         * there's no way for us to change it. Rely on cgroups (if available
-         * and enabled in the config) rather than virNuma*. */
-        VIR_DEBUG("Relying on CGroups for memory binding");
-    } else {
+    if (virDomainNumatuneGetMode(ctrl->def->numa, -1, &mode) == 0) {
+        if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
+            virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
+            /* Use virNuma* API iff necessary. Once set and child is exec()-ed,
+             * there's no way for us to change it. Rely on cgroups (if available
+             * and enabled in the config) rather than virNuma*. */
+            VIR_DEBUG("Relying on CGroups for memory binding");
+        } else {
 
-        VIR_DEBUG("Setting up process resource limits");
+            VIR_DEBUG("Setting up process resource limits");
 
-        if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0)
-            goto cleanup;
+            if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0)
+                goto cleanup;
 
-        nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1);
+            nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1);
 
-        if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
-            goto cleanup;
+            if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
+                goto cleanup;
+        }
     }
 
     if (virLXCControllerSetupCpuAffinity(ctrl) < 0)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 786e0adace5fe56f038717ccc471654b8e16e1a0..2a4450413e024e488008f6bf426ad8a713925307 100644 (file)
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1845,6 +1845,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def)
     size_t i;
     PRL_VM_TYPE vmType;
     PRL_RESULT pret;
+    virDomainNumatuneMemMode memMode;
 
     if (def->title) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -1924,8 +1925,8 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def)
      * virDomainDefPtr always contain non zero NUMA configuration
      * So, just make sure this configuration does't differ from auto generated.
      */
-    if ((virDomainNumatuneGetMode(def->numa, -1) !=
-         VIR_DOMAIN_NUMATUNE_MEM_STRICT) ||
+    if ((virDomainNumatuneGetMode(def->numa, -1, &memMode) == 0 &&
+         memMode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) ||
          virDomainNumatuneHasPerNodeBinding(def->numa)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                         _("numa parameters are not supported "

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e24989b33bff8a6619cc6d831e267a82995dc35e..96677dd0403ecaca7ef00a7177b2fdef8a7a4cd1 100644 (file)
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -613,14 +613,15 @@ qemuSetupCpusetMems(virDomainObjPtr vm)
 {
     virCgroupPtr cgroup_temp = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    virDomainNumatuneMemMode mode;
     char *mem_mask = NULL;
     int ret = -1;
 
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
         return 0;
 
-    if (virDomainNumatuneGetMode(vm->def->numa, -1) !=
-        VIR_DOMAIN_NUMATUNE_MEM_STRICT)
+    if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) < 0 ||
+        mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT)
         return 0;
 
     if (virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
@@ -979,6 +980,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
     unsigned long long period = vm->def->cputune.period;
     long long quota = vm->def->cputune.quota;
     char *mem_mask = NULL;
+    virDomainNumatuneMemMode mem_mode;
 
     if ((period || quota) &&
         !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
@@ -1009,8 +1011,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
         return 0;
     }
 
-    if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
-        VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
+    if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
+        mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
         virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
                                             priv->autoNodeset,
                                             &mem_mask, -1) < 0)
@@ -1153,6 +1155,7 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
     unsigned long long period = vm->def->cputune.period;
     long long quota = vm->def->cputune.quota;
     char *mem_mask = NULL;
+    virDomainNumatuneMemMode mem_mode;
 
     if ((period || quota) &&
         !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
@@ -1176,8 +1179,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
     if (priv->cgroup == NULL)
         return 0;
 
-    if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
-        VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
+    if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
+        mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
         virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
                                             priv->autoNodeset,
                                             &mem_mask, -1) < 0)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3ccd35d5704d174edc1958f069c18e4e3ae4c816..30597087d8b71e16929ed0da9d5a13308d567afb 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4707,7 +4707,9 @@ qemuBuildMemoryBackendStr(unsigned long long size,
         return -1;
 
     memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
-    mode = virDomainNumatuneGetMode(def->numa, guestNode);
+    if (virDomainNumatuneGetMode(def->numa, guestNode, &mode) < 0 &&
+        virDomainNumatuneGetMode(def->numa, -1, &mode) < 0)
+        mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
 
     if (pagesize == 0 || pagesize != system_page_size) {
         /* Find the huge page size we want to use */

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a2f8e24f771ccd1183e8a7f2bff7497538a826ea..31cc2eea8b1cbe92a702cb3c598c5782a04c6964 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4737,6 +4737,7 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
     int ncpupids;
     virCgroupPtr cgroup_vcpu = NULL;
     char *mem_mask = NULL;
+    virDomainNumatuneMemMode mem_mode;
 
     qemuDomainObjEnterMonitor(driver, vm);
 
@@ -4804,8 +4805,8 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
         goto cleanup;
     }
 
-    if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
-        VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
+    if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
+        mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
         virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
                                             priv->autoNodeset,
                                             &mem_mask, -1) < 0)
@@ -6113,6 +6114,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
     qemuMonitorIOThreadInfoPtr *new_iothreads = NULL;
     virCgroupPtr cgroup_iothread = NULL;
     char *mem_mask = NULL;
+    virDomainNumatuneMemMode mode;
     virDomainIOThreadIDDefPtr iothrid;
     virBitmapPtr cpumask;
 
@@ -6154,8 +6156,8 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
     }
     vm->def->iothreads = exp_niothreads;
 
-    if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
-        VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
+    if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) == 0 &&
+        mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
         virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
                                             priv->autoNodeset,
                                             &mem_mask, -1) < 0)
@@ -10331,11 +10333,12 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm,
     virCgroupPtr cgroup_temp = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char *nodeset_str = NULL;
+    virDomainNumatuneMemMode mode;
     size_t i = 0;
     int ret = -1;
 
-    if (virDomainNumatuneGetMode(vm->def->numa, -1) !=
-        VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+    if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) < 0 ||
+        mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("change of nodeset for running domain "
                          "requires strict numa mode"));
@@ -10392,6 +10395,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
     virCapsPtr caps = NULL;
     qemuDomainObjPrivatePtr priv;
     virBitmapPtr nodeset = NULL;
+    virDomainNumatuneMemMode config_mode;
     int mode = -1;
 
     virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@@ -10467,7 +10471,8 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         if (mode != -1 &&
-            virDomainNumatuneGetMode(vm->def->numa, -1) != mode) {
+            virDomainNumatuneGetMode(vm->def->numa, -1, &config_mode) == 0 &&
+            config_mode != mode) {
             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                            _("can't change numatune mode for running domain"));
             goto endjob;
@@ -10568,7 +10573,8 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
                                         VIR_TYPED_PARAM_INT, 0) < 0)
                 goto cleanup;
 
-            param->value.i = virDomainNumatuneGetMode(def->numa, -1);
+            virDomainNumatuneGetMode(def->numa, -1,
+                                     (virDomainNumatuneMemMode *) &param->value.i);
             break;
 
         case 1: /* fill numa nodeset here */

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2b3d9b5171e9fe2f8e7731e6d37e3c70af17b3cf..118fc524572d23175f960a7d5df451ae22dfefd0 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3135,21 +3135,21 @@ static int qemuProcessHook(void *data)
     if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0)
         goto cleanup;
 
-    mode = virDomainNumatuneGetMode(h->vm->def->numa, -1);
-
-    if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
-        h->cfg->cgroupControllers & (1 << VIR_CGROUP_CONTROLLER_CPUSET) &&
-        virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
-        /* Use virNuma* API iff necessary. Once set and child is exec()-ed,
-         * there's no way for us to change it. Rely on cgroups (if available
-         * and enabled in the config) rather than virNuma*. */
-        VIR_DEBUG("Relying on CGroups for memory binding");
-    } else {
-        nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa,
-                                              priv->autoNodeset, -1);
+    if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) == 0) {
+        if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
+            h->cfg->cgroupControllers & (1 << VIR_CGROUP_CONTROLLER_CPUSET) &&
+            virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
+            /* Use virNuma* API iff necessary. Once set and child is exec()-ed,
+             * there's no way for us to change it. Rely on cgroups (if available
+             * and enabled in the config) rather than virNuma*. */
+            VIR_DEBUG("Relying on CGroups for memory binding");
+        } else {
+            nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa,
+                                                  priv->autoNodeset, -1);
 
-        if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
-            goto cleanup;
+            if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
+                goto cleanup;
+        }
     }
 
     ret = 0;