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

[PATCH 2/2] tools/libs/light: don't touch nr_vcpus_out if listing vcpus and returning NULL



If we are in libvxl_list_vcpu() and we are returning NULL, let's avoid
touching the output parameter *nr_vcpus_out (which should contain the
number of vcpus in the list). Ideally, the caller initialized it to 0,
which is therefore consistent with us returning NULL (or, as an alternative,
we can explicitly set it to 0 if we're returning null... But just not
touching it seems the best behavior).

In fact, the current behavior is especially problematic if, for
instance, a domain is destroyed after we have done some steps of the
for() loop. In which case, calls like xc_vcpu_getinfo() or
xc_vcpu_getaffinity() will start to fail, and we return back to the
caller inconsistent information, such as a NULL list of vcpus, but a
modified and not 0 any longer, number of vcpus in the list.

Signed-off-by: Dario Faggioli <dfaggioli@xxxxxxxx>
Tested-by: James Fehlig <jfehlig@xxxxxxxx>
---
Cc: Wei Liu <wl@xxxxxxx>
Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Cc: Juergen Gross <jgross@xxxxxxxx>
---
 tools/libs/light/libxl_domain.c |   14 ++++++++------
 tools/libs/light/libxl_numa.c   |    4 +++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 544a9bf59d..aabc264e16 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -1661,6 +1661,7 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t 
domid,
     libxl_vcpuinfo *ptr, *ret;
     xc_domaininfo_t domaininfo;
     xc_vcpuinfo_t vcpuinfo;
+    int nr_vcpus;
 
     if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
         LOGED(ERROR, domid, "Getting infolist");
@@ -1677,27 +1678,27 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, 
uint32_t domid,
     ret = ptr = libxl__calloc(NOGC, domaininfo.max_vcpu_id + 1,
                               sizeof(libxl_vcpuinfo));
 
-    for (*nr_vcpus_out = 0;
-         *nr_vcpus_out <= domaininfo.max_vcpu_id;
-         ++*nr_vcpus_out, ++ptr) {
+    for (nr_vcpus = 0;
+         nr_vcpus <= domaininfo.max_vcpu_id;
+         ++nr_vcpus, ++ptr) {
         libxl_bitmap_init(&ptr->cpumap);
         if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0))
             goto err;
         libxl_bitmap_init(&ptr->cpumap_soft);
         if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap_soft, 0))
             goto err;
-        if (xc_vcpu_getinfo(ctx->xch, domid, *nr_vcpus_out, &vcpuinfo) == -1) {
+        if (xc_vcpu_getinfo(ctx->xch, domid, nr_vcpus, &vcpuinfo) == -1) {
             LOGED(ERROR, domid, "Getting vcpu info");
             goto err;
         }
 
-        if (xc_vcpu_getaffinity(ctx->xch, domid, *nr_vcpus_out,
+        if (xc_vcpu_getaffinity(ctx->xch, domid, nr_vcpus,
                                 ptr->cpumap.map, ptr->cpumap_soft.map,
                                 XEN_VCPUAFFINITY_SOFT|XEN_VCPUAFFINITY_HARD) 
== -1) {
             LOGED(ERROR, domid, "Getting vcpu affinity");
             goto err;
         }
-        ptr->vcpuid = *nr_vcpus_out;
+        ptr->vcpuid = nr_vcpus;
         ptr->cpu = vcpuinfo.cpu;
         ptr->online = !!vcpuinfo.online;
         ptr->blocked = !!vcpuinfo.blocked;
@@ -1705,6 +1706,7 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t 
domid,
         ptr->vcpu_time = vcpuinfo.cpu_time;
     }
     GC_FREE;
+    *nr_vcpus_out = nr_vcpus;
     return ret;
 
 err:
diff --git a/tools/libs/light/libxl_numa.c b/tools/libs/light/libxl_numa.c
index 3679028c79..b04e3917a0 100644
--- a/tools/libs/light/libxl_numa.c
+++ b/tools/libs/light/libxl_numa.c
@@ -219,8 +219,10 @@ static int nr_vcpus_on_nodes(libxl__gc *gc, 
libxl_cputopology *tinfo,
             goto next;
 
         vinfo = libxl_list_vcpu(CTX, dinfo[i].domid, &nr_dom_vcpus, &nr_cpus);
-        if (vinfo == NULL)
+        if (vinfo == NULL) {
+            assert(nr_dom_vcpus == 0);
             goto next;
+        }
 
         /* Retrieve the domain's node-affinity map */
         libxl_domain_get_nodeaffinity(CTX, dinfo[i].domid, &dom_nodemap);





 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.