Re: [Xen-devel] [PATCH v4 12/15] libxl: get and set soft affinity

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On mer, 2013-11-27 at 14:45 +0000, Ian Campbell wrote:
> On Fri, 2013-11-22 at 19:58 +0100, Dario Faggioli wrote:
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 961d55e..4b871d7 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -4538,6 +4538,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, 
> > uint32_t domid,
> >      for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu, 
> > ++ptr) {
> >          if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0))
> >              return NULL;
> > +        if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap_soft, 0))
> > +            return NULL;
> 
> Leaks ptr and ptr->cpumap...
> 
Addressed in a separate patch (still part of the new version, v5, of the
series).

> > + */
> > +#define LIBXL_HAVE_SOFT_AFFINITY 1
> > +
> > +/*
> >   * LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
> >   * libxl_vendor_device field is present in the hvm sections of
> >   * libxl_domain_build_info. This field tells libxl which
> > @@ -994,9 +1002,35 @@ int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t 
> > domid,
> >  
> >  int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo);
> >  int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
> > -                           libxl_bitmap *cpumap);
> > +                           const libxl_bitmap *cpumap_hard,
> > +                           const libxl_bitmap *cpumap_soft);
> >  int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid,
> > -                               unsigned int max_vcpus, libxl_bitmap 
> > *cpumap);
> > +                               unsigned int max_vcpus,
> > +                               const libxl_bitmap *cpumap_hard,
> > +                               const libxl_bitmap *cpumap_soft);
> > +
> > +#if defined (LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x040400
> > +
> > +static inline
> > +int libxl_set_vcpuaffinity_0x040200(libxl_ctx *ctx, uint32_t domid,
> > +                                    uint32_t vcpuid, libxl_bitmap *cpumap)
> > +{
> > +    return libxl_set_vcpuaffinity(ctx, domid, vcpuid, cpumap, NULL);
> > +}
> > +
> > +static inline
> > +int libxl_set_vcpuaffinity_all_0x040200(libxl_ctx *ctx, uint32_t domid,
> > +                                        unsigned int max_vcpus,
> > +                                        libxl_bitmap *cpumap)
> > +{
> > +    return libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, cpumap, NULL);
> > +}
> > +
> > +#define libxl_set_vcpuaffinity libxl_set_vcpuaffinity_0x040200
> > +#define libxl_set_vcpuaffinity_all libxl_set_vcpuaffinity_all_0x040200
> 
> This looks correct, thanks. It could also have jsut been #defined since
> the inline is only used in the libxl_domain_create_restore_0x040200 case
> because there is a local variable.
>
Yes, I saw that.

You mean something like this:

#define libxl_set_vcpuaffinity(_ctx, _domid, _vcpuid, _map) \
    libxl_set_vcpuaffinity((_ctx), (_domid), (_vcpuid), (_map), NULL)

?

Well, I guess I can, but don't I more protection for the arguments /
risk to clash with the app namespace, if I do?

That's the main reason why I used the inline variant, and that is true
for v5 as well. If you prefer to go with the `#define' only, I can
respin (either the series or the patch).

Thanks and 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