On Fri, 2018-11-30 at 12:41 -0500, Scott Mayhew wrote:
> There are some NFS v4.0 servers that request confirmation on
> reclaim-type opens even though it is disallowed by RFC 7530.
>
> The Linux NFS client used to cope with those okay due to
> 1) nfs4_opendata_alloc() setting the filehandle in the args for the
> OPEN_CONFIRM to point to the filehandle in the OPEN results, and
> 2) nfs4_open_prepare() copying the filehandle from the OPEN args to
> the
> OPEN results when doing an open by filehandle
>
> The latter part was removed by commit 4e2fcac77390 ("nfsv4: Use
> correct
> inode in _nfs4_opendata_to_nfs4_state()")... and since there's no
> GETFH
> in the OPEN compound when doing a reclaim-type open, if one of these
> servers does request confirmation we wind up sending a zero
> filehandle
> in the PUTFH of the OPEN_CONFIRM compound and the OPEN_CONFIRM fails
> with NFS4ERR_BADHANDLE, causing the reclaim to fail.
>
> Add the copying of the filehandle back to nfs4_open_prepare() and
> log a warning in the event that the client does get a reply to
> OPEN(CLAIM_PREVIOUS) that has NFS4_OPEN_RESULT_CONFIRM set.
>
> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 867457d6dfbe..5e521a506da3 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2323,6 +2323,7 @@ static void nfs4_open_prepare(struct rpc_task
> *task, void *calldata)
> /* Fall through */
> case NFS4_OPEN_CLAIM_FH:
> task->tk_msg.rpc_proc =
> &nfs4_procedures[NFSPROC4_CLNT_OPEN_NOATTR];
> + nfs_copy_fh(&data->o_res.fh, data->o_arg.fh);
> }
> data->timestamp = jiffies;
> if (nfs4_setup_sequence(data->o_arg.server->nfs_client,
> @@ -2470,8 +2471,14 @@ static int _nfs4_recover_proc_open(struct
> nfs4_opendata *data)
>
> nfs_fattr_map_and_free_names(NFS_SERVER(dir), &data->f_attr);
>
> - if (o_res->rflags & NFS4_OPEN_RESULT_CONFIRM)
> + if (o_res->rflags & NFS4_OPEN_RESULT_CONFIRM) {
> + if (data->o_arg.claim == NFS4_OPEN_CLAIM_PREVIOUS)
> + pr_err_ratelimited("NFS: Broken NFSv4 server %s
> is "
> + "requesting open confirmation
> for "
> + "OPEN(CLAIM_PREVIOUS)\n",
> + NFS_SERVER(dir)->nfs_client-
> >cl_hostname);
> status = _nfs4_proc_open_confirm(data);
> + }
>
> return status;
> }
If the spec says that the server is broken, then we don't change the
client. We already have one ugly hack like this, but that is no excuse
for adding a second.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
