| Message ID | 1474057631-31209-3-git-send-email-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Sep 16, 2016, at 17:46, Jeff Layton <jlayton@redhat.com> wrote: > > 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. Right. You also have: 15.1.8.7. NFS4ERR_LOCK_RANGE (Error Code 10028) A LOCK operation is operating on a range that overlaps in part a currently held byte-range lock for the current lock-owner and does not precisely match a single such byte-range lock where the server does not support this type of request, and thus does not implement POSIX locking semantics [24]. See Sections 18.10.4, 18.11.4, and 18.12.4 for a discussion of how this applies to LOCK, LOCKT, and LOCKU respectively. > > 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. The current code was designed, as I said, to allow flock() locks at least to work with a server that only supports Windows stacked locks and/or native BSD locks.
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);
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(-)