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

["J. Bruce Fields" <bfields@fieldses.org>]

On Sat, Jun 30, 2007 at 10:25:16AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:30PM -0400, J. Bruce Fields wrote:
> > Define an nfs setlease method that just returns -EOPNOTSUPP.
> > 
> > If someone can demonstrate a real need, perhaps we could reenable
> > them in the presence of the "nolock" mount option.
> 
> I'm not a big fan of default methods that do the wrong thing instead
> of just missing functionality.  Would you mind just returning
> -EOPNOTSUPP if ->setlease is not implemented and add it to all
> the local filesystems while all the network/distributed filesystems
> should not have it, not just nfs.

OK, after looking at this a little more, I'm less happy about the idea
of erroring out by default:

	- There are a ton of filesystems that probably should allow
	  leases, and only a few (network filesystems) that shouldn't,
	  so leaving leases on by default seems simpler.
	- We already fall back on the local method by default in the case
	  of locks, and I don't see a reason to treat leases differently.
	- The patch to add
		.setlease = setlease,
	  to all the file_operations is going to be a big patch that
	  changes behavior in a way that might be easy to miss (because
	  it changes behavior exactly on those filesystems it *doesn't*
	  touch.)  I think it'll be easier to get better review on the
	  patch that adds a method just to those filesystems that we're
	  disabling leases for.

I agree about dealing with the other network filesystems, though, and
not just NFS and GFS2; I'll look into that.

--b.