[systemd-devel] [PATCH v2] nspawn: move container wait logic into wait_for_container()

[Lennart Poettering]

On Fri, 02.05.14 12:05, Djalal Harouni (tixxdz at opendz.org) wrote:

Ah, thanks for changing the clobbering thing. Should have read the rest
of the thread before replying to the earlier patch... (making the enum
anonymous might still be a an option though, but I will merge it even at is).

Sorry for the late review!

Lennart

> Move the container wait logic into its own wait_for_container() function
> and add two status codes: CONTAINER_TERMINATED or CONTAINER_REBOOTED.
> 
> These status codes are used to terminate nspawn or loop again in case of
> CONTAINER_REBOOTED.
> ---
> v1 -> v2:
> Apply Tom Gundersen notes.
> 
> src/nspawn/nspawn.c | 113 +++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 76 insertions(+), 37 deletions(-)
> 
> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
> index 0bd52da..5bd2b34 100644
> --- a/src/nspawn/nspawn.c
> +++ b/src/nspawn/nspawn.c
> @@ -92,6 +92,11 @@
>  #include "seccomp-util.h"
>  #endif
>  
> +typedef enum ContainerStatus {
> +        CONTAINER_TERMINATED,
> +        CONTAINER_REBOOTED
> +} ContainerStatus;
> +
>  typedef enum LinkJournal {
>          LINK_NO,
>          LINK_AUTO,
> @@ -2565,6 +2570,72 @@ static int change_uid_gid(char **_home) {
>          return 0;
>  }
>  
> +/* Return 0 in case the container is being rebooted, has been shut
> + * down or exited successfully. On failures a non-zero value is
> + * returned.
> + *
> + * The status of the container "CONTAINER_TERMINATED" or
> + * "CONTAINER_REBOOTED" will be saved in the container argument */
> +static int wait_for_container(pid_t pid, ContainerStatus *container) {
> +        int r;
> +        siginfo_t status;
> +
> +        r = wait_for_terminate(pid, &status);
> +        if (r < 0)
> +                return r;
> +
> +        /* If the reboot conditions are met it will be updated to
> +         * CONTAINER_REBOOTED, otherwise on all success or failure
> +         * paths, the container is put in the CONTAINER_TERMINATED
> +         * state. */
> +        *container = CONTAINER_TERMINATED;
> +
> +        switch (status.si_code) {
> +        case CLD_EXITED:
> +                r = status.si_status;
> +                if (r != 0) {
> +                        log_error("Container %s failed with error code %i.",
> +                                  arg_machine, status.si_status);
> +                        r = -1;
> +                } else if (!arg_quiet) {
> +                        log_debug("Container %s exited successfully.",
> +                                  arg_machine);
> +                }
> +                break;
> +
> +        case CLD_KILLED:
> +                if (status.si_status == SIGINT) {
> +                        if (!arg_quiet)
> +                                log_info("Container %s has been shut down.",
> +                                         arg_machine);
> +                        r = 0;
> +                        break;
> +                } else if (status.si_status == SIGHUP) {
> +                        if (!arg_quiet)
> +                                log_info("Container %s is being rebooted.",
> +                                         arg_machine);
> +
> +                        *container = CONTAINER_REBOOTED;
> +                        r = 0;
> +                        break;
> +                }
> +
> +        case CLD_DUMPED:
> +                log_error("Container %s terminated by signal %s.",
> +                          arg_machine, signal_to_string(status.si_status));
> +                r = -1;
> +                break;
> +
> +        default:
> +                log_error("Container %s failed due to unknown reason.",
> +                          arg_machine);
> +                r = -1;
> +                break;
> +        }
> +
> +        return r;
> +}
> +
>  int main(int argc, char *argv[]) {
>  
>          _cleanup_free_ char *kdbus_domain = NULL, *device_path = NULL, *root_device = NULL, *home_device = NULL, *srv_device = NULL;
> @@ -2743,8 +2814,8 @@ int main(int argc, char *argv[]) {
>          assert_se(sigprocmask(SIG_BLOCK, &mask, NULL) == 0);
>  
>          for (;;) {
> +                ContainerStatus container_status;
>                  int parent_ready_fd = -1, child_ready_fd = -1;
> -                siginfo_t status;
>                  eventfd_t x;
>  
>                  parent_ready_fd = eventfd(0, EFD_CLOEXEC);
> @@ -3094,48 +3165,16 @@ int main(int argc, char *argv[]) {
>                  /* Redundant, but better safe than sorry */
>                  kill(pid, SIGKILL);
>  
> -                k = wait_for_terminate(pid, &status);
> +                r = wait_for_container(pid, &container_status);
>                  pid = 0;
>  
> -                if (k < 0) {
> +                if (r < 0) {
>                          r = EXIT_FAILURE;
>                          break;
> -                }
> -
> -                if (status.si_code == CLD_EXITED) {
> -                        r = status.si_status;
> -                        if (status.si_status != 0) {
> -                                log_error("Container %s failed with error code %i.", arg_machine, status.si_status);
> -                                break;
> -                        }
> -
> -                        if (!arg_quiet)
> -                                log_debug("Container %s exited successfully.", arg_machine);
> +                } else if (container_status == CONTAINER_TERMINATED)
>                          break;
> -                } else if (status.si_code == CLD_KILLED &&
> -                           status.si_status == SIGINT) {
>  
> -                        if (!arg_quiet)
> -                                log_info("Container %s has been shut down.", arg_machine);
> -                        r = 0;
> -                        break;
> -                } else if (status.si_code == CLD_KILLED &&
> -                           status.si_status == SIGHUP) {
> -
> -                        if (!arg_quiet)
> -                                log_info("Container %s is being rebooted.", arg_machine);
> -                        continue;
> -                } else if (status.si_code == CLD_KILLED ||
> -                           status.si_code == CLD_DUMPED) {
> -
> -                        log_error("Container %s terminated by signal %s.", arg_machine, signal_to_string(status.si_status));
> -                        r = EXIT_FAILURE;
> -                        break;
> -                } else {
> -                        log_error("Container %s failed due to unknown reason.", arg_machine);
> -                        r = EXIT_FAILURE;
> -                        break;
> -                }
> +                /* CONTAINER_REBOOTED, loop again */
>          }
>  
>  finish:


Lennart

-- 
Lennart Poettering, Red Hat