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

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> On Fri, 7 Mar 2014 18:08:19 -0600
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > 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?
> > 
> > Yup, that sounds good, thanks.
> > 
> > -serge
> 
> I tried doing the close in the child in lxcapi_create() and also in the
> child in create_run_template(), but valgrind was still reporting a leak
> which I'm not sure where it is coming from. So I think we can just drop
> this, there isn't a leak in the parent path.

Odd.  I wonder if there's a more serious problem in there...  But you
say the original patch which immediately closed that fd fixed it, right?
So it must just valgrind being paranoid...  weird.

-serge