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

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Dario Faggioli writes ("[PATCH v2 09/10] libxl: allow to set the ratelimit 
value online for Credit2"):
> This is the remaining part of the plumbing (the libxl
> one) necessary to be able to change the value of the
> ratelimit_us parameter online, for Credit2 (like it is
> already for Credit1).

Thanks.  I have some coding style nits, all but one of which are
utterly trivial.  If it hadn't been for the `rc=0' one I would have
acked this patch as-is.

> +int libxl_sched_credit2_params_set(libxl_ctx *ctx, uint32_t poolid,
> +                                   libxl_sched_credit2_params *scinfo)
> +{
> +    struct xen_sysctl_credit2_schedule sparam;
> +    int r, rc;
> +    GC_INIT(ctx);
> +
> +    rc = sched_ratelimit_check(gc, scinfo->ratelimit_us);
> +    if (rc) {
> +        goto out;
> +    }

When you respin this, I think IWBNI you could change this to
      if (rc)
          goto out;
or
      if (rc) goto out;
both of which are encouraged by the coding style, and are more common
inside libxl.

An earlier hunk in this patch does not adjust from a similar construct
where the { } are becoming newly unnecessary.  You may want to adjust
that too, but I won't object if you don't feel like it.

> +
> +    sparam.ratelimit_us = scinfo->ratelimit_us;
> +
> +    r = xc_sched_credit2_params_set(ctx->xch, poolid, &sparam);
> +    if ( r < 0 ) {
           ^     ^
Coding style: libxl does not use the spaces just inside the ( ).

> +        LOGE(ERROR, "Setting Credit2 scheduler parameters");
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    scinfo->ratelimit_us = sparam.ratelimit_us;
> +
> + out:
> +    GC_FREE;
> +    return rc;

This should have an explicit assignment rc=0 before the out label.  As
it is, it will happen to be 0 anyway, so that's a stylistic fix.

Thanks,
Ian.

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