[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