Re: [PATCH v5 2/5] xl: improve return and exit codes of scheduling related functions

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

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

On Tue, 2015-10-27 at 21:08 +0530, Harmandeep Kaur wrote:
> Turning scheduling related functions exit codes towards using the
> EXIT_[SUCCESS|FAILURE] constants, instead of instead of arbitrary
> numbers
> or libxl return codes.
> 	- for main_*: arbitrary -> EXIT_SUCCESS|EXIT_FAILURE.
> 	- for internal fucntion: arbitrary -> 0/1.
> 
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v4->v5: Corrected coding style.
> v3->v4: No changes.
> v2->v3: Reverted removal of 'rc' in sched_domain_output().
>
Would you believe? When double checking specifically for coding style
issues, I found one more. :-/

Harman, do you mind sending v6 with this fixed as well. Keep the tags
(mine for sure, and I'm actually 99% sure of Wei's too, but, Wei, feel
free to say otherwise).

Sorry for noticing only now.

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 646b281..b101aff 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c

> @@ -6118,21 +6108,22 @@ int main_sched_credit(int argc, char **argv)
>              if (opt_r)
>                  scparam.ratelimit_us = ratelimit;
>  
> -            rc = sched_credit_params_set(poolid, &scparam);
> -            if (rc)
> -                return -rc;
> +            if (sched_credit_params_set(poolid, &scparam))
> +                return EXIT_FAILURE;
>          }
>      } else if (!dom) { /* list all domain's credit scheduler info */
> -        return -sched_domain_output(LIBXL_SCHEDULER_CREDIT,
> +        if (sched_domain_output(LIBXL_SCHEDULER_CREDIT,
>                                      sched_credit_domain_output,
>                                      sched_credit_pool_output,
> -                                    cpupool);
> +                                    cpupool))
>
Here. Parameters on new lines need to be aligned with the function's
opening '('.  I.e.:

if (sched_domain_output(LIBXL_SCHEDULER_CREDIT,
                        sched_credit_domain_output,
                        sched_credit_pool_output,
                        cpupool))

Of course, this applies to the other two call sites of
sched_domain_output() being refactored by the patch.

Oh, BTW, it seems that something happened to threading in v5 (patch 4
was not in reply to the cover letter, at least this is how I see it).
Previous versions were properly threaded, so I'm quite sure you know
how to do it properly, and wanted only to draw your attention on
it. :-)

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: 181 bytes --]

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

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