thr3ads.net - Xen devel - [PATCH]libxl: rewrite libxl_cpumap

If this information is useful, please help other people find it:
Share via:

Zhang, Yang Z

2012-May-18 02:53 UTC

[PATCH]libxl: rewrite libxl_cpumap_alloc()

Allow libxl_cpumap_alloc to allocate memory of specific size. 
Max_cpus equals to zero means allocate the biggest possibly required map.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>

diff -r 28831853d1a8 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Thu May 17 14:45:43 2012 +0100
+++ b/tools/libxl/libxl.c       Fri May 18 10:47:59 2012 +0800
@@ -572,7 +572,7 @@ libxl_cpupoolinfo * libxl_list_cpupool(l
         ptr[i].poolid = info->cpupool_id;
         ptr[i].sched = info->sched_id;
         ptr[i].n_dom = info->n_dom;
-        if (libxl_cpumap_alloc(ctx, &ptr[i].cpumap)) {
+        if (libxl_cpumap_alloc(ctx, &ptr[i].cpumap, 0)) {
             xc_cpupool_infofree(ctx->xch, info);
             break;
         }
@@ -3036,7 +3036,7 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
     }

     for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu,
++ptr) {
-        if (libxl_cpumap_alloc(ctx, &ptr->cpumap)) {
+        if (libxl_cpumap_alloc(ctx, &ptr->cpumap, 0)) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating
cpumap");
             return NULL;
         }
@@ -3719,8 +3719,8 @@ int libxl_cpupool_destroy(libxl_ctx *ctx
     if ((info->cpupool_id != poolid) || (info->n_dom))
         goto out;

-    rc = ERROR_NOMEM;
-    if (libxl_cpumap_alloc(ctx, &cpumap))
+    rc = libxl_cpumap_alloc(ctx, &cpumap, 0);
+    if (rc)
         goto out;

     memcpy(cpumap.map, info->cpumap, cpumap.size);
diff -r 28831853d1a8 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c        Thu May 17 14:45:43 2012 +0100
+++ b/tools/libxl/libxl_create.c        Fri May 18 10:47:59 2012 +0800
@@ -146,8 +146,8 @@ int libxl__domain_build_info_setdefault(
         b_info->cur_vcpus = 1;

     if (!b_info->cpumap.size) {
-        if (libxl_cpumap_alloc(CTX, &b_info->cpumap))
-            return ERROR_NOMEM;
+        if (libxl_cpumap_alloc(CTX, &b_info->cpumap, 0))
+            return ERROR_FAIL;
         libxl_cpumap_set_any(&b_info->cpumap);
     }

diff -r 28831853d1a8 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c Thu May 17 14:45:43 2012 +0100
+++ b/tools/libxl/libxl_utils.c Fri May 18 10:47:59 2012 +0800
@@ -488,19 +488,20 @@ int libxl_mac_to_device_nic(libxl_ctx *c
     return rc;
 }

-int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap)
+int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap, int max_cpus)
 {
-    int max_cpus;
     int sz;

-    max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
-        return ERROR_FAIL;
+    if (max_cpus < 0) {
+        return ERROR_INVAL;
+    } else if (max_cpus == 0) {
+        max_cpus = libxl_get_max_cpus(ctx);
+        if (max_cpus == 0)
+            return ERROR_FAIL;
+    }

     sz = (max_cpus + 7) / 8;
-    cpumap->map = calloc(sz, sizeof(*cpumap->map));
-    if (!cpumap->map)
-        return ERROR_NOMEM;
+    cpumap->map = libxl__zalloc(NULL, sz * sizeof(*cpumap->map));
     cpumap->size = sz;
     return 0;
 }
diff -r 28831853d1a8 tools/libxl/libxl_utils.h
--- a/tools/libxl/libxl_utils.h Thu May 17 14:45:43 2012 +0100
+++ b/tools/libxl/libxl_utils.h Fri May 18 10:47:59 2012 +0800
@@ -63,7 +63,7 @@ int libxl_devid_to_device_nic(libxl_ctx
 int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid, const char *vdev,
                                libxl_device_disk *disk);

-int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap);
+int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap, int max_cpus);
 int libxl_cpumap_test(libxl_cpumap *cpumap, int cpu);
 void libxl_cpumap_set(libxl_cpumap *cpumap, int cpu);
 void libxl_cpumap_reset(libxl_cpumap *cpumap, int cpu);
diff -r 28831853d1a8 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Thu May 17 14:45:43 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Fri May 18 10:47:59 2012 +0800
@@ -502,7 +502,7 @@ static int vcpupin_parse(char *cpu, libx
         return 0;
     }

