|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 ]libxl: allow to set more than 31 vcpus
On Fri, 2012-06-01 at 03:48 +0100, Zhang, Yang Z wrote:
> Change from v2:
> Add function libxl_cpumap_to_hex_string to covert cpumap to hex string.
> According to Ian's comments, modified some codes to make the logic more
> reasonable.
>
> In current implementation, it uses integer to record current avail cpus and
> this only allows user to specify 31 vcpus.
> In following patch, it uses cpumap instead integer which make more sense than
> before. Also there is no limit to the max vcpus.
>
> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>
> diff -r 3b0eed731020 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c Fri Jun 01 09:27:17 2012 +0800
> +++ b/tools/libxl/libxl_create.c Fri Jun 01 10:34:13 2012 +0800
> @@ -146,8 +146,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(CTX, &b_info->avail_vcpus, 1))
> + return ERROR_FAIL;
> + libxl_cpumap_set(&b_info->avail_vcpus, 0);
> + }
>
> if (!b_info->cpumap.size) {
> if (libxl_cpumap_alloc(CTX, &b_info->cpumap, 0))
> diff -r 3b0eed731020 tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c Fri Jun 01 09:27:17 2012 +0800
> +++ b/tools/libxl/libxl_dm.c Fri Jun 01 10:34:13 2012 +0800
> @@ -160,6 +160,8 @@ static char ** libxl__build_device_model
> }
> if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> int ioemu_vifs = 0;
> + int nr_set_cpus = 0;
> + char *s;
>
> if (b_info->u.hvm.serial) {
> flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial,
> NULL);
> @@ -200,11 +202,14 @@ static char ** libxl__build_device_model
> libxl__sprintf(gc, "%d", b_info->max_vcpus),
> NULL);
> }
> - if (b_info->cur_vcpus) {
> - flexarray_vappend(dm_args, "-vcpu_avail",
> - libxl__sprintf(gc, "0x%x", b_info->cur_vcpus),
> - NULL);
> - }
> +
> + nr_set_cpus = libxl_cpumap_count_set(&b_info->avail_vcpus);
> + s = libxl_cpumap_to_hex_string(&b_info->avail_vcpus);
> + flexarray_vappend(dm_args, "-vcpu_avail",
> + libxl__sprintf(gc, "0x%s", s),
You might as well make to_hex_string include the 0x?
> + NULL);
> + free(s);
> +
> for (i = 0; i < num_vifs; i++) {
> if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
> char *smac = libxl__sprintf(gc,
> diff -r 3b0eed731020 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c Fri Jun 01 09:27:17 2012 +0800
> +++ b/tools/libxl/libxl_dom.c Fri Jun 01 10:34:13 2012 +0800
> @@ -195,7 +195,7 @@ 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 && !libxl_cpumap_test(&info->avail_vcpus, i))
> ? "offline" : "online";
Weren't you going to drop the "i &&" and invert the ternary?
> }
>
> @@ -350,7 +350,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);
This needs some sort of limit check, probably in terms of HVM_MAX_VCPUS.
otherwise a particularly large vcpus= in the config will cause mayhem...
I guess this should probably be done in
libxl__domain_build_info_setdefaults.
> diff -r 3b0eed731020 tools/libxl/libxl_utils.c
> --- a/tools/libxl/libxl_utils.c Fri Jun 01 09:27:17 2012 +0800
> +++ b/tools/libxl/libxl_utils.c Fri Jun 01 10:34:13 2012 +0800
> @@ -533,6 +533,28 @@ void libxl_cpumap_reset(libxl_cpumap *cp
> cpumap->map[cpu / 8] &= ~(1 << (cpu & 7));
> }
>
> +int libxl_cpumap_count_set(const libxl_cpumap *cpumap)
> +{
> + int i, nr_set_cpus = 0;
> + libxl_for_each_set_cpu(i, *((libxl_cpumap *)cpumap))
Please stop this habit of yours of casting away the const to make the
warning go away, it is almost always wrong and on the odd occasion that
it is right it should have a comment explaining why...
In this case please make libxl_cpumap_test const correct instead.
> + nr_set_cpus++;
> +
> + return nr_set_cpus;
> +}
> +
> +char *libxl_cpumap_to_hex_string(const libxl_cpumap *cpumap)
> +{
> + int i = cpumap->size;
> + char *p = libxl__zalloc(NULL, cpumap->size * 2 + 1);
> + char *q = p;
> + while(--i >= 0) {
> + sprintf((char *)p, "%02x", cpumap->map[i]);
Why this cast?
> + p += 2;
> + }
> + *p = '\0';
> + return q;
> +}
> +
> int libxl_get_max_cpus(libxl_ctx *ctx)
> {
> return xc_get_max_cpus(ctx->xch);
> diff -r 3b0eed731020 tools/libxl/libxl_utils.h
> --- a/tools/libxl/libxl_utils.h Fri Jun 01 09:27:17 2012 +0800
> +++ b/tools/libxl/libxl_utils.h Fri Jun 01 10:34:13 2012 +0800
> @@ -67,6 +67,8 @@ int libxl_cpumap_alloc(libxl_ctx *ctx, l
> 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(const libxl_cpumap *cpumap);
Please add a comment describing this function, it should remind the
caller that they are responsible for freeing the result.
> +char *libxl_cpumap_to_hex_string(const libxl_cpumap *cpumap);
> static inline void libxl_cpumap_set_any(libxl_cpumap *cpumap)
> {
> memset(cpumap->map, -1, cpumap->size);
> diff -r 3b0eed731020 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Fri Jun 01 09:27:17 2012 +0800
> +++ b/tools/libxl/xl_cmdimpl.c Fri Jun 01 10:34:13 2012 +0800
> @@ -650,7 +650,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(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);
This while loop is == libxl_cpumap_set_any.
> }
>
> if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |