Re: [Xen-devel] [PATCH v3 1/7] libxl: get rid of the SEDF scheduler

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Mon, 2015-07-06 at 18:17 +0200, Dario Faggioli wrote:
> > > @@ -356,9 +357,13 @@ libxl_domain_sched_params = 
> > > Struct("domain_sched_params",[
> > >      ("weight",       integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
> > >      ("cap",          integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT'}),
> > >      ("period",       integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT'}),
> > > -    ("slice",        integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),
> > > -    ("latency",      integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
> > > -    ("extratime",    integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
> > > +    # The following three parameters ('slice', 'latency' and 
> > > 'extratime') are deprecated,
> > > +    # and will have no effect if used, since the SEDF scheduler has been 
> > > removed.
> > > +    # Note that 'period' was an SDF parameter too, but it is still 
> > > effective as it is
> > > +    # now used (together with 'budget') by the RTDS scheduler.
> > > +    ("slice",        integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),     # deprecated
> > > +    ("latency",      integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),   # deprecated
> > > +    ("extratime",    integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}), # deprecated
> > >      ("budget",       integer, {'init_val': 
> > > 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT'}),
> > 
> > Since we're aiming for API compatibility rather than ABI compatibility,
> > is it allowable to move 'budget' up above the comment, so that it's more
> > obvious that it hasn't been deprecated?
> > 
> It's tool's people call, I guess. My opinion is that, yes, it should be
> possible without any issue, and yes, I also would like the end result
> better.

Yes, I think you can move it up.

You should also add a blank line before "# The following three" and you
can now also drop the per line "# deprecated" since it will be visually
obvious which three the bigger comment refers to.

Nit:
>     (-25, "FEATURE_REMOVED"), # For functionallities that are no longer there

The correct spelling would be "functionalities", but the correct meaning
would be "functionality that is", I'd probably also go with "that has
been removed".

Unless there is some reason to resend I can fix that on commit:
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Ian.




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