[PATCH v4 0/5] xl: convert exit codes to EXIT_[SUCCESS|FAILURE]

[PATCH v4 0/5] xl: convert exit codes to EXIT_[SUCCESS|FAILURE]

[Harmandeep Kaur]

*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/xen-devel@lists.xen.org/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

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
---

[PATCH v4 1/5] xl: convert main() exit codes to EXIT_[SUCCESS|FAILURE]

[Harmandeep Kaur]

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)

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
---
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(-)

diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 5316ad9..dfae84a 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -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:
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 0021112..c6fb1da 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -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);
-- 
1.9.1

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

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

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
---
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(-)

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
@@ -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)
-- 
1.9.1

[PATCH v4 3/5] xl: improve return and exit codes of vcpu related functions

[Harmandeep Kaur]

Turning vcpu manipulation functions exit codes toward using the
EXIT_[SUCCESS|FAILURE] constants, instead of instead of arbitrary numbers
or libxl return codes.

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
---
v3->v4: No changes.
v2->v3: Fixed return of main_vcpuset.

 tools/libxl/xl_cmdimpl.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index b101aff..1dae28f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -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)
-- 
1.9.1

[PATCH v4 4/5] xl: improve return and exit codes of cpupool related functions

[Harmandeep Kaur]

Turning  cpupools related functions exit codes towards using the
EXIT_[SUCCESS|FAILURE] constants, instead of instead of arbitrary numbers
or libxl return codes.

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
---
v3->v4: No changes.
v2->v3: Edited changelog.

 tools/libxl/xl_cmdimpl.c | 52 ++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1dae28f..76d9af6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7314,7 +7314,7 @@ int main_cpupoolcreate(int argc, char **argv)
     libxl_bitmap cpumap;
     libxl_uuid uuid;
     libxl_cputopology *topology;
-    int rc = 1;
+    int rc = EXIT_FAILURE;
 
     SWITCH_FOREACH_OPT(opt, "nf:", opts, "cpupool-create", 0) {
     case 'f':
@@ -7484,7 +7484,7 @@ int main_cpupoolcreate(int argc, char **argv)
         }
     }
     /* We made it! */
-    rc = 0;
+    rc = EXIT_SUCCESS;
    
 out_cfg:
     xlu_cfg_destroy(config);
@@ -7521,14 +7521,14 @@ int main_cpupoollist(int argc, char **argv)
         pool = argv[optind];
         if (libxl_name_to_cpupoolid(ctx, pool, &poolid)) {
             fprintf(stderr, "Pool \'%s\' does not exist\n", pool);
-            return 1;
+            return EXIT_FAILURE;
         }
     }
 
     poolinfo = libxl_list_cpupool(ctx, &n_pools);
     if (!poolinfo) {
         fprintf(stderr, "error getting cpupool info\n");
-        return 1;
+        return EXIT_FAILURE;
     }
 
     printf("%-19s", "Name");
@@ -7559,7 +7559,7 @@ int main_cpupoollist(int argc, char **argv)
 
     libxl_cpupoolinfo_list_free(poolinfo, n_pools);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_cpupooldestroy(int argc, char **argv)
@@ -7577,13 +7577,13 @@ int main_cpupooldestroy(int argc, char **argv)
     if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
         !libxl_cpupoolid_is_valid(ctx, poolid)) {
         fprintf(stderr, "unknown cpupool '%s'\n", pool);
-        return 1;
+        return EXIT_FAILURE;
     }
 
     if (libxl_cpupool_destroy(ctx, poolid))
-        return 1;
+        return EXIT_FAILURE;
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_cpupoolrename(int argc, char **argv)
@@ -7602,17 +7602,17 @@ int main_cpupoolrename(int argc, char **argv)
     if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
         !libxl_cpupoolid_is_valid(ctx, poolid)) {
         fprintf(stderr, "unknown cpupool '%s'\n", pool);
-        return 1;
+        return EXIT_FAILURE;
     }
 
     new_name = argv[optind];
 
     if (libxl_cpupool_rename(ctx, new_name, poolid)) {
         fprintf(stderr, "Can't rename cpupool '%s'\n", pool);
-        return 1;
+        return EXIT_FAILURE;
     }
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_cpupoolcpuadd(int argc, char **argv)
@@ -7621,7 +7621,7 @@ int main_cpupoolcpuadd(int argc, char **argv)
     const char *pool;
     uint32_t poolid;
     libxl_bitmap cpumap;
