Re: [PATCH v4 13/15] xl: enable getting and setting soft

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2637 bytes --]

On mer, 2013-11-27 at 14:57 +0000, Ian Campbell wrote:
> On Fri, 2013-11-22 at 19:58 +0100, Dario Faggioli wrote:
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -4630,23 +4649,29 @@ static int vcpupin(uint32_t domid, const char *vcpu, char *cpu)
> >      }
> >  
> >      if (vcpuid != -1) {
> > -        if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap, NULL)) {
> > -            fprintf(stderr, "Could not set affinity for vcpu `%u'.\n", vcpuid);
> > +        if (!soft_affinity &&
> > +            libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap, NULL)) {
> > +            fprintf(stderr, "Could not set hard affinity for vcpu `%u'.\n",
> > +                    vcpuid);
> > +            goto out;
> > +        } else if (soft_affinity &&
> > +                   libxl_set_vcpuaffinity(ctx, domid, vcpuid, NULL, &cpumap)) {
> > +            fprintf(stderr, "Could not set soft affinity for vcpu `%u'.\n",
> > +                    vcpuid);
> 
> I think I would have written this as:
> 	libxl_bitmap *soft = NULL, *hard = NULL;
> 	if (soft_affinity)
> 		soft = &cpumap
> 	else
> 		hard = &cpumap
> 
> The in both of the branches "if (vcpu != -1)":
> 	libxl_set_vcpuaffinity(..., hard, soft)
> 		error handling
> 
> I think you can omit the soft/hard from the log message other logging
> and/or the fact that they know what they asked for will cover the
> distinction. If you really wanted the const char *what = soft_affinity ?
> "soft" : "hard" to use.
> 
Done this, and all the that was suggested in this e-mail.

> Is there no option to set both?
> 
No, not for now. Do you think there should be one?

Personally, I think we should keep `xl vcpu-pin <dom> <vcpu> <affinity>'
working, and doing the thing that people are most used with, i.e.,
setting the pinning (hard affinity). At that point, the most natural
thing to do seemed to add a switch to have this deal with soft affinity,
which is what happens in this patch.

I of course can come up with pretty much all sort of behavior, e.g.,
setting both hard and soft affinity to the same cpumap, if no switch is
provided, and then introduce both a '-h' and a '-s', for specifying
different cpumaps, at the same time, etc.

Of course, that could well happen during 4.5, I guess. :-P

So, what do we want?

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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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