Re: [Xen-devel] [PATCH 14/24] libxl: allow to set the ratelimit value online for Credit2

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Dario Faggioli writes ("[PATCH 14/24] libxl: allow to set the ratelimit value 
online for Credit2"):
...
> -    rc = xc_sched_credit_params_set(ctx->xch, poolid, &sparam);
> -    if ( rc < 0 ) {
> -        LOGE(ERROR, "setting sched credit param");
> -        GC_FREE;
> -        return ERROR_FAIL;
> +    r = xc_sched_credit_params_set(ctx->xch, poolid, &sparam);
> +    if ( r < 0 ) {
> +        LOGE(ERROR, "Setting Credit scheduler parameters");
> +        rc = ERROR_FAIL;
> +        goto out;

I had to read this three times to figure out what the change was.

It is good that you are fixing the coding style but can you please put
it in a separate patch ?

In general I was surprised at how large this patch, and the
corresponding xl one, was.  Perhaps after the coding style fixes are
split off the functional patches will be smaller.

But I wonder whether there will still be lots of rather formulaic code
that could profitably be generalised somehow.  I'd appreciate your
views on whether that would be possible, and whether it would be a
good idea..

For now I will stop my review here.

Thanks,
Ian.

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