libxc/libxl: sanitize error handling in *_get_max_{cpus, nodes}

In libxc, make xc_get_max_{cpus,node}() always return either a
positive number or -1, and change all the callers to deal with
that.

In libxl, make libxl_get_max_{cpus,nodes}() always return either a
positive number or a libxl error code. Thanks to that, it is also
possible to fix loggig for libxl_{cpu,node}_bitmap_alloc(), which
now happens inside the functions themselves, more accurately
reporting what happened.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 56efe6a28bff6dbd995730c283b668edfb6e0aae..c7714699e54c2d5360bcd7fd8438cae7ee961095 100644 (file)
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -30,9 +30,12 @@ int xc_get_max_cpus(xc_interface *xch)
         return max_cpus;
 
     if ( !xc_physinfo(xch, &physinfo) )
+    {
         max_cpus = physinfo.max_cpu_id + 1;
+        return max_cpus;
+    }
 
-    return max_cpus;
+    return -1;
 }
 
 int xc_get_max_nodes(xc_interface *xch)
@@ -44,19 +47,30 @@ int xc_get_max_nodes(xc_interface *xch)
         return max_nodes;
 
     if ( !xc_physinfo(xch, &physinfo) )
+    {
         max_nodes = physinfo.max_node_id + 1;
+        return max_nodes;
+    }
 
-    return max_nodes;
+    return -1;
 }
 
 int xc_get_cpumap_size(xc_interface *xch)
 {
-    return (xc_get_max_cpus(xch) + 7) / 8;
+    int max_cpus = xc_get_max_cpus(xch);
+
+    if ( max_cpus < 0 )
+        return -1;
+    return (max_cpus + 7) / 8;
 }
 
 int xc_get_nodemap_size(xc_interface *xch)
 {
-    return (xc_get_max_nodes(xch) + 7) / 8;
+    int max_nodes = xc_get_max_nodes(xch);
+
+    if ( max_nodes < 0 )
+        return -1;
+    return (max_nodes + 7) / 8;
 }
 
 xc_cpumap_t xc_cpumap_alloc(xc_interface *xch)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2925e1ec5e6b80a679f144c72a71659375ba13a1..fd8b988feae9ca260cfc160d2ebe699db22feec6 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -615,10 +615,8 @@ static int cpupool_info(libxl__gc *gc,
     info->n_dom = xcinfo->n_dom;
     rc = libxl_cpu_bitmap_alloc(CTX, &info->cpumap, 0);
     if (rc)
-    {
-        LOG(ERROR, "unable to allocate cpumap %d\n", rc);
         goto out;
-    }
+
     memcpy(info->cpumap.map, xcinfo->cpumap, info->cpumap.size);
 
     rc = 0;
@@ -4355,7 +4353,7 @@ libxl_cputopology *libxl_get_cpu_topology(libxl_ctx *ctx, int *nb_cpu_out)
     int max_cpus;
 
     max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
+    if (max_cpus < 0)
     {
         LIBXL__LOG(ctx, XTL_ERROR, "Unable to determine number of CPUS");
         ret = NULL;
@@ -4420,7 +4418,7 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
     int i, j, max_nodes;
 
     max_nodes = libxl_get_max_nodes(ctx);
-    if (max_nodes == 0)
+    if (max_nodes < 0)
     {
         LIBXL__LOG(ctx, XTL_ERROR, "Unable to determine number of NODES");
         ret = NULL;
@@ -4545,10 +4543,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
          *nr_vcpus_out <= domaininfo.max_vcpu_id;
          ++*nr_vcpus_out, ++ptr) {
         libxl_bitmap_init(&ptr->cpumap);
-        if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0)) {
-            LOGE(ERROR, "allocating cpumap");
+        if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0))
             goto err;
