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

[Dario Faggioli <raistlin@xxxxxxxx>]

On Thu, 2012-01-12 at 08:43 +0000, Ian Campbell wrote: 
> > 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?
> 
Having this set to "none" here simplifies a bit the code when we came to
config file parsing (if I set it to "any CPU" here, I'll have to reset
to "none" before parsing). Also, default is exactly what you're
suggesting already, as I set the map to "any CPU" if no "cpus" config
option is found.

Anyway, I guess I can do as you suggest if you really think it's
better. :-)

> > +static void print_bitmap(uint8_t *map, int maplen, FILE *stream);
>
> [..] 
>   
> > +static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap);
> 
> Again, maybe just reorder the functions?
> 
Ok, fine, will do that.

> >  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?
> 
It is different, and that was intentional. What I wanted, was the syntax
to be exactly the same of `xl vcpu-pin', which is the command line
equivalent of this option, much more than cpupool-*. If you want, I
think I can easily enable list-like CPU specification as in cpupool
config file so that _both_ syntax are allowed. What do you think?


> > +        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.
> 
Ok.

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)

Attachment: signature.asc
Description: This is a digitally signed message part

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