-    int rc = 1;
+    int rc = EXIT_FAILURE;
 
     SWITCH_FOREACH_OPT(opt, "", NULL, "cpupool-cpu-add", 2) {
         /* No options */
@@ -7630,7 +7630,7 @@ int main_cpupoolcpuadd(int argc, char **argv)
     libxl_bitmap_init(&cpumap);
     if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) {
         fprintf(stderr, "Unable to allocate cpumap");
-        return 1;
+        return EXIT_FAILURE;
     }
 
     pool = argv[optind++];
@@ -7646,7 +7646,7 @@ int main_cpupoolcpuadd(int argc, char **argv)
     if (libxl_cpupool_cpuadd_cpumap(ctx, poolid, &cpumap))
         fprintf(stderr, "some cpus may not have been added to %s\n", pool);
 
-    rc = 0;
+    rc = EXIT_SUCCESS;
 
 out:
     libxl_bitmap_dispose(&cpumap);
@@ -7659,12 +7659,12 @@ int main_cpupoolcpuremove(int argc, char **argv)
     const char *pool;
     uint32_t poolid;
     libxl_bitmap cpumap;
-    int rc = 1;
+    int rc = EXIT_FAILURE;
 
     libxl_bitmap_init(&cpumap);
     if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) {
         fprintf(stderr, "Unable to allocate cpumap");
-        return 1;
+        return EXIT_FAILURE;
     }
 
     SWITCH_FOREACH_OPT(opt, "", NULL, "cpupool-cpu-remove", 2) {
@@ -7684,7 +7684,7 @@ int main_cpupoolcpuremove(int argc, char **argv)
     if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, &cpumap))
         fprintf(stderr, "some cpus may not have been removed from %s\n", pool);
 
-    rc = 0;
+    rc = EXIT_SUCCESS;
 
 out:
     libxl_bitmap_dispose(&cpumap);
@@ -7709,19 +7709,19 @@ int main_cpupoolmigrate(int argc, char **argv)
     if (libxl_domain_qualifier_to_domid(ctx, dom, &domid) ||
         !libxl_domid_to_name(ctx, domid)) {
         fprintf(stderr, "unknown domain '%s'\n", dom);
-        return 1;
+        return EXIT_FAILURE;
     }
 
     if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
         !libxl_cpupoolid_is_valid(ctx, poolid)) {
         fprintf(stderr, "unknown cpupool '%s'\n", pool);
-        return 1;
+        return EXIT_FAILURE;
     }
 
     if (libxl_cpupool_movedomain(ctx, poolid, domid))
-        return 1;
+        return EXIT_FAILURE;
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_cpupoolnumasplit(int argc, char **argv)
@@ -7749,13 +7749,13 @@ int main_cpupoolnumasplit(int argc, char **argv)
 
     libxl_dominfo_init(&info);
 
-    rc = 1;
+    rc = EXIT_FAILURE;
 
     libxl_bitmap_init(&cpumap);
     poolinfo = libxl_list_cpupool(ctx, &n_pools);
     if (!poolinfo) {
         fprintf(stderr, "error getting cpupool info\n");
-        return 1;
+        return EXIT_FAILURE;
     }
     poolid = poolinfo[0].poolid;
     sched = poolinfo[0].sched;
@@ -7763,13 +7763,13 @@ int main_cpupoolnumasplit(int argc, char **argv)
 
     if (n_pools > 1) {
         fprintf(stderr, "splitting not possible, already cpupools in use\n");
-        return 1;
+        return EXIT_FAILURE;
     }
 
     topology = libxl_get_cpu_topology(ctx, &n_cpus);
     if (topology == NULL) {
         fprintf(stderr, "libxl_get_topologyinfo failed\n");
-        return 1;
+        return EXIT_FAILURE;
     }
 
     if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) {
@@ -7858,7 +7858,7 @@ int main_cpupoolnumasplit(int argc, char **argv)
         }
     }
 
-    rc = 0;
+    rc = EXIT_SUCCESS;
 
 out:
     libxl_cputopology_list_free(topology, n_cpus);
