[lxc-devel] [PATCH] shutdown: Rework API and lxc-stop

Serge Hallyn serge.hallyn at ubuntu.com
Fri Jan 31 17:41:02 UTC 2014


Quoting Stéphane Graber (stgraber at ubuntu.com):
> With this change, shutdown() will no longer call stop() after the
> timeout, instead it'll just return false and it's up to the caller to
> then call stop() if appropriate.
> 
> This also updates the bindings, tests and other scripts.
> 
> lxc-stop is then updated to do proper option checking and use shutdown,
> stop or reboot as appropriate.
> 
> Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>

Two questions below.  Provided the answer is "that's what I wanted",
then

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

> ---
>  src/lxc/lxc-start-ephemeral.in      |  6 ++--
>  src/lxc/lxc_stop.c                  | 64 ++++++++++++++++++++++++++++++++-----
>  src/lxc/lxccontainer.c              |  4 ---
>  src/lxc/lxccontainer.h              |  8 ++---
>  src/python-lxc/examples/api_test.py |  3 +-
>  src/python-lxc/lxc.c                |  5 ++-
>  src/tests/shutdowntest.c            |  3 ++
>  7 files changed, 70 insertions(+), 23 deletions(-)
> 
> diff --git a/src/lxc/lxc-start-ephemeral.in b/src/lxc/lxc-start-ephemeral.in
> index d40ce4e..62a6194 100644
> --- a/src/lxc/lxc-start-ephemeral.in
> +++ b/src/lxc/lxc-start-ephemeral.in
> @@ -270,7 +270,8 @@ if not dest.start() or not dest.wait("RUNNING", timeout=5):
>  # Deal with the case where we just attach to the container's console
>  if not args.command and not args.daemon:
>      dest.console()
> -    dest.shutdown(timeout=5)
> +    if not dest.shutdown(timeout=5):
> +        dest.stop()
>      sys.exit(0)
>  
>  # Try to get the IP addresses
> @@ -318,6 +319,7 @@ for ip in ips:
>      break
>  
>  # Shutdown the container
> -dest.shutdown(timeout=5)
> +if not dest.shutdown(timeout=5):
> +    dest.stop()
>  
>  sys.exit(retval)
> diff --git a/src/lxc/lxc_stop.c b/src/lxc/lxc_stop.c
> index d8de2b0..47db9e8 100644
> --- a/src/lxc/lxc_stop.c
> +++ b/src/lxc/lxc_stop.c
> @@ -77,7 +77,7 @@ Options :\n\
>  	.options  = my_longopts,
>  	.parser   = my_parser,
>  	.checker  = NULL,
> -	.timeout = 60,
> +	.timeout  = -2,
>  };
>  
>  /* returns -1 on failure, 0 on success */
> @@ -95,7 +95,7 @@ static int do_reboot_and_check(struct lxc_arguments *a, struct lxc_container *c)
>  		return -1;
>  	if (a->nowait)
>  		return 0;
> -	if (timeout <= 0)
> +	if (timeout == 0)
>  		goto out;
>  
>  	for (;;) {
> @@ -118,7 +118,7 @@ static int do_reboot_and_check(struct lxc_arguments *a, struct lxc_container *c)
>  		if (ret)
>  			break;
>  		elapsed_time = tv.tv_sec - curtime;
> -		if (timeout - elapsed_time <= 0)
> +		if (timeout != -1 && timeout - elapsed_time <= 0)
>  			break;
>  		timeout -= elapsed_time;
>  	}
> @@ -145,6 +145,51 @@ int main(int argc, char *argv[])
>  			 my_args.progname, my_args.quiet, my_args.lxcpath[0]))
>  		return 1;
>  
> +	/* Set default timeout */
> +	if (my_args.timeout == -2) {
> +		if (my_args.hardstop) {
> +			my_args.timeout = 0;
> +		}
> +		else {
> +			my_args.timeout = 60;
> +		}
> +	}
> +
> +	if (my_args.nowait) {
> +		my_args.timeout = 0;
> +	}
> +
> +	/* some checks */
> +	if (!my_args.hardstop && my_args.timeout < -1) {

Is this (< -1) what you actually meant?  What other value besides
-2 that is < -1 is valid here?

> +		fprintf(stderr, "invalid timeout\n");
> +		return 1;
> +	}
> +
> +	if (my_args.hardstop && my_args.nokill) {
> +		fprintf(stderr, "-k can't be used with --nokill\n");
> +		return 1;
> +	}
> +
> +	if (my_args.hardstop && my_args.reboot) {
> +		fprintf(stderr, "-k can't be used with -r\n");
> +		return 1;
> +	}
> +
> +	if (my_args.hardstop && my_args.timeout) {
> +		fprintf(stderr, "-k doesn't allow timeouts\n");
> +		return 1;
> +	}
> +
> +	if (my_args.nolock && !my_args.hardstop) {
> +		fprintf(stderr, "--nolock may only be used with -k\n");
> +		return 1;
> +	}
> +
> +	if (my_args.nokill && my_args.timeout < 1) {
> +		fprintf(stderr, "--nokill requires a positive timeout\n");
> +		return 1;
> +	}