-        }
         if (xc_vcpu_getinfo(ctx->xch, domid, *nr_vcpus_out, &vcpuinfo) == -1) {
             LOGE(ERROR, "getting vcpu info");
             goto err;
@@ -5308,8 +5304,8 @@ int libxl_get_freecpus(libxl_ctx *ctx, libxl_bitmap *cpumap)
     int ncpus;
 
     ncpus = libxl_get_max_cpus(ctx);
-    if (ncpus == 0)
-        return ERROR_FAIL;
+    if (ncpus < 0)
+        return ncpus;
 
     cpumap->map = xc_cpupool_freeinfo(ctx->xch);
     if (cpumap->map == NULL)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 93f7a87629412ed7e14a4277fb1fd2f06aa6fe32..0833de2cc30ac9079fe95ae04512e7cdf11123ef 100644 (file)
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -653,27 +653,54 @@ char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap)
 
 int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus)
 {
-    if (max_cpus < 0)
-        return ERROR_INVAL;
+    GC_INIT(ctx);
+    int rc = 0;
+
+    if (max_cpus < 0) {
+        rc = ERROR_INVAL;
+        LOG(ERROR, "invalid number of cpus provided");
+        goto out;
+    }
     if (max_cpus == 0)
         max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
-        return ERROR_FAIL;
+    if (max_cpus < 0) {
+        LOG(ERROR, "failed to retrieve the maximum number of cpus");
+        rc = max_cpus;
+        goto out;
+    }
+    /* This can't fail: no need to check and log */
+    libxl_bitmap_alloc(ctx, cpumap, max_cpus);
 
-    return libxl_bitmap_alloc(ctx, cpumap, max_cpus);
+ out:
+    GC_FREE;
+    return rc;
 }
 
 int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
                             int max_nodes)
 {
-    if (max_nodes < 0)
-        return ERROR_INVAL;
+    GC_INIT(ctx);
+    int rc = 0;
+
+    if (max_nodes < 0) {
+        rc = ERROR_INVAL;
+        LOG(ERROR, "invalid number of nodes provided");
+        goto out;
+    }
+
     if (max_nodes == 0)
         max_nodes = libxl_get_max_nodes(ctx);
-    if (max_nodes == 0)
-        return ERROR_FAIL;
+    if (max_nodes < 0) {
+        LOG(ERROR, "failed to retrieve the maximum number of nodes");
+        rc = max_nodes;
+        goto out;
+    }
+    /* This can't fail: no need to check and log */
+    libxl_bitmap_alloc(ctx, nodemap, max_nodes);
 
-    return libxl_bitmap_alloc(ctx, nodemap, max_nodes);
+ out:
+    GC_FREE;
+    return rc;
 }
 
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
@@ -744,12 +771,16 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
 
 int libxl_get_max_cpus(libxl_ctx *ctx)
 {
-    return xc_get_max_cpus(ctx->xch);
+    int max_cpus = xc_get_max_cpus(ctx->xch);
+
+    return max_cpus < 0 ? ERROR_FAIL : max_cpus;
 }
 
 int libxl_get_max_nodes(libxl_ctx *ctx)
 {
-    return xc_get_max_nodes(ctx->xch);
+    int max_nodes = xc_get_max_nodes(ctx->xch);
+
+    return max_nodes < 0 ? ERROR_FAIL : max_nodes;
 }
 
 int libxl__enum_from_string(const libxl_enum_string_table *t,

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 2625fc41c403a5b0b1b91c912ff3708379f4bdcd..737bdac279a30ce5600b13e6664d4590e772412b 100644 (file)
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -233,7 +233,7 @@ static PyObject *pyxc_vcpu_setaffinity(XcObject *self,
         return NULL;
 
     nr_cpus = xc_get_max_cpus(self->xc_handle);
-    if ( nr_cpus == 0 )
+    if ( nr_cpus < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     cpumap = xc_cpumap_alloc(self->xc_handle);
@@ -392,7 +392,7 @@ static PyObject *pyxc_vcpu_getinfo(XcObject *self,
         return NULL;
 
     nr_cpus = xc_get_max_cpus(self->xc_handle);
-    if ( nr_cpus == 0 )
+    if ( nr_cpus < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     rc = xc_vcpu_getinfo(self->xc_handle, dom, vcpu, &info);
@@ -1923,7 +1923,7 @@ static PyObject *cpumap_to_cpulist(XcObject *self, xc_cpumap_t cpumap)
     int nr_cpus;
 
     nr_cpus = xc_get_max_cpus(self->xc_handle);
-    if ( nr_cpus == 0 )
+    if ( nr_cpus < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     cpulist = PyList_New(0);