-- 
1.9.1

[PATCH v4 5/5] xl: improve return and exit codes of parse related functions

[Harmandeep Kaur]

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

Don't touch parse_config_data() which is big enough to deserve its own patch.

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
---
v3->v4: Exculded out of scope changes.
v2->v3: Reverted return of parse_mem_size_kb() to -1.
	And exculded out of scope changes.

 tools/libxl/xl_cmdimpl.c | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 76d9af6..06a1531 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -578,10 +578,10 @@ static void parse_disk_config_multistring(XLU_Config **config,
     }
 
     e = xlu_disk_parse(*config, nspecs, specs, disk);
-    if (e == EINVAL) exit(-1);
+    if (e == EINVAL) exit(EXIT_FAILURE);
     if (e) {
         fprintf(stderr,"xlu_disk_parse failed: %s\n",strerror(errno));
-        exit(-1);
+        exit(EXIT_FAILURE);
     }
 }
 
@@ -597,10 +597,10 @@ static void parse_vif_rate(XLU_Config **config, const char *rate,
     int e;
 
     e = xlu_vif_parse_rate(*config, rate, nic);
-    if (e == EINVAL || e == EOVERFLOW) exit(-1);
+    if (e == EINVAL || e == EOVERFLOW) exit(EXIT_FAILURE);
     if (e) {
         fprintf(stderr,"xlu_vif_parse_rate failed: %s\n",strerror(errno));
-        exit(-1);
+        exit(EXIT_FAILURE);
     }
 }
 
@@ -741,19 +741,19 @@ static int parse_range(const char *str, unsigned long *a, unsigned long *b)
 
     *a = *b = strtoul(str, &endptr, 10);
     if (endptr == str || *a == ULONG_MAX)
-        return ERROR_INVAL;
+        return 1;
 
     if (*endptr == '-') {
         nstr = endptr + 1;
 
         *b = strtoul(nstr, &endptr, 10);
         if (endptr == nstr || *b == ULONG_MAX || *b < *a)
-            return ERROR_INVAL;
+            return 1;
     }
 
     /* Valid value or range so far, but we also don't want junk after that */
     if (*endptr != '\0')
-        return ERROR_INVAL;
+        return 1;
 
     return 0;
 }