Oh, I thought you were going to have --no-kill -t 0 simply wait
forever for the shutdown to complete.  This is fine, but I just
want to make sure it's what you wanted.

> +
>  	/* shortcut - if locking is bogus, we should be able to kill
>  	 * containers at least */
>  	if (my_args.nolock)
> @@ -167,22 +212,25 @@ int main(int argc, char *argv[])
>  		goto out;
>  	}
>  
> +	/* kill */
>  	if (my_args.hardstop) {
>  		ret = c->stop(c) ? 0 : 1;
>  		goto out;
>  	}
> +
> +	/* reboot */
>  	if (my_args.reboot) {
>  		ret = do_reboot_and_check(&my_args, c);
>  		goto out;
>  	}
>  
> -	if (my_args.nokill)
> -		my_args.timeout = 0;
> -
> +	/* shutdown */
>  	s = c->shutdown(c, my_args.timeout);
>  	if (!s) {
> -		if (!my_args.shutdown)
> -			ret = c->wait(c, "STOPPED", -1) ? 0 : 1;
> +		if (!my_args.nokill)
> +			ret = c->stop(c) ? 0 : 1;
> +		else if (my_args.timeout == 0)
> +			ret = 0;
>  		else
>  			ret = 1; // fail
>  	} else
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index a0a5af5..85c644a 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -1348,10 +1348,6 @@ static bool lxcapi_shutdown(struct lxc_container *c, int timeout)
>  		haltsignal = c->lxc_conf->haltsignal;
>  	kill(pid, haltsignal);
>  	retv = c->wait(c, "STOPPED", timeout);
> -	if (!retv && timeout > 0) {
> -		c->stop(c);
> -		retv = c->wait(c, "STOPPED", 0); // 0 means don't wait
> -	}
>  	return retv;
>  }
>  
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index 5f5d9b2..92c76b4 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -362,12 +362,10 @@ struct lxc_container {
>  	 * SIGPWR.
>  	 *
>  	 * \param c Container.
> -	 * \param timeout Seconds to wait before forcing a hard stop
> -	 *  (value must be >0).
> +	 * \param timeout Seconds to wait before returning false.
> +	 *  (-1 to wait forever, 0 to avoid waiting).
>  	 *
> -	 * \return \c true if configuration was loaded successfully, else \c false.
> -	 *
> -	 * \note A \p timeout of \c 0 means do not wait.
> +	 * \return \c true if the container was shutdown successfully, else \c false.
>  	 */
>  	bool (*shutdown)(struct lxc_container *c, int timeout);
>  
> diff --git a/src/python-lxc/examples/api_test.py b/src/python-lxc/examples/api_test.py
> index f72cc56..a1fe6f9 100755
> --- a/src/python-lxc/examples/api_test.py
> +++ b/src/python-lxc/examples/api_test.py
> @@ -144,7 +144,8 @@ if len(sys.argv) > 1 and sys.argv[1] == "--with-console":
>  
>  ## Shutting down the container
>  print("Shutting down the container")
> -container.shutdown(3)
> +if not container.shutdown(3):
> +    container.stop()
>  
>  if container.running:
>      print("Stopping the container")
> diff --git a/src/python-lxc/lxc.c b/src/python-lxc/lxc.c
> index 05727bf..9fdc27f 100644
> --- a/src/python-lxc/lxc.c
> +++ b/src/python-lxc/lxc.c
> @@ -1591,9 +1591,8 @@ static PyMethodDef Container_methods[] = {
>       METH_VARARGS|METH_KEYWORDS,
>       "shutdown(timeout = -1) -> boolean\n"
>       "\n"
> -     "Sends SIGPWR to the container and wait for it to shutdown "
> -     "unless timeout is set to a positive value, in which case "
> -     "the container will be killed when the timeout is reached."
> +     "Sends SIGPWR to the container and wait for it to shutdown."
> +     "-1 means wait forever, 0 means skip waiting."
>      },
>      {"snapshot", (PyCFunction)Container_snapshot,
>       METH_VARARGS|METH_KEYWORDS,
> diff --git a/src/tests/shutdowntest.c b/src/tests/shutdowntest.c
> index 6e4bb31..c5d609d 100644
> --- a/src/tests/shutdowntest.c
> +++ b/src/tests/shutdowntest.c
> @@ -69,6 +69,9 @@ int main(int argc, char *argv[])
>  		goto out;
>  	}
>  
> +	/* Wait for init to be ready for SIGPWR */
> +	sleep(10);
> +
>  	if (!c->shutdown(c, 60)) {
>  		fprintf(stderr, "%d: failed to shut down %s\n", __LINE__, MYNAME);
>  		goto out;
> -- 
> 1.9.rc1
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel


More information about the lxc-devel mailing list