[PATCH] Convert Samba to OFD locking - version 2

[Jeremy Allison]

On Sat, May 21, 2016 at 07:36:57AM -0700, Richard Sharpe wrote:
> On Thu, May 19, 2016 at 2:12 PM, Jeremy Allison <jra at samba.org> wrote:
> > OK, here is version 2 with Uri and Simo's changes
> > incorporated. Compile-time checks now run the test
> > code rather than just compiling, and there's a
> > tuneable "smbd:force process locks" that will force
> > the old-style process specific locks. Advice to
> > use it gets printed in the log if fcntl returns
> > EINVAL.
> >
> > Please review and push if happy !
> 
> First few comment, perhaps a bit picky:
> 
> Since we always expose OFD locks to clients, if available, why do we
> need to still reference count them?

We need to reference count for the same reason
we reference count the Windows locks - if the
underlying system doesn't support OFD locks then
we need to remember if there are any outstanding
locks on close as we need to keep the closed fd
still around to stop process oriented locks from
being dropped.

We always expose OFD lock semantics to clients now
(which is the same as the Windows locks semantics),
but the underlying system may not have them.

What this code is doing is unifying the logic
between unix extensions locks and Windows locks.

> Since you are touching the line, can we get spaces after the commas?
> There is another example in that same file change.

Code already went in, sorry. Next time I touch
this code I'll try and remember to fix this.

> According to https://www.gnu.org/software/libc/manual/html_node/Open-File-Description-Locks.html
> OFD locks on an file description (pointer?) are only freed when the
> last file descriptor that refers to the file description (pointer?) is
> closed.
> 
> If that is true, the comment seems a little misleading. At that point
> do we know we only have one FD for that file? If we never dup the the
> FD, I guess we do but it would be nice to see that mentioned for the
> newbies looking at that code.

Yes, we never dup the fd - we always open() new fd's.
I'm not sure we need a comment, as dup()'ing an fd
would be an unusual case in a file server (at least
for Samba, where we've never done this).

Thanks for looking over this though, much
appreciated !