File libxl-dont-touch-nr_vcpus_out-if-listing-vcpus-... of Package xen.30824

Overview Repositories Revisions Requests Users Attributes Meta

File libxl-dont-touch-nr_vcpus_out-if-listing-vcpus-and-returning-NULL.patch of Package xen.30824

References: bsc#1191668, bsc#1194267

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(-)

--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1443,6 +1443,7 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
     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");
@@ -1459,27 +1460,27 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
     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;
@@ -1487,6 +1488,7 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
         ptr->vcpu_time = vcpuinfo.cpu_time;
     }
     GC_FREE;
+    *nr_vcpus_out = nr_vcpus;
     return ret;
 
 err:
--- a/tools/libxl/libxl_numa.c
+++ b/tools/libxl/libxl_numa.c
@@ -219,8 +219,10 @@ static int nr_vcpus_on_nodes(libxl__gc *
             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);

Places

File libxl-dont-touch-nr_vcpus_out-if-listing-vcpus-and-returning-NULL.patch of Package xen.30824

Places