Re: [PATCH 10 of 10] xl: Implement sched-credit schedule parameter command-line interface

[George Dunlap <george.dunlap@citrix.com>]

On Tue, 2012-02-21 at 17:56 +0000, Ian Jackson wrote:
> George Dunlap writes ("[Xen-devel] [PATCH 10 of 10] xl: Implement sched-credit schedule parameter command-line interface"):
> > Add features to the sched-credit interface to allow querying and
> > displaying scheduler parameters.  New interface works as follows:
> > 
> >  <nothing>             : List all domain params and sched params from all pools
> >  -d [domid]            : List domain params for domain
> >  -d [domid] [params]   : Set domain params for domain
> >  -p [pool]             : list all domains and sched params for pool
> >  -s                    : List sched params for poolid 0
> >  -s [params]           : Set sched params for poolid 0
> >  -p [pool] -s          : List sched params for pool
> >  -p [pool] -s [params] : Set sched params for pool
> >  -p [pool] -d...       : Illegal
> 
> This information should be in the xl manpage, rather than in the
> commit message and a code comment.

Of course.  And, is there some other documentation that this should be
put into as well?  

> 
> > +static int sched_credit_param_set(
> > +    int poolid, libxl_sched_credit_param *scinfo)
> > +{
> 
> Formatting.  It should be
> 
> static int sched_credit_param_set(int poolid, libxl_sched_credit_param *scinfo)
> 
> (79 columns) or
> 
> static int sched_credit_param_set(int poolid,
>                                   libxl_sched_credit_param *scinfo)
> 
> (67 columns).
> 
> This should be changed throughout I'm afraid.

Ack.

> 
> > +        } else { /* Set scheduling parameters*/
> > +            rc = sched_credit_param_get(poolid, &scparam);
> 
> What happens if the pool isn't using the credit scheduler ?

Ah, good point.  I suppose it should just print the pool name.

What happens to output of domains when you use xl sched-credit and there
is a pool that's not using the credit scheduler?  It looks like it will
stop at that point and quit processing.  It seems like having it print
the domain ID and "[not sched-credit]" or something like that would be
better.

What would be *even* better is to unify these all somehow, so that xl
would automatically detect the appropriate scheduler for the pool and
behave accordingly.  But that's a patch series for another time, I
think.

 -George