xl: convert exit codes to EXIT_[SUCCESS|FAILURE]
v5 cover xl: convert exit codes to EXIT_[SUCCESS|FAILURE]
*Its my "bite size contribution" that is required for applying to the Outreachy program.
*main_foo() is treated somewhat as a regular main(), it is changed to return EXIT_SUCCESS or EXIT_FAILURE.
*Functions that are not main_foo(), are changed to do 'return 0' or 'return 1', and then 0/1 is taken care in the main_foo() functions that calls them.
*Functions in xl.c and xl_cmdimpl.c are changed.
*Functions related to scheduling, vcpu, cpupool and parsing are currently changed excluding parse_config_data() which is big enough to deserve its own patch.
*Some discussions about this patch:
https://www.mail-archive.com/[email protected]/msg42850.html
*v1 of this patch:
http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02497.html
*v2 of this patch:
http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02623.html
*v3 of this patch:
http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02872.html
*v4 of this patch:
http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02947.html
---
PATCH v5 1/5 xl: convert main() exit codes to EXIT_[SUCCESS|FAILURE]
Turning main() function exit codes towards using the EXIT_[SUCCESS|FAILURE] constants, instead of instead of arbitrary numbers or libxl return codes.
Also includes a document comment in xl.h stating xl process should always return EXIT_FOO and main_* can be treated as main() as if they are returning a process exit status and not a function return value)
---v4->v5: No changes.v3->v4: Aligned comment correctly.v2->v3: Commented more precisely and aligned comment properly. v1->v2: Applied new approach (main() functions return
EXIT_SUCCESS/FAILURE and other functions return 0/1).
tools/libxl/xl.c | 12 ++++++------ tools/libxl/xl.h | 7 +++++++ 2 files changed, 13 insertions(+), 6 deletions(-)
tools/libxl/xl.c +6 -6
@@ -318,7 +318,7 @@ int main(int argc, char **argv)break ;default :fprintf (stderr,"unknown global option\n" ); -exit (2 ); +exit (EXIT_FAILURE ); } }
@@ -326,13 +326,13 @@ int main(int argc, char **argv)if (!cmd) {help (NULL ); -exit (1 ); +exit (EXIT_FAILURE ); } opterr =0 ; logger =xtl_createlogger_stdiostream (stderr, minmsglevel, (progress_use_cr ?XTL_STDIOSTREAM_PROGRESS_USE_CR :0 )); -if (!logger)exit (1 ); +if (!logger)exit (EXIT_FAILURE );atexit (xl_ctx_free);
@@ -355,16 +355,16 @@ int main(int argc, char **argv)if (cspec) {if (dryrun_only && !cspec->can_dryrun) {fprintf (stderr,"command does not implement -N (dryrun) option\n" ); - ret =1 ; + ret =EXIT_FAILURE ; goto xit; } ret = cspec->cmd_impl (argc, argv); }else if (!strcmp (cmd,"help" )) {help (argv[1 ]); - ret =0 ; + ret =EXIT_SUCCESS ; }else {fprintf (stderr,"command not implemented\n" ); - ret =1 ; + ret =EXIT_FAILURE ; } xit:
tools/libxl/xl.h +7 -0
@@ -30,6 +30,13 @@ struct cmd_spec {char *cmd_option; }; +/* +* The xl process should always return either EXIT_SUCCESS or +* EXIT_FAILURE. main_* functions, implementing the various xl +* commands, can be treated as main() as if they are returning +* a process exit status and not a function return value. +*/ +int main_vcpulist (int argc,char **argv);int main_info (int argc,char **argv);int main_sharing (int argc,char **argv);
Signature · 1 line
1.9.1
PATCH v5 2/5 xl: improve return and exit codes of scheduling related functions
In reply to Harmandeep KaurTurning 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.
---v4->v5: Corrected coding style.v3->v4: No changes.v2->v3: Reverted removal of 'rc' in sched_domain_output().
tools/libxl/xl_cmdimpl.c | 139 +++++++++++++++++++++++------------------------ 1 file changed, 67 insertions(+), 72 deletions(-)
tools/libxl/xl_cmdimpl.c +67 -72
@@ -5793,18 +5793,15 @@ int main_sharing(int argc, char **argv)static int sched_domain_get (libxl_scheduler sched,int domid,libxl_domain_sched_params *scinfo) { -int rc; - - rc =libxl_domain_sched_params_get (ctx, domid, scinfo); -if (rc) { +if (libxl_domain_sched_params_get (ctx, domid, scinfo)) {fprintf (stderr,"libxl_domain_sched_params_get failed.\n" ); -return rc; +return 1 ; }if (scinfo->sched != sched) {fprintf (stderr,"libxl_domain_sched_params_get returned %s not %s.\n" ,libxl_scheduler_to_string (scinfo->sched),libxl_scheduler_to_string (sched)); -return ERROR_INVAL ; +return 1 ; }return 0 ;
@@ -5812,42 +5809,38 @@ static int sched_domain_get(libxl_scheduler sched, int domid,static int sched_domain_set (int domid,const libxl_domain_sched_params *scinfo) { -int rc; - - rc =libxl_domain_sched_params_set (ctx, domid, scinfo); -if (rc) +if (libxl_domain_sched_params_set (ctx, domid, scinfo)) {fprintf (stderr,"libxl_domain_sched_params_set failed.\n" ); +return 1 ; + } -return rc; +return 0 ; }static int sched_credit_params_set (int poolid,libxl_sched_credit_params *scinfo) { -int rc; - - rc =libxl_sched_credit_params_set (ctx, poolid, scinfo); -if (rc) +if (libxl_sched_credit_params_set (ctx, poolid, scinfo)) {fprintf (stderr,"libxl_sched_credit_params_set failed.\n" ); +return 1 ; + } -return rc; +return 0 ; }static int sched_credit_params_get (int poolid,libxl_sched_credit_params *scinfo) { -int rc; - - rc =libxl_sched_credit_params_get (ctx, poolid, scinfo); -if (rc) +if (libxl_sched_credit_params_get (ctx, poolid, scinfo)) {fprintf (stderr,"libxl_sched_credit_params_get failed.\n" ); +return 1 ; + } -return rc; +return 0 ; }static int sched_credit_domain_output (int domid) {char *domname;libxl_domain_sched_params scinfo; -int rc;if (domid <0 ) {printf ("%-33s %4s %6s %4s\n" ,"Name" ,"ID" ,"Weight" ,"Cap" );
@@ -5855,9 +5848,10 @@ static int sched_credit_domain_output(int domid) }libxl_domain_sched_params_init (&scinfo); - rc =sched_domain_get (LIBXL_SCHEDULER_CREDIT , domid, &scinfo); -if (rc) -return rc; +if (sched_domain_get (LIBXL_SCHEDULER_CREDIT , domid, &scinfo)) { +libxl_domain_sched_params_dispose (&scinfo); +return 1 ; + } domname =libxl_domid_to_name (ctx, domid); printf("%-33s %4d %6d %4d\n", domname,
@@ -5873,11 +5867,9 @@ static int sched_credit_pool_output(uint32_t poolid) {libxl_sched_credit_params scparam;char *poolname; -int rc; poolname =libxl_cpupoolid_to_name (ctx, poolid); - rc =sched_credit_params_get (poolid, &scparam); -if (rc) { +if (sched_credit_params_get (poolid, &scparam)) {printf ("Cpupool %s: [sched params unavailable]\n" , poolname); }else {
@@ -5895,7 +5887,6 @@ static int sched_credit2_domain_output( {char *domname;libxl_domain_sched_params scinfo; -int rc;if (domid <0 ) {printf ("%-33s %4s %6s\n" ,"Name" ,"ID" ,"Weight" );
@@ -5903,9 +5894,10 @@ static int sched_credit2_domain_output( }libxl_domain_sched_params_init (&scinfo); - rc =sched_domain_get (LIBXL_SCHEDULER_CREDIT2 , domid, &scinfo); -if (rc) -return rc; +if (sched_domain_get (LIBXL_SCHEDULER_CREDIT2 , domid, &scinfo)) { +libxl_domain_sched_params_dispose (&scinfo); +return 1 ; + } domname =libxl_domid_to_name (ctx, domid); printf("%-33s %4d %6d\n", domname,
@@ -5921,7 +5913,6 @@ static int sched_rtds_domain_output( {char *domname;libxl_domain_sched_params scinfo; -int rc =0 ;if (domid <0 ) {printf ("%-33s %4s %9s %9s\n" ,"Name" ,"ID" ,"Period" ,"Budget" );
@@ -5929,9 +5920,10 @@ static int sched_rtds_domain_output( }libxl_domain_sched_params_init (&scinfo); - rc =sched_domain_get (LIBXL_SCHEDULER_RTDS , domid, &scinfo); -if (rc) - goto out; +if (sched_domain_get (LIBXL_SCHEDULER_RTDS , domid, &scinfo)) { +libxl_domain_sched_params_dispose (&scinfo); +return 1 ; + } domname =libxl_domid_to_name (ctx, domid); printf("%-33s %4d %9d %9d\n",
@@ -5940,10 +5932,8 @@ static int sched_rtds_domain_output( scinfo.period, scinfo.budget);free (domname); - -out:libxl_domain_sched_params_dispose (&scinfo); -return rc; +return 0 ; }static int sched_rtds_pool_output (uint32_t poolid)
@@ -5981,7 +5971,7 @@ static int sched_domain_output(libxl_scheduler sched, int (*output)(int),if (libxl_cpupool_qualifier_to_cpupoolid (ctx, cpupool, &poolid,NULL ) || !libxl_cpupoolid_is_valid (ctx, poolid)) {fprintf (stderr,"unknown cpupool \'%s\'\n" , cpupool); -return -ERROR_FAIL ; +return 1 ; } }
@@ -5994,7 +5984,7 @@ static int sched_domain_output(libxl_scheduler sched, int (*output)(int),if (!poolinfo) {fprintf (stderr,"error getting cpupool info\n" );libxl_dominfo_list_free (info, nb_domain); -return -ERROR_NOMEM ; +return 1 ; }for (p =0 ; !rc && (p < n_pools); p++) {
@@ -6080,16 +6070,16 @@ int main_sched_credit(int argc, char **argv)if ((cpupool || opt_s) && (dom || opt_w || opt_c)) {fprintf (stderr,"Specifying a cpupool or schedparam is not " "allowed with domain options.\n" ); -return 1 ; +return EXIT_FAILURE ; }if (!dom && (opt_w || opt_c)) {fprintf (stderr,"Must specify a domain.\n" ); -return 1 ; +return EXIT_FAILURE ; }if (!opt_s && (opt_t || opt_r)) {fprintf (stderr,"Must specify schedparam to set schedule " "parameter values.\n" ); -return 1 ; +return EXIT_FAILURE ; }if (opt_s) {
@@ -6101,16 +6091,16 @@ int main_sched_credit(int argc, char **argv) &poolid, NULL) || !libxl_cpupoolid_is_valid(ctx, poolid)) {fprintf (stderr,"unknown cpupool \'%s\'\n" , cpupool); -return -ERROR_FAIL ; +return EXIT_FAILURE ; } }if (!opt_t && !opt_r) {/* Output scheduling parameters */ -return -sched_credit_pool_output (poolid); +if (sched_credit_pool_output (poolid)) +return EXIT_FAILURE ; }else {/* Set scheduling parameters*/ - rc =sched_credit_params_get (poolid, &scparam); -if (rc) -return -rc; +if (sched_credit_params_get (poolid, &scparam)) +return EXIT_FAILURE ;if (opt_t) scparam.tslice_ms = tslice;
@@ -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)) +return EXIT_FAILURE ; }else {uint32_t domid =find_domain (dom);if (!opt_w && !opt_c) {/* output credit scheduler info */ sched_credit_domain_output (-1 ); -return -sched_credit_domain_output (domid); +if (sched_credit_domain_output (domid)) +return EXIT_FAILURE ; }else {/* set credit scheduler paramaters */ libxl_domain_sched_params scinfo;libxl_domain_sched_params_init (&scinfo);
@@ -6144,11 +6135,11 @@ int main_sched_credit(int argc, char **argv) rc =sched_domain_set (domid, &scinfo);libxl_domain_sched_params_dispose (&scinfo);if (rc) -return -rc; +return EXIT_FAILURE ; } } -return 0 ; +return EXIT_SUCCESS ; }int main_sched_credit2 (int argc,char **argv)
@@ -6180,24 +6171,26 @@ int main_sched_credit2(int argc, char **argv)if (cpupool && (dom || opt_w)) {fprintf (stderr,"Specifying a cpupool is not allowed with other " "options.\n" ); -return 1 ; +return EXIT_FAILURE ; }if (!dom && opt_w) {fprintf (stderr,"Must specify a domain.\n" ); -return 1 ; +return EXIT_FAILURE ; }if (!dom) {/* list all domain's credit scheduler info */ -return -sched_domain_output (LIBXL_SCHEDULER_CREDIT2 , +if (sched_domain_output (LIBXL_SCHEDULER_CREDIT2 , sched_credit2_domain_output, sched_default_pool_output, - cpupool); + cpupool)) +return EXIT_FAILURE ; }else {uint32_t domid =find_domain (dom);if (!opt_w) {/* output credit2 scheduler info */ sched_credit2_domain_output (-1 ); -return -sched_credit2_domain_output (domid); +if (sched_credit2_domain_output (domid)) +return EXIT_FAILURE ; }else {/* set credit2 scheduler paramaters */ libxl_domain_sched_params scinfo;libxl_domain_sched_params_init (&scinfo);
@@ -6207,11 +6200,11 @@ int main_sched_credit2(int argc, char **argv) rc =sched_domain_set (domid, &scinfo);libxl_domain_sched_params_dispose (&scinfo);if (rc) -return -rc; +return EXIT_FAILURE ; } } -return 0 ; +return EXIT_SUCCESS ; }/*
@@ -6256,27 +6249,29 @@ int main_sched_rtds(int argc, char **argv)if (cpupool && (dom || opt_p || opt_b)) {fprintf (stderr,"Specifying a cpupool is not allowed with " "other options.\n" ); -return 1 ; +return EXIT_FAILURE ; }if (!dom && (opt_p || opt_b)) {fprintf (stderr,"Must specify a domain.\n" ); -return 1 ; +return EXIT_FAILURE ; }if (opt_p != opt_b) {fprintf (stderr,"Must specify period and budget\n" ); -return 1 ; +return EXIT_FAILURE ; }if (!dom) {/* list all domain's rt scheduler info */ -return -sched_domain_output (LIBXL_SCHEDULER_RTDS , +if (sched_domain_output (LIBXL_SCHEDULER_RTDS , sched_rtds_domain_output, sched_rtds_pool_output, - cpupool); + cpupool)) +return EXIT_FAILURE ; }else {uint32_t domid =find_domain (dom);if (!opt_p && !opt_b) {/* output rt scheduler info */ sched_rtds_domain_output (-1 ); -return -sched_rtds_domain_output (domid); +if (sched_rtds_domain_output (domid)) +return EXIT_FAILURE ; }else {/* set rt scheduler paramaters */ libxl_domain_sched_params scinfo;libxl_domain_sched_params_init (&scinfo);
@@ -6287,11 +6282,11 @@ int main_sched_rtds(int argc, char **argv) rc =sched_domain_set (domid, &scinfo);libxl_domain_sched_params_dispose (&scinfo);if (rc) -return -rc; +return EXIT_FAILURE ; } } -return 0 ; +return EXIT_SUCCESS ; }int main_domid (int argc,char **argv)
Signature · 1 line
1.9.1
Re: [PATCH v5 2/5] xl: improve return and exit codes of scheduling related functions
Quoted 7 lines · Harmandeep Kaur
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.
Quoted 8 lines
Signed-off-by Harmandeep Kaur <[email protected]>Reviewed-by Dario Faggioli <[email protected]>Acked-by Wei Liu <[email protected]>---
v4->v5: Corrected coding style.v3->v4: No changes.v2->v3: Reverted removal of 'rc' insched_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.
Quoted 13 lines
tools/libxl/xl_cmdimpl.c +1
-3@@ -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))
tools/libxl/xl_cmdimpl.c int main_sched_credit(int argc, char **argv) · line 6119
Quoted 10 lines
tools/libxl/xl_cmdimpl.c +3
-2+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
Signature · 4 lines
<<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)
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
PATCH v5 3/5 xl: improve return and exit codes of vcpu related functions
In reply to Harmandeep KaurTurning vcpu manipulation functions exit codes toward using the EXIT_[SUCCESS|FAILURE] constants, instead of instead of arbitrary numbers or libxl return codes.
---v4->v5: Corrected coding style.v3->v4: No changes.v2->v3: Fixed return of main_vcpuset.
tools/libxl/xl_cmdimpl.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
tools/libxl/xl_cmdimpl.c +11 -10
@@ -5316,7 +5316,7 @@ int main_vcpulist(int argc, char **argv) }vcpulist (argc - optind, argv + optind); -return 0 ; +return EXIT_SUCCESS ; }int main_vcpupin (int argc,char **argv)
@@ -5332,7 +5332,7 @@ int main_vcpupin(int argc, char **argv)long vcpuid;const char *vcpu, *hard_str, *soft_str;char *endptr; -int opt, nb_cpu, nb_vcpu, rc =-1 ; +int opt, nb_cpu, nb_vcpu, rc =EXIT_FAILURE ;libxl_bitmap_init (&cpumap_hard);libxl_bitmap_init (&cpumap_soft);
@@ -5407,10 +5407,10 @@ int main_vcpupin(int argc, char **argv)if (ferror (stdout) ||fflush (stdout)) {perror ("stdout" ); -exit (-1 ); +exit (EXIT_FAILURE ); } - rc =0 ; + rc =EXIT_SUCCESS ; goto out; }
@@ -5430,7 +5430,7 @@ int main_vcpupin(int argc, char **argv)libxl_vcpuinfo_list_free (vcpuinfo, nb_vcpu); } - rc =0 ; + rc =EXIT_SUCCESS ; out:libxl_bitmap_dispose (&cpumap_soft);libxl_bitmap_dispose (&cpumap_hard);
@@ -5459,8 +5459,7 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)unsigned int host_cpu =libxl_get_max_cpus (ctx);libxl_dominfo dominfo; - rc =libxl_domain_info (ctx, &dominfo, domid); -if (rc) +if (libxl_domain_info (ctx, &dominfo, domid))return 1 ;if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) {
@@ -5473,8 +5472,7 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)if (rc)return 1 ; } - rc =libxl_cpu_bitmap_alloc (ctx, &cpumap, max_vcpus); -if (rc) { +if (libxl_cpu_bitmap_alloc (ctx, &cpumap, max_vcpus)) {fprintf (stderr,"libxl_cpu_bitmap_alloc failed, rc: %d\n" , rc);return 1 ; }
@@ -5508,7 +5506,10 @@ int main_vcpuset(int argc, char **argv)break ; } -return vcpuset (find_domain (argv[optind]), argv[optind +1 ], check_host); +if (vcpuset (find_domain (argv[optind]), argv[optind +1 ], check_host)) +return EXIT_FAILURE ; + +return EXIT_SUCCESS ; }static void output_xeninfo (void )
Signature · 1 line
1.9.1