Re: [PATCH] libxl: libxl_domain_sched_params_set case for ARINC 653 scheduler

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

On 24/07/12 23:48, Dario Faggioli wrote:
> On Tue, 2012-07-24 at 15:09 -0400, Andrew Kane wrote:
>>>> +static int sched_arinc653_domain_set(libxl__gc *gc, uint32_t domid,
>>>> +                                     const libxl_domain_sched_params *scinfo)
>>>> +{
>>>> +    // Currently, the ARINC 653 scheduler does not take any domain-specific
>>>> +    //     configuration, so we simply return success.
>>>>
>>> I think using C (/* */) style for comment is highly recommended, if not
>>> required. :-)
>> Oops. That's what I get for trusting the editor with comments. =)
>>
> :-)
>
>> Our thought was to define this following the structure that exists for the
>> other schedulers, both for consistency and to facilitate future work
>> on the ARINC 653 scheduler.
>>
> Yeah, I got that, and it's not bad thinking actually.
>
> Thinking a bit more about this, right below
> libxl_domain_sched_params_set() (in libxl.c) there is another function
> called libxl_domain_sched_params_get(), doing pretty much the same
> thing, although of course it retrieves instead of setting.
>
> Shouldn't you be doing something similar to that too?
>
>> If/when we actually need domain-specific configuration like this,
>> it would only involve changes in the sched_arinc653_domain_set
>> function, and wouldn't require any changes to
>> libxl_domain_sched_params_set.
>>
>> If the preference is to hold off on implementing a
>> sched_arinc653_domain_set function until there's actually something
>> to put in it, I'm happy to change it. =)
>>
> It's mostly a matter of taste I guess.
>
> The way I pointed is my preference, but I really don't care that much.
> If you send a patch with proper commenting (and perhaps dealing with the
> *_get() path), I'll ack it no matter if you have those empty functions
> or not... Which will then mean it'll be up to Goerge and Ian (added to
> the Cc list) to decide what they like better. :-)
Yeah, the comment needs to be changed to C-style, but I don't think 
calling a function or not is worth the trouble of talking about. :-)

  -George