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

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Dario Faggioli writes ("[PATCH v2 10/10] xl: allow to set the ratelimit value 
online for Credit2"):
> Last part of the wiring necessary for allowing to
> change the value of the ratelimit_us parameter online,
> for Credit2 (like it is already for Credit1).
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Although I do have a comment:

> +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.

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.
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.

But it was a thing I noticed which I wanted to write down and/or have
your opinion on.

Thanks,
Ian.

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