Re: [Xen-devel] [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-01-11 at 18:00 +0000, Dario Faggioli wrote:
> Enable CPU affinity specification in a VM's config file with the
> exact syntax `xl vcpu-pin' provides.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> 
> diff -r 9ce68a4036b1 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c      Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/libxl_create.c      Wed Jan 11 17:40:45 2012 +0000
> @@ -78,6 +78,8 @@ int libxl_init_build_info(libxl_ctx *ctx
>      memset(b_info, '\0', sizeof(*b_info));
>      b_info->max_vcpus = 1;
>      b_info->cur_vcpus = 1;
> +    if (libxl_cpumap_alloc(ctx, &b_info->cpumap))
> +        return ERROR_NOMEM;

Should probably set all here since that is the best default?

>      b_info->max_memkb = 32 * 1024;
>      b_info->target_memkb = b_info->max_memkb;
>      b_info->disable_migrate = 0;
> diff -r 9ce68a4036b1 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/libxl_dom.c Wed Jan 11 17:40:45 2012 +0000
> @@ -72,8 +72,14 @@ int libxl__build_pre(libxl__gc *gc, uint
>                libxl_domain_build_info *info, libxl__domain_build_state 
> *state)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
> -    int tsc_mode;
> +    int i, tsc_mode;
>      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
> +    for (i = 0; i < info->max_vcpus; i++) {
> +        if (libxl_set_vcpuaffinity(ctx, domid, i, &info->cpumap) == -1) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "libxl_set_vcpuaffinity 
> failed. "
> +                           "Going ahead without setting affinity for cpu 
> %d.\n", i);
> +        }
> +    }4b
>      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + 
> LIBXL_MAXMEM_CONSTANT);
>      if (info->type == LIBXL_DOMAIN_TYPE_PV)
>          xc_domain_set_memmap_limit(ctx->xch, domid,
> diff -r 9ce68a4036b1 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl     Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/libxl_types.idl     Wed Jan 11 17:40:45 2012 +0000
> @@ -162,6 +162,7 @@ libxl_domain_create_info = Struct("domai
>  libxl_domain_build_info = Struct("domain_build_info",[
>      ("max_vcpus",       integer),
>      ("cur_vcpus",       integer),
> +    ("cpumap",          libxl_cpumap),
>      ("tsc_mode",        libxl_tsc_mode),
>      ("max_memkb",       uint32),
>      ("target_memkb",    uint32),
> diff -r 9ce68a4036b1 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c        Wed Jan 11 17:40:45 2012 +0000
> @@ -288,16 +288,24 @@ static void dolog(const char *file, int 
>          libxl_write_exactly(NULL, logfile, s, rc, NULL, NULL);
>  }
>  
> +static void print_bitmap(uint8_t *map, int maplen, FILE *stream);

Just move the function up? If we are going to do this sort of forward
declaration there should be single block of them near the top somewhere.

> +
>  static void printf_info(int domid,
>                          libxl_domain_config *d_config,
>                          libxl_device_model_info *dm_info)
>  {
> -    int i;
> +    int i, nr_cpus = -1;
>      libxl_dominfo info;
> +    libxl_physinfo physinfo;
>  
>      libxl_domain_create_info *c_info = &d_config->c_info;
>      libxl_domain_build_info *b_info = &d_config->b_info;
>  
> +    if (libxl_get_physinfo(ctx, &physinfo) == 0)
> +        nr_cpus = physinfo.nr_cpus;
> +    else
> +        fprintf(stderr, "libxl_physinfo failed.\n");
> +
>      printf("(domain\n\t(domid %d)\n", domid);
>      printf("\t(create_info)\n");
>      printf("\t(hvm %d)\n", c_info->type == LIBXL_DOMAIN_TYPE_HVM);
> @@ -328,6 +336,9 @@ static void printf_info(int domid,
>  
>      printf("\t(build_info)\n");
>      printf("\t(max_vcpus %d)\n", b_info->max_vcpus);
> +    printf("\t(CPU affinity ");
> +    print_bitmap(b_info->cpumap.map, nr_cpus, stdout);
> +    printf(")\n");
>      printf("\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
>      printf("\t(max_memkb %d)\n", b_info->max_memkb);
>      printf("\t(target_memkb %d)\n", b_info->target_memkb);
> @@ -569,6 +580,8 @@ static void split_string_into_string_lis
>      free(s);
>  }
>  
> +static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap);

Again, maybe just reorder the functions?

>  static void parse_config_data(const char *configfile_filename_report,
>                                const char *configfile_data,
>                                int configfile_len,
> @@ -661,6 +674,13 @@ static void parse_config_data(const char
>      if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
>          b_info->max_vcpus = l;
>  
> +    if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {

The syntax supported here is different to that in a cpupool cfg? (see
main_cpupoolcreate) Perhaps they should be similar?

> +        char *buf2 = strdup(buf);
> +        vcpupin_parse(buf2, &b_info->cpumap);
> +    } else {
> +        memset(b_info->cpumap.map, -1, b_info->cpumap.size);

This could do with a helper function in libxl_utils.h.

Ian.


> +    }
> +
>      if (!xlu_cfg_get_long (config, "memory", &l, 0)) {
>          b_info->max_memkb = l * 1024;
>          b_info->target_memkb = b_info->max_memkb;
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel