Re: [Xen-devel] [PATCH V5 03/32] xl / libxl: push VCPU affinity pinning down to libxl

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On gio, 2014-05-15 at 10:24 +0100, Wei Liu wrote:
> > One question: I've skimmed through the rest of the series, and it looks
> > like you are not doing anything particular with the other maps, already
> > existing in b_info (cpumap and nodemap) yet, is that right?
> > 
> 
> Correct. I found out it's not necessary to touch them.
> 
Great! I looked at the code and gave it a quick try and came to similar
conclusions.

> > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > > index 661999c..b818815 100644
> > > --- a/tools/libxl/libxl_dom.c
> > > +++ b/tools/libxl/libxl_dom.c
> > > @@ -263,6 +263,54 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> > >      libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);
> > >      libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, 
> > > &info->cpumap);
> > >  
> > > +    /* If we have vcpu affinity list, pin vcpu to pcpu. */
> > > +    if (d_config->b_info.vcpu_affinity) {
> > > +        int i;
> > > +        libxl_bitmap vcpu_cpumap;
> > > +        int *vcpu_to_pcpu, sz = sizeof(int) * d_config->b_info.max_vcpus;
> > > +
> > > +        vcpu_to_pcpu = libxl__zalloc(gc, sz);
> > > +        memset(vcpu_to_pcpu, -1, sz);
> > > +
> > > +        for (i = 0; i < d_config->b_info.max_vcpus; i++) {
> > > +            libxl_key_value_list kvl = d_config->b_info.vcpu_affinity;
> > > +            const char *key, *val;
> > > +            int k, v;
> > > +
> > > +            key = kvl[i * 2];
> > > +            if (!key)
> > > +                break;
> > > +            val = kvl[i * 2 + 1];
> > > +
> > > +            k = atoi(key);
> > > +            v = atoi(val);
> > > +            vcpu_to_pcpu[k] = v;
> > > +        }
> > > +
> > > +        rc = libxl_cpu_bitmap_alloc(ctx, &vcpu_cpumap, 0);
> > > +        if (rc) {
> > > +            libxl_bitmap_dispose(&vcpu_cpumap);
> > > +            return ERROR_FAIL;
> > > +        }
> > > +
> > Perhaps you can move this up, i.e., try the allocation first so that, if
> > it fails, you can bail before getting into decoding the key/value list?
> > 
> 
> That's more or less a personal taste...
> 
Indeed it is.

Regards,
Dario

> > Also, if libxl_cpu_bitmap_alloc() fails, do you really have to call
> > libxl_bitmap_dispose()? I thought not.
> > 
> 
> The paradigm of using libxl_type is to always call dispose. 
> 
Alright, according to that same usage paradigm, aren't you missing an
libxl_bitmap_init(&vcpu_cpumap) then?

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel