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

Dwight Engen dwight.engen at oracle.com
Sat Mar 8 00:03:55 UTC 2014


On Fri, 7 Mar 2014 17:27:38 -0600
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:

> 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.

Got it, thanks, sorry for the noise. I missed the unlock on close
thing. Do you think its worthwhile to close it just in the child since
the lock won't be inherited across fork?

> > 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
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel



More information about the lxc-devel mailing list