2013-03-19 13:07:48

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 01/11] NFSv4: Fail I/O if the state recovery fails irrevocably

If state recovery fails with an ESTALE or a ENOENT, then we shouldn't
keep retrying. Instead, mark the stateid as being invalid and
fail the I/O with an EIO error.
For other operations such as POSIX and BSD file locking, truncate
etc, fail with an EBADF to indicate that this file descriptor is no
longer valid.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4_fs.h | 8 +++++++-
fs/nfs/nfs4filelayout.c | 12 +++++++++---
fs/nfs/nfs4proc.c | 37 +++++++++++++++++++++++++++++--------
fs/nfs/nfs4state.c | 19 ++++++++++++++-----
fs/nfs/pnfs.c | 2 ++
5 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 944c9a5..9ce9013 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -149,6 +149,7 @@ enum {
NFS_STATE_RECLAIM_REBOOT, /* OPEN stateid server rebooted */
NFS_STATE_RECLAIM_NOGRACE, /* OPEN stateid needs to recover state */
NFS_STATE_POSIX_LOCKS, /* Posix locks are supported */
+ NFS_STATE_RECOVERY_FAILED, /* OPEN stateid state recovery failed */
};

struct nfs4_state {
@@ -347,7 +348,7 @@ extern int nfs4_wait_clnt_recover(struct nfs_client *clp);
extern int nfs4_client_recover_expired_lease(struct nfs_client *clp);
extern void nfs4_schedule_state_manager(struct nfs_client *);
extern void nfs4_schedule_path_down_recovery(struct nfs_client *clp);
-extern void nfs4_schedule_stateid_recovery(const struct nfs_server *, struct nfs4_state *);
+extern int nfs4_schedule_stateid_recovery(const struct nfs_server *, struct nfs4_state *);
extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags);
extern void nfs41_handle_server_scope(struct nfs_client *,
struct nfs41_server_scope **);
@@ -412,6 +413,11 @@ static inline bool nfs4_stateid_match(const nfs4_stateid *dst, const nfs4_statei
return memcmp(dst, src, sizeof(*dst)) == 0;
}

+static inline bool nfs4_valid_open_stateid(const struct nfs4_state *state)
+{
+ return test_bit(NFS_STATE_RECOVERY_FAILED, &state->flags) == 0;
+}
+
#else

#define nfs4_close_state(a, b) do { } while (0)
diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index 49eeb04..e4a29fb 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -159,11 +159,14 @@ static int filelayout_async_handle_error(struct rpc_task *task,
case -NFS4ERR_OPENMODE:
if (state == NULL)
break;
- nfs4_schedule_stateid_recovery(mds_server, state);
+ if (nfs4_schedule_stateid_recovery(mds_server, state) < 0)
+ goto out_bad_stateid;
goto wait_on_recovery;
case -NFS4ERR_EXPIRED:
- if (state != NULL)
- nfs4_schedule_stateid_recovery(mds_server, state);
+ if (state != NULL) {
+ if (nfs4_schedule_stateid_recovery(mds_server, state) < 0)
+ goto out_bad_stateid;
+ }
nfs4_schedule_lease_recovery(mds_client);
goto wait_on_recovery;
/* DS session errors */
@@ -227,6 +230,9 @@ reset:
out:
task->tk_status = 0;
return -EAGAIN;
+out_bad_stateid:
+ task->tk_status = -EIO;
+ return 0;
wait_on_recovery:
rpc_sleep_on(&mds_client->cl_rpcwaitq, task, NULL);
if (test_bit(NFS4CLNT_MANAGER_RUNNING, &mds_client->cl_state) == 0)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b2671cb..5a6b3c9 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -295,7 +295,9 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
}
if (state == NULL)
break;
- nfs4_schedule_stateid_recovery(server, state);
+ ret = nfs4_schedule_stateid_recovery(server, state);
+ if (ret < 0)
+ break;
goto wait_on_recovery;
case -NFS4ERR_DELEG_REVOKED:
case -NFS4ERR_ADMIN_REVOKED:
@@ -303,11 +305,16 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
if (state == NULL)
break;
nfs_remove_bad_delegation(state->inode);
- nfs4_schedule_stateid_recovery(server, state);
+ ret = nfs4_schedule_stateid_recovery(server, state);
+ if (ret < 0)
+ break;
goto wait_on_recovery;
case -NFS4ERR_EXPIRED:
- if (state != NULL)
- nfs4_schedule_stateid_recovery(server, state);
+ if (state != NULL) {
+ ret = nfs4_schedule_stateid_recovery(server, state);
+ if (ret < 0)
+ break;
+ }
case -NFS4ERR_STALE_STATEID:
case -NFS4ERR_STALE_CLIENTID:
nfs4_schedule_lease_recovery(clp);
@@ -2053,7 +2060,7 @@ static int _nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,

nfs_fattr_init(fattr);

- if (state != NULL) {
+ if (state != NULL && nfs4_valid_open_stateid(state)) {
struct nfs_lockowner lockowner = {
.l_owner = current->files,
.l_pid = current->tgid,
@@ -2201,6 +2208,8 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
calldata->arg.fmode &= ~FMODE_WRITE;
}
}
+ if (!nfs4_valid_open_stateid(state))
+ call_close = 0;
spin_unlock(&state->owner->so_lock);

if (!call_close) {
@@ -3980,11 +3989,14 @@ nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server,
case -NFS4ERR_OPENMODE:
if (state == NULL)
break;
- nfs4_schedule_stateid_recovery(server, state);
+ if (nfs4_schedule_stateid_recovery(server, state) < 0)
+ goto stateid_invalid;
goto wait_on_recovery;
case -NFS4ERR_EXPIRED:
- if (state != NULL)
- nfs4_schedule_stateid_recovery(server, state);
+ if (state != NULL) {
+ if (nfs4_schedule_stateid_recovery(server, state) < 0)
+ goto stateid_invalid;
+ }
case -NFS4ERR_STALE_STATEID:
case -NFS4ERR_STALE_CLIENTID:
nfs4_schedule_lease_recovery(clp);
@@ -4016,6 +4028,9 @@ nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server,
}
task->tk_status = nfs4_map_errors(task->tk_status);
return 0;
+stateid_invalid:
+ task->tk_status = -EIO;
+ return 0;
wait_on_recovery:
rpc_sleep_on(&clp->cl_rpcwaitq, task, NULL);
if (test_bit(NFS4CLNT_MANAGER_RUNNING, &clp->cl_state) == 0)
@@ -4632,12 +4647,18 @@ static void nfs4_lock_prepare(struct rpc_task *task, void *calldata)
data->res.open_seqid = data->arg.open_seqid;
} else
data->arg.new_lock_owner = 0;
+ if (!nfs4_valid_open_stateid(state)) {
+ data->rpc_status = -EBADF;
+ task->tk_action = NULL;
+ goto out_release_open_seqid;
+ }
data->timestamp = jiffies;
if (nfs4_setup_sequence(data->server,
&data->arg.seq_args,
&data->res.seq_res,
task) == 0)
return;
+out_release_open_seqid:
nfs_release_seqid(data->arg.open_seqid);
out_release_lock_seqid:
nfs_release_seqid(data->arg.lock_seqid);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 6ace365..fec1c5b 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -699,6 +699,8 @@ __nfs4_find_state_byowner(struct inode *inode, struct nfs4_state_owner *owner)
list_for_each_entry(state, &nfsi->open_states, inode_states) {
if (state->owner != owner)
continue;
+ if (!nfs4_valid_open_stateid(state))
+ continue;
if (atomic_inc_not_zero(&state->count))
return state;
}
@@ -1286,14 +1288,17 @@ static int nfs4_state_mark_reclaim_nograce(struct nfs_client *clp, struct nfs4_s
return 1;
}

-void nfs4_schedule_stateid_recovery(const struct nfs_server *server, struct nfs4_state *state)
+int nfs4_schedule_stateid_recovery(const struct nfs_server *server, struct nfs4_state *state)
{
struct nfs_client *clp = server->nfs_client;

+ if (!nfs4_valid_open_stateid(state))
+ return -EBADF;
nfs4_state_mark_reclaim_nograce(clp, state);
dprintk("%s: scheduling stateid recovery for server %s\n", __func__,
clp->cl_hostname);
nfs4_schedule_state_manager(clp);
+ return 0;
}
EXPORT_SYMBOL_GPL(nfs4_schedule_stateid_recovery);

@@ -1323,6 +1328,11 @@ void nfs_inode_find_state_and_recover(struct inode *inode,
nfs4_schedule_state_manager(clp);
}

+static void nfs4_state_mark_recovery_failed(struct nfs4_state *state, int error)
+{
+ set_bit(NFS_STATE_RECOVERY_FAILED, &state->flags);
+}
+

