Re: [PATCH 6/6] nfs: disable leases over NFS

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Fri, 2007-06-29 at 17:39 -0400, J. Bruce Fields wrote:
> On Fri, Jun 29, 2007 at 05:16:19PM -0400, Peter Staubach wrote:
> > First, there is already some support to disable leases for NFS mounted
> > file systems in -mm, I think.
> 
> Oops, sorry; my fault for not checking -mm before sending....
> 
> > Are you planning on removing it?
> 
> I'd rather do that, yes.  Any objection?
> 
> > Second, it seems to me that EINVAL would be a better error to return
> > than EOPNOTSUPP.  This is an invalid operation to apply to this file
> > and might match POSIX style specs better.
> 
> I'm not sure what you mean by "might match POSIX style specs better"?
> 
> From a quick check, other reasons we'd get EINVAL in this case:
> 
> 	- attempt to get a lease on something other than a regular file.
> 	- leases disabled with /proc/sys/fs/leases-enable
> 
> So if the application calling fcntl knows it was calling it on a regular
> file, then with your proposal an EINVAL return would mean leases were
> disabled for one reason or another, and it could take that as a sign to
> fall back on some other behavior.  And I can't see any reason it would
> need to distinguish between those two remaining cases (filesystem
> doesn't support leases, or leases are disabled by the sysctl).  So,
> OK, EINVAL sounds fine to me.
> 
> But I don't have a really strong opinion.  I think the suggestion of
> EOPNOTSUPP was from Steven Whitehouse; Steven, do you care?
> 
> --b.

EINVAL is fine by me, just so long as its not EAGAIN then it gets my
blessing :-)

Steve.