On Fri, 2012-06-01 at 11:38 -0400, Chuck Lever wrote:
> The result of a TEST_STATEID operation can indicate a few different
> things. If NFS_OK is returned, then the client can continue using the
> state ID under test, and skip recovery. RFC 5661 says that if and
> only if the state ID was revoked, then the client must perform an
> explicit FREE_STATEID before trying to re-open.
>
> However, if the server doesn't recognize the state ID at all, then the
> client should immediately continue with open recovery.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>
> fs/nfs/nfs4proc.c | 38 ++++++++++++++++++++++++++++++++++++--
> 1 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 4f1e824..39d641a 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1733,6 +1733,14 @@ static int nfs4_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *sta
> }
>
> #if defined(CONFIG_NFS_V4_1)
> +/**
> + * nfs41_check_expired_stateid - does a state ID need recovery?
> + *
> + * @state: NFSv4 open state for a file
> + *
> + * Returns NFS_OK if recovery for this state ID is now finished.
> + * Otherwise a negative NFS4ERR value is returned.
> + */
> static int nfs41_check_expired_stateid(struct nfs4_state *state, nfs4_stateid *stateid, unsigned int flags)
> {
> int status = NFS_OK;
> @@ -1740,9 +1748,19 @@ static int nfs41_check_expired_stateid(struct nfs4_state *state, nfs4_stateid *s
>
> if (state->flags & flags) {
> status = nfs41_test_stateid(server, stateid);
> - if (status != NFS_OK) {
> + switch (status) {
> + case NFS_OK:
> + /* server recognizes this one, don't re-open */
> + break;
> + case -NFS4ERR_ADMIN_REVOKED:
> + case -NFS4ERR_DELEG_REVOKED:
> + /* state was revoked, free then re-open */
> nfs41_free_stateid(server, stateid);
> state->flags &= ~flags;
> + break;
> + default:
Please don't add empty "default:" cases.
> + /* anything else: go ahead and re-open it */
> + break;
That last "break" is unnecessary too in standard C99.
> }
> }
> return status;
> @@ -4642,6 +4660,14 @@ out:
> }
>
> #if defined(CONFIG_NFS_V4_1)
> +/**
> + * nfs41_check_expired_locks - clear lock state IDs
> + *
> + * @state: NFSv4 open state for a file
> + *
> + * Returns NFS_OK if recovery for this state ID is now finished.
> + * Otherwise a negative NFS4ERR value is returned.
> + */
> static int nfs41_check_expired_locks(struct nfs4_state *state)
> {
> int status, ret = NFS_OK;
> @@ -4651,9 +4677,17 @@ static int nfs41_check_expired_locks(struct nfs4_state *state)
> list_for_each_entry(lsp, &state->lock_states, ls_locks) {
> if (lsp->ls_flags & NFS_LOCK_INITIALIZED) {
> status = nfs41_test_stateid(server, &lsp->ls_stateid);
> - if (status != NFS_OK) {
> + switch (status) {
> + case NFS_OK:
> + /* server recognizes this one, don't re-lock */
> + break;
> + case -NFS4ERR_ADMIN_REVOKED:
> + case -NFS4ERR_DELEG_REVOKED:
> + /* lock was revoked, free then re-lock */
> nfs41_free_stateid(server, &lsp->ls_stateid);
> lsp->ls_flags &= ~NFS_LOCK_INITIALIZED;
> + /*FALLTHROUGH*/
Why do we need this comment? It should be obvious.
> + default:
> ret = status;
> }
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
