nspawn: let's avoid using goto to wildly for non-cleanup purposes

[elogind.git] / src / nspawn / nspawn.c

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0a8dc0cf301cd7bf047487e75fed5cc33db06c44..fd61d07761fc9353852b6ca3ca7bee79a5878f28 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2645,20 +2645,31 @@ static int change_uid_gid(char **_home) {
 }
 
 /*
- * Return 0 in case the container is being rebooted, has been shut
- * down or exited successfully. On failures a negative value is
- * returned.
+ * Return values:
+ * < 0 : wait_for_terminate() failed to get the state of the
+ *       container, the container was terminated by a signal, or
+ *       failed for an unknown reason.  No change is made to the
+ *       container argument.
+ * > 0 : The program executed in the container terminated with an
+ *       error.  The exit code of the program executed in the
+ *       container is returned.  No change is made to the container
+ *       argument.
+ *   0 : The container is being rebooted, has been shut down or exited
+ *       successfully.  The container argument has been set to either
+ *       CONTAINER_TERMINATED or CONTAINER_REBOOTED.
  *
- * The status of the container "CONTAINER_TERMINATED" or
- * "CONTAINER_REBOOTED" will be saved in the container argument
+ * That is, success is indicated by a return value of zero, and an
+ * error is indicated by a non-zero value.
  */
 static int wait_for_container(pid_t pid, ContainerStatus *container) {
         int r;
         siginfo_t status;
 
         r = wait_for_terminate(pid, &status);
-        if (r < 0)
+        if (r < 0) {
+                log_warning("Failed to wait for container: %s", strerror(-r));
                 return r;
+        }
 
         switch (status.si_code) {
         case CLD_EXITED:
@@ -2672,7 +2683,6 @@ static int wait_for_container(pid_t pid, ContainerStatus *container) {
                 } else {
                         log_error("Container %s failed with error code %i.",
                                   arg_machine, status.si_status);
-                        r = -1;
                 }
                 break;
 
@@ -3237,72 +3247,75 @@ int main(int argc, char *argv[]) {
                  * join its cgroup which might limit what it can do */
                 r = eventfd_child_succeeded(eventfds[1]);
                 eventfds[1] = safe_close(eventfds[1]);
-                if (r < 0)
-                        goto check_container_status;
 
-                r = register_machine(pid);
-                if (r < 0)
-                        goto finish;
+                if (r >= 0) {
+                        r = register_machine(pid);
+                        if (r < 0)
+                                goto finish;
 
-                r = move_network_interfaces(pid);
-                if (r < 0)
-                        goto finish;
+                        r = move_network_interfaces(pid);
+                        if (r < 0)
+                                goto finish;
 
-                r = setup_veth(pid, veth_name);
-                if (r < 0)
-                        goto finish;
+                        r = setup_veth(pid, veth_name);
+                        if (r < 0)
+                                goto finish;
 
-                r = setup_bridge(veth_name);
-                if (r < 0)
-                        goto finish;
+                        r = setup_bridge(veth_name);
+                        if (r < 0)
+                                goto finish;
 
-                r = setup_macvlan(pid);
-                if (r < 0)
-                        goto finish;
+                        r = setup_macvlan(pid);
+                        if (r < 0)
+                                goto finish;
 
-                /* Block SIGCHLD here, before notifying child.
-                 * process_pty() will handle it with the other signals. */
-                r = sigprocmask(SIG_BLOCK, &mask_chld, NULL);
-                if (r < 0)
-                        goto finish;
+                        /* Block SIGCHLD here, before notifying child.
+                         * process_pty() will handle it with the other signals. */
+                        r = sigprocmask(SIG_BLOCK, &mask_chld, NULL);
+                        if (r < 0)
+                                goto finish;
 
-                /* Reset signal to default */
-                r = default_signals(SIGCHLD, -1);
-                if (r < 0)
-                        goto finish;
+                        /* Reset signal to default */
+                        r = default_signals(SIGCHLD, -1);
+                        if (r < 0)
+                                goto finish;
 
-                /* Notify the child that the parent is ready with all
-                 * its setup, and that the child can now hand over
-                 * control to the code to run inside the container. */
-                r = eventfd_send_state(eventfds[0],
-                                       EVENTFD_PARENT_SUCCEEDED);
-                eventfds[0] = safe_close(eventfds[0]);
-                if (r < 0)
-                        goto finish;
+                        /* Notify the child that the parent is ready with all
+                         * its setup, and that the child can now hand over
+                         * control to the code to run inside the container. */
+                        r = eventfd_send_state(eventfds[0], EVENTFD_PARENT_SUCCEEDED);
+                        eventfds[0] = safe_close(eventfds[0]);
+                        if (r < 0)
+                                goto finish;
 
-                k = process_pty(master, &mask, arg_boot ? pid : 0, SIGRTMIN+3);
-                if (k < 0) {
-                        r = EXIT_FAILURE;
-                        break;
-                }
+                        k = process_pty(master, &mask, arg_boot ? pid : 0, SIGRTMIN+3);
+                        if (k < 0) {
+                                r = EXIT_FAILURE;
+                                break;
+                        }
 
-                if (!arg_quiet)
-                        putc('\n', stdout);
+                        if (!arg_quiet)
+                                putc('\n', stdout);
 
-                /* Kill if it is not dead yet anyway */
-                terminate_machine(pid);
+                        /* Kill if it is not dead yet anyway */
+                        terminate_machine(pid);
+                }
 
-check_container_status:
-                /* Redundant, but better safe than sorry */
+                /* Normally redundant, but better safe than sorry */
                 kill(pid, SIGKILL);
 
                 r = wait_for_container(pid, &container_status);
                 pid = 0;
 
                 if (r < 0) {
+                        /* We failed to wait for the container, or the
+                         * container exited abnormally */
                         r = EXIT_FAILURE;
                         break;
-                } else if (container_status == CONTAINER_TERMINATED)
+                } else if (r > 0 || container_status == CONTAINER_TERMINATED)
+                        /* The container exited with a non-zero
+                         * status, or with zero status and no reboot
+                         * was requested. */
                         break;
 
                 /* CONTAINER_REBOOTED, loop again */