From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] libxl: libxl_domain_sched_params_set case for ARINC 653 scheduler Date: Wed, 25 Jul 2012 10:15:21 +0100 Message-ID: <500FB929.6070501@eu.citrix.com> References: <857a035d6a4a835bb258.1343149194@arlpc33> <1343154690.13198.6.camel@Abyss> <8625277B-3B64-4473-A1AB-765E561DDC09@dornerworks.com> <1343170114.13198.49.camel@Abyss> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1343170114.13198.49.camel@Abyss> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: Andrew Kane , "xen-devel@lists.xensource.com" , Steve VanderLeest , Ian Campbell List-Id: xen-devel@lists.xenproject.org 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