[3/9] xl: Improve return and exit codes of migrate related functions.

Commit Message

Harmandeep Kaur Feb. 24, 2016, 12:53 p.m. UTC

Signed-off-by: Harmandeep Kaur <[e-mail]>
---
 tools/libxl/xl_cmdimpl.c | 56 ++++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

Comments

Dario Faggioli March 2, 2016, 5:53 p.m. UTC | #1

On Wed, 2016-02-24 at 18:23 +0530, Harmandeep Kaur wrote:
> @@ -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);                                     
>     \
>          }                                                           
>     \
>      })
>  
Right below this, there are two more macros, CHK_SYSCALL and MUST,
which also need "fixing"

> @@ -4152,7 +4152,7 @@ static pid_t create_migration_child(const char
> *rune, int *send_fd,
>      pid_t child;
>  
>      if (!rune || !send_fd || !recv_fd)
> -        return -1;
> +        return EXIT_FAILURE;
> 
Err.. no, create_migration_child() is an internal function, so it is ok
for it to return -1/0, isn't it?
>  
>      MUST( libxl_pipe(ctx, sendpipe) );
>      MUST( libxl_pipe(ctx, recvpipe) );
> @@ -4166,7 +4166,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);
>      }
Of course, in this specific case, since it's an exit() and not a
'return', it is ok to convert this to EXIT_FAILURE, like you're doing.

> @@ -4189,16 +4189,16 @@ 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 EXIT_FAILURE;
>  
Internal too, so ok changing it, but to -1/0.

Thanks and Regards,
Dario

Patch

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e5bb41f..2092c80 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);                                         \
         }                                                               \
     })
 
@@ -4036,7 +4036,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);
@@ -4045,14 +4045,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' */
@@ -4152,7 +4152,7 @@  static pid_t create_migration_child(const char *rune, int *send_fd,
     pid_t child;
 
     if (!rune || !send_fd || !recv_fd)
-        return -1;
+        return EXIT_FAILURE;
 
     MUST( libxl_pipe(ctx, sendpipe) );
     MUST( libxl_pipe(ctx, recvpipe) );
@@ -4166,7 +4166,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]);
@@ -4189,16 +4189,16 @@  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 EXIT_FAILURE;
 
     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 EXIT_FAILURE;
     }
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 static void migration_child_report(int recv_fd) {
@@ -4272,7 +4272,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,
@@ -4281,7 +4281,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",
@@ -4306,7 +4306,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);
@@ -4393,26 +4393,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,
@@ -4425,7 +4425,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,
@@ -4460,7 +4460,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;
@@ -4497,7 +4497,7 @@  static void migrate_receive(int debug, int daemonize, int monitor,
                     "Failed to unpause domain %s (id: %u):%d\n",
                     common_domname, domid, rc);
 
-        exit(rc ? -ERROR_FAIL: 0);
+        exit(rc ? EXIT_FAILURE : EXIT_SUCCESS);
     }
 
     fprintf(stderr, "migration target: Transfer complete,"
@@ -4507,7 +4507,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),
@@ -4532,14 +4532,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");
@@ -4548,7 +4548,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"
@@ -4559,10 +4559,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)
@@ -4656,13 +4656,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,
                     remus);
 
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 int main_save(int argc, char **argv)
@@ -4761,7 +4761,7 @@  int main_migrate(int argc, char **argv)
     }
 
     migrate_domain(domid, rune, debug, config_filename);
-    return 0;
+    return EXIT_SUCCESS;
 }
 #endif