[lxc-devel] [PATCH] shutdown: Rework API and lxc-stop
Stéphane Graber
stgraber at ubuntu.com
Fri Jan 31 17:51:47 UTC 2014
On Fri, Jan 31, 2014 at 05:41:02PM +0000, Serge Hallyn wrote:
> 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?
None which is why it prints an error message if a negative value other
than -1 is passed.
>
> > + 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.
lxc-stop -n p1 -t 0 is what you're describing (sends SIGPWR and exits 0
immediately).
>
> > +
> > /* 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
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140131/228fa9a3/attachment-0001.pgp>
More information about the lxc-devel
mailing list