[PATCH v5 04/17] libxc/libxl: sanitize error handling in *_get_max_{cpus, nodes}

[Dario Faggioli <dario.faggioli@citrix.com>]

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.

Note that libxl_{cpu,node}_bitmap_alloc() are also moved to
libxl_utils.c as, apart from having become less trivial and hence
better suited there, that's required for using GC_INIT()/GC_FREE,
which in turn is needed for the LOG()/LOGE() macros.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
---
Changes from v4:
 * fix error handling in xc_get_max_{cpus,nodes}() as well, as
   suggested during review;
 * no longer move libxl_{cpu,node}_bitmap_alloc() from .h to .c,
   as requested during review;
 * add more logging in libxl_{cpu,node}_bitmap_alloc(), as
   suggested during review;
 * propagate the error from libxl_get_max_{cpus,nodes}, as
   suggested during review;

Changes from v3:
 * switch the functions to LOG() / LOGE().
 * take care of the callers of libxl_get_max_{cpus,nodes}() too.

Changes from v2:
 * this wasn't there in v2, but fixing this for v3 was requested
   during v2 review.
---
 tools/libxc/xc_misc.c             |   22 +++++++++++---
 tools/libxl/libxl.c               |   16 ++++------
 tools/libxl/libxl_utils.c         |   60 ++++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_utils.h         |   29 ++----------------
 tools/python/xen/lowlevel/xc/xc.c |    6 ++--
 5 files changed, 88 insertions(+), 45 deletions(-)

diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 56efe6a..c771469 100644
--- 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 028106b..d3180dc 100644
--- 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;
@@ -4542,10 +4540,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
 
     for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu, ++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, *nb_vcpu, &vcpuinfo) == -1) {
             LOGE(ERROR, "getting vcpu info");
             goto err;
@@ -5315,8 +5311,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 9f5f589..0833de2 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -651,6 +651,58 @@ char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap)
     return q;
 }
 
+int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus)
+{
+    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) {
+        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);
+
+ out:
+    GC_FREE;
+    return rc;
+}
+
+int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
+                            int max_nodes)
+{
+    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) {
+        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);
+
+ out:
+    GC_FREE;
+    return rc;
+}
+
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
                             const libxl_bitmap *nodemap,
                             libxl_bitmap *cpumap)
@@ -719,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/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 7b84e6a..e37fb89 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -98,32 +98,9 @@ static inline int libxl_bitmap_cpu_valid(libxl_bitmap *bitmap, int bit)
 #define libxl_for_each_set_bit(v, m) for (v = 0; v < (m).size * 8; v++) \
                                              if (libxl_bitmap_test(&(m), v))
 
-static inline int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap,
-                                         int max_cpus)
-{
-    if (max_cpus < 0)
-        return ERROR_INVAL;
-    if (max_cpus == 0)
-        max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
-        return ERROR_FAIL;
-
-    return libxl_bitmap_alloc(ctx, cpumap, max_cpus);
-}
-
-static inline int libxl_node_bitmap_alloc(libxl_ctx *ctx,
-                                          libxl_bitmap *nodemap,
-                                          int max_nodes)
-{
-    if (max_nodes < 0)
-        return ERROR_INVAL;
-    if (max_nodes == 0)
-        max_nodes = libxl_get_max_nodes(ctx);
-    if (max_nodes == 0)
-        return ERROR_FAIL;
-
-    return libxl_bitmap_alloc(ctx, nodemap, max_nodes);
-}
+int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus);
+int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
+                            int max_nodes);
 
 /* Populate cpumap with the cpus spanned by the nodes in nodemap */
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 2625fc4..737bdac 100644
--- 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);