In current implementation, it uses integer for record current avail cpus and
this only allows user to specify 31 vcpus.
In following patch, it uses cpumap instead interger which make more sense than
before. Also there is no limit to the max vcpus.
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
diff -r c8a01e966ec8 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Thu May 17 12:59:49 2012 +0800
+++ b/tools/libxl/libxl_create.c Thu May 17 13:01:26 2012 +0800
@@ -142,8 +142,11 @@ int libxl__domain_build_info_setdefault(
if (!b_info->max_vcpus)
b_info->max_vcpus = 1;
- if (!b_info->cur_vcpus)
- b_info->cur_vcpus = 1;
+ if (!b_info->avail_vcpus.size) {
+ if (libxl_cpumap_alloc_size(CTX, &b_info->avail_vcpus, 1))
+ return ERROR_NOMEM;
+ libxl_cpumap_set(&b_info->avail_vcpus, 0);
+ }
if (!b_info->cpumap.size) {
if (libxl_cpumap_alloc(CTX, &b_info->cpumap))
diff -r c8a01e966ec8 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Thu May 17 12:59:49 2012 +0800
+++ b/tools/libxl/libxl_dm.c Thu May 17 13:01:26 2012 +0800
@@ -200,10 +200,19 @@ static char ** libxl__build_device_model
libxl__sprintf(gc, "%d",
b_info->max_vcpus),
NULL);
}
- if (b_info->cur_vcpus) {
+ if (b_info->avail_vcpus.size) {
+ int nr_set_cpus = 0;
+ char *s;
+
+ nr_set_cpus = libxl_cpumap_count_set(
+ &(((libxl_domain_build_info
*)b_info)->avail_vcpus));
+
+ s = libxl_cpumap_to_hex_string(
+ &(((libxl_domain_build_info
*)b_info)->avail_vcpus));
flexarray_vappend(dm_args, "-vcpu_avail",
- libxl__sprintf(gc, "0x%x",
b_info->cur_vcpus),
+ libxl__sprintf(gc, "0x%s", s),
NULL);
+ free(s);
}
for (i = 0; i < num_vifs; i++) {
if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
@@ -441,11 +450,15 @@ static char ** libxl__build_device_model
}
if (b_info->max_vcpus > 1) {
flexarray_append(dm_args, "-smp");
- if (b_info->cur_vcpus)
+ if (b_info->avail_vcpus.size) {
+ int nr_set_cpus = 0;
+ nr_set_cpus = libxl_cpumap_count_set(
+ &(((libxl_domain_build_info
*)b_info)->avail_vcpus));
+
flexarray_append(dm_args, libxl__sprintf(gc,
"%d,maxcpus=%d",
b_info->max_vcpus,
-
b_info->cur_vcpus));
- else
+ nr_set_cpus));
+ } else
flexarray_append(dm_args, libxl__sprintf(gc, "%d",
b_info->max_vcpus));
}
diff -r c8a01e966ec8 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Thu May 17 12:59:49 2012 +0800
+++ b/tools/libxl/libxl_dom.c Thu May 17 13:01:26 2012 +0800
@@ -195,7 +195,8 @@ int libxl__build_post(libxl__gc *gc, uin
ents[11] = libxl__sprintf(gc, "%lu", state->store_mfn);
for (i = 0; i < info->max_vcpus; i++) {
ents[12+(i*2)] = libxl__sprintf(gc, "cpu/%d/availability",
i);
- ents[12+(i*2)+1] = (i && info->cur_vcpus &&
!(info->cur_vcpus & (1 << i)))
+ ents[12+(i*2)+1] = (i && info->avail_vcpus.size
+ &&
!libxl_cpumap_test(&info->avail_vcpus, i))
? "offline" : "online";
}
@@ -346,7 +347,7 @@ static int hvm_build_set_params(xc_inter
va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
va_hvm->nr_vcpus = info->max_vcpus;
- memcpy(va_hvm->vcpu_online, &info->cur_vcpus,
sizeof(info->cur_vcpus));
+ memcpy(va_hvm->vcpu_online, info->avail_vcpus.map,
info->avail_vcpus.size);
for (i = 0, sum = 0; i < va_hvm->length; i++)
sum += ((uint8_t *) va_hvm)[i];
va_hvm->checksum -= sum;
diff -r c8a01e966ec8 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Thu May 17 12:59:49 2012 +0800
+++ b/tools/libxl/libxl_types.idl Thu May 17 13:01:26 2012 +0800
@@ -242,7 +242,7 @@ libxl_sched_params = Struct("sched_param
# libxl_file_reference_unmap.
libxl_domain_build_info = Struct("domain_build_info",[
("max_vcpus", integer),
- ("cur_vcpus", integer),
+ ("avail_vcpus", libxl_cpumap),
("cpumap", libxl_cpumap),
("tsc_mode", libxl_tsc_mode),
("max_memkb", MemKB),
diff -r c8a01e966ec8 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c Thu May 17 12:59:49 2012 +0800
+++ b/tools/libxl/libxl_utils.c Thu May 17 13:01:26 2012 +0800
@@ -503,6 +503,19 @@ int libxl_cpumap_alloc(libxl_ctx *ctx, l
return 0;
}
+int libxl_cpumap_alloc_size(libxl_ctx *ctx, libxl_cpumap *cpumap, int max_cpus)
+{
+ int sz;
+
+ if (max_cpus == 0)
+ return ERROR_FAIL;
+
+ sz = (max_cpus + 7) / 8;
+ cpumap->map = libxl__zalloc(NULL, sz * sizeof(*cpumap->map));
+ cpumap->size = sz;
+ return 0;
+}
+
void libxl_cpumap_dispose(libxl_cpumap *map)
{
free(map->map);
@@ -529,6 +542,29 @@ void libxl_cpumap_reset(libxl_cpumap *cp
cpumap->map[cpu / 8] &= ~(1 << (cpu & 7));
}
+int libxl_cpumap_count_set(libxl_cpumap *cpumap)
+{
+ int i, nr_set_cpus = 0;
+ libxl_for_each_set_cpu(i, *cpumap)
+ nr_set_cpus++;
+
+ return nr_set_cpus;
+}
+
+void *libxl_cpumap_to_hex_string(libxl_cpumap *cpumap)
+{
+ int i = cpumap->size;
+ uint8_t *p = libxl__zalloc(NULL, cpumap->size * 2 + 1);
+ uint8_t *q = p;
+ while(--i >= 0) {
+ fprintf(stderr,"i:%d, p :%p, map[%d]: %x\n", i, p, i,
cpumap->map[i]);
+ sprintf((char *)p, "%02x", cpumap->map[i]);
+ p += 2;
+ }
+ *p = ''\0'';
+ return q;
+}
+
int libxl_get_max_cpus(libxl_ctx *ctx)
{
return xc_get_max_cpus(ctx->xch);
diff -r c8a01e966ec8 tools/libxl/libxl_utils.h
--- a/tools/libxl/libxl_utils.h Thu May 17 12:59:49 2012 +0800
+++ b/tools/libxl/libxl_utils.h Thu May 17 13:01:26 2012 +0800
@@ -64,9 +64,12 @@ int libxl_vdev_to_device_disk(libxl_ctx
libxl_device_disk *disk);
int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap);
+int libxl_cpumap_alloc_size(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);
+int libxl_cpumap_count_set(libxl_cpumap *cpumap);
+void *libxl_cpumap_to_hex_string(libxl_cpumap *cpumap);
static inline void libxl_cpumap_set_any(libxl_cpumap *cpumap)
{
memset(cpumap->map, -1, cpumap->size);
diff -r c8a01e966ec8 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Thu May 17 12:59:49 2012 +0800
+++ b/tools/libxl/xl_cmdimpl.c Thu May 17 13:01:26 2012 +0800
@@ -647,7 +647,14 @@ static void parse_config_data(const char
if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
b_info->max_vcpus = l;
- b_info->cur_vcpus = (1 << l) - 1;
+
+ if (libxl_cpumap_alloc_size(ctx, &b_info->avail_vcpus, l)) {
+ fprintf(stderr, "Unable to allocate cpumap\n");
+ exit(1);
+ }
+ libxl_cpumap_set_none(&b_info->avail_vcpus);
+ while (l-- > 0)
+ libxl_cpumap_set((&b_info->avail_vcpus), l);
}
if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
On Thu, 2012-05-17 at 06:02 +0100, Zhang, Yang Z wrote:> In current implementation, it uses integer for record current avail cpus and this only allows user to specify 31 vcpus. > In following patch, it uses cpumap instead interger which make more sense than before. Also there is no limit to the max vcpus. > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > > diff -r c8a01e966ec8 tools/libxl/libxl_create.c > --- a/tools/libxl/libxl_create.c Thu May 17 12:59:49 2012 +0800 > +++ b/tools/libxl/libxl_create.c Thu May 17 13:01:26 2012 +0800 > @@ -142,8 +142,11 @@ int libxl__domain_build_info_setdefault( > > if (!b_info->max_vcpus) > b_info->max_vcpus = 1; > - if (!b_info->cur_vcpus) > - b_info->cur_vcpus = 1; > + if (!b_info->avail_vcpus.size) { > + if (libxl_cpumap_alloc_size(CTX, &b_info->avail_vcpus, 1)) > + return ERROR_NOMEM;After your previous patch the only thing cpumap_alloc_size can fail with is ERROR_FAIL (inability to determine number of processors), converting that into ERROR_NOMEM seems inappropriate. This should propagate the actual error code. It might also be reasonable for libxl to abort/crash if libxl_get_max_cpus fails in which case cpumap_alloc_size would return void.> + libxl_cpumap_set(&b_info->avail_vcpus, 0); > + } > > if (!b_info->cpumap.size) { > if (libxl_cpumap_alloc(CTX, &b_info->cpumap)) > diff -r c8a01e966ec8 tools/libxl/libxl_dm.c > --- a/tools/libxl/libxl_dm.c Thu May 17 12:59:49 2012 +0800 > +++ b/tools/libxl/libxl_dm.c Thu May 17 13:01:26 2012 +0800 > @@ -200,10 +200,19 @@ static char ** libxl__build_device_model > libxl__sprintf(gc, "%d", b_info->max_vcpus), > NULL); > } > - if (b_info->cur_vcpus) { > + if (b_info->avail_vcpus.size) {Can never be false, due to allocation in setdefaults?> + int nr_set_cpus = 0; > + char *s; > + > + nr_set_cpus = libxl_cpumap_count_set( > + &(((libxl_domain_build_info *)b_info)->avail_vcpus)); > + > + s = libxl_cpumap_to_hex_string( > + &(((libxl_domain_build_info *)b_info)->avail_vcpus));Why the casts to (libxl_domain_build_info *)?> flexarray_vappend(dm_args, "-vcpu_avail", > - libxl__sprintf(gc, "0x%x", b_info->cur_vcpus), > + libxl__sprintf(gc, "0x%s", s), > NULL); > + free(s); > } > for (i = 0; i < num_vifs; i++) { > if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) { > @@ -441,11 +450,15 @@ static char ** libxl__build_device_model > } > if (b_info->max_vcpus > 1) { > flexarray_append(dm_args, "-smp"); > - if (b_info->cur_vcpus) > + if (b_info->avail_vcpus.size) { > + int nr_set_cpus = 0; > + nr_set_cpus = libxl_cpumap_count_set( > + &(((libxl_domain_build_info *)b_info)->avail_vcpus));Another strange looking cast. Are you perhaps casting away the const-ness of b_info? You most likely want to make libxl_cpumap_count_set const-correct instead.> + > flexarray_append(dm_args, libxl__sprintf(gc, "%d,maxcpus=%d", > b_info->max_vcpus, > - b_info->cur_vcpus)); > - else > + nr_set_cpus)); > + } else > flexarray_append(dm_args, libxl__sprintf(gc, "%d", > b_info->max_vcpus)); > } > diff -r c8a01e966ec8 tools/libxl/libxl_dom.c > --- a/tools/libxl/libxl_dom.c Thu May 17 12:59:49 2012 +0800 > +++ b/tools/libxl/libxl_dom.c Thu May 17 13:01:26 2012 +0800 > @@ -195,7 +195,8 @@ int libxl__build_post(libxl__gc *gc, uin > ents[11] = libxl__sprintf(gc, "%lu", state->store_mfn); > for (i = 0; i < info->max_vcpus; i++) { > ents[12+(i*2)] = libxl__sprintf(gc, "cpu/%d/availability", i); > - ents[12+(i*2)+1] = (i && info->cur_vcpus && !(info->cur_vcpus & (1 << i))) > + ents[12+(i*2)+1] = (i && info->avail_vcpus.size"!info->avail_vcpus.size" should always be a bug here since you always allocate one in setdefaults. I think you can also stop special casing i == 0, that would be a user error to supply a cpumap without cpu0 set in it. Might be worth checking that in the setdefaults fn.> + && !libxl_cpumap_test(&info->avail_vcpus, i)) > ? "offline" : "online"; > } > > @@ -346,7 +347,7 @@ static int hvm_build_set_params(xc_inter > va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET); > va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic); > va_hvm->nr_vcpus = info->max_vcpus; > - memcpy(va_hvm->vcpu_online, &info->cur_vcpus, sizeof(info->cur_vcpus)); > + memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size); > for (i = 0, sum = 0; i < va_hvm->length; i++) > sum += ((uint8_t *) va_hvm)[i]; > va_hvm->checksum -= sum; > diff -r c8a01e966ec8 tools/libxl/libxl_types.idl > --- a/tools/libxl/libxl_types.idl Thu May 17 12:59:49 2012 +0800 > +++ b/tools/libxl/libxl_types.idl Thu May 17 13:01:26 2012 +0800 > @@ -242,7 +242,7 @@ libxl_sched_params = Struct("sched_param > # libxl_file_reference_unmap. > libxl_domain_build_info = Struct("domain_build_info",[ > ("max_vcpus", integer), > - ("cur_vcpus", integer), > + ("avail_vcpus", libxl_cpumap), > ("cpumap", libxl_cpumap), > ("tsc_mode", libxl_tsc_mode), > ("max_memkb", MemKB), > diff -r c8a01e966ec8 tools/libxl/libxl_utils.c > --- a/tools/libxl/libxl_utils.c Thu May 17 12:59:49 2012 +0800 > +++ b/tools/libxl/libxl_utils.c Thu May 17 13:01:26 2012 +0800 > @@ -503,6 +503,19 @@ int libxl_cpumap_alloc(libxl_ctx *ctx, l > return 0; > } > > +int libxl_cpumap_alloc_size(libxl_ctx *ctx, libxl_cpumap *cpumap, int max_cpus) > +{ > + int sz; > + > + if (max_cpus == 0)<= ?> + return ERROR_FAIL; > + > + sz = (max_cpus + 7) / 8; > + cpumap->map = libxl__zalloc(NULL, sz * sizeof(*cpumap->map)); > + cpumap->size = sz; > + return 0; > +}Can you reimplement libxl_cpumap_alloc in terms of this function? Alternatively add a new max_cpus param to the existing function and have it do: if (max_cpus == 0) max_cpus = libxl_get_max_cpus(ctx); (IOW libxl_cpumap_alloc_size(ctx, &map, 0) means allocate the biggest possibly required map).> + > void libxl_cpumap_dispose(libxl_cpumap *map) > { > free(map->map); > @@ -529,6 +542,29 @@ void libxl_cpumap_reset(libxl_cpumap *cp > cpumap->map[cpu / 8] &= ~(1 << (cpu & 7)); > } > > +int libxl_cpumap_count_set(libxl_cpumap *cpumap) > +{ > + int i, nr_set_cpus = 0; > + libxl_for_each_set_cpu(i, *cpumap) > + nr_set_cpus++; > + > + return nr_set_cpus; > +} > + > +void *libxl_cpumap_to_hex_string(libxl_cpumap *cpumap)shouldn''t this return char *? Other libxl types provide a _to_string method -- is the fact that this one happens to be hex formatted especially relevant? The general form of those is (const?) char *libxl_FOO_to_string(libxl_FOO *foo);> +{ > + int i = cpumap->size; > + uint8_t *p = libxl__zalloc(NULL, cpumap->size * 2 + 1); > + uint8_t *q = p;Why are the uint8_t and not char *?> + while(--i >= 0) { > + fprintf(stderr,"i:%d, p :%p, map[%d]: %x\n", i, p, i, cpumap->map[i]);libxl shouldn''t be logging to stderr. If this function needs to log then it should take a ctx and use the logging primitives. I expect this is just left over debugging which can be removed (it''d be rather verbose for the final verison)> + sprintf((char *)p, "%02x", cpumap->map[i]); > + p += 2; > + } > + *p = ''\0''; > + return q; > +} > + > int libxl_get_max_cpus(libxl_ctx *ctx) > { > return xc_get_max_cpus(ctx->xch); > diff -r c8a01e966ec8 tools/libxl/libxl_utils.h > --- a/tools/libxl/libxl_utils.h Thu May 17 12:59:49 2012 +0800 > +++ b/tools/libxl/libxl_utils.h Thu May 17 13:01:26 2012 +0800 > @@ -64,9 +64,12 @@ int libxl_vdev_to_device_disk(libxl_ctx > libxl_device_disk *disk); > > int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap); > +int libxl_cpumap_alloc_size(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); > +int libxl_cpumap_count_set(libxl_cpumap *cpumap); > +void *libxl_cpumap_to_hex_string(libxl_cpumap *cpumap); > static inline void libxl_cpumap_set_any(libxl_cpumap *cpumap) > { > memset(cpumap->map, -1, cpumap->size); > diff -r c8a01e966ec8 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Thu May 17 12:59:49 2012 +0800 > +++ b/tools/libxl/xl_cmdimpl.c Thu May 17 13:01:26 2012 +0800 > @@ -647,7 +647,14 @@ static void parse_config_data(const char > > if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) { > b_info->max_vcpus = l; > - b_info->cur_vcpus = (1 << l) - 1; > + > + if (libxl_cpumap_alloc_size(ctx, &b_info->avail_vcpus, l)) { > + fprintf(stderr, "Unable to allocate cpumap\n"); > + exit(1); > + } > + libxl_cpumap_set_none(&b_info->avail_vcpus); > + while (l-- > 0) > + libxl_cpumap_set((&b_info->avail_vcpus), l); > } > > if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0)) > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
> -----Original Message----- > From: Ian Campbell [mailto:Ian.Campbell@citrix.com] > Sent: Thursday, May 17, 2012 5:45 PM > > > > if (!b_info->max_vcpus) > > b_info->max_vcpus = 1; > > - if (!b_info->cur_vcpus) > > - b_info->cur_vcpus = 1; > > + if (!b_info->avail_vcpus.size) { > > + if (libxl_cpumap_alloc_size(CTX, &b_info->avail_vcpus, 1)) > > + return ERROR_NOMEM; > > After your previous patch the only thing cpumap_alloc_size can fail with > is ERROR_FAIL (inability to determine number of processors), converting > that into ERROR_NOMEM seems inappropriate. > > This should propagate the actual error code. > > It might also be reasonable for libxl to abort/crash if > libxl_get_max_cpus fails in which case cpumap_alloc_size would return > void.Agree.> > diff -r c8a01e966ec8 tools/libxl/libxl_dm.c > > --- a/tools/libxl/libxl_dm.c Thu May 17 12:59:49 2012 +0800 > > +++ b/tools/libxl/libxl_dm.c Thu May 17 13:01:26 2012 +0800 > > @@ -200,10 +200,19 @@ static char ** libxl__build_device_model > > libxl__sprintf(gc, "%d", > b_info->max_vcpus), > > NULL); > > } > > - if (b_info->cur_vcpus) { > > + if (b_info->avail_vcpus.size) { > > Can never be false, due to allocation in setdefaults? > > > @@ -441,11 +450,15 @@ static char ** libxl__build_device_model > > } > > if (b_info->max_vcpus > 1) { > > flexarray_append(dm_args, "-smp"); > > - if (b_info->cur_vcpus) > > + if (b_info->avail_vcpus.size) { > > + int nr_set_cpus = 0; > > + nr_set_cpus = libxl_cpumap_count_set( > > + &(((libxl_domain_build_info > *)b_info)->avail_vcpus)); > > Another strange looking cast. > > Are you perhaps casting away the const-ness of b_info? > > You most likely want to make libxl_cpumap_count_set const-correct > instead.Correct! Your point is the right way.> > diff -r c8a01e966ec8 tools/libxl/libxl_dom.c > > --- a/tools/libxl/libxl_dom.c Thu May 17 12:59:49 2012 +0800 > > +++ b/tools/libxl/libxl_dom.c Thu May 17 13:01:26 2012 +0800 > > @@ -195,7 +195,8 @@ int libxl__build_post(libxl__gc *gc, uin > > ents[11] = libxl__sprintf(gc, "%lu", state->store_mfn); > > for (i = 0; i < info->max_vcpus; i++) { > > ents[12+(i*2)] = libxl__sprintf(gc, "cpu/%d/availability", i); > > - ents[12+(i*2)+1] = (i && info->cur_vcpus && !(info->cur_vcpus & (1 > << i))) > > + ents[12+(i*2)+1] = (i && info->avail_vcpus.size > > "!info->avail_vcpus.size" should always be a bug here since you always > allocate one in setdefaults. > > I think you can also stop special casing i == 0, that would be a user > error to supply a cpumap without cpu0 set in it. Might be worth checking > that in the setdefaults fn.Agree.> > +int libxl_cpumap_alloc_size(libxl_ctx *ctx, libxl_cpumap *cpumap, int > max_cpus) > > +{ > > + int sz; > > + > > + if (max_cpus == 0) > > <= ? > > > + return ERROR_FAIL; > > + > > + sz = (max_cpus + 7) / 8; > > + cpumap->map = libxl__zalloc(NULL, sz * sizeof(*cpumap->map)); > > + cpumap->size = sz; > > + return 0; > > +} > > Can you reimplement libxl_cpumap_alloc in terms of this function?Sure, I will send a separated patch to do it.> Alternatively add a new max_cpus param to the existing function and have > it do: > if (max_cpus == 0) > max_cpus = libxl_get_max_cpus(ctx); > > (IOW libxl_cpumap_alloc_size(ctx, &map, 0) means allocate the biggest > possibly required map). > > > + > > +void *libxl_cpumap_to_hex_string(libxl_cpumap *cpumap) > > shouldn''t this return char *?> Other libxl types provide a _to_string method -- is the fact that this > one happens to be hex formatted especially relevant? The general form of > those is > (const?) char *libxl_FOO_to_string(libxl_FOO *foo); > > > +{ > > + int i = cpumap->size; > > + uint8_t *p = libxl__zalloc(NULL, cpumap->size * 2 + 1); > > + uint8_t *q = p; > > Why are the uint8_t and not char *? > > > + while(--i >= 0) { > > + fprintf(stderr,"i:%d, p :%p, map[%d]: %x\n", i, p, i, > cpumap->map[i]); > > libxl shouldn''t be logging to stderr. If this function needs to log then > it should take a ctx and use the logging primitives. I expect this is > just left over debugging which can be removed (it''d be rather verbose > for the final verison)Sorry, this only for debugging. I forget to delete it before sending the patch. :) best regards yang