static int nfs4_reclaim_locks(struct nfs4_state *state, const struct nfs4_state_recovery_ops *ops)
{
@@ -1398,6 +1408,8 @@ restart:
list_for_each_entry(state, &sp->so_states, open_states) {
if (!test_and_clear_bit(ops->state_flag_bit, &state->flags))
continue;
+ if (!nfs4_valid_open_stateid(state))
+ continue;
if (state->state == 0)
continue;
atomic_inc(&state->count);
@@ -1430,10 +1442,7 @@ restart:
* Open state on this file cannot be recovered
* All we can do is revert to using the zero stateid.
*/
- memset(&state->stateid, 0,
- sizeof(state->stateid));
- /* Mark the file as being 'closed' */
- state->state = 0;
+ nfs4_state_mark_recovery_failed(state, status);
break;
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_STALE_STATEID:
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 48ac5aa..e86e35f 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -711,6 +711,8 @@ pnfs_choose_layoutget_stateid(nfs4_stateid *dst, struct pnfs_layout_hdr *lo,
spin_lock(&lo->plh_inode->i_lock);
if (pnfs_layoutgets_blocked(lo, 1)) {
status = -EAGAIN;
+ } else if (!nfs4_valid_open_stateid(open_state)) {
+ status = -EBADF;
} else if (list_empty(&lo->plh_segs)) {
int seq;

--
1.8.1.4



2013-03-19 15:23:07

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, Mar 19, 2013 at 03:16:46PM +0000, Myklebust, Trond wrote:
> On Tue, 2013-03-19 at 10:50 -0400, J. Bruce Fields wrote:
> > On Tue, Mar 19, 2013 at 02:41:42PM +0000, Myklebust, Trond wrote:
> > > On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> > > > (Sorry for the digression, I was just trying to decide whether I can get
> > > > away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> > > > GSS on the backchannel....)
> > >
> > > If you don't, could you please at least offer up a module parameter that
> > > allows me to do that? The current thing is a PITA, since it isn't
> > > supported by any distro scripts, and it requires you to switch off the
> > > server in order to change the version pseudofile contents.
> >
> > The module parameter on its own wouldn't help with that.
> >
> > We could probably fix the pseudofile interface to allow changing the
> > version on the fly.
> >
> > My vague plan was to teach rpc.nfsd to read this (and other parameters)
> > from a config file, so then it'd be "vi /etc/nfsd && service nfsd
> > restart" or equivalent.
> >
> > So is the main thing you want a more convenient interface or a way to
> > change it without rebooting?
>
> Basically, all I want is to enable 4.1 permanently on those servers that
> I want to use for NFSv4.1 testing.
>
> An --enable-nfs-version option for rpc.nfsd that acts together with
> --no-nfs-version as a convenient interface for /proc/fs/nfsd/versions
> would work fine too. All distro configuration scripts that I'm aware of
> allow you to specify optional arguments to rpc.nfsd.

Right, got it. Yes, that's on my todo list.

(But, of course, patches welcomed if someone wants to beat me to it.)

--b.

2013-03-19 14:32:38

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, Mar 19, 2013 at 02:16:15PM +0000, Myklebust, Trond wrote:
> On Tue, 2013-03-19 at 10:09 -0400, J. Bruce Fields wrote:
> > On Tue, Mar 19, 2013 at 09:07:42AM -0400, Trond Myklebust wrote:
> > > Sometimes, we actually _want_ to do open-by-filehandle, for instance
> > > when recovering opens after a network partition, or when called
> > > from nfs4_file_open.
> > > Enable that functionality using a new capability NFS_CAP_ATOMIC_OPEN_V1,
> > > and which is only enabled for NFSv4.1 servers that support it.
> >
> > So you're assuming NFS4ERR_INVAL is how the server indicates lack of
> > support?
>
> Looking at the list of valid errors for OPEN in section 15.2 of RFC5661,
> I don't see what else fits the bill.

OK, fair enough.

> > Looking back at NFS server history.... I think that's what it did before
> > supporting these types, but I wonder if that was really right. Possibly
> > it's just a bug not to support the new claim types in a 4.1 server.
>
> I've assumed that it isn't. NFSv4.1 is the very first minor version, so
> it's not supposed to contain any mandatory new features. Yes, I know we
> broke the rules on that one in spectacular fashion with sessions, but
> I'm assuming that is the only exception...

It's really unclear. There's a lot of stuff like this that aren't
clearly identified as optional or given an obvious way to negotiate.
And which sessions features are mandatory? Even where the spec clearly
makes things mandatory for servers to implement (SSV, trunking) I wonder
whether servers are actually complying.

(Sorry for the digression, I was just trying to decide whether I can get
away with turning on 4.1 by default before implementing SP4_MACH_CRED or
GSS on the backchannel....)

--b.

2013-03-19 15:16:35

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, Mar 19, 2013 at 02:45:00PM +0000, Myklebust, Trond wrote:
> On Tue, 2013-03-19 at 10:41 -0400, Trond Myklebust wrote:
> > On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> > > (Sorry for the digression, I was just trying to decide whether I can get
> > > away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> > > GSS on the backchannel....)
> >
> > If you don't, could you please at least offer up a module parameter that
> > allows me to do that? The current thing is a PITA, since it isn't
> > supported by any distro scripts, and it requires you to switch off the
> > server in order to change the version pseudofile contents.
> >
> BTW: you may recall that SPKM3 and UTF-8 support was mandatory in
> RFC3530,

Yes.

> as was RPCSEC_GSS support on the back channel.

I don't agree there--maybe the spec says it's mandatory, I don't
remember, but in practice 4.0 degrades gracefully without callbacks. In
the 4.1 case the create_session's just going to fail with an unspecified
error.

> That didn't stop anybody from shipping clients and servers that didn't
> support them...

Yeah, I may just resign myself to erroring out in some more cases.

--b.

2013-03-19 14:41:44

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> (Sorry for the digression, I was just trying to decide whether I can get
> away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> GSS on the backchannel....)

If you don't, could you please at least offer up a module parameter that
allows me to do that? The current thing is a PITA, since it isn't
supported by any distro scripts, and it requires you to switch off the
server in order to change the version pseudofile contents.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-03-19 13:07:53

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 09/11] NFSv4.1: Add xdr support for CLAIM_FH and CLAIM_DELEG_CUR_FH opens

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4xdr.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 9d32877..650fa32 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1459,6 +1459,23 @@ static inline void encode_claim_delegate_cur(struct xdr_stream *xdr, const struc
encode_string(xdr, name->len, name->name);
}

+static inline void encode_claim_fh(struct xdr_stream *xdr)
+{
+ __be32 *p;
+
+ p = reserve_space(xdr, 4);
+ *p = cpu_to_be32(NFS4_OPEN_CLAIM_FH);
+}
+
+static inline void encode_claim_delegate_cur_fh(struct xdr_stream *xdr, const nfs4_stateid *stateid)
+{
+ __be32 *p;
+
+ p = reserve_space(xdr, 4);
+ *p = cpu_to_be32(NFS4_OPEN_CLAIM_DELEG_CUR_FH);
+ encode_nfs4_stateid(xdr, stateid);
+}
+
static void encode_open(struct xdr_stream *xdr, const struct nfs_openargs *arg, struct compound_hdr *hdr)
{
encode_op_hdr(xdr, OP_OPEN, decode_open_maxsz, hdr);
@@ -1474,6 +1491,12 @@ static void encode_open(struct xdr_stream *xdr, const struct nfs_openargs *arg,
case NFS4_OPEN_CLAIM_DELEGATE_CUR:
encode_claim_delegate_cur(xdr, arg->name, &arg->u.delegation);
break;
+ case NFS4_OPEN_CLAIM_FH:
+ encode_claim_fh(xdr);
+ break;
+ case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
+ encode_claim_delegate_cur_fh(xdr, &arg->u.delegation);
+ break;
default:
BUG();
}
--
1.8.1.4


2013-03-19 13:07:52

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 07/11] NFSv4.1: Select the "most recent locking state" for read/write/setattr stateids

Follow the practice described in section 8.2.2 of RFC5661: When sending a
read/write or setattr stateid, set the seqid field to zero in order to
signal that the NFS server should apply the most recent locking state.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 3 ++-
fs/nfs/nfs4state.c | 2 ++
include/linux/nfs_fs_sb.h | 1 +
3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 4fc1496..c5cc4a8 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6855,7 +6855,8 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
.init_caps = NFS_CAP_READDIRPLUS
| NFS_CAP_ATOMIC_OPEN
| NFS_CAP_CHANGE_ATTR
- | NFS_CAP_POSIX_LOCK,
+ | NFS_CAP_POSIX_LOCK
+ | NFS_CAP_STATEID_NFSV41,
.call_sync = nfs4_call_sync_sequence,
.match_stateid = nfs41_match_stateid,
.find_root_sec = nfs41_find_root_sec,
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 4e95bd7..685b1e9 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1053,6 +1053,8 @@ int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
goto out;
ret = nfs4_copy_open_stateid(dst, state);
out:
+ if (nfs_server_capable(state->inode, NFS_CAP_STATEID_NFSV41))
+ dst->seqid = 0;
return ret;
}

diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 6c6ed15..74c9e52 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -197,5 +197,6 @@ struct nfs_server {
#define NFS_CAP_MTIME (1U << 13)
#define NFS_CAP_POSIX_LOCK (1U << 14)
#define NFS_CAP_UIDGID_NOMAP (1U << 15)
+#define NFS_CAP_STATEID_NFSV41 (1U << 16)

#endif
--
1.8.1.4


2013-03-19 13:07:55

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

Sometimes, we actually _want_ to do open-by-filehandle, for instance
when recovering opens after a network partition, or when called
from nfs4_file_open.
Enable that functionality using a new capability NFS_CAP_ATOMIC_OPEN_V1,
and which is only enabled for NFSv4.1 servers that support it.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 2 ++
fs/nfs/nfs4proc.c | 53 ++++++++++++++++++++++++++++++++++++++++-------
include/linux/nfs_fs_sb.h | 1 +
3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index f23f455..e093e73 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1486,6 +1486,8 @@ static int nfs4_lookup_revalidate(struct dentry *dentry, unsigned int flags)
goto no_open;
if (d_mountpoint(dentry))
goto no_open;
+ if (NFS_SB(dentry->d_sb)->caps & NFS_CAP_ATOMIC_OPEN_V1)
+ goto no_open;

inode = dentry->d_inode;
parent = dget_parent(dentry);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 712eb70..480b5d0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -767,6 +767,35 @@ struct nfs4_opendata {
int cancelled;
};

+static bool nfs4_clear_cap_atomic_open_v1(struct nfs_server *server,
+ int err, struct nfs4_exception *exception)
+{
+ if (err != -EINVAL)
+ return false;
+ if (!(server->caps & NFS_CAP_ATOMIC_OPEN_V1))
+ return false;
+ server->caps &= ~NFS_CAP_ATOMIC_OPEN_V1;
+ exception->retry = 1;
+ return true;
+}
+
+static enum open_claim_type4
+nfs4_map_atomic_open_claim(struct nfs_server *server,
+ enum open_claim_type4 claim)
+{
+ if (server->caps & NFS_CAP_ATOMIC_OPEN_V1)
+ return claim;
+ switch (claim) {
+ default:
+ return claim;
+ case NFS4_OPEN_CLAIM_FH:
+ return NFS4_OPEN_CLAIM_NULL;
+ case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
+ return NFS4_OPEN_CLAIM_DELEGATE_CUR;
+ case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
+ return NFS4_OPEN_CLAIM_DELEGATE_PREV;
+ }
+}

static void nfs4_init_opendata_res(struct nfs4_opendata *p)
{
@@ -818,8 +847,8 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
p->o_arg.server = server;
p->o_arg.bitmask = server->attr_bitmask;
p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
- p->o_arg.claim = claim;
- switch (claim) {
+ p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
+ switch (p->o_arg.claim) {
case NFS4_OPEN_CLAIM_NULL:
case NFS4_OPEN_CLAIM_DELEGATE_CUR:
case NFS4_OPEN_CLAIM_DELEGATE_PREV:
@@ -1326,6 +1355,8 @@ static int nfs4_do_open_reclaim(struct nfs_open_context *ctx, struct nfs4_state
int err;
do {
err = _nfs4_do_open_reclaim(ctx, state);
+ if (nfs4_clear_cap_atomic_open_v1(server, err, &exception))
+ continue;
if (err != -NFS4ERR_DELAY)
break;
nfs4_handle_exception(server, err, &exception);
@@ -1741,7 +1772,7 @@ static int _nfs4_open_expired(struct nfs_open_context *ctx, struct nfs4_state *s
int ret;

opendata = nfs4_open_recoverdata_alloc(ctx, state,
- NFS4_OPEN_CLAIM_NULL);
+ NFS4_OPEN_CLAIM_FH);
if (IS_ERR(opendata))
return PTR_ERR(opendata);
ret = nfs4_open_recover(opendata, state);
@@ -1759,6 +1790,8 @@ static int nfs4_do_open_expired(struct nfs_open_context *ctx, struct nfs4_state

do {
err = _nfs4_open_expired(ctx, state);
+ if (nfs4_clear_cap_atomic_open_v1(server, err, &exception))
+ continue;
switch (err) {
default:
goto out;
@@ -1926,6 +1959,7 @@ static int _nfs4_do_open(struct inode *dir,
struct nfs4_state *state = NULL;
struct nfs_server *server = NFS_SERVER(dir);
struct nfs4_opendata *opendata;
+ enum open_claim_type4 claim = NFS4_OPEN_CLAIM_NULL;
int status;

/* Protect against reboot recovery conflicts */
@@ -1941,9 +1975,10 @@ static int _nfs4_do_open(struct inode *dir,
if (dentry->d_inode != NULL)
nfs4_return_incompatible_delegation(dentry->d_inode, fmode);
status = -ENOMEM;
+ if (dentry->d_inode)
+ claim = NFS4_OPEN_CLAIM_FH;
opendata = nfs4_opendata_alloc(dentry, sp, fmode, flags, sattr,
- NFS4_OPEN_CLAIM_NULL,
- GFP_KERNEL);
+ claim, GFP_KERNEL);
if (opendata == NULL)
goto err_put_state_owner;

@@ -2001,6 +2036,7 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir,
struct rpc_cred *cred,
struct nfs4_threshold **ctx_th)
{
+ struct nfs_server *server = NFS_SERVER(dir);
struct nfs4_exception exception = { };
struct nfs4_state *res;
int status;
@@ -2044,7 +2080,9 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir,
exception.retry = 1;
continue;
}
- res = ERR_PTR(nfs4_handle_exception(NFS_SERVER(dir),
+ if (nfs4_clear_cap_atomic_open_v1(server, status, &exception))
+ continue;
+ res = ERR_PTR(nfs4_handle_exception(server,
status, &exception));
} while (exception.retry);
return res;
@@ -6872,7 +6910,8 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
| NFS_CAP_ATOMIC_OPEN
| NFS_CAP_CHANGE_ATTR
| NFS_CAP_POSIX_LOCK
- | NFS_CAP_STATEID_NFSV41,
+ | NFS_CAP_STATEID_NFSV41
+ | NFS_CAP_ATOMIC_OPEN_V1,
.call_sync = nfs4_call_sync_sequence,
.match_stateid = nfs41_match_stateid,
.find_root_sec = nfs41_find_root_sec,
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 74c9e52..d8fdfdc 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -198,5 +198,6 @@ struct nfs_server {
#define NFS_CAP_POSIX_LOCK (1U << 14)
#define NFS_CAP_UIDGID_NOMAP (1U << 15)
#define NFS_CAP_STATEID_NFSV41 (1U << 16)
+#define NFS_CAP_ATOMIC_OPEN_V1 (1U << 17)

#endif
--
1.8.1.4


2013-03-19 14:50:36

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, Mar 19, 2013 at 02:41:42PM +0000, Myklebust, Trond wrote:
> On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> > (Sorry for the digression, I was just trying to decide whether I can get
> > away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> > GSS on the backchannel....)
>
> If you don't, could you please at least offer up a module parameter that
> allows me to do that? The current thing is a PITA, since it isn't
> supported by any distro scripts, and it requires you to switch off the
> server in order to change the version pseudofile contents.

The module parameter on its own wouldn't help with that.

We could probably fix the pseudofile interface to allow changing the
version on the fly.

My vague plan was to teach rpc.nfsd to read this (and other parameters)
from a config file, so then it'd be "vi /etc/nfsd && service nfsd
restart" or equivalent.

So is the main thing you want a more convenient interface or a way to
change it without rebooting?

--b.

2013-03-19 13:07:48

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 02/11] NFS: Don't accept more reads/writes if the open context recovery failed

If the state recovery failed, we want to ensure that the application
doesn't try to use the same file descriptor for more reads or writes.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4filelayout.c | 8 ++++++++
fs/nfs/nfs4state.c | 16 ++++++++++++++++
fs/nfs/pagelist.c | 2 ++
fs/nfs/read.c | 2 ++
fs/nfs/write.c | 2 ++
include/linux/nfs_fs.h | 1 +
6 files changed, 31 insertions(+)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index e4a29fb..2533ad4 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -306,6 +306,10 @@ static void filelayout_read_prepare(struct rpc_task *task, void *data)
{
struct nfs_read_data *rdata = data;

+ if (unlikely(test_bit(NFS_CONTEXT_BAD, &rdata->args.context->flags))) {
+ rpc_exit(task, -EIO);
+ return;
+ }
if (filelayout_reset_to_mds(rdata->header->lseg)) {
dprintk("%s task %u reset io to MDS\n", __func__, task->tk_pid);
filelayout_reset_read(rdata);
@@ -408,6 +412,10 @@ static void filelayout_write_prepare(struct rpc_task *task, void *data)
{
struct nfs_write_data *wdata = data;

+ if (unlikely(test_bit(NFS_CONTEXT_BAD, &wdata->args.context->flags))) {
+ rpc_exit(task, -EIO);
+ return;
+ }
if (filelayout_reset_to_mds(wdata->header->lseg)) {
dprintk("%s task %u reset io to MDS\n", __func__, task->tk_pid);
filelayout_reset_write(wdata);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index fec1c5b..8db102c 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1328,9 +1328,25 @@ void nfs_inode_find_state_and_recover(struct inode *inode,
nfs4_schedule_state_manager(clp);
}

+static void nfs4_state_mark_open_context_bad(struct nfs4_state *state)
+{
+ struct inode *inode = state->inode;
+ struct nfs_inode *nfsi = NFS_I(inode);
+ struct nfs_open_context *ctx;
+
+ spin_lock(&inode->i_lock);
+ list_for_each_entry(ctx, &nfsi->open_files, list) {
+ if (ctx->state != state)
+ continue;
+ set_bit(NFS_CONTEXT_BAD, &ctx->flags);
+ }
+ spin_unlock(&inode->i_lock);
+}
+
static void nfs4_state_mark_recovery_failed(struct nfs4_state *state, int error)
{
set_bit(NFS_STATE_RECOVERY_FAILED, &state->flags);
+ nfs4_state_mark_open_context_bad(state);
}


diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index e56e846..7f09330 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -104,6 +104,8 @@ nfs_create_request(struct nfs_open_context *ctx, struct inode *inode,
struct nfs_page *req;
struct nfs_lock_context *l_ctx;

+ if (test_bit(NFS_CONTEXT_BAD, &ctx->flags))
+ return ERR_PTR(-EBADF);
/* try to allocate the request struct */
req = nfs_page_alloc();
if (req == NULL)
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index a5e5d98..70a26c6 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -514,6 +514,8 @@ void nfs_read_prepare(struct rpc_task *task, void *calldata)
{
struct nfs_read_data *data = calldata;
NFS_PROTO(data->header->inode)->read_rpc_prepare(task, data);
+ if (unlikely(test_bit(NFS_CONTEXT_BAD, &data->args.context->flags)))
+ rpc_exit(task, -EIO);
}

static const struct rpc_call_ops nfs_read_common_ops = {
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index c483cc5..a2c7c28 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1251,6 +1251,8 @@ void nfs_write_prepare(struct rpc_task *task, void *calldata)
{
struct nfs_write_data *data = calldata;
NFS_PROTO(data->header->inode)->write_rpc_prepare(task, data);
+ if (unlikely(test_bit(NFS_CONTEXT_BAD, &data->args.context->flags)))
+ rpc_exit(task, -EIO);
}

void nfs_commit_prepare(struct rpc_task *task, void *calldata)
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 1cc2568..f6b1956 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -77,6 +77,7 @@ struct nfs_open_context {
unsigned long flags;
#define NFS_CONTEXT_ERROR_WRITE (0)
#define NFS_CONTEXT_RESEND_WRITES (1)
+#define NFS_CONTEXT_BAD (2)
int error;

struct list_head list;
--
1.8.1.4


2013-03-19 13:07:49

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 03/11] NFS: __nfs_find_lock_context needs to check ctx->lock_context for a match too

Currently, we're forcing an unnecessary duplication of the
initial nfs_lock_context in calls to nfs_get_lock_context, since
__nfs_find_lock_context ignores the ctx->lock_context.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/inode.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 1f94167..55b840f 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -565,16 +565,17 @@ static void nfs_init_lock_context(struct nfs_lock_context *l_ctx)

static struct nfs_lock_context *__nfs_find_lock_context(struct nfs_open_context *ctx)
{
- struct nfs_lock_context *pos;
+ struct nfs_lock_context *head = &ctx->lock_context;
+ struct nfs_lock_context *pos = head;

- list_for_each_entry(pos, &ctx->lock_context.list, list) {
+ do {
if (pos->lockowner.l_owner != current->files)
continue;
if (pos->lockowner.l_pid != current->tgid)
continue;
atomic_inc(&pos->count);
return pos;
- }
+ } while ((pos = list_entry(pos->list.next, typeof(*pos), list)) != head);
return NULL;
}

--
1.8.1.4


2013-03-19 13:07:50

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 05/11] NFSv4: Resend the READ/WRITE RPC call if a stateid change causes an error

Adds logic to ensure that if the server returns a BAD_STATEID,
or other state related error, then we check if the stateid has
already changed. If it has, then rather than start state recovery,
we should just resend the failed RPC call with the new stateid.

Allow nfs4_select_rw_stateid to notify that the stateid is unstable by
having it return -EWOULDBLOCK if an RPC is underway that might change the
stateid.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4_fs.h | 4 ++--
fs/nfs/nfs4proc.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++---
fs/nfs/nfs4state.c | 32 +++++++++++++++++++--------
3 files changed, 86 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 8309e98..627a74f 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -234,7 +234,7 @@ extern struct rpc_clnt *nfs4_proc_lookup_mountpoint(struct inode *, struct qstr
extern int nfs4_proc_secinfo(struct inode *, const struct qstr *, struct nfs4_secinfo_flavors *);
extern int nfs4_release_lockowner(struct nfs4_lock_state *);
extern const struct xattr_handler *nfs4_xattr_handlers[];
-extern void nfs4_set_rw_stateid(nfs4_stateid *stateid,
+extern int nfs4_set_rw_stateid(nfs4_stateid *stateid,
const struct nfs_open_context *ctx,
const struct nfs_lock_context *l_ctx,
fmode_t fmode);
@@ -358,7 +358,7 @@ extern void nfs41_handle_server_scope(struct nfs_client *,
struct nfs41_server_scope **);
extern void nfs4_put_lock_state(struct nfs4_lock_state *lsp);
extern int nfs4_set_lock_state(struct nfs4_state *state, struct file_lock *fl);
-extern void nfs4_select_rw_stateid(nfs4_stateid *, struct nfs4_state *,
+extern int nfs4_select_rw_stateid(nfs4_stateid *, struct nfs4_state *,
fmode_t, const struct nfs_lockowner *);

extern struct nfs_seqid *nfs_alloc_seqid(struct nfs_seqid_counter *counter, gfp_t gfp_mask);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index aaf935b..028bd52 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3454,7 +3454,7 @@ static int nfs4_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
return err;
}

-void nfs4_set_rw_stateid(nfs4_stateid *stateid,
+int nfs4_set_rw_stateid(nfs4_stateid *stateid,
const struct nfs_open_context *ctx,
const struct nfs_lock_context *l_ctx,
fmode_t fmode)
@@ -3463,10 +3463,37 @@ void nfs4_set_rw_stateid(nfs4_stateid *stateid,

if (l_ctx != NULL)
lockowner = &l_ctx->lockowner;
- nfs4_select_rw_stateid(stateid, ctx->state, fmode, lockowner);
+ return nfs4_select_rw_stateid(stateid, ctx->state, fmode, lockowner);
}
EXPORT_SYMBOL_GPL(nfs4_set_rw_stateid);

+static bool nfs4_stateid_is_current(nfs4_stateid *stateid,
+ const struct nfs_open_context *ctx,
+ const struct nfs_lock_context *l_ctx,
+ fmode_t fmode)
+{
+ nfs4_stateid current_stateid;
+
+ if (nfs4_set_rw_stateid(&current_stateid, ctx, l_ctx, fmode))
+ return false;
+ return nfs4_stateid_match(stateid, &current_stateid);
+}
+
+static bool nfs4_error_stateid_expired(int err)
+{
+ switch (err) {
+ case -NFS4ERR_DELEG_REVOKED:
+ case -NFS4ERR_ADMIN_REVOKED:
+ case -NFS4ERR_BAD_STATEID:
+ case -NFS4ERR_STALE_STATEID:
+ case -NFS4ERR_OLD_STATEID:
+ case -NFS4ERR_OPENMODE:
+ case -NFS4ERR_EXPIRED:
+ return true;
+ }
+ return false;
+}
+
void __nfs4_read_done_cb(struct nfs_read_data *data)
{
nfs_invalidate_atime(data->header->inode);
@@ -3487,6 +3514,20 @@ static int nfs4_read_done_cb(struct rpc_task *task, struct nfs_read_data *data)
return 0;
}

+static bool nfs4_read_stateid_changed(struct rpc_task *task,
+ struct nfs_readargs *args)
+{
+
+ if (!nfs4_error_stateid_expired(task->tk_status) ||
+ nfs4_stateid_is_current(&args->stateid,
+ args->context,
+ args->lock_context,
+ FMODE_READ))
+ return false;
+ rpc_restart_call_prepare(task);
+ return true;
+}
+
static int nfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)
{

@@ -3494,7 +3535,8 @@ static int nfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)

if (!nfs4_sequence_done(task, &data->res.seq_res))
return -EAGAIN;
-
+ if (nfs4_read_stateid_changed(task, &data->args))
+ return -EAGAIN;
return data->read_done_cb ? data->read_done_cb(task, data) :
nfs4_read_done_cb(task, data);
}
@@ -3533,10 +3575,26 @@ static int nfs4_write_done_cb(struct rpc_task *task, struct nfs_write_data *data
return 0;
}

+static bool nfs4_write_stateid_changed(struct rpc_task *task,
+ struct nfs_writeargs *args)
+{
+
+ if (!nfs4_error_stateid_expired(task->tk_status) ||
+ nfs4_stateid_is_current(&args->stateid,
+ args->context,
+ args->lock_context,
+ FMODE_WRITE))
+ return false;
+ rpc_restart_call_prepare(task);
+ return true;
+}
+
static int nfs4_write_done(struct rpc_task *task, struct nfs_write_data *data)
{
if (!nfs4_sequence_done(task, &data->res.seq_res))
return -EAGAIN;
+ if (nfs4_write_stateid_changed(task, &data->args))
+ return -EAGAIN;
return data->write_done_cb ? data->write_done_cb(task, data) :
nfs4_write_done_cb(task, data);
}
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 8db102c..4e95bd7 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -989,13 +989,14 @@ int nfs4_set_lock_state(struct nfs4_state *state, struct file_lock *fl)
return 0;
}

-static bool nfs4_copy_lock_stateid(nfs4_stateid *dst, struct nfs4_state *state,
+static int nfs4_copy_lock_stateid(nfs4_stateid *dst,
+ struct nfs4_state *state,
const struct nfs_lockowner *lockowner)
{
struct nfs4_lock_state *lsp;
fl_owner_t fl_owner;
pid_t fl_pid;
- bool ret = false;
+ int ret = -ENOENT;


if (lockowner == NULL)
@@ -1010,7 +1011,10 @@ static bool nfs4_copy_lock_stateid(nfs4_stateid *dst, struct nfs4_state *state,
lsp = __nfs4_find_lock_state(state, fl_owner, fl_pid, NFS4_ANY_LOCK_TYPE);
if (lsp != NULL && test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) != 0) {
nfs4_stateid_copy(dst, &lsp->ls_stateid);
- ret = true;
+ ret = 0;
+ smp_rmb();
+ if (!list_empty(&lsp->ls_seqid.list))
+ ret = -EWOULDBLOCK;
}
spin_unlock(&state->state_lock);
nfs4_put_lock_state(lsp);
@@ -1018,28 +1022,38 @@ out:
return ret;
}

-static void nfs4_copy_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
+static int nfs4_copy_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
{
+ int ret;
int seq;

do {
seq = read_seqbegin(&state->seqlock);
nfs4_stateid_copy(dst, &state->stateid);
+ ret = 0;
+ smp_rmb();
+ if (!list_empty(&state->owner->so_seqid.list))
+ ret = -EWOULDBLOCK;
} while (read_seqretry(&state->seqlock, seq));
+ return ret;
}

/*
* Byte-range lock aware utility to initialize the stateid of read/write
* requests.
*/
-void nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
+int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
fmode_t fmode, const struct nfs_lockowner *lockowner)
{
+ int ret = 0;
if (nfs4_copy_delegation_stateid(dst, state->inode, fmode))
- return;
- if (nfs4_copy_lock_stateid(dst, state, lockowner))
- return;
- nfs4_copy_open_stateid(dst, state);
+ goto out;
+ ret = nfs4_copy_lock_stateid(dst, state, lockowner);
+ if (ret != -ENOENT)
+ goto out;
+ ret = nfs4_copy_open_stateid(dst, state);
+out:
+ return ret;
}

struct nfs_seqid *nfs_alloc_seqid(struct nfs_seqid_counter *counter, gfp_t gfp_mask)
--
1.8.1.4


2013-03-19 16:37:19

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, 2013-03-19 at 11:23 -0400, J. Bruce Fields wrote:
> On Tue, Mar 19, 2013 at 03:16:46PM +0000, Myklebust, Trond wrote:
> > On Tue, 2013-03-19 at 10:50 -0400, J. Bruce Fields wrote:
> > > On Tue, Mar 19, 2013 at 02:41:42PM +0000, Myklebust, Trond wrote:
> > > > On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> > > > > (Sorry for the digression, I was just trying to decide whether I can get
> > > > > away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> > > > > GSS on the backchannel....)
> > > >
> > > > If you don't, could you please at least offer up a module parameter that
> > > > allows me to do that? The current thing is a PITA, since it isn't
> > > > supported by any distro scripts, and it requires you to switch off the
> > > > server in order to change the version pseudofile contents.
> > >
> > > The module parameter on its own wouldn't help with that.
> > >
> > > We could probably fix the pseudofile interface to allow changing the
> > > version on the fly.
> > >
> > > My vague plan was to teach rpc.nfsd to read this (and other parameters)
> > > from a config file, so then it'd be "vi /etc/nfsd && service nfsd
> > > restart" or equivalent.
> > >
> > > So is the main thing you want a more convenient interface or a way to
> > > change it without rebooting?
> >
> > Basically, all I want is to enable 4.1 permanently on those servers that
> > I want to use for NFSv4.1 testing.
> >
> > An --enable-nfs-version option for rpc.nfsd that acts together with
> > --no-nfs-version as a convenient interface for /proc/fs/nfsd/versions
> > would work fine too. All distro configuration scripts that I'm aware of
> > allow you to specify optional arguments to rpc.nfsd.
>
> Right, got it. Yes, that's on my todo list.
>
> (But, of course, patches welcomed if someone wants to beat me to it.)

Here you go...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


Attachments:
0001-nfsd-Add-support-for-the-V-and-nfs-version-optional-.patch (3.85 kB)
0001-nfsd-Add-support-for-the-V-and-nfs-version-optional-.patch

2013-03-19 14:16:21

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, 2013-03-19 at 10:09 -0400, J. Bruce Fields wrote:
> On Tue, Mar 19, 2013 at 09:07:42AM -0400, Trond Myklebust wrote:
> > Sometimes, we actually _want_ to do open-by-filehandle, for instance
> > when recovering opens after a network partition, or when called
> > from nfs4_file_open.
> > Enable that functionality using a new capability NFS_CAP_ATOMIC_OPEN_V1,
> > and which is only enabled for NFSv4.1 servers that support it.
>
> So you're assuming NFS4ERR_INVAL is how the server indicates lack of
> support?

Looking at the list of valid errors for OPEN in section 15.2 of RFC5661,
I don't see what else fits the bill.

> Looking back at NFS server history.... I think that's what it did before
> supporting these types, but I wonder if that was really right. Possibly
> it's just a bug not to support the new claim types in a 4.1 server.

I've assumed that it isn't. NFSv4.1 is the very first minor version, so
it's not supposed to contain any mandatory new features. Yes, I know we
broke the rules on that one in spectacular fashion with sessions, but
I'm assuming that is the only exception...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-03-19 13:07:56

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 11/11] NFSv4.1: Use CLAIM_DELEG_CUR_FH opens when available

Now that we do CLAIM_FH opens, we may run into situations where we
get a delegation but don't have perfect knowledge of the file path.
When returning the delegation, we might therefore not be able to
us CLAIM_DELEGATE_CUR opens to convert the delegation into OPEN
stateids and locks.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 480b5d0..88c4015 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1383,7 +1383,7 @@ static int _nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs
int ret;

opendata = nfs4_open_recoverdata_alloc(ctx, state,
- NFS4_OPEN_CLAIM_DELEGATE_CUR);
+ NFS4_OPEN_CLAIM_DELEG_CUR_FH);
if (IS_ERR(opendata))
return PTR_ERR(opendata);
nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid);
--
1.8.1.4


2013-03-19 13:07:50

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 04/11] NFSv4: The stateid must remain the same for replayed RPC calls

If we replay a READ or WRITE call, we should not be changing the
stateid. Currently, we may end up doing so, because the stateid
is only selected at xdr encode time.

This patch ensures that we select the stateid after we get an NFSv4.1
session slot, and that we keep that same stateid across retries.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4_fs.h | 4 ++++
fs/nfs/nfs4filelayout.c | 14 ++++++++++----
fs/nfs/nfs4proc.c | 27 +++++++++++++++++++++++----
fs/nfs/nfs4xdr.c | 28 ++--------------------------
include/linux/nfs_xdr.h | 2 ++
5 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 9ce9013..8309e98 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -234,6 +234,10 @@ extern struct rpc_clnt *nfs4_proc_lookup_mountpoint(struct inode *, struct qstr
extern int nfs4_proc_secinfo(struct inode *, const struct qstr *, struct nfs4_secinfo_flavors *);
extern int nfs4_release_lockowner(struct nfs4_lock_state *);
extern const struct xattr_handler *nfs4_xattr_handlers[];
+extern void nfs4_set_rw_stateid(nfs4_stateid *stateid,
+ const struct nfs_open_context *ctx,
+ const struct nfs_lock_context *l_ctx,
+ fmode_t fmode);

#if defined(CONFIG_NFS_V4_1)
static inline struct nfs4_session *nfs4_get_session(const struct nfs_server *server)
diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index 2533ad4..c5656c5 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -318,10 +318,13 @@ static void filelayout_read_prepare(struct rpc_task *task, void *data)
}
rdata->read_done_cb = filelayout_read_done_cb;

- nfs41_setup_sequence(rdata->ds_clp->cl_session,
+ if (nfs41_setup_sequence(rdata->ds_clp->cl_session,
&rdata->args.seq_args,
&rdata->res.seq_res,
- task);
+ task))
+ return;
+ nfs4_set_rw_stateid(&rdata->args.stateid, rdata->args.context,
+ rdata->args.lock_context, FMODE_READ);
}

static void filelayout_read_call_done(struct rpc_task *task, void *data)
@@ -422,10 +425,13 @@ static void filelayout_write_prepare(struct rpc_task *task, void *data)
rpc_exit(task, 0);
return;
}
- nfs41_setup_sequence(wdata->ds_clp->cl_session,
+ if (nfs41_setup_sequence(wdata->ds_clp->cl_session,
&wdata->args.seq_args,
&wdata->res.seq_res,
- task);
+ task))
+ return;
+ nfs4_set_rw_stateid(&wdata->args.stateid, wdata->args.context,
+ wdata->args.lock_context, FMODE_WRITE);
}

static void filelayout_write_call_done(struct rpc_task *task, void *data)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5a6b3c9..aaf935b 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3454,6 +3454,19 @@ static int nfs4_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
return err;
}

+void nfs4_set_rw_stateid(nfs4_stateid *stateid,
+ const struct nfs_open_context *ctx,
+ const struct nfs_lock_context *l_ctx,
+ fmode_t fmode)
+{
+ const struct nfs_lockowner *lockowner = NULL;
+
+ if (l_ctx != NULL)
+ lockowner = &l_ctx->lockowner;
+ nfs4_select_rw_stateid(stateid, ctx->state, fmode, lockowner);
+}
+EXPORT_SYMBOL_GPL(nfs4_set_rw_stateid);
+
void __nfs4_read_done_cb(struct nfs_read_data *data)
{
nfs_invalidate_atime(data->header->inode);
@@ -3496,10 +3509,13 @@ static void nfs4_proc_read_setup(struct nfs_read_data *data, struct rpc_message

static void nfs4_proc_read_rpc_prepare(struct rpc_task *task, struct nfs_read_data *data)
{
- nfs4_setup_sequence(NFS_SERVER(data->header->inode),
+ if (nfs4_setup_sequence(NFS_SERVER(data->header->inode),
&data->args.seq_args,
&data->res.seq_res,
- task);
+ task))
+ return;
+ nfs4_set_rw_stateid(&data->args.stateid, data->args.context,
+ data->args.lock_context, FMODE_READ);
}

static int nfs4_write_done_cb(struct rpc_task *task, struct nfs_write_data *data)
@@ -3560,10 +3576,13 @@ static void nfs4_proc_write_setup(struct nfs_write_data *data, struct rpc_messag

static void nfs4_proc_write_rpc_prepare(struct rpc_task *task, struct nfs_write_data *data)
{
- nfs4_setup_sequence(NFS_SERVER(data->header->inode),
+ if (nfs4_setup_sequence(NFS_SERVER(data->header->inode),
&data->args.seq_args,
&data->res.seq_res,
- task);
+ task))
+ return;
+ nfs4_set_rw_stateid(&data->args.stateid, data->args.context,
+ data->args.lock_context, FMODE_WRITE);
}

static void nfs4_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit_data *data)
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index e3edda5..9d32877 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1506,35 +1506,12 @@ static void encode_putrootfh(struct xdr_stream *xdr, struct compound_hdr *hdr)
encode_op_hdr(xdr, OP_PUTROOTFH, decode_putrootfh_maxsz, hdr);
}

-static void encode_open_stateid(struct xdr_stream *xdr,
- const struct nfs_open_context *ctx,
- const struct nfs_lock_context *l_ctx,
- fmode_t fmode,
- int zero_seqid)
-{
- nfs4_stateid stateid;
-
- if (ctx->state != NULL) {
- const struct nfs_lockowner *lockowner = NULL;
-
- if (l_ctx != NULL)
- lockowner = &l_ctx->lockowner;
- nfs4_select_rw_stateid(&stateid, ctx->state,
- fmode, lockowner);
- if (zero_seqid)
- stateid.seqid = 0;
- encode_nfs4_stateid(xdr, &stateid);
- } else
- encode_nfs4_stateid(xdr, &zero_stateid);
-}
-
static void encode_read(struct xdr_stream *xdr, const struct nfs_readargs *args, struct compound_hdr *hdr)
{
__be32 *p;

encode_op_hdr(xdr, OP_READ, decode_read_maxsz, hdr);
- encode_open_stateid(xdr, args->context, args->lock_context,
- FMODE_READ, hdr->minorversion);
+ encode_nfs4_stateid(xdr, &args->stateid);

p = reserve_space(xdr, 12);
p = xdr_encode_hyper(p, args->offset);
@@ -1670,8 +1647,7 @@ static void encode_write(struct xdr_stream *xdr, const struct nfs_writeargs *arg
__be32 *p;

encode_op_hdr(xdr, OP_WRITE, decode_write_maxsz, hdr);
- encode_open_stateid(xdr, args->context, args->lock_context,
- FMODE_WRITE, hdr->minorversion);
+ encode_nfs4_stateid(xdr, &args->stateid);

p = reserve_space(xdr, 16);
p = xdr_encode_hyper(p, args->offset);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 4b993d3..90a4aa1 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -486,6 +486,7 @@ struct nfs_readargs {
struct nfs_fh * fh;
struct nfs_open_context *context;
struct nfs_lock_context *lock_context;
+ nfs4_stateid stateid;
__u64 offset;
__u32 count;
unsigned int pgbase;
@@ -507,6 +508,7 @@ struct nfs_writeargs {
struct nfs_fh * fh;
struct nfs_open_context *context;
struct nfs_lock_context *lock_context;
+ nfs4_stateid stateid;
__u64 offset;
__u32 count;
enum nfs3_stable_how stable;
--
1.8.1.4


2013-03-19 13:07:53

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 08/11] NFSv4: Clean up nfs4_opendata_alloc in preparation for NFSv4.1 open modes

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 38 +++++++++++++++++++++++++++-----------
1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c5cc4a8..712eb70 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -782,6 +782,7 @@ static void nfs4_init_opendata_res(struct nfs4_opendata *p)
static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
struct nfs4_state_owner *sp, fmode_t fmode, int flags,
const struct iattr *attrs,
+ enum open_claim_type4 claim,
gfp_t gfp_mask)
{
struct dentry *parent = dget_parent(dentry);
@@ -800,7 +801,6 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
p->dir = parent;
p->owner = sp;
atomic_inc(&sp->so_count);
- p->o_arg.fh = NFS_FH(dir);
p->o_arg.open_flags = flags;
p->o_arg.fmode = fmode & (FMODE_READ|FMODE_WRITE);
/* don't put an ACCESS op in OPEN compound if O_EXCL, because ACCESS
@@ -818,7 +818,19 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
p->o_arg.server = server;
p->o_arg.bitmask = server->attr_bitmask;
p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
- p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
+ p->o_arg.claim = claim;
+ switch (claim) {
+ case NFS4_OPEN_CLAIM_NULL:
+ case NFS4_OPEN_CLAIM_DELEGATE_CUR:
+ case NFS4_OPEN_CLAIM_DELEGATE_PREV:
+ p->o_arg.fh = NFS_FH(dir);
+ break;
+ case NFS4_OPEN_CLAIM_PREVIOUS:
+ case NFS4_OPEN_CLAIM_FH:
+ case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
+ case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
+ p->o_arg.fh = NFS_FH(dentry->d_inode);
+ }
if (attrs != NULL && attrs->ia_valid != 0) {
__be32 verf[2];

@@ -1200,11 +1212,13 @@ static struct nfs_open_context *nfs4_state_find_open_context(struct nfs4_state *
return ERR_PTR(-ENOENT);
}

-static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context *ctx, struct nfs4_state *state)
+static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context *ctx,
+ struct nfs4_state *state, enum open_claim_type4 claim)
{
struct nfs4_opendata *opendata;

- opendata = nfs4_opendata_alloc(ctx->dentry, state->owner, 0, 0, NULL, GFP_NOFS);
+ opendata = nfs4_opendata_alloc(ctx->dentry, state->owner, 0, 0,
+ NULL, claim, GFP_NOFS);
if (opendata == NULL)
return ERR_PTR(-ENOMEM);
opendata->state = state;
@@ -1290,11 +1304,10 @@ static int _nfs4_do_open_reclaim(struct nfs_open_context *ctx, struct nfs4_state
fmode_t delegation_type = 0;
int status;

- opendata = nfs4_open_recoverdata_alloc(ctx, state);
+ opendata = nfs4_open_recoverdata_alloc(ctx, state,
+ NFS4_OPEN_CLAIM_PREVIOUS);
if (IS_ERR(opendata))
return PTR_ERR(opendata);
- opendata->o_arg.claim = NFS4_OPEN_CLAIM_PREVIOUS;
- opendata->o_arg.fh = NFS_FH(state->inode);
rcu_read_lock();
delegation = rcu_dereference(NFS_I(state->inode)->delegation);
if (delegation != NULL && test_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags) != 0)
@@ -1338,10 +1351,10 @@ static int _nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs
struct nfs4_opendata *opendata;
int ret;

- opendata = nfs4_open_recoverdata_alloc(ctx, state);
+ opendata = nfs4_open_recoverdata_alloc(ctx, state,
+ NFS4_OPEN_CLAIM_DELEGATE_CUR);
if (IS_ERR(opendata))
return PTR_ERR(opendata);
- opendata->o_arg.claim = NFS4_OPEN_CLAIM_DELEGATE_CUR;
nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid);
ret = nfs4_open_recover(opendata, state);
nfs4_opendata_put(opendata);
@@ -1727,7 +1740,8 @@ static int _nfs4_open_expired(struct nfs_open_context *ctx, struct nfs4_state *s
struct nfs4_opendata *opendata;
int ret;

- opendata = nfs4_open_recoverdata_alloc(ctx, state);
+ opendata = nfs4_open_recoverdata_alloc(ctx, state,
+ NFS4_OPEN_CLAIM_NULL);
if (IS_ERR(opendata))
return PTR_ERR(opendata);
ret = nfs4_open_recover(opendata, state);
@@ -1927,7 +1941,9 @@ static int _nfs4_do_open(struct inode *dir,
if (dentry->d_inode != NULL)
nfs4_return_incompatible_delegation(dentry->d_inode, fmode);
status = -ENOMEM;
- opendata = nfs4_opendata_alloc(dentry, sp, fmode, flags, sattr, GFP_KERNEL);
+ opendata = nfs4_opendata_alloc(dentry, sp, fmode, flags, sattr,
+ NFS4_OPEN_CLAIM_NULL,
+ GFP_KERNEL);
if (opendata == NULL)
goto err_put_state_owner;

--
1.8.1.4


2013-03-19 17:29:40

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

Hi Trond

You need to declare the new NFS_OPEN_CLAIMs.

From: Andy Adamson <[email protected]>
Date: Tue, 19 Mar 2013 13:03:36 -0400
Subject: [PATCH 17/21] NFSv4.1 declare new NFS4_OPEN_CLAIM * FH

Signed-off-by: Andy Adamson <[email protected]>
---
include/linux/nfs4.h | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 7552bb5..380d944 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -388,7 +388,10 @@ enum open_claim_type4 {
NFS4_OPEN_CLAIM_NULL = 0,
NFS4_OPEN_CLAIM_PREVIOUS = 1,
NFS4_OPEN_CLAIM_DELEGATE_CUR = 2,
- NFS4_OPEN_CLAIM_DELEGATE_PREV = 3
+ NFS4_OPEN_CLAIM_DELEGATE_PREV = 3,
+ NFS4_OPEN_CLAIM_FH = 4,
+ NFS4_OPEN_CLAIM_DELEG_CUR_FH = 5,
+ NFS4_OPEN_CLAIM_DELEG_PREV_FH = 6
};

enum opentype4 {

-->Andy

On Mar 19, 2013, at 9:07 AM, Trond Myklebust <[email protected]> wrote:

> Sometimes, we actually _want_ to do open-by-filehandle, for instance
> when recovering opens after a network partition, or when called
> from nfs4_file_open.
> Enable that functionality using a new capability NFS_CAP_ATOMIC_OPEN_V1,
> and which is only enabled for NFSv4.1 servers that support it.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/dir.c | 2 ++
> fs/nfs/nfs4proc.c | 53 ++++++++++++++++++++++++++++++++++++++++-------
> include/linux/nfs_fs_sb.h | 1 +
> 3 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index f23f455..e093e73 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1486,6 +1486,8 @@ static int nfs4_lookup_revalidate(struct dentry *dentry, unsigned int flags)
> goto no_open;
> if (d_mountpoint(dentry))
> goto no_open;
> + if (NFS_SB(dentry->d_sb)->caps & NFS_CAP_ATOMIC_OPEN_V1)
> + goto no_open;
>
> inode = dentry->d_inode;
> parent = dget_parent(dentry);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 712eb70..480b5d0 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -767,6 +767,35 @@ struct nfs4_opendata {
> int cancelled;
> };
>
> +static bool nfs4_clear_cap_atomic_open_v1(struct nfs_server *server,
> + int err, struct nfs4_exception *exception)
> +{
> + if (err != -EINVAL)
> + return false;
> + if (!(server->caps & NFS_CAP_ATOMIC_OPEN_V1))
> + return false;
> + server->caps &= ~NFS_CAP_ATOMIC_OPEN_V1;
> + exception->retry = 1;
> + return true;
> +}
> +
> +static enum open_claim_type4
> +nfs4_map_atomic_open_claim(struct nfs_server *server,
> + enum open_claim_type4 claim)
> +{
> + if (server->caps & NFS_CAP_ATOMIC_OPEN_V1)
> + return claim;
> + switch (claim) {
> + default:
> + return claim;
> + case NFS4_OPEN_CLAIM_FH:
> + return NFS4_OPEN_CLAIM_NULL;
> + case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
> + return NFS4_OPEN_CLAIM_DELEGATE_CUR;
> + case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
> + return NFS4_OPEN_CLAIM_DELEGATE_PREV;
> + }
> +}
>
> static void nfs4_init_opendata_res(struct nfs4_opendata *p)
> {
> @@ -818,8 +847,8 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
> p->o_arg.server = server;
> p->o_arg.bitmask = server->attr_bitmask;
> p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
> - p->o_arg.claim = claim;
> - switch (claim) {
> + p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
> + switch (p->o_arg.claim) {
> case NFS4_OPEN_CLAIM_NULL:
> case NFS4_OPEN_CLAIM_DELEGATE_CUR:
> case NFS4_OPEN_CLAIM_DELEGATE_PREV:
> @@ -1326,6 +1355,8 @@ static int nfs4_do_open_reclaim(struct nfs_open_context *ctx, struct nfs4_state
> int err;
> do {
> err = _nfs4_do_open_reclaim(ctx, state);
> + if (nfs4_clear_cap_atomic_open_v1(server, err, &exception))
> + continue;
> if (err != -NFS4ERR_DELAY)
> break;
> nfs4_handle_exception(server, err, &exception);
> @@ -1741,7 +1772,7 @@ static int _nfs4_open_expired(struct nfs_open_context *ctx, struct nfs4_state *s
> int ret;
>
> opendata = nfs4_open_recoverdata_alloc(ctx, state,
> - NFS4_OPEN_CLAIM_NULL);
> + NFS4_OPEN_CLAIM_FH);
> if (IS_ERR(opendata))
> return PTR_ERR(opendata);
> ret = nfs4_open_recover(opendata, state);
> @@ -1759,6 +1790,8 @@ static int nfs4_do_open_expired(struct nfs_open_context *ctx, struct nfs4_state
>
> do {
> err = _nfs4_open_expired(ctx, state);
> + if (nfs4_clear_cap_atomic_open_v1(server, err, &exception))
> + continue;
> switch (err) {
> default:
> goto out;
> @@ -1926,6 +1959,7 @@ static int _nfs4_do_open(struct inode *dir,
> struct nfs4_state *state = NULL;
> struct nfs_server *server = NFS_SERVER(dir);
> struct nfs4_opendata *opendata;
> + enum open_claim_type4 claim = NFS4_OPEN_CLAIM_NULL;
> int status;
>
> /* Protect against reboot recovery conflicts */
> @@ -1941,9 +1975,10 @@ static int _nfs4_do_open(struct inode *dir,
> if (dentry->d_inode != NULL)
> nfs4_return_incompatible_delegation(dentry->d_inode, fmode);
> status = -ENOMEM;
> + if (dentry->d_inode)
> + claim = NFS4_OPEN_CLAIM_FH;
> opendata = nfs4_opendata_alloc(dentry, sp, fmode, flags, sattr,
> - NFS4_OPEN_CLAIM_NULL,
> - GFP_KERNEL);
> + claim, GFP_KERNEL);
> if (opendata == NULL)
> goto err_put_state_owner;
>
> @@ -2001,6 +2036,7 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir,
> struct rpc_cred *cred,
> struct nfs4_threshold **ctx_th)
> {
> + struct nfs_server *server = NFS_SERVER(dir);
> struct nfs4_exception exception = { };
> struct nfs4_state *res;
> int status;
> @@ -2044,7 +2080,9 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir,
> exception.retry = 1;
> continue;
> }
> - res = ERR_PTR(nfs4_handle_exception(NFS_SERVER(dir),
> + if (nfs4_clear_cap_atomic_open_v1(server, status, &exception))
> + continue;
> + res = ERR_PTR(nfs4_handle_exception(server,
> status, &exception));
> } while (exception.retry);
> return res;
> @@ -6872,7 +6910,8 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
> | NFS_CAP_ATOMIC_OPEN
> | NFS_CAP_CHANGE_ATTR
> | NFS_CAP_POSIX_LOCK
> - | NFS_CAP_STATEID_NFSV41,
> + | NFS_CAP_STATEID_NFSV41
> + | NFS_CAP_ATOMIC_OPEN_V1,
> .call_sync = nfs4_call_sync_sequence,
> .match_stateid = nfs41_match_stateid,
> .find_root_sec = nfs41_find_root_sec,
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 74c9e52..d8fdfdc 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -198,5 +198,6 @@ struct nfs_server {
> #define NFS_CAP_POSIX_LOCK (1U << 14)
> #define NFS_CAP_UIDGID_NOMAP (1U << 15)
> #define NFS_CAP_STATEID_NFSV41 (1U << 16)
> +#define NFS_CAP_ATOMIC_OPEN_V1 (1U << 17)
>
> #endif
> --
> 1.8.1.4
>


2013-03-19 14:09:03

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, Mar 19, 2013 at 09:07:42AM -0400, Trond Myklebust wrote:
> Sometimes, we actually _want_ to do open-by-filehandle, for instance
> when recovering opens after a network partition, or when called
> from nfs4_file_open.
> Enable that functionality using a new capability NFS_CAP_ATOMIC_OPEN_V1,
> and which is only enabled for NFSv4.1 servers that support it.

So you're assuming NFS4ERR_INVAL is how the server indicates lack of
support?

Looking back at NFS server history.... I think that's what it did before
supporting these types, but I wonder if that was really right. Possibly
it's just a bug not to support the new claim types in a 4.1 server.

--b.

>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/dir.c | 2 ++
> fs/nfs/nfs4proc.c | 53 ++++++++++++++++++++++++++++++++++++++++-------
> include/linux/nfs_fs_sb.h | 1 +
> 3 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index f23f455..e093e73 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1486,6 +1486,8 @@ static int nfs4_lookup_revalidate(struct dentry *dentry, unsigned int flags)
> goto no_open;
> if (d_mountpoint(dentry))
> goto no_open;
> + if (NFS_SB(dentry->d_sb)->caps & NFS_CAP_ATOMIC_OPEN_V1)
> + goto no_open;
>
> inode = dentry->d_inode;
> parent = dget_parent(dentry);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 712eb70..480b5d0 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -767,6 +767,35 @@ struct nfs4_opendata {
> int cancelled;
> };
>
> +static bool nfs4_clear_cap_atomic_open_v1(struct nfs_server *server,
> + int err, struct nfs4_exception *exception)
> +{
> + if (err != -EINVAL)
> + return false;
> + if (!(server->caps & NFS_CAP_ATOMIC_OPEN_V1))
> + return false;
> + server->caps &= ~NFS_CAP_ATOMIC_OPEN_V1;
> + exception->retry = 1;
> + return true;
> +}
> +
> +static enum open_claim_type4
> +nfs4_map_atomic_open_claim(struct nfs_server *server,
> + enum open_claim_type4 claim)
> +{
> + if (server->caps & NFS_CAP_ATOMIC_OPEN_V1)
> + return claim;
> + switch (claim) {
> + default:
> + return claim;
> + case NFS4_OPEN_CLAIM_FH:
> + return NFS4_OPEN_CLAIM_NULL;
> + case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
> + return NFS4_OPEN_CLAIM_DELEGATE_CUR;
> + case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
> + return NFS4_OPEN_CLAIM_DELEGATE_PREV;
> + }
> +}
>
> static void nfs4_init_opendata_res(struct nfs4_opendata *p)
> {
> @@ -818,8 +847,8 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
> p->o_arg.server = server;
> p->o_arg.bitmask = server->attr_bitmask;
> p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
> - p->o_arg.claim = claim;
> - switch (claim) {
> + p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
> + switch (p->o_arg.claim) {
> case NFS4_OPEN_CLAIM_NULL:
> case NFS4_OPEN_CLAIM_DELEGATE_CUR:
> case NFS4_OPEN_CLAIM_DELEGATE_PREV:
> @@ -1326,6 +1355,8 @@ static int nfs4_do_open_reclaim(struct nfs_open_context *ctx, struct nfs4_state
> int err;
> do {
> err = _nfs4_do_open_reclaim(ctx, state);
> + if (nfs4_clear_cap_atomic_open_v1(server, err, &exception))
> + continue;
> if (err != -NFS4ERR_DELAY)
> break;
> nfs4_handle_exception(server, err, &exception);
> @@ -1741,7 +1772,7 @@ static int _nfs4_open_expired(struct nfs_open_context *ctx, struct nfs4_state *s
> int ret;
>
> opendata = nfs4_open_recoverdata_alloc(ctx, state,
> - NFS4_OPEN_CLAIM_NULL);
> + NFS4_OPEN_CLAIM_FH);
> if (IS_ERR(opendata))
> return PTR_ERR(opendata);
> ret = nfs4_open_recover(opendata, state);
> @@ -1759,6 +1790,8 @@ static int nfs4_do_open_expired(struct nfs_open_context *ctx, struct nfs4_state
>
> do {
> err = _nfs4_open_expired(ctx, state);
> + if (nfs4_clear_cap_atomic_open_v1(server, err, &exception))
> + continue;
> switch (err) {
> default:
> goto out;
> @@ -1926,6 +1959,7 @@ static int _nfs4_do_open(struct inode *dir,
> struct nfs4_state *state = NULL;
> struct nfs_server *server = NFS_SERVER(dir);
> struct nfs4_opendata *opendata;
> + enum open_claim_type4 claim = NFS4_OPEN_CLAIM_NULL;
> int status;
>
> /* Protect against reboot recovery conflicts */
> @@ -1941,9 +1975,10 @@ static int _nfs4_do_open(struct inode *dir,
> if (dentry->d_inode != NULL)
> nfs4_return_incompatible_delegation(dentry->d_inode, fmode);
> status = -ENOMEM;
> + if (dentry->d_inode)
> + claim = NFS4_OPEN_CLAIM_FH;
> opendata = nfs4_opendata_alloc(dentry, sp, fmode, flags, sattr,
> - NFS4_OPEN_CLAIM_NULL,
> - GFP_KERNEL);
> + claim, GFP_KERNEL);
> if (opendata == NULL)
> goto err_put_state_owner;
>
> @@ -2001,6 +2036,7 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir,
> struct rpc_cred *cred,
> struct nfs4_threshold **ctx_th)
> {
> + struct nfs_server *server = NFS_SERVER(dir);
> struct nfs4_exception exception = { };
> struct nfs4_state *res;
> int status;
> @@ -2044,7 +2080,9 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir,
> exception.retry = 1;
> continue;
> }
> - res = ERR_PTR(nfs4_handle_exception(NFS_SERVER(dir),
> + if (nfs4_clear_cap_atomic_open_v1(server, status, &exception))
> + continue;
> + res = ERR_PTR(nfs4_handle_exception(server,
> status, &exception));
> } while (exception.retry);
> return res;
> @@ -6872,7 +6910,8 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
> | NFS_CAP_ATOMIC_OPEN
> | NFS_CAP_CHANGE_ATTR
> | NFS_CAP_POSIX_LOCK
> - | NFS_CAP_STATEID_NFSV41,
> + | NFS_CAP_STATEID_NFSV41
> + | NFS_CAP_ATOMIC_OPEN_V1,
> .call_sync = nfs4_call_sync_sequence,
> .match_stateid = nfs41_match_stateid,
> .find_root_sec = nfs41_find_root_sec,
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 74c9e52..d8fdfdc 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -198,5 +198,6 @@ struct nfs_server {
> #define NFS_CAP_POSIX_LOCK (1U << 14)
> #define NFS_CAP_UIDGID_NOMAP (1U << 15)
> #define NFS_CAP_STATEID_NFSV41 (1U << 16)
> +#define NFS_CAP_ATOMIC_OPEN_V1 (1U << 17)
>
> #endif
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-03-21 15:25:19

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, Mar 19, 2013 at 04:37:17PM +0000, Myklebust, Trond wrote:
> On Tue, 2013-03-19 at 11:23 -0400, J. Bruce Fields wrote:
> > On Tue, Mar 19, 2013 at 03:16:46PM +0000, Myklebust, Trond wrote:
> > > On Tue, 2013-03-19 at 10:50 -0400, J. Bruce Fields wrote:
> > > > On Tue, Mar 19, 2013 at 02:41:42PM +0000, Myklebust, Trond wrote:
> > > > > On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> > > > > > (Sorry for the digression, I was just trying to decide whether I can get
> > > > > > away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> > > > > > GSS on the backchannel....)
> > > > >
> > > > > If you don't, could you please at least offer up a module parameter that
> > > > > allows me to do that? The current thing is a PITA, since it isn't
> > > > > supported by any distro scripts, and it requires you to switch off the
> > > > > server in order to change the version pseudofile contents.
> > > >
> > > > The module parameter on its own wouldn't help with that.
> > > >
> > > > We could probably fix the pseudofile interface to allow changing the
> > > > version on the fly.
> > > >
> > > > My vague plan was to teach rpc.nfsd to read this (and other parameters)
> > > > from a config file, so then it'd be "vi /etc/nfsd && service nfsd
> > > > restart" or equivalent.
> > > >
> > > > So is the main thing you want a more convenient interface or a way to
> > > > change it without rebooting?
> > >
> > > Basically, all I want is to enable 4.1 permanently on those servers that
> > > I want to use for NFSv4.1 testing.
> > >
> > > An --enable-nfs-version option for rpc.nfsd that acts together with
> > > --no-nfs-version as a convenient interface for /proc/fs/nfsd/versions
> > > would work fine too. All distro configuration scripts that I'm aware of
> > > allow you to specify optional arguments to rpc.nfsd.
> >
> > Right, got it. Yes, that's on my todo list.
> >
> > (But, of course, patches welcomed if someone wants to beat me to it.)
>
> Here you go...

Looks fine to me.--b.

>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com

> From 3beebc45d59aa990cb57efbfb55b62f69182d40a Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <[email protected]>
> Date: Tue, 19 Mar 2013 12:28:35 -0400
> Subject: [PATCH] nfsd: Add support for the -V and --nfs-version optional
> arguments
>
> Add command line options to enable those NFS versions that are
> currently disabled by default. We choose to use the options '-V'
> and '--nfs-version' for compatibility with rpc.mountd.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> support/include/nfs/nfs.h | 1 +
> utils/nfsd/nfsd.c | 26 ++++++++++++++++++++++++--
> utils/nfsd/nfsd.man | 9 ++++++++-
> 3 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index 320880e..174c2dd 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -52,6 +52,7 @@ struct nfs_fh_old {
> #define NFSCTL_UDPISSET(_cltbits) ((_cltbits) & NFSCTL_UDPBIT)
> #define NFSCTL_TCPISSET(_cltbits) ((_cltbits) & NFSCTL_TCPBIT)
>
> +#define NFSCTL_VERSET(_cltbits, _v) ((_cltbits) |= (1 << ((_v) - 1)))
> #define NFSCTL_UDPSET(_cltbits) ((_cltbits) |= NFSCTL_UDPBIT)
> #define NFSCTL_TCPSET(_cltbits) ((_cltbits) |= NFSCTL_TCPBIT)
>
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 2a3f5cc..e87c0a9 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -38,6 +38,7 @@ static struct option longopts[] =
> { "host", 1, 0, 'H' },
> { "help", 0, 0, 'h' },
> { "no-nfs-version", 1, 0, 'N' },
> + { "nfs-version", 1, 0, 'V' },
> { "no-tcp", 0, 0, 'T' },
> { "no-udp", 0, 0, 'U' },
> { "port", 1, 0, 'P' },
> @@ -119,7 +120,7 @@ main(int argc, char **argv)
> xlog_syslog(0);
> xlog_stderr(1);
>
> - while ((c = getopt_long(argc, argv, "dH:hN:p:P:sTU", longopts, NULL)) != EOF) {
> + while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTU", longopts, NULL)) != EOF) {
> switch(c) {
> case 'd':
> xlog_config(D_ALL, 1);
> @@ -175,6 +176,27 @@ main(int argc, char **argv)
> exit(1);
> }
> break;
> + case 'V':
> + switch((c = strtol(optarg, &p, 0))) {
> + case 4:
> + if (*p == '.') {
> + int i = atoi(p+1);
> + if (i != 1) {
> + fprintf(stderr, "%s: unsupported minor version\n", optarg);
> + exit(1);
> + }
> + minorvers41 = 1;
> + break;
> + }
> + case 3:
> + case 2:
> + NFSCTL_VERSET(versbits, c);
> + break;
> + default:
> + fprintf(stderr, "%s: Unsupported version\n", optarg);
> + exit(1);
> + }
> + break;
> case 's':
> xlog_syslog(1);
> xlog_stderr(0);
> @@ -312,7 +334,7 @@ static void
> usage(const char *prog)
> {
> fprintf(stderr, "Usage:\n"
> - "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
> + "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version] [-V|--nfs-version version] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
> prog);
> exit(2);
> }
> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> index 1cf9296..7de0867 100644
> --- a/utils/nfsd/nfsd.man
> +++ b/utils/nfsd/nfsd.man
> @@ -47,7 +47,7 @@ This option can be used to request that
> .B rpc.nfsd
> does not offer certain versions of NFS. The current version of
> .B rpc.nfsd
> -can support both NFS version 2,3 and the newer version 4.
> +can support NFS versions 2,3,4 and the newer version 4.1.
> .TP
> .B \-s " or " \-\-syslog
> By default,
> @@ -67,6 +67,13 @@ Disable
> .B rpc.nfsd
> from accepting UDP connections from clients.
> .TP
> +.B \-V " or " \-\-nfs-version vers
> +This option can be used to request that
> +.B rpc.nfsd
> +offer certain versions of NFS. The current version of
> +.B rpc.nfsd
> +can support NFS versions 2,3,4 and the newer version 4.1.
> +.TP
> .I nproc
> specify the number of NFS server threads. By default, just one
> thread is started. However, for optimum performance several threads
> --
> 1.8.1.4
>


2013-03-19 17:42:45

by Myklebust, Trond

[permalink] [raw]
Subject: RE: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

> -----Original Message-----
> From: Adamson, Andy
> Sent: Tuesday, March 19, 2013 1:30 PM
> To: Myklebust, Trond
> Cc: <[email protected]>
> Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle
>
> Hi Trond
>
> You need to declare the new NFS_OPEN_CLAIMs.
>
> From: Andy Adamson <[email protected]>
> Date: Tue, 19 Mar 2013 13:03:36 -0400
> Subject: [PATCH 17/21] NFSv4.1 declare new NFS4_OPEN_CLAIM * FH
>
> Signed-off-by: Andy Adamson <[email protected]>
> ---
> include/linux/nfs4.h | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h index
> 7552bb5..380d944 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -388,7 +388,10 @@ enum open_claim_type4 {
> NFS4_OPEN_CLAIM_NULL = 0,
> NFS4_OPEN_CLAIM_PREVIOUS = 1,
> NFS4_OPEN_CLAIM_DELEGATE_CUR = 2,
> - NFS4_OPEN_CLAIM_DELEGATE_PREV = 3
> + NFS4_OPEN_CLAIM_DELEGATE_PREV = 3,
> + NFS4_OPEN_CLAIM_FH = 4,
> + NFS4_OPEN_CLAIM_DELEG_CUR_FH = 5,
> + NFS4_OPEN_CLAIM_DELEG_PREV_FH = 6
> };
>
> enum opentype4 {
>
> -->Andy

See commit 8b289b2c2355c3bea75f3e499b4aa251a3191382 (nfsd4: implement new 4.1 open reclaim types). It was upstreamed in Linux-3.2.

Cheers
Trond

2013-03-25 20:12:42

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle



On 21/03/13 11:25, J. Bruce Fields wrote:
> On Tue, Mar 19, 2013 at 04:37:17PM +0000, Myklebust, Trond wrote:
>> On Tue, 2013-03-19 at 11:23 -0400, J. Bruce Fields wrote:
>>> On Tue, Mar 19, 2013 at 03:16:46PM +0000, Myklebust, Trond wrote:
>>>> On Tue, 2013-03-19 at 10:50 -0400, J. Bruce Fields wrote:
>>>>> On Tue, Mar 19, 2013 at 02:41:42PM +0000, Myklebust, Trond wrote:
>>>>>> On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
>>>>>>> (Sorry for the digression, I was just trying to decide whether I can get
>>>>>>> away with turning on 4.1 by default before implementing SP4_MACH_CRED or
>>>>>>> GSS on the backchannel....)
>>>>>>
>>>>>> If you don't, could you please at least offer up a module parameter that
>>>>>> allows me to do that? The current thing is a PITA, since it isn't
>>>>>> supported by any distro scripts, and it requires you to switch off the
>>>>>> server in order to change the version pseudofile contents.
>>>>>
>>>>> The module parameter on its own wouldn't help with that.
>>>>>
>>>>> We could probably fix the pseudofile interface to allow changing the
>>>>> version on the fly.
>>>>>
>>>>> My vague plan was to teach rpc.nfsd to read this (and other parameters)
>>>>> from a config file, so then it'd be "vi /etc/nfsd && service nfsd
>>>>> restart" or equivalent.
>>>>>
>>>>> So is the main thing you want a more convenient interface or a way to
>>>>> change it without rebooting?
>>>>
>>>> Basically, all I want is to enable 4.1 permanently on those servers that
>>>> I want to use for NFSv4.1 testing.
>>>>
>>>> An --enable-nfs-version option for rpc.nfsd that acts together with
>>>> --no-nfs-version as a convenient interface for /proc/fs/nfsd/versions
>>>> would work fine too. All distro configuration scripts that I'm aware of
>>>> allow you to specify optional arguments to rpc.nfsd.
>>>
>>> Right, got it. Yes, that's on my todo list.
>>>
>>> (But, of course, patches welcomed if someone wants to beat me to it.)
>>
>> Here you go...
>
> Looks fine to me.--b.
>
>>
>> --
>> Trond Myklebust
>> Linux NFS client maintainer
>>
>> NetApp
>> [email protected]
>> http://www.netapp.com
>
>> From 3beebc45d59aa990cb57efbfb55b62f69182d40a Mon Sep 17 00:00:00 2001
>> From: Trond Myklebust <[email protected]>
>> Date: Tue, 19 Mar 2013 12:28:35 -0400
>> Subject: [PATCH] nfsd: Add support for the -V and --nfs-version optional
>> arguments
>>
>> Add command line options to enable those NFS versions that are
>> currently disabled by default. We choose to use the options '-V'
>> and '--nfs-version' for compatibility with rpc.mountd.
>>
>> Signed-off-by: Trond Myklebust <[email protected]>

BTW, A good way to hide patch is to make it and attachment in one
of the last emails of a fairly lengthy thread.... 8-)

Committed!

steved.

>> ---
>> support/include/nfs/nfs.h | 1 +
>> utils/nfsd/nfsd.c | 26 ++++++++++++++++++++++++--
>> utils/nfsd/nfsd.man | 9 ++++++++-
>> 3 files changed, 33 insertions(+), 3 deletions(-)
>>
>> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
>> index 320880e..174c2dd 100644
>> --- a/support/include/nfs/nfs.h
>> +++ b/support/include/nfs/nfs.h
>> @@ -52,6 +52,7 @@ struct nfs_fh_old {
>> #define NFSCTL_UDPISSET(_cltbits) ((_cltbits) & NFSCTL_UDPBIT)
>> #define NFSCTL_TCPISSET(_cltbits) ((_cltbits) & NFSCTL_TCPBIT)
>>
>> +#define NFSCTL_VERSET(_cltbits, _v) ((_cltbits) |= (1 << ((_v) - 1)))
>> #define NFSCTL_UDPSET(_cltbits) ((_cltbits) |= NFSCTL_UDPBIT)
>> #define NFSCTL_TCPSET(_cltbits) ((_cltbits) |= NFSCTL_TCPBIT)
>>
>> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
>> index 2a3f5cc..e87c0a9 100644
>> --- a/utils/nfsd/nfsd.c
>> +++ b/utils/nfsd/nfsd.c
>> @@ -38,6 +38,7 @@ static struct option longopts[] =
>> { "host", 1, 0, 'H' },
>> { "help", 0, 0, 'h' },
>> { "no-nfs-version", 1, 0, 'N' },
>> + { "nfs-version", 1, 0, 'V' },
>> { "no-tcp", 0, 0, 'T' },
>> { "no-udp", 0, 0, 'U' },
>> { "port", 1, 0, 'P' },
>> @@ -119,7 +120,7 @@ main(int argc, char **argv)
>> xlog_syslog(0);
>> xlog_stderr(1);
>>
>> - while ((c = getopt_long(argc, argv, "dH:hN:p:P:sTU", longopts, NULL)) != EOF) {
>> + while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTU", longopts, NULL)) != EOF) {
>> switch(c) {
>> case 'd':
>> xlog_config(D_ALL, 1);
>> @@ -175,6 +176,27 @@ main(int argc, char **argv)
>> exit(1);
>> }
>> break;
>> + case 'V':
>> + switch((c = strtol(optarg, &p, 0))) {
>> + case 4:
>> + if (*p == '.') {
>> + int i = atoi(p+1);
>> + if (i != 1) {
>> + fprintf(stderr, "%s: unsupported minor version\n", optarg);
>> + exit(1);
>> + }
>> + minorvers41 = 1;
>> + break;
>> + }
>> + case 3:
>> + case 2:
>> + NFSCTL_VERSET(versbits, c);
>> + break;
>> + default:
>> + fprintf(stderr, "%s: Unsupported version\n", optarg);
>> + exit(1);
>> + }
>> + break;
>> case 's':
>> xlog_syslog(1);
>> xlog_stderr(0);
>> @@ -312,7 +334,7 @@ static void
>> usage(const char *prog)
>> {
>> fprintf(stderr, "Usage:\n"
>> - "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
>> + "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version] [-V|--nfs-version version] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
>> prog);
>> exit(2);
>> }
>> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
>> index 1cf9296..7de0867 100644
>> --- a/utils/nfsd/nfsd.man
>> +++ b/utils/nfsd/nfsd.man
>> @@ -47,7 +47,7 @@ This option can be used to request that
>> .B rpc.nfsd
>> does not offer certain versions of NFS. The current version of
>> .B rpc.nfsd
>> -can support both NFS version 2,3 and the newer version 4.
>> +can support NFS versions 2,3,4 and the newer version 4.1.
>> .TP
>> .B \-s " or " \-\-syslog
>> By default,
>> @@ -67,6 +67,13 @@ Disable
>> .B rpc.nfsd
>> from accepting UDP connections from clients.
>> .TP
>> +.B \-V " or " \-\-nfs-version vers
>> +This option can be used to request that
>> +.B rpc.nfsd
>> +offer certain versions of NFS. The current version of
>> +.B rpc.nfsd
>> +can support NFS versions 2,3,4 and the newer version 4.1.
>> +.TP
>> .I nproc
>> specify the number of NFS server threads. By default, just one
>> thread is started. However, for optimum performance several threads
>> --
>> 1.8.1.4
>>
>

2013-03-19 15:16:49

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, 2013-03-19 at 10:50 -0400, J. Bruce Fields wrote:
> On Tue, Mar 19, 2013 at 02:41:42PM +0000, Myklebust, Trond wrote:
> > On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> > > (Sorry for the digression, I was just trying to decide whether I can get
> > > away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> > > GSS on the backchannel....)
> >
> > If you don't, could you please at least offer up a module parameter that
> > allows me to do that? The current thing is a PITA, since it isn't
> > supported by any distro scripts, and it requires you to switch off the
> > server in order to change the version pseudofile contents.
>
> The module parameter on its own wouldn't help with that.
>
> We could probably fix the pseudofile interface to allow changing the
> version on the fly.
>
> My vague plan was to teach rpc.nfsd to read this (and other parameters)
> from a config file, so then it'd be "vi /etc/nfsd && service nfsd
> restart" or equivalent.
>
> So is the main thing you want a more convenient interface or a way to
> change it without rebooting?

Basically, all I want is to enable 4.1 permanently on those servers that
I want to use for NFSv4.1 testing.

An --enable-nfs-version option for rpc.nfsd that acts together with
--no-nfs-version as a convenient interface for /proc/fs/nfsd/versions
would work fine too. All distro configuration scripts that I'm aware of
allow you to specify optional arguments to rpc.nfsd.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-03-19 14:45:02

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 10/11] NFSv4.1: Enable open-by-filehandle

On Tue, 2013-03-19 at 10:41 -0400, Trond Myklebust wrote:
> On Tue, 2013-03-19 at 10:32 -0400, J. Bruce Fields wrote:
> > (Sorry for the digression, I was just trying to decide whether I can get
> > away with turning on 4.1 by default before implementing SP4_MACH_CRED or
> > GSS on the backchannel....)
>
> If you don't, could you please at least offer up a module parameter that
> allows me to do that? The current thing is a PITA, since it isn't
> supported by any distro scripts, and it requires you to switch off the
> server in order to change the version pseudofile contents.
>
BTW: you may recall that SPKM3 and UTF-8 support was mandatory in
RFC3530, as was RPCSEC_GSS support on the back channel. That didn't stop
anybody from shipping clients and servers that didn't support them...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-03-19 13:07:51

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 06/11] NFSv4: Prepare for minorversion-specific nfs_server capabilities

Clean up the setting of the nfs_server->caps, by shoving it all
into nfs4_server_common_setup().
Then add an 'initial capabilities' field into struct nfs4_minor_version_ops.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4client.c | 24 +++++++++++++-----------
fs/nfs/nfs4proc.c | 8 ++++++++
3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 627a74f..7ef19ce 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -36,6 +36,7 @@ enum nfs4_client_state {

struct nfs4_minor_version_ops {
u32 minor_version;
+ unsigned init_caps;

int (*call_sync)(struct rpc_clnt *clnt,
struct nfs_server *server,
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index ac4fc9a..17b34b2 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -717,6 +717,19 @@ static int nfs4_server_common_setup(struct nfs_server *server,
if (error < 0)
goto out;

+ /* Set the basic capabilities */
+ server->caps |= server->nfs_client->cl_mvops->init_caps;
+ if (server->flags & NFS_MOUNT_NORDIRPLUS)
+ server->caps &= ~NFS_CAP_READDIRPLUS;
+ /*
+ * Don't use NFS uid/gid mapping if we're using AUTH_SYS or lower
+ * authentication.
+ */
+ if (nfs4_disable_idmapping &&
+ server->client->cl_auth->au_flavor == RPC_AUTH_UNIX)
+ server->caps |= NFS_CAP_UIDGID_NOMAP;
+
+
/* Probe the root fh to retrieve its FSID and filehandle */
error = nfs4_get_rootfh(server, mntfh);
if (error < 0)
@@ -760,9 +773,6 @@ static int nfs4_init_server(struct nfs_server *server,

/* Initialise the client representation from the mount data */
server->flags = data->flags;
- server->caps |= NFS_CAP_ATOMIC_OPEN|NFS_CAP_CHANGE_ATTR|NFS_CAP_POSIX_LOCK;
- if (!(data->flags & NFS_MOUNT_NORDIRPLUS))
- server->caps |= NFS_CAP_READDIRPLUS;
server->options = data->options;

/* Get a client record */
@@ -779,13 +789,6 @@ static int nfs4_init_server(struct nfs_server *server,
if (error < 0)
goto error;

- /*
- * Don't use NFS uid/gid mapping if we're using AUTH_SYS or lower
- * authentication.
- */
- if (nfs4_disable_idmapping && data->auth_flavors[0] == RPC_AUTH_UNIX)
- server->caps |= NFS_CAP_UIDGID_NOMAP;
-
if (data->rsize)
server->rsize = nfs_block_size(data->rsize, NULL);
if (data->wsize)
@@ -863,7 +866,6 @@ struct nfs_server *nfs4_create_referral_server(struct nfs_clone_mount *data,

/* Initialise the client representation from the parent server */
nfs_server_copy_userdata(server, parent_server);
- server->caps |= NFS_CAP_ATOMIC_OPEN|NFS_CAP_CHANGE_ATTR;

/* Get a client representation.
* Note: NFSv4 always uses TCP, */
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 028bd52..4fc1496 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6837,6 +6837,10 @@ static const struct nfs4_state_maintenance_ops nfs41_state_renewal_ops = {

static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
.minor_version = 0,
+ .init_caps = NFS_CAP_READDIRPLUS
+ | NFS_CAP_ATOMIC_OPEN
+ | NFS_CAP_CHANGE_ATTR
+ | NFS_CAP_POSIX_LOCK,
.call_sync = _nfs4_call_sync,
.match_stateid = nfs4_match_stateid,
.find_root_sec = nfs4_find_root_sec,
@@ -6848,6 +6852,10 @@ static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
#if defined(CONFIG_NFS_V4_1)
static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
.minor_version = 1,
+ .init_caps = NFS_CAP_READDIRPLUS
+ | NFS_CAP_ATOMIC_OPEN
+ | NFS_CAP_CHANGE_ATTR
+ | NFS_CAP_POSIX_LOCK,
.call_sync = nfs4_call_sync_sequence,
.match_stateid = nfs41_match_stateid,
.find_root_sec = nfs41_find_root_sec,
--
1.8.1.4