[lxc-devel] [PATCH 2/2] daemon: remove pidfile when daemon container is stopped

[Stéphane Graber]

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.huangqiang at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140118/3dcb1b2a/attachment.pgp>