[systemd-devel] [PATCH 1/3] nspawn: move container wait logic into wait_for_container() function

[Lennart Poettering]

On Fri, 11.04.14 01:45, Djalal Harouni (tixxdz at opendz.org) wrote:

> 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.
> ---
>  src/nspawn/nspawn.c | 114 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 75 insertions(+), 39 deletions(-)
> 
> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
> index 0bd52da..d606bf2 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 succesfully. 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, k;
> +        siginfo_t status;
> +
> +        /* Explicitly set this to CONTAINER_TERMINATED. If the reboot
> +         * conditions are met it will be updated to CONTAINER_REBOOTED */
> +        *container = CONTAINER_TERMINATED;

Ad a matter of coding style we try to avoid lobbering ret-by-ref
variables if we fail. Even though this is an internal function and not
exposed outside of this file I'd really prefer if we could adhere to the
coding style. Or in other words, please reset this only where necessary.

That all said, I'd probably simplify thise, and define
CONTAINER_TERMINATED and CONTAINER_REBOOTED as anonymous enums, and
return this in the "int" return value of the function, so that no
ret-by-ref parameter is required.

Otherwise looks good!

Lennart

-- 
Lennart Poettering, Red Hat