[lxc-devel] [PATCH] simplify remove_partial by closing fd early

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> This came up as I was chasing an fd leak with valgrind that showed up
> in the fork()ed child. There is no reason we can't just close the fd
> before the fork which also simplifies the code.

There is though - the ongoing_create() functions checks the pid of the
task holding the lock on the /partial file.  If you close it
immediately, then a racing create or start will assume that the create
failed and remove the container.

So unfortunately we can't close that fd until we're done.

> Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> ---
>  src/lxc/lxccontainer.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index c90b564..6ae6c12 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -177,14 +177,13 @@ static int create_partial(struct lxc_container *c)
>  	return fd;
>  }
>  
> -static void remove_partial(struct lxc_container *c, int fd)
> +static void remove_partial(struct lxc_container *c)
>  {
>  	// $lxcpath + '/' + $name + '/partial' + \0
>  	int len = strlen(c->config_path) + strlen(c->name) + 10;
>  	char *path = alloca(len);
>  	int ret;
>  
> -	close(fd);
>  	ret = snprintf(path, len, "%s/%s/partial", c->config_path, c->name);
>  	if (ret < 0 || ret >= len) {
>  		ERROR("Error writing partial pathname");
> @@ -1287,6 +1286,7 @@ static bool lxcapi_create(struct lxc_container *c, const char *t,
>  	/* Mark that this container is being created */
>  	if ((partial_fd = create_partial(c)) < 0)
>  		goto out;
> +	close(partial_fd);
>  
>  	/* no need to get disk lock bc we have the partial locked */
>  
> @@ -1347,8 +1347,7 @@ static bool lxcapi_create(struct lxc_container *c, const char *t,
>  	ret = load_config_locked(c, c->configfile);
>  
>  out_unlock:
> -	if (partial_fd >= 0)
> -		remove_partial(c, partial_fd);
> +	remove_partial(c);
>  out:
>  	if (!ret && c)
>  		lxcapi_destroy(c);
> -- 
> 1.8.5.3
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel