xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: xen-devel@lists.xenproject.org
Cc: Wei Liu <wei.liu2@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: [PATCH 1/2] libxl: turn NUMA placement misconfigs into warnings
Date: Wed, 01 Jul 2015 16:03:07 +0200	[thread overview]
Message-ID: <20150701140306.15951.83934.stgit@Solace.station> (raw)
In-Reply-To: <20150701135059.15951.39938.stgit@Solace.station>

instead than errors. More specifically, in libxl,
b_info->numa_autoplacement is meant as a way to
disable automatic NUMA placement, if one does not
want it to happen. It is, however, useful for
consistency checking as well, i.e., to ensure that
the user provided configuration (such as, for instance,
vcpu hard or soft affinity) and NUMA placement itself
will not clash.

However, right now, if such a clash happens we abort
domain creation and error out, which is too much! It
is, in fact, enough to infom the user/caller that NUMA
placement won't be performed, with a WARN, and that's
what this commit does.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_dom.c |   76 +++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 39 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 1c9418a..b9a77a5 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -330,50 +330,48 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
      * reflect the placement result if that is the case
      */
     if (libxl_defbool_val(info->numa_placement)) {
-        libxl_bitmap cpumap_soft;
-
-        if (info->cpumap.size ||
-            info->num_vcpu_hard_affinity || info->num_vcpu_soft_affinity) {
-            LOG(ERROR, "Can run NUMA placement only if no vcpu "
-                       "(hard or soft) affinity is specified explicitly");
-            return ERROR_INVAL;
-        }
-        if (info->nodemap.size) {
-            LOG(ERROR, "Can run NUMA placement only if the domain does not "
-                       "have any NUMA node affinity set already");
-            return ERROR_INVAL;
-        }
-
-        rc = libxl_node_bitmap_alloc(ctx, &info->nodemap, 0);
-        if (rc)
-            return rc;
-        libxl_bitmap_set_any(&info->nodemap);
+        if (info->cpumap.size || info->num_vcpu_hard_affinity ||
+            info->num_vcpu_soft_affinity)
+            LOG(WARN, "Can't run NUMA placement, as an (hard or soft) "
+                      "affinity has been specified explicitly");
+        else if (info->nodemap.size)
+            LOG(WARN, "Can't run NUMA placement, as the domain has "
+                      "NUMA node affinity set already");
+        else {
+            libxl_bitmap cpumap_soft;
+
+            rc = libxl_node_bitmap_alloc(ctx, &info->nodemap, 0);
+            if (rc)
+                return rc;
+            libxl_bitmap_set_any(&info->nodemap);
+
+            rc = libxl_cpu_bitmap_alloc(ctx, &cpumap_soft, 0);
+            if (rc)
+                return rc;
+
+            rc = numa_place_domain(gc, domid, info);
+            if (rc) {
+                libxl_bitmap_dispose(&cpumap_soft);
+                return rc;
+            }
 
-        rc = libxl_cpu_bitmap_alloc(ctx, &cpumap_soft, 0);
-        if (rc)
-            return rc;
+            /*
+             * All we need to do now is converting the result of automatic
+             * placement from nodemap to cpumap, and then use such cpumap
+             * as the soft affinity for all the vcpus of the domain.
+             *
+             * When calling libxl_set_vcpuaffinity_all(), it is ok to use
+             * NULL as hard affinity, as we know we don't have one, or we
+             * won't be here.
+             */
+            libxl_nodemap_to_cpumap(ctx, &info->nodemap, &cpumap_soft);
+            libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,
+                                       NULL, &cpumap_soft);
 
-        rc = numa_place_domain(gc, domid, info);
-        if (rc) {
             libxl_bitmap_dispose(&cpumap_soft);
-            return rc;
         }
-
-        /*
-         * All we need to do now is converting the result of automatic
-         * placement from nodemap to cpumap, and then use such cpumap as
-         * the soft affinity for all the vcpus of the domain.
-         *
-         * When calling libxl_set_vcpuaffinity_all(), it is ok to use NULL
-         * as hard affinity, as we know we don't have one, or we won't be
-         * here.
-         */
-        libxl_nodemap_to_cpumap(ctx, &info->nodemap, &cpumap_soft);
-        libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,
-                                   NULL, &cpumap_soft);
-
-        libxl_bitmap_dispose(&cpumap_soft);
     }
+
     if (info->nodemap.size)
         libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);

  reply	other threads:[~2015-07-01 14:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-01 14:02 [PATCH 0/2] libxl: fix NUMA placement preventing domain config to be re-used Dario Faggioli
2015-07-01 14:03 ` Dario Faggioli [this message]
2015-07-01 14:38   ` [PATCH 1/2] libxl: turn NUMA placement misconfigs into warnings Ian Jackson
2015-07-03 10:35     ` Ian Campbell
2015-07-03 11:09       ` Ian Jackson
2015-07-03 14:52         ` Dario Faggioli
2015-07-01 14:03 ` [PATCH 2/2] libxl: unset info->numa_placement upon successful placement Dario Faggioli
2015-07-01 14:39   ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150701140306.15951.83934.stgit@Solace.station \
    --to=dario.faggioli@citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).