[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