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

[Peter Staubach <staubach@redhat.com>]

J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@citi.umich.edu>
>
> As Peter Staubach says elsewhere
> (http://marc.info/?l=linux-kernel&m=118113649526444&w=2):
>
>   
>> The problem is that some file system such as NFSv2 and NFSv3 do
>> not have sufficient support to be able to support leases correctly.
>> In particular for these two file systems, there is no over the wire
>> protocol support.
>>
>> Currently, these two file systems fail the fcntl(F_SETLEASE) call
>> accidentally, due to a reference counting difference.  These file
>> systems should fail more consciously, with a proper error to
>> indicate that the call is invalid for them.
>>     
>
> 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.
>
> Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
> Cc: Peter Staubach <staubach@redhat.com>
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  fs/nfs/file.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 9eb8eb4..97c1a3d 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -51,6 +51,7 @@ static int  nfs_fsync(struct file *, struct dentry *dentry, int datasync);
>  static int nfs_check_flags(int flags);
>  static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl);
>  static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl);
> +static int nfs_setlease(struct file *file, long arg, struct file_lock **fl);
>  
>  const struct file_operations nfs_file_operations = {
>  	.llseek		= nfs_file_llseek,
> @@ -67,6 +68,7 @@ const struct file_operations nfs_file_operations = {
>  	.flock		= nfs_flock,
>  	.sendfile	= nfs_file_sendfile,
>  	.check_flags	= nfs_check_flags,
> +	.setlease	= nfs_setlease,
>  };
>  
>  const struct inode_operations nfs_file_inode_operations = {
> @@ -555,3 +557,8 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  		return do_unlk(filp, cmd, fl);
>  	return do_setlk(filp, cmd, fl);
>  }
> +
> +static int nfs_setlease(struct file *file, long arg, struct file_lock **fl)
> +{
> +	return -EOPNOTSUPP;
> +}
>   

A couple of things --

First, there is already some support to disable leases for NFS mounted
file systems in -mm, I think.  Are you planning on removing it?

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.

    Thanx...

       ps