Re: [Cluster-devel] [PATCH v2 01/17] locks: consolidate nolease routines
On Thu, 4 Sep 2014 13:12:00 -0700 Christoph Hellwig h...@infradead.org wrote: On Thu, Sep 04, 2014 at 02:25:35PM -0400, Trond Myklebust wrote: Actually, it looks as if when you compile with !CONFIG_FILE_LOCKING, then fcntl_setlease() returns the value '0' (which would be success!). The word confusing only begins to describe it all. That's incorrect for sure, we should agree on a single sensible code for: 1) !CONFIG_FILE_LOCKING 2) !lease_enable 3) filesystem doesn't support leases. Agreed. I think -ENOLCK is really better than -EINVAL. I usually take -EINVAL to mean you sent me something bogus. Whereas -ENOLCK just says locking doesn't work. -ENOLCK seems closer to the situation in all 3 cases above. That said, this is a user-visible change. The main userland consumer of leases (AFAIK) is samba, so I'll take a peek at that code and run it by them before merging anything. -- Jeff Layton jlay...@primarydata.com
Re: [Cluster-devel] [PATCH v2 01/17] locks: consolidate nolease routines
On Thu, Sep 4, 2014 at 8:38 AM, Jeff Layton jlay...@primarydata.com wrote: GFS2 and NFS have setlease routines that always just return -EINVAL. Turn that into a generic routine that can live in fs/libfs.c. Cc: Trond Myklebust trond.mykleb...@primarydata.com Cc: linux-...@vger.kernel.org Cc: Steven Whitehouse swhit...@redhat.com Cc: cluster-devel@redhat.com Signed-off-by: Jeff Layton jlay...@primarydata.com --- fs/gfs2/file.c | 22 +- fs/libfs.c | 16 fs/nfs/file.c | 13 + fs/nfs/internal.h | 1 - fs/nfs/nfs4file.c | 2 +- include/linux/fs.h | 1 + 6 files changed, 20 insertions(+), 35 deletions(-) Acked-by: Trond Myklebust trond.mykleb...@primarydata.com -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.mykleb...@primarydata.com
Re: [Cluster-devel] [PATCH v2 01/17] locks: consolidate nolease routines
On Thu, 4 Sep 2014 08:41:51 -0400 Trond Myklebust trond.mykleb...@primarydata.com wrote: On Thu, Sep 4, 2014 at 8:38 AM, Jeff Layton jlay...@primarydata.com wrote: GFS2 and NFS have setlease routines that always just return -EINVAL. Turn that into a generic routine that can live in fs/libfs.c. Cc: Trond Myklebust trond.mykleb...@primarydata.com Cc: linux-...@vger.kernel.org Cc: Steven Whitehouse swhit...@redhat.com Cc: cluster-devel@redhat.com Signed-off-by: Jeff Layton jlay...@primarydata.com --- fs/gfs2/file.c | 22 +- fs/libfs.c | 16 fs/nfs/file.c | 13 + fs/nfs/internal.h | 1 - fs/nfs/nfs4file.c | 2 +- include/linux/fs.h | 1 + 6 files changed, 20 insertions(+), 35 deletions(-) Acked-by: Trond Myklebust trond.mykleb...@primarydata.com Thanks. While spinning this up, I did have a momentary pause to wonder if -ENOLCK would be a better return value here. It would make it easier to distinguish this from from oops, I passed in bogus arguments. For now, I'll leave it as -EINVAL, but it's something to consider... -- Jeff Layton jlay...@primarydata.com
Re: [Cluster-devel] [PATCH v2 01/17] locks: consolidate nolease routines
On Thu, Sep 04, 2014 at 08:38:27AM -0400, Jeff Layton wrote: GFS2 and NFS have setlease routines that always just return -EINVAL. Turn that into a generic routine that can live in fs/libfs.c. Looks good, Reviewed-by: Christoph Hellwig h...@lst.de
Re: [Cluster-devel] [PATCH v2 01/17] locks: consolidate nolease routines
On Thu, Sep 4, 2014 at 8:49 AM, Jeff Layton jeff.lay...@primarydata.com wrote: On Thu, 4 Sep 2014 08:41:51 -0400 Trond Myklebust trond.mykleb...@primarydata.com wrote: On Thu, Sep 4, 2014 at 8:38 AM, Jeff Layton jlay...@primarydata.com wrote: GFS2 and NFS have setlease routines that always just return -EINVAL. Turn that into a generic routine that can live in fs/libfs.c. Cc: Trond Myklebust trond.mykleb...@primarydata.com Cc: linux-...@vger.kernel.org Cc: Steven Whitehouse swhit...@redhat.com Cc: cluster-devel@redhat.com Signed-off-by: Jeff Layton jlay...@primarydata.com --- fs/gfs2/file.c | 22 +- fs/libfs.c | 16 fs/nfs/file.c | 13 + fs/nfs/internal.h | 1 - fs/nfs/nfs4file.c | 2 +- include/linux/fs.h | 1 + 6 files changed, 20 insertions(+), 35 deletions(-) Acked-by: Trond Myklebust trond.mykleb...@primarydata.com Thanks. While spinning this up, I did have a momentary pause to wonder if -ENOLCK would be a better return value here. It would make it easier to distinguish this from from oops, I passed in bogus arguments. For now, I'll leave it as -EINVAL, but it's something to consider... Actually, it looks as if when you compile with !CONFIG_FILE_LOCKING, then fcntl_setlease() returns the value '0' (which would be success!). The word confusing only begins to describe it all. Cheers Trond
Re: [Cluster-devel] [PATCH v2 01/17] locks: consolidate nolease routines
On Thu, Sep 04, 2014 at 02:25:35PM -0400, Trond Myklebust wrote: Actually, it looks as if when you compile with !CONFIG_FILE_LOCKING, then fcntl_setlease() returns the value '0' (which would be success!). The word confusing only begins to describe it all. That's incorrect for sure, we should agree on a single sensible code for: 1) !CONFIG_FILE_LOCKING 2) !lease_enable 3) filesystem doesn't support leases.
