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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-01-23 at 18:22 +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 d76603510485 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c     Mon Jan 23 18:02:42 2012 +0000
> +++ b/tools/libxl/libxl.c     Mon Jan 23 18:02:47 2012 +0000
> @@ -2663,6 +2663,20 @@ int libxl_set_vcpuaffinity(libxl_ctx *ct
>      return 0;
>  }
>  
> +int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid,
> +                               unsigned int max_vcpus, libxl_cpumap *cpumap)
> +{
> +    int i, rc = 0;
> +
> +    for (i = 0; i < max_vcpus; i++) 
> +        if (libxl_set_vcpuaffinity(ctx, domid, i, cpumap)) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "no affinity for cpu %d", i);

"failed to set affinity for ..." would better describe what had happened.

> +            rc = ERROR_FAIL;
> +        }
> +    }
> +    return rc;
> +}
> +
>  int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_cpumap 
> *cpumap)
>  {
>      GC_INIT(ctx);
[...]
>                                           if (libxl_cpumap_test(&(m), v))
> diff -r d76603510485 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Mon Jan 23 18:02:42 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c        Mon Jan 23 18:02:47 2012 +0000
> @@ -288,16 +288,72 @@ 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)
> +{
[...]

I assumed this was pure code motion so I didn't read it.

> +}
> +
>  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 +384,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);

If libxl_get_physinfo failed then nr_cpus would == -1 here.

However I don't think it is even necessary to extend this (legacy) sexp
any further. I'm about to send a patch which will use the print the json
representation (which is autogenerated).

If we were to extend it then it should match the xend/xm output since
it's only purpose is for xm compatibility.

> +    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 +628,8 @@ static void split_string_into_string_lis
>      free(s);
>  }
>  
> +static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap);

There are no calls to vcpupin_parse added after this point by this patch
so is this necessary? I'd prefer moving the function up in any case or
at least declaring all the forward references near the top of the file.

Ian.


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