-    if (libxl_cpumap_alloc(ctx, &exclude_cpumap)) {
+    if (libxl_cpumap_alloc(ctx, &exclude_cpumap, 0)) {
         fprintf(stderr, "Error: Failed to allocate cpumap.\n");
         return ENOMEM;
     }
@@ -656,7 +656,7 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_list (config, "cpus", &cpus, 0, 1)) {
         int i, n_cpus = 0;

-        if (libxl_cpumap_alloc(ctx, &b_info->cpumap)) {
+        if (libxl_cpumap_alloc(ctx, &b_info->cpumap, 0)) {
             fprintf(stderr, "Unable to allocate cpumap\n");
             exit(1);
         }
@@ -692,7 +692,7 @@ static void parse_config_data(const char
     else if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
         char *buf2 = strdup(buf);

-        if (libxl_cpumap_alloc(ctx, &b_info->cpumap)) {
+        if (libxl_cpumap_alloc(ctx, &b_info->cpumap, 0)) {
             fprintf(stderr, "Unable to allocate cpumap\n");
             exit(1);
         }
@@ -1767,7 +1767,9 @@ start:
     if (vcpu_to_pcpu) {
         libxl_cpumap vcpu_cpumap;

-        libxl_cpumap_alloc(ctx, &vcpu_cpumap);
+        ret = libxl_cpumap_alloc(ctx, &vcpu_cpumap, 0);
+        if (ret)
+            goto error_out;
         for (i = 0; i < d_config.b_info.max_vcpus; i++) {

             if (vcpu_to_pcpu[i] != -1) {
@@ -3975,7 +3977,7 @@ static void vcpupin(const char *d, const

     find_domain(d);

-    if (libxl_cpumap_alloc(ctx, &cpumap)) {
+    if (libxl_cpumap_alloc(ctx, &cpumap, 0)) {
         goto vcpupin_out;
     }

@@ -4029,7 +4031,7 @@ static void vcpuset(const char *d, const

     find_domain(d);

-    if (libxl_cpumap_alloc(ctx, &cpumap)) {
+    if (libxl_cpumap_alloc(ctx, &cpumap, 0)) {
         fprintf(stderr, "libxl_cpumap_alloc failed\n");
         return;
     }
@@ -5887,7 +5889,7 @@ int main_cpupoolcreate(int argc, char **
         fprintf(stderr, "libxl_get_freecpus failed\n");
         goto out_cfg;
     }
-    if (libxl_cpumap_alloc(ctx, &cpumap)) {
+    if (libxl_cpumap_alloc(ctx, &cpumap, 0)) {
         fprintf(stderr, "Failed to allocate cpumap\n");
         goto out_cfg;
     }
@@ -6254,7 +6256,7 @@ int main_cpupoolnumasplit(int argc, char
         return -ERROR_FAIL;
     }

-    if (libxl_cpumap_alloc(ctx, &cpumap)) {
+    if (libxl_cpumap_alloc(ctx, &cpumap, 0)) {
         fprintf(stderr, "Failed to allocate cpumap\n");
         libxl_cputopology_list_free(topology, n_cpus);
         return -ERROR_FAIL;

Ian Campbell

2012-May-23 09:50 UTC

head link

Re: [PATCH]libxl: rewrite libxl_cpumap_alloc()

On Fri, 2012-05-18 at 03:53 +0100, Zhang, Yang Z wrote:> Allow libxl_cpumap_alloc to allocate memory of specific size. 
> Max_cpus equals to zero means allocate the biggest possibly required map.
Can we get a comment to that effect in the header please.
> -    int max_cpus;
>      int sz;
> 
> -    max_cpus = libxl_get_max_cpus(ctx);
> -    if (max_cpus == 0)
> -        return ERROR_FAIL;
> +    if (max_cpus < 0) {
> +        return ERROR_INVAL;
> +    } else if (max_cpus == 0) {
> +        max_cpus = libxl_get_max_cpus(ctx);
> +        if (max_cpus == 0)
> +            return ERROR_FAIL;
> +    }
I would have written this as

+    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;

and avoided the nested if and ifelse logic.
> 
>      sz = (max_cpus + 7) / 8;
> -    cpumap->map = calloc(sz, sizeof(*cpumap->map));
> -    if (!cpumap->map)
> -        return ERROR_NOMEM;
> +    cpumap->map = libxl__zalloc(NULL, sz * sizeof(*cpumap->map));
You might as well use libxl__calloc here.

Should I be expecting a v3 of "libxl: allow to set more than 31 vcpus"
which uses this patch? If you already sent it then I may have missed it.

Cheers,
Ian.

Zhang, Yang Z

2012-May-28 01:25 UTC

head link

Re: [PATCH]libxl: rewrite libxl_cpumap_alloc()

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Sent: Wednesday, May 23, 2012 5:51 PM
> On Fri, 2012-05-18 at 03:53 +0100, Zhang, Yang Z wrote:
> > Allow libxl_cpumap_alloc to allocate memory of specific size.
> > Max_cpus equals to zero means allocate the biggest possibly required
map.
> 
> Can we get a comment to that effect in the header please.
Sure.
> > -    int max_cpus;
> >      int sz;
> >
> > -    max_cpus = libxl_get_max_cpus(ctx);
> > -    if (max_cpus == 0)
> > -        return ERROR_FAIL;
> > +    if (max_cpus < 0) {
> > +        return ERROR_INVAL;
> > +    } else if (max_cpus == 0) {
> > +        max_cpus = libxl_get_max_cpus(ctx);
> > +        if (max_cpus == 0)
> > +            return ERROR_FAIL;
> > +    }
> 
> I would have written this as
> 
> +    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;
> 
> and avoided the nested if and ifelse logic.
Agree.
 > >
> >      sz = (max_cpus + 7) / 8;
> > -    cpumap->map = calloc(sz, sizeof(*cpumap->map));
> > -    if (!cpumap->map)
> > -        return ERROR_NOMEM;
> > +    cpumap->map = libxl__zalloc(NULL, sz *
sizeof(*cpumap->map));
> 
> You might as well use libxl__calloc here.
Agree.
> Should I be expecting a v3 of "libxl: allow to set more than 31
vcpus"
> which uses this patch? If you already sent it then I may have missed it.
Yes, I will send it. 

best regards
yang

Xen devel - May 2012 - [PATCH]libxl: rewrite libxl_cpumap_alloc()

[PATCH]libxl: rewrite libxl_cpumap_alloc()

Re: [PATCH]libxl: rewrite libxl_cpumap_alloc()

Re: [PATCH]libxl: rewrite libxl_cpumap_alloc()