Re: [PATCH v3 2/9] nfs: check for POSIX lock capability on server even for flock locks

[Jeff Layton <jlayton@redhat.com>]

On Fri, 2016-09-16 at 21:14 +0000, Trond Myklebust wrote:
> > 
> > > > On Sep 16, 2016, at 16:27, Jeff Layton <jlayton@redhat.com> wrote:
> > 
> > We may end up in here with a FL_FLOCK lock request. We translate those
> > to whole-file NFSv4 locks and send them on to the server, so we need to
> > verify that the server supports them no matter what sort of lock request
> > this is.
> > 
> > > > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfs/nfs4proc.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 9d38366666f4..a0f25185c78c 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -6135,8 +6135,7 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
> > 	unsigned char fl_flags = request->fl_flags;
> > 	int status = -ENOLCK;
> > 
> > > > -	if ((fl_flags & FL_POSIX) &&
> > > > -			!test_bit(NFS_STATE_POSIX_LOCKS, &state->flags))
> > > > +	if (!test_bit(NFS_STATE_POSIX_LOCKS, &state->flags))
> > > > 		goto out;
> > 	/* Is this a delegated open? */
> > 	status = nfs4_set_lock_state(state, request);
> > -- 
> > 2.7.4
> 
> The ability to support FL_FLOCK locks does not depend on the server’s
> support for POSIX locking semantics. FL_FLOCK can also use stacked
> lock semantics, precisely because they always cover the whole file.

Oh! I had always thought this flags absence basically meant "I don't
support file locking at all, so don't bother sending any LOCK
requests". Now that I look though, all RFC5661 says is:


   o  OPEN4_RESULT_LOCKTYPE_POSIX indicates that the server's byte-range
      locking behavior supports the complete set of POSIX locking
      techniques [24].  From this, the client can choose to manage byte-
      range locking state in a way to handle a mismatch of byte-range
      locking management.

If this flag isn't there, I guess we can't infer anything about how the
server's locks are implemented. That's just super.

So, ok. If you think this logic is more correct as-is, then I'm fine
with dropping this patch. This check gets moved in a later patch
though, so I'll need to fix that up as well.

-- 
Jeff Layton <jlayton@redhat.com>