All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anna Schumaker <Anna.Schumaker@netapp.com>
To: "J. Bruce Fields" <bfields@fieldses.org>,
	Trond Myklebust <trond.myklebust@primarydata.com>,
	Anna Schumaker <schumakeranna@gmail.com>
Cc: <linux-nfs@vger.kernel.org>, Benjamin Coddington <bcodding@redhat.com>
Subject: Re: [PATCH] NFSv4.2: handle NFS-specific llseek errors
Date: Thu, 23 Jul 2015 11:25:23 -0400	[thread overview]
Message-ID: <55B10763.3020203@Netapp.com> (raw)
In-Reply-To: <20150723150843.GA12894@fieldses.org>

Hi Bruce,

On 07/23/2015 11:08 AM, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> Handle NFS-specific llseek errors instead of letting them leak out to
> userspace.
> 
> Reported-by: Benjamin Coddington <bcodding@redhat.com>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/nfs/nfs42proc.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> I don't actually have a test case for this, but it looks right....
> 
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index f486b80f927a..2fec410bc50f 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -135,7 +135,7 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
>  	return err;
>  }
>  
> -loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> +loff_t _nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
>  {
>  	struct inode *inode = file_inode(filep);
>  	struct nfs42_seek_args args = {
> @@ -171,6 +171,23 @@ loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
>  	return vfs_setpos(filep, res.sr_offset, inode->i_sb->s_maxbytes);
>  }
>  
> +loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> +{
> +	struct nfs_server *server = NFS_SERVER(file_inode(filep));
> +	struct nfs4_exception exception = { };
> +	int err;

Can you move the nfs_server_capable() check to before the do {} while loop?  I think it's cleaner if we check for support before ever trying to call _nfs42_proc_llseek().

> +
> +	do {
> +		err = _nfs42_proc_llseek(filep, offset, whence);
> +		if (err == -ENOTSUPP)
> +			return -EOPNOTSUPP;

With the change to check support moved to this function, we should also unset NFS_CAP_SEEK if we see -ENOTSUPP here.  I think you'll also need to change nfs4_file_llseek() (in nfs4file.c) to check for -EOPNOTSUPP instead of -ENOTSUPP.

Thanks,
Anna

> +		err = nfs4_handle_exception(server, err, &exception);
> +	} while (exception.retry);
> +
> +	return err;
> +}
> +
> +
>  static void
>  nfs42_layoutstat_prepare(struct rpc_task *task, void *calldata)
>  {
> 

  reply	other threads:[~2015-07-23 15:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-23 15:08 [PATCH] NFSv4.2: handle NFS-specific llseek errors J. Bruce Fields
2015-07-23 15:25 ` Anna Schumaker [this message]
2015-07-23 19:21   ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55B10763.3020203@Netapp.com \
    --to=anna.schumaker@netapp.com \
    --cc=bcodding@redhat.com \
    --cc=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=schumakeranna@gmail.com \
    --cc=trond.myklebust@primarydata.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.