On Sat, Jan 18, 2014 at 03:00:00PM +0800, Qiang Huang wrote: > This is for bug: https://github.com/lxc/lxc/issues/89 > > When start container with daemon model, the daemon process's > father will return back to main in start time, and pidfile > is removed then, that's not right. > So we store pidfile to lxc_container, and remove it when > lxc_container_free.
That one looks wrong to me, removing the pid file on lxc_container_free
is wrong. We want the pidfile removed when the background lxc-start
exits, not whenever a random API client flushes the container from
memory.
With your patch, doing something like:
- lxc-start -n p1 -d -p /tmp/pid
- python3
import lxc
p1 = lxc.Container("p1")
p1 = None
Or the equivalent with any binding, or directly in C, will destroy the
pid file even though the container is clearly still running.
>
> Signed-off-by: Qiang Huang <h.huangqi...@huawei.com>
> ---
> src/lxc/lxc_start.c | 9 +++++----
> src/lxc/lxccontainer.c | 6 ++++++
> src/lxc/lxccontainer.h | 6 ++++++
> 3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
> index d5379da..fd2dc6e 100644
> --- a/src/lxc/lxc_start.c
> +++ b/src/lxc/lxc_start.c
> @@ -302,6 +302,11 @@ int main(int argc, char *argv[])
> }
>
> if (my_args.pidfile != NULL) {
> + if (ensure_path(&c->pidfile, my_args.pidfile) < 0) {
> + ERROR("failed to ensure pidfile '%s'", my_args.pidfile);
> + goto out;
> + }
> +
> pid_fp = fopen(my_args.pidfile, "w");
> if (pid_fp == NULL) {
> SYSERROR("failed to create pidfile '%s' for '%s'",
> @@ -342,10 +347,6 @@ int main(int argc, char *argv[])
> c->want_close_all_fds(c, true);
>
> err = c->start(c, 0, args) ? 0 : -1;
> -
> - if (my_args.pidfile)
> - unlink(my_args.pidfile);
> -
> out:
> lxc_container_put(c);
> if (pid_fp)
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index ddea0d7..d742781 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -231,6 +231,11 @@ static void lxc_container_free(struct lxc_container *c)
> free(c->config_path);
> c->config_path = NULL;
> }
> + if (c->pidfile) {
> + unlink(c->pidfile);
> + free(c->pidfile);
> + c->pidfile = NULL;
> + }
> free(c);
> }
>
> @@ -3051,6 +3056,7 @@ struct lxc_container *lxc_container_new(const char
> *name, const char *configpath
> lxcapi_clear_config(c);
> }
> c->daemonize = true;
> + c->pidfile = NULL;
>
> // assign the member functions
> c->is_defined = lxcapi_is_defined;
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index 921e47d..a860648 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -68,6 +68,12 @@ struct lxc_container {
>
> /*!
> * \private
> + * File to store pid.
> + */
> + char *pidfile;
> +
> + /*!
> + * \private
> * Container semaphore lock.
> */
> struct lxc_lock *slock;
> --
> 1.8.3
>
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
signature.asc
Description: Digital signature
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
