Re: [PATCH v2 10/10] xl: allow to set the ratelimit value online for Credit2

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2697 bytes --]

On Fri, 2016-09-30 at 11:34 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("[PATCH v2 10/10] xl: allow to set
> theratelimit value online for Credit2"):
> >
> > +static int sched_credit2_params_set(int poolid,
> > +                                    libxl_sched_credit2_params
> > *scinfo)
> 
> Each of this and the corresponding _get calls the corresponding libxl
> function, prints a message, and adjusts the return value.
> 
> But: AFAICT each of these has only one call site; and each of the
> underlying libxl functions logs an error too; and each of the call
> sites prints a message too.
> 
> So I question whether these functions are valuable.
> 
So, about this last point (utility of sched_*_params_{get,set}() ), I
totally agree. I think the reason why the functions exist is for the
most part related to convenience and style. E.g., maybe
indentation/nesting level was getting too deep within the various
main_sched_*().

As said in another email, I don't especially like those functions, and
I have the feeling they're a better way to write them, but I haven't
really try. I'll put having a look at that in my todo list, but not at
super high priority. :-P

> At this stage of the release cycle, and the maturity of this series,
> I
> guess you probably don't want to start messing with the error paths.
>
Yeah, indeed, thanks for understanding! :-P

Although...

> The messages that will come out will be overly verbose rather than
> inadequate, so the code as it is fine if not as good as it could be.
> 
...I'm not entirely sure I get what you're saying here.

I mean, AFAICT, there's xl calling a libxl function and printing an
error if it fails. The libxl function also logs an error --true-- but
this seems rather common to me, and apart from that, it looks
reasonable as well, isn't it so?

Avoiding double/overly verbose logging from either within xl or within
libxl is something I totally understand and agree. But when the
'border' is crossed, is it that terrible that both do log? After all,
how can xl be sure that libxl will log something? And even if he could,
I see the two as different kind and different level of logging.

Or was it something else that you were referring to?

> But it was a thing I noticed which I wanted to write down and/or have
> your opinion on.
> 
Sure! :-)

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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