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

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

On Tue, 2015-07-07 at 15:19 +0100, George Dunlap wrote:
> On 07/07/2015 03:15 PM, Ian Campbell wrote:
> > 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!
> 
> Oh, took you to mean that you were offering to make both modifications
> on check-in, and I thought "Well, that's generous of him."  I guess
> you're not so generous after all. ;-)

Not so much generous as absent minded I'm afraid, sorry Dario...