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

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-07-07 at 14:48 +0100, Ian Campbell wrote:
> 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

Which of course I gave myself in the first couple of paras!

>  I can fix that on commit:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Ian.
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel