[PATCH v3 0/6] xl: convert exit codes related to domain subcommands to EXIT_[SUCCESS|FAILURE]

[PATCH v3 0/6] xl: convert exit codes related to domain subcommands to EXIT_[SUCCESS|FAILURE]

[Dario Faggioli]

Hi tools maintainers,

So, this series from Harmandeep seems to have fallen through the cracks (and
that's my fault, as I said I'd have handled it). In any case, it was posted
before last posting day, it looks simple enough to me and it's a nice cleanup.

This is about using EXIT_SUCCESS or EXIT_FAILURE when calling exit(), in xl.
main_foo() functions are treated as they were main(), and so they also return
EXIT_SUCCESS or EXIT_FAILURE. Internal functions are also refactored. The idea
was to have them return either 0 or 1, but there needs to be exceptions (that
are documented). TBF, this series mostly deals with exit codes, and the amount
of internal function refactoring is rather limited.

Unfortunately, even after this series, there will still be more similar cleanup
to do... but that would have to wait 4.8.

Version 2 was already almost ok, but there were a few adjustments needed.
Therefore, instead than asking for them, I've done them myself. That basically
means that I've reorganized the series a bit, squashing some patches together.
I've also reduced the scope of "xl: improve return and exit codes of dom create
related functions" to only deal with exit()-s from within that function, as
quite a few more work than what was being done in that patch in v2 was
necessary to propetly refactoring the internal return paths.

I've also added patch "xl: make return type of create_domain() more
consistent.", doing what we agreed upon on IRC (about making the return type
'int'). I could have folded that change in the other patch, but I thought the
thing would deserve its own changelog.

v2 is here:
 http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg01037.html

and there is a git branch with the series applied here:
 git://xenbits.xen.org/people/dariof/xen.git   rel/xl/exit-code-cleanup-v3
 http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/xl/exit-code-cleanup-v3

Let me know what you think.

Thanks and Regards,
Dario
---
Dario Faggioli (2):
      xl: make return type of create_domain() more consistent.
      xl: improve return and exit codes of dom create related functions

Harmandeep Kaur (4):
      xl: improve return and exit codes of memory related functions
      xl: improve exit codes of save/restore and migration functions
      xl: improve exit codes of some of the domain handling functions
      xl: improve exit codes of debug related functions


 tools/libxl/xl_cmdimpl.c |  241 +++++++++++++++++++++++-----------------------
 1 file changed, 123 insertions(+), 118 deletions(-)
--
<<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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v3 1/6] xl: improve return and exit codes of memory related functions

[Dario Faggioli]

From: Harmandeep Kaur <write.harmandeep@gmail.com>

by making them more consistent with other examples in xl.

While there, make freemem() of boolean return type, which
looks more natural, and add comment explaining why
parse_mem_size_kb() needs to diverge from the pattern.

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
v3: Shorten changelog.
    Make freemem() boolean return type.

v2: Add comment to explain return vaule of parse_mem_size_kb().
    Add freemem() and main_sharing().
    Remove find_domain().
---
 tools/libxl/xl_cmdimpl.c |   39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2ee6c74..9612b00 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2680,40 +2680,45 @@ static int preserve_domain(uint32_t *r_domid, libxl_event *event,
     return rc == 0 ? 1 : 0;
 }
 
-static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
+/*
+ * Returns false if memory can't be freed, but also if we encounter errors.
+ * Returns true in case there is already, or we manage to free it, enough
+ * memory, but also if autoballoon is false.
+ */
+static bool freemem(uint32_t domid, libxl_domain_build_info *b_info)
 {
     int rc, retries = 3;
     uint32_t need_memkb, free_memkb;
 
     if (!autoballoon)
-        return 0;
+        return true;
 
     rc = libxl_domain_need_memory(ctx, b_info, &need_memkb);
     if (rc < 0)
-        return rc;
+        return false;
 
     do {
         rc = libxl_get_free_memory(ctx, &free_memkb);
         if (rc < 0)
-            return rc;
+            return false;
 
         if (free_memkb >= need_memkb)
-            return 0;
+            return true;
 
         rc = libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0);
         if (rc < 0)
-            return rc;
+            return false;
 
         /* wait until dom0 reaches its target, as long as we are making
          * progress */
         rc = libxl_wait_for_memory_target(ctx, 0, 10);
         if (rc < 0)
-            return rc;
+            return false;
 
         retries--;
     } while (retries > 0);
 
-    return ERROR_NOMEM;
+    return false;
 }
 
 static void autoconnect_console(libxl_ctx *ctx_ignored,
@@ -2980,8 +2985,7 @@ start:
         goto error_out;
 
     if (domid_soft_reset == INVALID_DOMID) {
-        ret = freemem(domid, &d_config.b_info);
-        if (ret < 0) {
+        if (!freemem(domid, &d_config.b_info)) {
             fprintf(stderr, "failed to free memory for the domain\n");
             ret = ERROR_FAIL;
             goto error_out;
@@ -3245,6 +3249,7 @@ void help(const char *command)
     }
 }
 
+/* Returns -1 on failure; the amount of memory on success. */
 static int64_t parse_mem_size_kb(const char *mem)
 {
     char *endptr;
@@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const char *mem)
     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1) {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        exit(EXIT_FAILURE);
     }
 
     if (libxl_domain_setmaxmem(ctx, domid, memorykb)) {
@@ -3425,7 +3430,7 @@ static int set_memory_target(uint32_t domid, const char *mem)
     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1)  {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        exit(EXIT_FAILURE);
     }
 
     if (libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1)) {
@@ -6132,7 +6137,7 @@ int main_sharing(int argc, char **argv)
         info = libxl_list_domain(ctx, &nb_domain);
         if (!info) {
             fprintf(stderr, "libxl_list_domain failed.\n");
-            return 1;
+            return EXIT_FAILURE;
         }
         info_free = info;
     } else if (optind == argc-1) {
@@ -6141,17 +6146,17 @@ int main_sharing(int argc, char **argv)
         if (rc == ERROR_DOMAIN_NOTFOUND) {
             fprintf(stderr, "Error: Domain \'%s\' does not exist.\n",
                 argv[optind]);
-            return -rc;
+            return EXIT_FAILURE;
         }
         if (rc) {
             fprintf(stderr, "libxl_domain_info failed (code %d).\n", rc);
-            return -rc;
+            return EXIT_FAILURE;
         }
         info = &info_buf;
         nb_domain = 1;
     } else {
         help("sharing");
-        return 2;
+        return EXIT_FAILURE;
     }
 
     sharing(info, nb_domain);
@@ -6161,7 +6166,7 @@ int main_sharing(int argc, char **argv)
     else
         libxl_dominfo_dispose(info);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 static int sched_domain_get(libxl_scheduler sched, int domid,


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

[PATCH v3 2/6] xl: improve exit codes of save/restore and migration functions

[Dario Faggioli]

From: Harmandeep Kaur <write.harmandeep@gmail.com>

by making them more consistent with other examples in xl.

Macros CHK_ERRNOVAL, CHK_SYSCALL, MUST are also updated.

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
v3: This is patches 2 and 3 squashed together.
    Shorten changelog.

v2: Add main_remus().
    Remove create_domain().
---
 tools/libxl/xl_cmdimpl.c |   80 +++++++++++++++++++++++-----------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9612b00..9cd3144 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -50,7 +50,7 @@
         else if (chk_errnoval > 0) {                                    \
             fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n",          \
                     __FILE__,__LINE__, strerror(chk_errnoval), #call);  \
-            exit(-ERROR_FAIL);                                          \
+            exit(EXIT_FAILURE);                                         \
         }                                                               \
     })
 
@@ -59,7 +59,7 @@
         if ((call) == -1) {                                             \
             fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n",          \
                     __FILE__,__LINE__, strerror(errno), #call);         \
-            exit(-ERROR_FAIL);                                          \
+            exit(EXIT_FAILURE);                                         \
         }                                                               \
     })
 
@@ -68,7 +68,7 @@
         if (must_rc < 0) {                                                  \
             fprintf(stderr,"xl: fatal error: %s:%d, rc=%d: %s\n",       \
                     __FILE__,__LINE__, must_rc, #call);                 \
-            exit(-must_rc);                                             \
+            exit(EXIT_FAILURE);                                         \
         }                                                               \
     })
 
@@ -376,7 +376,7 @@ static void xvasprintf(char **strp, const char *fmt, va_list ap)
     int r = vasprintf(strp, fmt, ap);
     if (r == -1) {
         perror("asprintf failed");
-        exit(-ERROR_FAIL);
+        exit(EXIT_FAILURE);
     }
 }
 
@@ -4350,7 +4350,7 @@ static void save_domain_core_begin(uint32_t domid,
                                       &config_v, config_len_r);
         if (rc) {
             fprintf(stderr, "unable to read overridden config file\n");
-            exit(2);
+            exit(EXIT_FAILURE);
         }
         parse_config_data(override_config_file, config_v, *config_len_r,
                           &d_config);
@@ -4359,14 +4359,14 @@ static void save_domain_core_begin(uint32_t domid,
         rc = libxl_retrieve_domain_configuration(ctx, domid, &d_config);
         if (rc) {
             fprintf(stderr, "unable to retrieve domain configuration\n");
-            exit(2);
+            exit(EXIT_FAILURE);
         }
     }
 
     config_c = libxl_domain_config_to_json(ctx, &d_config);
     if (!config_c) {
         fprintf(stderr, "unable to convert config file to JSON\n");
-        exit(2);
+        exit(EXIT_FAILURE);
     }
     *config_data_r = (uint8_t *)config_c;
     *config_len_r = strlen(config_c) + 1; /* including trailing '\0' */
@@ -4436,7 +4436,7 @@ static int save_domain(uint32_t domid, const char *filename, int checkpoint,
     fd = open(filename, O_WRONLY|O_CREAT|O_TRUNC, 0644);
     if (fd < 0) {
         fprintf(stderr, "Failed to open temp file %s for writing\n", filename);
-        exit(2);
+        exit(EXIT_FAILURE);
     }
 
     save_domain_core_writeconfig(fd, filename, config_data, config_len);
@@ -4456,7 +4456,7 @@ static int save_domain(uint32_t domid, const char *filename, int checkpoint,
     else
         libxl_domain_destroy(ctx, domid, 0);
 
-    exit(rc < 0 ? 1 : 0);
+    exit(rc < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
 static pid_t create_migration_child(const char *rune, int *send_fd,
@@ -4480,7 +4480,7 @@ static pid_t create_migration_child(const char *rune, int *send_fd,
         close(recvpipe[0]); close(recvpipe[1]);
         execlp("sh","sh","-c",rune,(char*)0);
         perror("failed to exec sh");
-        exit(-1);
+        exit(EXIT_FAILURE);
     }
 
     close(sendpipe[0]);
@@ -4503,14 +4503,14 @@ static int migrate_read_fixedmessage(int fd, const void *msg, int msgsz,
 
     stream = rune ? "migration receiver stream" : "migration stream";
     rc = libxl_read_exactly(ctx, fd, buf, msgsz, stream, what);
-    if (rc) return ERROR_FAIL;
+    if (rc) return 1;
 
     if (memcmp(buf, msg, msgsz)) {
         fprintf(stderr, "%s contained unexpected data instead of %s\n",
                 stream, what);
         if (rune)
             fprintf(stderr, "(command run was: %s )\n", rune);
-        return ERROR_FAIL;
+        return 1;
     }
     return 0;
 }
@@ -4586,7 +4586,7 @@ static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child,
 
     if (send_fd < 0 || recv_fd < 0) {
         fprintf(stderr, "migrate_do_preamble: invalid file descriptors\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     rc = migrate_read_fixedmessage(recv_fd, migrate_receiver_banner,
@@ -4595,7 +4595,7 @@ static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child,
     if (rc) {
         close(send_fd);
         migration_child_report(recv_fd);
-        exit(-rc);
+        exit(EXIT_FAILURE);
     }
 
     save_domain_core_writeconfig(send_fd, "migration stream",
@@ -4620,7 +4620,7 @@ static void migrate_domain(uint32_t domid, const char *rune, int debug,
     if (!config_len) {
         fprintf(stderr, "No config file stored for running domain and "
                 "none supplied - cannot migrate.\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     child = create_migration_child(rune, &send_fd, &recv_fd);
@@ -4707,26 +4707,26 @@ static void migrate_domain(uint32_t domid, const char *rune, int debug,
         if (!rc) fprintf(stderr, "migration sender: Resumed OK.\n");
 
         fprintf(stderr, "Migration failed due to problems at target.\n");
-        exit(-ERROR_FAIL);
+        exit(EXIT_FAILURE);
     }
 
     fprintf(stderr, "migration sender: Target reports successful startup.\n");
     libxl_domain_destroy(ctx, domid, 0); /* bang! */
     fprintf(stderr, "Migration successful.\n");
-    exit(0);
+    exit(EXIT_SUCCESS);
 
  failed_suspend:
     close(send_fd);
     migration_child_report(recv_fd);
     fprintf(stderr, "Migration failed, failed to suspend at sender.\n");
-    exit(-ERROR_FAIL);
+    exit(EXIT_FAILURE);
 
  failed_resume:
     close(send_fd);
     migration_child_report(recv_fd);
     fprintf(stderr, "Migration failed, resuming at sender.\n");
     libxl_domain_resume(ctx, domid, 1, 0);
-    exit(-ERROR_FAIL);
+    exit(EXIT_FAILURE);
 
  failed_badly:
     fprintf(stderr,
@@ -4739,7 +4739,7 @@ static void migrate_domain(uint32_t domid, const char *rune, int debug,
 
     close(send_fd);
     migration_child_report(recv_fd);
-    exit(-ERROR_BADFAIL);
+    exit(EXIT_FAILURE);
 }
 
 static void migrate_receive(int debug, int daemonize, int monitor,
@@ -4780,7 +4780,7 @@ static void migrate_receive(int debug, int daemonize, int monitor,
     if (rc < 0) {
         fprintf(stderr, "migration target: Domain creation failed"
                 " (code %d).\n", rc);
-        exit(-rc);
+        exit(EXIT_FAILURE);
     }
 
     domid = rc;
@@ -4823,7 +4823,7 @@ static void migrate_receive(int debug, int daemonize, int monitor,
                     "Failed to unpause domain %s (id: %u):%d\n",
                     ha, common_domname, domid, rc);
 
-        exit(rc ? -ERROR_FAIL: 0);
+        exit(rc ? EXIT_FAILURE : EXIT_SUCCESS);
     default:
         /* do nothing */
         break;
@@ -4836,7 +4836,7 @@ static void migrate_receive(int debug, int daemonize, int monitor,
                              migrate_receiver_ready,
                              sizeof(migrate_receiver_ready),
                              "migration ack stream", "ready message");
-    if (rc) exit(-rc);
+    if (rc) exit(EXIT_FAILURE);
 
     rc = migrate_read_fixedmessage(recv_fd, migrate_permission_to_go,
                                    sizeof(migrate_permission_to_go),
@@ -4861,14 +4861,14 @@ static void migrate_receive(int debug, int daemonize, int monitor,
                               migrate_report, sizeof(migrate_report),
                               "migration ack stream",
                               "success/failure report");
-    if (rc2) exit(-ERROR_BADFAIL);
+    if (rc2) exit(EXIT_FAILURE);
 
     rc_buf = -rc;
     assert(!!rc_buf == !!rc);
     rc2 = libxl_write_exactly(ctx, send_fd, &rc_buf, 1,
                               "migration ack stream",
                               "success/failure code");
-    if (rc2) exit(-ERROR_BADFAIL);
+    if (rc2) exit(EXIT_FAILURE);
 
     if (rc) {
         fprintf(stderr, "migration target: Failure, destroying our copy.\n");
@@ -4877,7 +4877,7 @@ static void migrate_receive(int debug, int daemonize, int monitor,
         if (rc2) {
             fprintf(stderr, "migration target: Failed to destroy our copy"
                     " (code %d).\n", rc2);
-            exit(-ERROR_BADFAIL);
+            exit(EXIT_FAILURE);
         }
 
         fprintf(stderr, "migration target: Cleanup OK, granting sender"
@@ -4888,10 +4888,10 @@ static void migrate_receive(int debug, int daemonize, int monitor,
                                   sizeof(migrate_permission_to_go),
                                   "migration ack stream",
                                   "permission to sender to have domain back");
-        if (rc2) exit(-ERROR_BADFAIL);
+        if (rc2) exit(EXIT_FAILURE);
     }
 
-    exit(0);
+    exit(EXIT_SUCCESS);
 }
 
 int main_restore(int argc, char **argv)
@@ -4940,7 +4940,7 @@ int main_restore(int argc, char **argv)
         checkpoint_file = argv[optind + 1];
     } else {
         help("restore");
-        return 2;
+        return EXIT_FAILURE;
     }
 
     memset(&dom_info, 0, sizeof(dom_info));
@@ -4958,9 +4958,9 @@ int main_restore(int argc, char **argv)
 
     rc = create_domain(&dom_info);
     if (rc < 0)
-        return -rc;
+        return EXIT_FAILURE;
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_migrate_receive(int argc, char **argv)
@@ -5000,13 +5000,13 @@ int main_migrate_receive(int argc, char **argv)
 
     if (argc-optind != 0) {
         help("migrate-receive");
-        return 2;
+        return EXIT_FAILURE;
     }
     migrate_receive(debug, daemonize, monitor,
                     STDOUT_FILENO, STDIN_FILENO,
                     checkpointed, script);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_save(int argc, char **argv)
@@ -5029,7 +5029,7 @@ int main_save(int argc, char **argv)
 
     if (argc-optind > 3) {
         help("save");
-        return 2;
+        return EXIT_FAILURE;
     }
 
     domid = find_domain(argv[optind]);
@@ -5038,7 +5038,7 @@ int main_save(int argc, char **argv)
         config_filename = argv[optind + 2];
 
     save_domain(domid, filename, checkpoint, leavepaused, config_filename);
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_migrate(int argc, char **argv)
@@ -5105,7 +5105,7 @@ int main_migrate(int argc, char **argv)
     }
 
     migrate_domain(domid, rune, debug, config_filename);
-    return 0;
+    return EXIT_SUCCESS;
 }
 #endif
 
@@ -8756,7 +8756,7 @@ int main_remus(int argc, char **argv)
         send_fd = open("/dev/null", O_RDWR, 0644);
         if (send_fd < 0) {
             perror("failed to open /dev/null");
-            exit(-1);
+            exit(EXIT_FAILURE);
         }
     } else {
 
@@ -8783,7 +8783,7 @@ int main_remus(int argc, char **argv)
         if (!config_len) {
             fprintf(stderr, "No config file stored for running domain and "
                     "none supplied - cannot start remus.\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
 
         child = create_migration_child(rune, &send_fd, &recv_fd);
@@ -8805,7 +8805,7 @@ int main_remus(int argc, char **argv)
         fprintf(stderr, "%s: Primary domain has been destroyed.\n",
                 libxl_defbool_val(r_info.colo) ? "COLO" : "Remus");
         close(send_fd);
-        return 0;
+        return EXIT_SUCCESS;
     }
 
     /* If we are here, it means remus setup/domain suspend/backup has
@@ -8821,7 +8821,7 @@ int main_remus(int argc, char **argv)
     }
 
     close(send_fd);
-    return -ERROR_FAIL;
+    return EXIT_FAILURE;
 }
 #endif
 


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

[PATCH v3 3/6] xl: improve exit codes of some of the domain handling functions

[Dario Faggioli]

From: Harmandeep Kaur <write.harmandeep@gmail.com>

by making them more consistent with other examples in xl.

Affected functions are the ones related to console, vnc,
dump, destroy, shutdown, list, domid and domname.

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Signed-off-by: Dario Fagggioli <dario.faggioli@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
V3: This is patches 4, 5, 6 and 9 squashed into one.
    Shorten changelog.

V2: Add autoconnect_vncviewer().
---
 tools/libxl/xl_cmdimpl.c |   72 +++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9cd3144..4a560d7 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -174,7 +174,7 @@ static uint32_t find_domain(const char *p)
     rc = libxl_domain_qualifier_to_domid(ctx, p, &domid);
     if (rc) {
         fprintf(stderr, "%s is an invalid domain identifier (rc=%d)\n", p, rc);
-        exit(2);
+        exit(EXIT_FAILURE);
     }
     common_domname = libxl_domid_to_name(ctx, domid);
     return domid;
@@ -221,7 +221,7 @@ static void autoconnect_vncviewer(uint32_t domid, int autopass)
 
     sleep(1);
     vncviewer(domid, autopass);
-    _exit(1);
+    _exit(EXIT_FAILURE);
 }
 
 static int acquire_lock(void)
@@ -435,7 +435,7 @@ static void flush_stream(FILE *fh)
 
     if (ferror(fh) || fflush(fh)) {
         perror(fh_name);
-        exit(-1);
+        exit(EXIT_FAILURE);
     }
 }
 
@@ -3735,7 +3735,7 @@ int main_console(int argc, char **argv)
             type = LIBXL_CONSOLE_TYPE_SERIAL;
         else {
             fprintf(stderr, "console type supported are: pv, serial\n");
-            return 2;
+            return EXIT_FAILURE;
         }
         break;
     case 'n':
@@ -3749,7 +3749,7 @@ int main_console(int argc, char **argv)
     else
         libxl_console_exec(ctx, domid, num, type);
     fprintf(stderr, "Unable to attach console\n");
-    return 1;
+    return EXIT_FAILURE;
 }
 
 int main_vncviewer(int argc, char **argv)
@@ -3771,8 +3771,8 @@ int main_vncviewer(int argc, char **argv)
     domid = find_domain(argv[optind]);
 
     if (vncviewer(domid, autopass))
-        return 1;
-    return 0;
+        return EXIT_FAILURE;
+    return EXIT_SUCCESS;
 }
 
 static void pcilist(uint32_t domid)
@@ -4010,10 +4010,10 @@ static void destroy_domain(uint32_t domid, int force)
         fprintf(stderr, "Not destroying domain 0; use -f to force.\n"
                         "This can only be done when using a disaggregated "
                         "hardware domain and toolstack.\n\n");
-        exit(-1);
+        exit(EXIT_FAILURE);
     }
     rc = libxl_domain_destroy(ctx, domid, 0);
-    if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(-1); }
+    if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(EXIT_FAILURE); }
 }
 
 static void wait_for_domain_deaths(libxl_evgen_domain_death **deathws, int nr)
@@ -4025,7 +4025,7 @@ static void wait_for_domain_deaths(libxl_evgen_domain_death **deathws, int nr)
         rc = libxl_event_wait(ctx, &event, LIBXL_EVENTMASK_ALL, 0,0);
         if (rc) {
             LOG("Failed to get event, quitting (rc=%d)", rc);
-            exit(-1);
+            exit(EXIT_FAILURE);
         }
 
         switch (event->type) {
@@ -4070,14 +4070,14 @@ static void shutdown_domain(uint32_t domid,
     }
 
     if (rc) {
-        fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(-1);
+        fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(EXIT_FAILURE);
     }
 
     if (deathw) {
         rc = libxl_evenable_domain_death(ctx, domid, for_user, deathw);
         if (rc) {
             fprintf(stderr,"wait for death failed (evgen, rc=%d)\n",rc);
-            exit(-1);
+            exit(EXIT_FAILURE);
         }
     }
 }
@@ -4101,14 +4101,14 @@ static void reboot_domain(uint32_t domid, libxl_evgen_domain_death **deathw,
         }
     }
     if (rc) {
-        fprintf(stderr,"reboot failed (rc=%d)\n",rc);exit(-1);
+        fprintf(stderr,"reboot failed (rc=%d)\n",rc);exit(EXIT_FAILURE);
     }
 
     if (deathw) {
         rc = libxl_evenable_domain_death(ctx, domid, for_user, deathw);
         if (rc) {
             fprintf(stderr,"wait for death failed (evgen, rc=%d)\n",rc);
-            exit(-1);
+            exit(EXIT_FAILURE);
         }
     }
 }
@@ -4241,12 +4241,12 @@ static void list_domains(bool verbose, bool context, bool claim, bool numa,
     if (numa) {
         if (libxl_node_bitmap_alloc(ctx, &nodemap, 0)) {
             fprintf(stderr, "libxl_node_bitmap_alloc_failed.\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
         if (libxl_get_physinfo(ctx, &physinfo) != 0) {
             fprintf(stderr, "libxl_physinfo failed.\n");
             libxl_bitmap_dispose(&nodemap);
-            exit(1);
+            exit(EXIT_FAILURE);
         }
 
         printf(" NODE Affinity");
@@ -4310,7 +4310,7 @@ static void list_vm(void)
 
     if (!info) {
         fprintf(stderr, "libxl_list_vm failed.\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
     printf("UUID                                  ID    name\n");
     for (i = 0; i < nb_vm; i++) {
@@ -4327,7 +4327,7 @@ static void core_dump_domain(uint32_t domid, const char *filename)
     int rc;
 
     rc=libxl_domain_core_dump(ctx, domid, filename, NULL);
-    if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); }
+    if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(EXIT_FAILURE); }
 }
 
 #ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
@@ -5118,7 +5118,7 @@ int main_dump_core(int argc, char **argv)
     }
 
     core_dump_domain(find_domain(argv[optind]), argv[optind + 1]);
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_pause(int argc, char **argv)
@@ -5131,7 +5131,7 @@ int main_pause(int argc, char **argv)
 
     pause_domain(find_domain(argv[optind]));
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_unpause(int argc, char **argv)
@@ -5144,7 +5144,7 @@ int main_unpause(int argc, char **argv)
 
     unpause_domain(find_domain(argv[optind]));
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_destroy(int argc, char **argv)
@@ -5159,7 +5159,7 @@ int main_destroy(int argc, char **argv)
     }
 
     destroy_domain(find_domain(argv[optind]), force);
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 static int main_shutdown_or_reboot(int do_reboot, int argc, char **argv)
@@ -5191,7 +5191,7 @@ static int main_shutdown_or_reboot(int do_reboot, int argc, char **argv)
 
     if (!argv[optind] && !all) {
         fprintf(stderr, "You must specify -a or a domain id.\n\n");
-        return opt;
+        return EXIT_FAILURE;
     }
 
     if (all) {
@@ -5199,7 +5199,7 @@ static int main_shutdown_or_reboot(int do_reboot, int argc, char **argv)
         libxl_evgen_domain_death **deathws = NULL;
         if (!(dominfo = libxl_list_domain(ctx, &nb_domain))) {
             fprintf(stderr, "libxl_list_domain failed.\n");
-            return -1;
+            return EXIT_FAILURE;
         }
 
         if (wait_for_it)
@@ -5230,7 +5230,7 @@ static int main_shutdown_or_reboot(int do_reboot, int argc, char **argv)
     }
 
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_shutdown(int argc, char **argv)
@@ -5288,7 +5288,7 @@ int main_list(int argc, char **argv)
         info = libxl_list_domain(ctx, &nb_domain);
         if (!info) {
             fprintf(stderr, "libxl_list_domain failed.\n");
-            return 1;
+            return EXIT_FAILURE;
         }
         info_free = info;
     } else if (optind == argc-1) {
@@ -5297,17 +5297,17 @@ int main_list(int argc, char **argv)
         if (rc == ERROR_DOMAIN_NOTFOUND) {
             fprintf(stderr, "Error: Domain \'%s\' does not exist.\n",
                 argv[optind]);
-            return -rc;
+            return EXIT_FAILURE;
         }
         if (rc) {
             fprintf(stderr, "libxl_domain_info failed (code %d).\n", rc);
-            return -rc;
+            return EXIT_FAILURE;
         }
         info = &info_buf;
         nb_domain = 1;
     } else {
         help("list");
-        return 2;
+        return EXIT_FAILURE;
     }
 
     if (details)
@@ -5321,7 +5321,7 @@ int main_list(int argc, char **argv)
 
     libxl_dominfo_dispose(&info_buf);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_vm_list(int argc, char **argv)
@@ -5333,7 +5333,7 @@ int main_vm_list(int argc, char **argv)
     }
 
     list_vm();
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 static void string_realloc_append(char **accumulate, const char *more)
@@ -6980,12 +6980,12 @@ int main_domid(int argc, char **argv)
 
     if (libxl_name_to_domid(ctx, domname, &domid)) {
         fprintf(stderr, "Can't get domid of domain name '%s', maybe this domain does not exist.\n", domname);
-        return 1;
+        return EXIT_FAILURE;
     }
 
     printf("%d\n", domid);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_domname(int argc, char **argv)
@@ -7003,19 +7003,19 @@ int main_domname(int argc, char **argv)
     if (domid == 0 && !strcmp(endptr, argv[optind])) {
         /*no digits at all*/
         fprintf(stderr, "Invalid domain id.\n\n");
-        return 1;
+        return EXIT_FAILURE;
     }
 
     domname = libxl_domid_to_name(ctx, domid);
     if (!domname) {
         fprintf(stderr, "Can't get domain name of domain id '%d', maybe this domain does not exist.\n", domid);
-        return 1;
+        return EXIT_FAILURE;
     }
 
     printf("%s\n", domname);
     free(domname);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_rename(int argc, char **argv)


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

[PATCH v3 4/6] xl : improve exit codes of debug related functions

[Dario Faggioli]

From: Harmandeep Kaur <write.harmandeep@gmail.com>

by making them more consistent with other examples in xl.

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
v3: Shorten changelog.

v2: Add main_sysrq(), main_debug_keys(), main_dmesg()
    Remove xvasprintf(), main_remus()
---
 tools/libxl/xl_cmdimpl.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4a560d7..823cb46 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5561,7 +5561,7 @@ static void button_press(uint32_t domid, const char *b)
         trigger = LIBXL_TRIGGER_SLEEP;
     } else {
         fprintf(stderr, "%s is an invalid button identifier\n", b);
-        exit(2);
+        exit(EXIT_FAILURE);
     }
 
     libxl_send_trigger(ctx, domid, trigger, 0);
@@ -7058,7 +7058,7 @@ int main_trigger(int argc, char **argv)
     trigger_name = argv[optind++];
     if (libxl_trigger_from_string(trigger_name, &trigger)) {
         fprintf(stderr, "Invalid trigger \"%s\"\n", trigger_name);
-        return -1;
+        return EXIT_FAILURE;
     }
 
     if (argv[optind]) {
@@ -7070,7 +7070,7 @@ int main_trigger(int argc, char **argv)
 
     libxl_send_trigger(ctx, domid, trigger, vcpuid);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 
@@ -7091,12 +7091,12 @@ int main_sysrq(int argc, char **argv)
     if (sysrq[1] != '\0') {
         fprintf(stderr, "Invalid sysrq.\n\n");
         help("sysrq");
-        return 1;
+        return EXIT_FAILURE;
     }
 
     libxl_send_sysrq(ctx, domid, sysrq[0]);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_debug_keys(int argc, char **argv)
@@ -7112,10 +7112,10 @@ int main_debug_keys(int argc, char **argv)
 
     if (libxl_send_debug_keys(ctx, keys)) {
         fprintf(stderr, "cannot send debug keys: %s\n", keys);
-        return 1;
+        return EXIT_FAILURE;
     }
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_dmesg(int argc, char **argv)
@@ -7141,7 +7141,7 @@ int main_dmesg(int argc, char **argv)
 finish:
     if (cr)
         libxl_xen_console_read_finish(ctx, cr);
-    return ret;
+    return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
 int main_top(int argc, char **argv)


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

[PATCH v3 5/6] xl: make return type of create_domain() more consistent.

[Dario Faggioli]

create_domain() is of uint32_t return type, because on
success it returns the domid of the new domain, and
uint32_t is what we typically use for domid-s.

However, on failure, it returns ERROR_FAIL or ERROR_INVAL,
which are -3 and -6. Callers assign the return value to an
'int rc' variable and then check for '(rc < 0)'.

Although things work, and no tool (compiler, Coverity, ecc.)
is complaining, using 'int' as return type seems better.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 823cb46..d2ad8c8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2777,7 +2777,7 @@ static void evdisable_disk_ejects(libxl_evgen_disk_eject **diskws,
     }
 }
 
-static uint32_t create_domain(struct domain_create *dom_info)
+static int create_domain(struct domain_create *dom_info)
 {
     uint32_t domid = INVALID_DOMID;
 


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

[PATCH v3 6/6] xl: improve exit codes of domain creation related functions

[Dario Faggioli]

by making them more consistent with other examples in xl.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
v3: In create_domain(), only deal with exit codes, internal returns
    need more work than what was being done in the original patch.
    Shorten changelog.

v2: Add create_domain()
    Remove main_sharing()
---
 tools/libxl/xl_cmdimpl.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d2ad8c8..31e2f30 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2964,7 +2964,7 @@ static int create_domain(struct domain_create *dom_info)
             if (!json) {
                 fprintf(stderr,
                         "Failed to convert domain configuration to JSON\n");
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             fputs(json, cfg_print_fh);
             free(json);
@@ -3212,7 +3212,7 @@ out:
      * already happened in the parent.
      */
     if ( daemonize && !need_daemon )
-        exit(ret);
+        exit(EXIT_SUCCESS);
 
     return ret;
 }
@@ -5345,7 +5345,7 @@ static void string_realloc_append(char **accumulate, const char *more)
     size_t morelen = strlen(more) + 1/*nul*/;
     if (oldlen > SSIZE_MAX || morelen > SSIZE_MAX - oldlen) {
         fprintf(stderr,"Additional config data far too large\n");
-        exit(-ERROR_FAIL);
+        exit(EXIT_FAILURE);
     }
 
     *accumulate = xrealloc(*accumulate, oldlen + morelen);
@@ -5420,7 +5420,7 @@ int main_create(int argc, char **argv)
         } else {
             help("create");
             free(dom_info.extra_config);
-            return 2;
+            return EXIT_FAILURE;
         }
     }
 
@@ -5440,11 +5440,11 @@ int main_create(int argc, char **argv)
     rc = create_domain(&dom_info);
     if (rc < 0) {
         free(dom_info.extra_config);
-        return -rc;
+        return EXIT_FAILURE;
     }
 
     free(dom_info.extra_config);
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_config_update(int argc, char **argv)
@@ -5465,7 +5465,7 @@ int main_config_update(int argc, char **argv)
     if (argc < 2) {
         fprintf(stderr, "xl config-update requires a domain argument\n");
         help("config-update");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     fprintf(stderr, "WARNING: xl now has better capability to manage domain configuration, "
@@ -5497,7 +5497,7 @@ int main_config_update(int argc, char **argv)
         } else {
             help("create");
             free(extra_config);
-            return 2;
+            return EXIT_FAILURE;
         }
     }
     if (filename) {
@@ -5506,25 +5506,25 @@ int main_config_update(int argc, char **argv)
                                       &config_data, &config_len);
         if (rc) { fprintf(stderr, "Failed to read config file: %s: %s\n",
                            filename, strerror(errno));
-                  free(extra_config); return ERROR_FAIL; }
+                  free(extra_config); return EXIT_FAILURE; }
         if (extra_config && strlen(extra_config)) {
             if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
                 fprintf(stderr, "Failed to attach extra configuration\n");
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             /* allocate space for the extra config plus two EOLs plus \0 */
             config_data = realloc(config_data, config_len
                 + strlen(extra_config) + 2 + 1);
             if (!config_data) {
                 fprintf(stderr, "Failed to realloc config_data\n");
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             config_len += sprintf(config_data + config_len, "\n%s\n",
                 extra_config);
         }
     } else {
         fprintf(stderr, "Config file not specified\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     libxl_domain_config_init(&d_config);
@@ -5540,7 +5540,7 @@ int main_config_update(int argc, char **argv)
                                    config_data, config_len);
         if (rc) {
             fprintf(stderr, "failed to update configuration\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
     }
 
@@ -5548,7 +5548,7 @@ int main_config_update(int argc, char **argv)
 
     free(config_data);
     free(extra_config);
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 static void button_press(uint32_t domid, const char *b)
@@ -7034,10 +7034,10 @@ int main_rename(int argc, char **argv)
     domid = find_domain(dom);
     if (libxl_domain_rename(ctx, domid, common_domname, new_name)) {
         fprintf(stderr, "Can't rename domain '%s'.\n", dom);
-        return 1;
+        return EXIT_FAILURE;
     }
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_trigger(int argc, char **argv)


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

Re: [PATCH v3 1/6] xl: improve return and exit codes of memory related functions

[Dario Faggioli]

[-- Attachment #1.1.1: Type: text/plain, Size: 7176 bytes --]

On Fri, 2016-04-08 at 04:23 +0200, Dario Faggioli wrote:
> 
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const
> char *mem)
>      memorykb = parse_mem_size_kb(mem);
>      if (memorykb == -1) {
>          fprintf(stderr, "invalid memory size: %s\n", mem);
> -        exit(3);
> +        exit(EXIT_FAILURE);
>
Actually, after George's 0614c4542, it's better if this is turned into
a return (and likewise in set_memory_target()).

The inlined (and attached) patch does that.

And there's an updated branch (labelled v4, and of course I can
resubmit the whole series from there, if necessary), with this version
of the patch in it, at:
 git://xenbits.xen.org/people/dariof/xen.git rel/xl/exit-code-cleanup-v4
 http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/xl/exit-code-cleanup-v4

Regards,
Dario
---
commit 43586222084638ec98b693702132f3412a63ddda
Author: Harmandeep Kaur <write.harmandeep@gmail.com>
Date:   Thu Apr 7 12:38:09 2016 +0200

    xl: improve return and exit codes of memory related functions
    
    by making them more consistent with other examples in xl.
    
    While there, make freemem() of boolean return type, which
    looks more natural, and add comment explaining why
    parse_mem_size_kb() needs to diverge from the pattern.
    
    Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
    Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
    ---
    Cc: Ian Jackson <ian.jackson@eu.citrix.com>
    Cc: Wei Liu <wei.liu2@citrix.com>
    Cc: George Dunlap <george.dunlap@citrix.com>
    ---
    v4: make set_memory_max() and set_memory_target() more
        consistent.
    
    v3: Shorten changelog.
        Make freemem() boolean return type.
    
    v2: Add comment to explain return vaule of parse_mem_size_kb().
        Add freemem() and main_sharing().
        Remove find_domain().

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2ee6c74..eab2abd 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2680,40 +2680,45 @@ static int preserve_domain(uint32_t *r_domid, libxl_event *event,
     return rc == 0 ? 1 : 0;
 }
 
-static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
+/*
+ * Returns false if memory can't be freed, but also if we encounter errors.
+ * Returns true in case there is already, or we manage to free it, enough
+ * memory, but also if autoballoon is false.
+ */
+static bool freemem(uint32_t domid, libxl_domain_build_info *b_info)
 {
     int rc, retries = 3;
     uint32_t need_memkb, free_memkb;
 
     if (!autoballoon)
-        return 0;
+        return true;
 
     rc = libxl_domain_need_memory(ctx, b_info, &need_memkb);
     if (rc < 0)
-        return rc;
+        return false;
 
     do {
         rc = libxl_get_free_memory(ctx, &free_memkb);
         if (rc < 0)
-            return rc;
+            return false;
 
         if (free_memkb >= need_memkb)
-            return 0;
+            return true;
 
         rc = libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0);
         if (rc < 0)
-            return rc;
+            return false;
 
         /* wait until dom0 reaches its target, as long as we are making
          * progress */
         rc = libxl_wait_for_memory_target(ctx, 0, 10);
         if (rc < 0)
-            return rc;
+            return false;
 
         retries--;
     } while (retries > 0);
 
-    return ERROR_NOMEM;
+    return false;
 }
 
 static void autoconnect_console(libxl_ctx *ctx_ignored,
@@ -2980,8 +2985,7 @@ start:
         goto error_out;
 
     if (domid_soft_reset == INVALID_DOMID) {
-        ret = freemem(domid, &d_config.b_info);
-        if (ret < 0) {
+        if (!freemem(domid, &d_config.b_info)) {
             fprintf(stderr, "failed to free memory for the domain\n");
             ret = ERROR_FAIL;
             goto error_out;
@@ -3245,6 +3249,7 @@ void help(const char *command)
     }
 }
 
+/* Returns -1 on failure; the amount of memory on success. */
 static int64_t parse_mem_size_kb(const char *mem)
 {
     char *endptr;
@@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const char *mem)
     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1) {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        return EXIT_FAILURE;
     }
 
     if (libxl_domain_setmaxmem(ctx, domid, memorykb)) {
@@ -3425,7 +3430,7 @@ static int set_memory_target(uint32_t domid, const char *mem)
     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1)  {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        return EXIT_FAILURE;
     }
 
     if (libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1)) {
@@ -6132,7 +6137,7 @@ int main_sharing(int argc, char **argv)
         info = libxl_list_domain(ctx, &nb_domain);
         if (!info) {
             fprintf(stderr, "libxl_list_domain failed.\n");
-            return 1;
+            return EXIT_FAILURE;
         }
         info_free = info;
     } else if (optind == argc-1) {
@@ -6141,17 +6146,17 @@ int main_sharing(int argc, char **argv)
         if (rc == ERROR_DOMAIN_NOTFOUND) {
             fprintf(stderr, "Error: Domain \'%s\' does not exist.\n",
                 argv[optind]);
-            return -rc;
+            return EXIT_FAILURE;
         }
         if (rc) {
             fprintf(stderr, "libxl_domain_info failed (code %d).\n", rc);
-            return -rc;
+            return EXIT_FAILURE;
         }
         info = &info_buf;
         nb_domain = 1;
     } else {
         help("sharing");
-        return 2;
+        return EXIT_FAILURE;
     }
 
     sharing(info, nb_domain);
@@ -6161,7 +6166,7 @@ int main_sharing(int argc, char **argv)
     else
         libxl_dominfo_dispose(info);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 static int sched_domain_get(libxl_scheduler sched, int domid,
-- 
<<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.1.2: xl-exit-memory-functions.patch --]
[-- Type: text/x-patch, Size: 5088 bytes --]

commit 43586222084638ec98b693702132f3412a63ddda
Author: Harmandeep Kaur <write.harmandeep@gmail.com>
Date:   Thu Apr 7 12:38:09 2016 +0200

    xl: improve return and exit codes of memory related functions
    
    by making them more consistent with other examples in xl.
    
    While there, make freemem() of boolean return type, which
    looks more natural, and add comment explaining why
    parse_mem_size_kb() needs to diverge from the pattern.
    
    Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
    Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
    ---
    Cc: Ian Jackson <ian.jackson@eu.citrix.com>
    Cc: Wei Liu <wei.liu2@citrix.com>
    Cc: George Dunlap <george.dunlap@citrix.com>
    ---
    v4: make set_memory_max() and set_memory_target() more
        consistent.
    
    v3: Shorten changelog.
        Make freemem() boolean return type.
    
    v2: Add comment to explain return vaule of parse_mem_size_kb().
        Add freemem() and main_sharing().
        Remove find_domain().

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2ee6c74..eab2abd 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2680,40 +2680,45 @@ static int preserve_domain(uint32_t *r_domid, libxl_event *event,
     return rc == 0 ? 1 : 0;
 }
 
-static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
+/*
+ * Returns false if memory can't be freed, but also if we encounter errors.
+ * Returns true in case there is already, or we manage to free it, enough
+ * memory, but also if autoballoon is false.
+ */
+static bool freemem(uint32_t domid, libxl_domain_build_info *b_info)
 {
     int rc, retries = 3;
     uint32_t need_memkb, free_memkb;
 
     if (!autoballoon)
-        return 0;
+        return true;
 
     rc = libxl_domain_need_memory(ctx, b_info, &need_memkb);
     if (rc < 0)
-        return rc;
+        return false;
 
     do {
         rc = libxl_get_free_memory(ctx, &free_memkb);
         if (rc < 0)
-            return rc;
+            return false;
 
         if (free_memkb >= need_memkb)
-            return 0;
+            return true;
 
         rc = libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0);
         if (rc < 0)
-            return rc;
+            return false;
 
         /* wait until dom0 reaches its target, as long as we are making
          * progress */
         rc = libxl_wait_for_memory_target(ctx, 0, 10);
         if (rc < 0)
-            return rc;
+            return false;
 
         retries--;
     } while (retries > 0);
 
-    return ERROR_NOMEM;
+    return false;
 }
 
 static void autoconnect_console(libxl_ctx *ctx_ignored,
@@ -2980,8 +2985,7 @@ start:
         goto error_out;
 
     if (domid_soft_reset == INVALID_DOMID) {
-        ret = freemem(domid, &d_config.b_info);
-        if (ret < 0) {
+        if (!freemem(domid, &d_config.b_info)) {
             fprintf(stderr, "failed to free memory for the domain\n");
             ret = ERROR_FAIL;
             goto error_out;
@@ -3245,6 +3249,7 @@ void help(const char *command)
     }
 }
 
+/* Returns -1 on failure; the amount of memory on success. */
 static int64_t parse_mem_size_kb(const char *mem)
 {
     char *endptr;
@@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const char *mem)
     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1) {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        return EXIT_FAILURE;
     }
 
     if (libxl_domain_setmaxmem(ctx, domid, memorykb)) {
@@ -3425,7 +3430,7 @@ static int set_memory_target(uint32_t domid, const char *mem)
     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1)  {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        return EXIT_FAILURE;
     }
 
     if (libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1)) {
@@ -6132,7 +6137,7 @@ int main_sharing(int argc, char **argv)
         info = libxl_list_domain(ctx, &nb_domain);
         if (!info) {
             fprintf(stderr, "libxl_list_domain failed.\n");
-            return 1;
+            return EXIT_FAILURE;
         }
         info_free = info;
     } else if (optind == argc-1) {
@@ -6141,17 +6146,17 @@ int main_sharing(int argc, char **argv)
         if (rc == ERROR_DOMAIN_NOTFOUND) {
             fprintf(stderr, "Error: Domain \'%s\' does not exist.\n",
                 argv[optind]);
-            return -rc;
+            return EXIT_FAILURE;
         }
         if (rc) {
             fprintf(stderr, "libxl_domain_info failed (code %d).\n", rc);
-            return -rc;
+            return EXIT_FAILURE;
         }
         info = &info_buf;
         nb_domain = 1;
     } else {
         help("sharing");
-        return 2;
+        return EXIT_FAILURE;
     }
 
     sharing(info, nb_domain);
@@ -6161,7 +6166,7 @@ int main_sharing(int argc, char **argv)
     else
         libxl_dominfo_dispose(info);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 static int sched_domain_get(libxl_scheduler sched, int domid,

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

Re: [PATCH v3 1/6] xl: improve return and exit codes of memory related functions

[Wei Liu]

On Fri, Apr 08, 2016 at 11:58:02AM +0200, Dario Faggioli wrote:
[...]
> commit 43586222084638ec98b693702132f3412a63ddda
> Author: Harmandeep Kaur <write.harmandeep@gmail.com>
> Date:   Thu Apr 7 12:38:09 2016 +0200
> 
>     xl: improve return and exit codes of memory related functions
>     
>     by making them more consistent with other examples in xl.
>     
>     While there, make freemem() of boolean return type, which
>     looks more natural, and add comment explaining why
>     parse_mem_size_kb() needs to diverge from the pattern.
>     
>     Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
>     Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
>     ---
>     Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>     Cc: Wei Liu <wei.liu2@citrix.com>
>     Cc: George Dunlap <george.dunlap@citrix.com>
>     ---

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 2/6] xl: improve exit codes of save/restore and migration functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:23:39AM +0200, Dario Faggioli wrote:
> From: Harmandeep Kaur <write.harmandeep@gmail.com>
> 
> by making them more consistent with other examples in xl.
> 
> Macros CHK_ERRNOVAL, CHK_SYSCALL, MUST are also updated.
> 
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 6/6] xl: improve exit codes of domain creation related functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:24:17AM +0200, Dario Faggioli wrote:
> by making them more consistent with other examples in xl.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: In create_domain(), only deal with exit codes, internal returns
>     need more work than what was being done in the original patch.
>     Shorten changelog.
> 
> v2: Add create_domain()
>     Remove main_sharing()
> ---
>  tools/libxl/xl_cmdimpl.c |   32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index d2ad8c8..31e2f30 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2964,7 +2964,7 @@ static int create_domain(struct domain_create *dom_info)
>              if (!json) {
>                  fprintf(stderr,
>                          "Failed to convert domain configuration to JSON\n");
> -                exit(1);
> +                exit(EXIT_FAILURE);
>              }
>              fputs(json, cfg_print_fh);
>              free(json);
> @@ -3212,7 +3212,7 @@ out:
>       * already happened in the parent.
>       */
>      if ( daemonize && !need_daemon )
> -        exit(ret);
> +        exit(EXIT_SUCCESS);

This is not entirely correct: ret can be non-zero here if I'm not
mistaken. Whether it is useful to exit with that value, I don't know.

Wei.

 

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

Re: [PATCH v3 5/6] xl: make return type of create_domain() more consistent.

[Wei Liu]

On Fri, Apr 08, 2016 at 04:24:07AM +0200, Dario Faggioli wrote:
> create_domain() is of uint32_t return type, because on
> success it returns the domid of the new domain, and
> uint32_t is what we typically use for domid-s.
> 
> However, on failure, it returns ERROR_FAIL or ERROR_INVAL,
> which are -3 and -6. Callers assign the return value to an
> 'int rc' variable and then check for '(rc < 0)'.
> 
> Although things work, and no tool (compiler, Coverity, ecc.)
> is complaining, using 'int' as return type seems better.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  tools/libxl/xl_cmdimpl.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 823cb46..d2ad8c8 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2777,7 +2777,7 @@ static void evdisable_disk_ejects(libxl_evgen_disk_eject **diskws,
>      }
>  }
>  
> -static uint32_t create_domain(struct domain_create *dom_info)
> +static int create_domain(struct domain_create *dom_info)
>  {
>      uint32_t domid = INVALID_DOMID;
>  
> 

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

Re: [PATCH v3 4/6] xl : improve exit codes of debug related functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:23:59AM +0200, Dario Faggioli wrote:
> From: Harmandeep Kaur <write.harmandeep@gmail.com>
> 
> by making them more consistent with other examples in xl.
> 
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 3/6] xl: improve exit codes of some of the domain handling functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:23:50AM +0200, Dario Faggioli wrote:
> From: Harmandeep Kaur <write.harmandeep@gmail.com>
> 
> by making them more consistent with other examples in xl.
> 
> Affected functions are the ones related to console, vnc,
> dump, destroy, shutdown, list, domid and domname.
> 
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> Signed-off-by: Dario Fagggioli <dario.faggioli@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 0/6] xl: convert exit codes related to domain subcommands to EXIT_[SUCCESS|FAILURE]

[Ian Jackson]

Dario Faggioli writes ("[PATCH v3 0/6] xl: convert exit codes related to domain subcommands to EXIT_[SUCCESS|FAILURE]"):
> This is about using EXIT_SUCCESS or EXIT_FAILURE when calling
> exit(), in xl.  main_foo() functions are treated as they were
> main(), and so they also return EXIT_SUCCESS or
> EXIT_FAILURE. Internal functions are also refactored. The idea was
> to have them return either 0 or 1, but there needs to be exceptions
> (that are documented). TBF, this series mostly deals with exit
> codes, and the amount of internal function refactoring is rather
> limited.

I have queued patches 1-5.  Thanks to Dario for the git branch.

Ian.

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