@@ -902,7 +902,7 @@ static char *parse_cmdline(XLU_Config *config)
 
     if ((buf || root || extra) && !cmdline) {
         fprintf(stderr, "Failed to allocate memory for cmdline\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     return cmdline;
@@ -946,11 +946,11 @@ static void parse_vcpu_affinity(libxl_domain_build_info *b_info,
             libxl_bitmap_init(&vcpu_affinity_array[j]);
             if (libxl_cpu_bitmap_alloc(ctx, &vcpu_affinity_array[j], 0)) {
                 fprintf(stderr, "Unable to allocate cpumap for vcpu %d\n", j);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
 
             if (cpurange_parse(buf, &vcpu_affinity_array[j]))
-                exit(1);
+                exit(EXIT_FAILURE);
 
             j++;
         }
@@ -963,17 +963,17 @@ static void parse_vcpu_affinity(libxl_domain_build_info *b_info,
         libxl_bitmap_init(&vcpu_affinity_array[0]);
         if (libxl_cpu_bitmap_alloc(ctx, &vcpu_affinity_array[0], 0)) {
             fprintf(stderr, "Unable to allocate cpumap for vcpu 0\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
 
         if (cpurange_parse(buf, &vcpu_affinity_array[0]))
-            exit(1);
+            exit(EXIT_FAILURE);
 
         for (i = 1; i < b_info->max_vcpus; i++) {
             libxl_bitmap_init(&vcpu_affinity_array[i]);
             if (libxl_cpu_bitmap_alloc(ctx, &vcpu_affinity_array[i], 0)) {
                 fprintf(stderr, "Unable to allocate cpumap for vcpu %d\n", i);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             libxl_bitmap_copy(ctx, &vcpu_affinity_array[i],
                               &vcpu_affinity_array[0]);
@@ -1064,7 +1064,7 @@ static unsigned long parse_ulong(const char *str)
     val = strtoul(str, &endptr, 10);
     if (endptr == str || val == ULONG_MAX) {
         fprintf(stderr, "xl: failed to convert \"%s\" to number\n", str);
-        exit(1);
+        exit(EXIT_FAILURE);
     }
     return val;
 }
@@ -1086,7 +1086,7 @@ static void parse_vnuma_config(const XLU_Config *config,
     if (libxl_get_physinfo(ctx, &physinfo) != 0) {
         libxl_physinfo_dispose(&physinfo);
         fprintf(stderr, "libxl_get_physinfo failed\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     nr_nodes = physinfo.nr_nodes;
@@ -1105,7 +1105,7 @@ static void parse_vnuma_config(const XLU_Config *config,
         libxl_bitmap_init(&vcpu_parsed[i]);
         if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], b_info->max_vcpus)) {
             fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
     }
 
@@ -1130,7 +1130,7 @@ static void parse_vnuma_config(const XLU_Config *config,
         xlu_cfg_value_get_list(config, vnode_spec, &vnode_config_list, 0);
         if (!vnode_config_list) {
             fprintf(stderr, "xl: cannot get vnode config option list\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
 
         for (conf_count = 0;
@@ -1152,7 +1152,7 @@ static void parse_vnuma_config(const XLU_Config *config,
                                            &value_untrimmed)) {
                     fprintf(stderr, "xl: failed to split \"%s\" into pair\n",
                             buf);
-                    exit(1);
+                    exit(EXIT_FAILURE);
                 }
                 trim(isspace, option_untrimmed, &option);
                 trim(isspace, value_untrimmed, &value);
@@ -1162,7 +1162,7 @@ static void parse_vnuma_config(const XLU_Config *config,
                     if (val >= nr_nodes) {
                         fprintf(stderr,
                                 "xl: invalid pnode number: %lu\n", val);
-                        exit(1);
+                        exit(EXIT_FAILURE);
                     }
                     p->pnode = val;
                     libxl_defbool_set(&b_info->numa_placement, false);
@@ -1218,20 +1218,20 @@ static void parse_vnuma_config(const XLU_Config *config,
     if (b_info->max_vcpus != 0) {
         if (b_info->max_vcpus != max_vcpus) {
             fprintf(stderr, "xl: vnuma vcpus and maxvcpus= mismatch\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
     } else {
         int host_cpus = libxl_get_online_cpus(ctx);
 
         if (host_cpus < 0) {
             fprintf(stderr, "Failed to get online cpus\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
 
         if (host_cpus < max_vcpus) {
             fprintf(stderr, "xl: vnuma specifies more vcpus than pcpus, "\
                     "use maxvcpus= to override this check.\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
 
         b_info->max_vcpus = max_vcpus;
@@ -1241,7 +1241,7 @@ static void parse_vnuma_config(const XLU_Config *config,
     if (b_info->max_memkb != LIBXL_MEMKB_DEFAULT &&
         b_info->max_memkb != max_memkb) {
         fprintf(stderr, "xl: maxmem and vnuma memory size mismatch\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     } else
         b_info->max_memkb = max_memkb;
 
-- 
1.9.1

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

[Wei Liu]

On Tue, Oct 27, 2015 at 06:18:55PM +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.
> 

I have one thing to complain. See below.

> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> ---
> 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(-)
> 
> 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
> @@ -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)){

This, and other instances in both patch 2 and 3, violates coding style.
You need to have a space before "{".

Please go through patch 2 and patch 3 to fix all such violations.

After that, you can add my ack to all your patches. I haven't carefully
read through every line of change, but I think Dario's review is sufficient
and thorough. Thanks Dario.

Wei.

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

[Dario Faggioli]

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

On Tue, 2015-10-27 at 13:50 +0000, Wei Liu wrote:
> On Tue, Oct 27, 2015 at 06:18:55PM +0530, Harmandeep Kaur wrote:

> > 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
> > @@ -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)){
> 
> This, and other instances in both patch 2 and 3, violates coding
> style.
> You need to have a space before "{".
> 
Bha, I guess I was concentrating on the "return / exit()" changes, and
managed to miss these! :-/

> After that, you can add my ack to all your patches. I haven't
> carefully
> read through every line of change, but I think Dario's review is
> sufficient
> and thorough. 
>
And yet, I managed to miss those coding style issues. :-(

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