[PATCH v2 0/2] NFSD: add FREE_STATEID and TEST_STATEID operations

[PATCH v2 0/2] NFSD: add FREE_STATEID and TEST_STATEID operations

[bjschuma]

From: Bryan Schumaker <bjschuma@netapp.com>

These patches add FREE_STATEID and TEST_STATEID to the NFS server.  I
appreciate any comments, especially if there is a better way of doing things.

v2:
- FREE_STATEID
  - Free open, lock, and delegation state ids correctly
  - Use the check_for_locks() function to check for locks

- TEST_STATEID
  - Remove openmode check
  - Sanity check on size of stateid list
  - Delay decoding stateid list until encoding the reply

v3:
- FREE_STATEID
  - Open stateids and delegations are never freeable (return nfserr_locks_held)

- TEST_STATEID
  - Remove unused arguments from a function call
  - Store and use the HAS_SESSION flag for validating stateids

Thanks!

- Bryan

[PATCH 1/2] NFSD: added FREE_STATEID operation

[bjschuma]

From: Bryan Schumaker <bjschuma@netapp.com>

This operation is used by the client to tell the server to free a
stateid.

Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
---
 fs/nfsd/nfs4proc.c  |    5 +++
 fs/nfsd/nfs4state.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   |   32 +++++++++++++++-
 fs/nfsd/xdr4.h      |    8 ++++
 4 files changed, 144 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 3a6dbd7..7e00116 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1412,6 +1412,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_SECINFO_NO_NAME",
 	},
+	[OP_FREE_STATEID] = {
+		.op_func = (nfsd4op_func)nfsd4_free_stateid,
+		.op_flags = ALLOWED_WITHOUT_FH,
+		.op_name = "OP_FREE_STATEID",
+	},
 };
 
 static const char *nfsd4_op_name(unsigned opnum)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a2ea14f..c8c1cad 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -60,9 +60,12 @@ static u64 current_sessionid = 1;
 
 /* forward declarations */
 static struct nfs4_stateid * find_stateid(stateid_t *stid, int flags);
+static struct nfs4_stateid * search_for_stateid(stateid_t *stid);
+static struct nfs4_delegation * search_for_delegation(stateid_t *stid);
 static struct nfs4_delegation * find_delegation_stateid(struct inode *ino, stateid_t *stid);
 static char user_recovery_dirname[PATH_MAX] = "/var/lib/nfs/v4recovery";
 static void nfs4_set_recdir(char *recdir);
+static int check_for_locks(struct nfs4_file *filp, struct nfs4_stateowner *lowner);
 
 /* Locking: */
 
@@ -3133,6 +3136,11 @@ static int is_delegation_stateid(stateid_t *stateid)
 	return stateid->si_fileid == 0;
 }
 
+static int is_open_stateid(struct nfs4_stateid *stateid)
+{
+	return stateid->st_openstp == NULL;
+}
+
 /*
 * Checks for stateid operations
 */
@@ -3212,6 +3220,53 @@ out:
 	return status;
 }
 
+static __be32
+nfsd4_free_delegation_stateid(stateid_t *stateid)
+{
+	struct nfs4_delegation *dp = search_for_delegation(stateid);
+	if (dp)
+		return nfserr_locks_held;
+	return nfserr_bad_stateid;
+}
+
+static __be32
+nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stateid *stp)
+{
+	if (check_for_locks(stp->st_file, stp->st_stateowner))
+		return nfserr_locks_held;
+	release_lock_stateid(stp);
+	return nfs_ok;
+}
+
+/*
+ * Free a state id
+ */
+__be32
+nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+		   struct nfsd4_free_stateid *free_stateid)
+{
+	stateid_t *stateid = &free_stateid->fr_stateid;
+	struct nfs4_stateid *stp;
+
+	if (is_delegation_stateid(stateid))
+		return nfsd4_free_delegation_stateid(stateid);
+
+	stp = search_for_stateid(stateid);
+	if (!stp)
+		return nfserr_bad_stateid;
+	if (stateid->si_generation != 0) {
+		if (stateid->si_generation < stp->st_stateid.si_generation)
+			return nfserr_old_stateid;
+		if (stateid->si_generation > stp->st_stateid.si_generation)
+			return nfserr_bad_stateid;
+	}
+
+	if (is_open_stateid(stp))
+		return nfserr_locks_held;
+	else
+		return nfsd4_free_lock_stateid(stateid, stp);
+}
+
 static inline int
 setlkflg (int type)
 {
@@ -3590,6 +3645,14 @@ static struct list_head lock_ownerid_hashtbl[LOCK_HASH_SIZE];
 static struct list_head	lock_ownerstr_hashtbl[LOCK_HASH_SIZE];
 static struct list_head lockstateid_hashtbl[STATEID_HASH_SIZE];
 
+static int
+same_stateid(stateid_t *id_one, stateid_t *id_two)
+{
+	if (id_one->si_stateownerid != id_two->si_stateownerid)
+		return 0;
+	return id_one->si_fileid == id_two->si_fileid;
+}
+
 static struct nfs4_stateid *
 find_stateid(stateid_t *stid, int flags)
 {
@@ -3619,6 +3682,44 @@ find_stateid(stateid_t *stid, int flags)
 	return NULL;
 }
 
+static struct nfs4_stateid *
+search_for_stateid(stateid_t *stid)
+{
+	struct nfs4_stateid *local;
+	unsigned int hashval = stateid_hashval(stid->si_stateownerid, stid->si_fileid);
+
+	list_for_each_entry(local, &lockstateid_hashtbl[hashval], st_hash) {
+		if (same_stateid(&local->st_stateid, stid))
+			return local;
+	}
+
+	list_for_each_entry(local, &stateid_hashtbl[hashval], st_hash) {
+		if (same_stateid(&local->st_stateid, stid))
+			return local;
+	}
+	return NULL;
+}
+
+static struct nfs4_delegation *
+search_for_delegation(stateid_t *stid)
+{
+	struct nfs4_file *fp;
+	struct nfs4_delegation *dp;
+	struct list_head *pos;
+	int i;
+
+	for (i = 0; i < FILE_HASH_SIZE; i++) {
+		list_for_each_entry(fp, &file_hashtbl[i], fi_hash) {
+			list_for_each(pos, &fp->fi_delegations) {
+				dp = list_entry(pos, struct nfs4_delegation, dl_perfile);
+				if (same_stateid(&dp->dl_stateid, stid))
+					return dp;
+			}
+		}
+	}
+	return NULL;
+}
+
 static struct nfs4_delegation *
 find_delegation_stateid(struct inode *ino, stateid_t *stid)
 {
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 195a91d..5da6874 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1246,6 +1246,19 @@ nfsd4_decode_destroy_session(struct nfsd4_compoundargs *argp,
 }
 
 static __be32
+nfsd4_decode_free_stateid(struct nfsd4_compoundargs *argp,
+			  struct nfsd4_free_stateid *free_stateid)
+{
+	DECODE_HEAD;
+
+	READ_BUF(sizeof(stateid_t));
+	READ32(free_stateid->fr_stateid.si_generation);
+	COPYMEM(&free_stateid->fr_stateid.si_opaque, sizeof(stateid_opaque_t));
+
+	DECODE_TAIL;
+}
+
+static __be32
 nfsd4_decode_sequence(struct nfsd4_compoundargs *argp,
 		      struct nfsd4_sequence *seq)
 {
@@ -1370,7 +1383,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
 	[OP_EXCHANGE_ID]	= (nfsd4_dec)nfsd4_decode_exchange_id,
 	[OP_CREATE_SESSION]	= (nfsd4_dec)nfsd4_decode_create_session,
 	[OP_DESTROY_SESSION]	= (nfsd4_dec)nfsd4_decode_destroy_session,
-	[OP_FREE_STATEID]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_FREE_STATEID]	= (nfsd4_dec)nfsd4_decode_free_stateid,
 	[OP_GET_DIR_DELEGATION]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_GETDEVICEINFO]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_GETDEVICELIST]	= (nfsd4_dec)nfsd4_decode_notsupp,
@@ -3115,6 +3128,21 @@ nfsd4_encode_destroy_session(struct nfsd4_compoundres *resp, int nfserr,
 	return nfserr;
 }
 
+static __be32
+nfsd4_encode_free_stateid(struct nfsd4_compoundres *resp, int nfserr,
+			  struct nfsd4_free_stateid *free_stateid)
+{
+	__be32 *p;
+
+	if (nfserr)
+		return nfserr;
+
+	RESERVE_SPACE(4);
+	WRITE32(nfserr);
+	ADJUST_ARGS();
+	return nfserr;
+}
+
 __be32
 nfsd4_encode_sequence(struct nfsd4_compoundres *resp, int nfserr,
 		      struct nfsd4_sequence *seq)
@@ -3196,7 +3224,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_EXCHANGE_ID]	= (nfsd4_enc)nfsd4_encode_exchange_id,
 	[OP_CREATE_SESSION]	= (nfsd4_enc)nfsd4_encode_create_session,
 	[OP_DESTROY_SESSION]	= (nfsd4_enc)nfsd4_encode_destroy_session,
-	[OP_FREE_STATEID]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_FREE_STATEID]	= (nfsd4_enc)nfsd4_encode_free_stateid,
 	[OP_GET_DIR_DELEGATION]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_GETDEVICEINFO]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_GETDEVICELIST]	= (nfsd4_enc)nfsd4_encode_noop,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 366401e..ed1784d 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -342,6 +342,11 @@ struct nfsd4_setclientid_confirm {
 	nfs4_verifier	sc_confirm;
 };
 
+struct nfsd4_free_stateid {
+	stateid_t	fr_stateid;         /* request */
+	__be32		fr_status;          /* response */
+};
+
 /* also used for NVERIFY */
 struct nfsd4_verify {
 	u32		ve_bmval[3];        /* request */
@@ -432,6 +437,7 @@ struct nfsd4_op {
 		struct nfsd4_destroy_session	destroy_session;
 		struct nfsd4_sequence		sequence;
 		struct nfsd4_reclaim_complete	reclaim_complete;
+		struct nfsd4_free_stateid	free_stateid;
 	} u;
 	struct nfs4_replay *			replay;
 };
@@ -564,6 +570,8 @@ extern __be32 nfsd4_delegreturn(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *, struct nfsd4_delegreturn *dr);
 extern __be32 nfsd4_renew(struct svc_rqst *rqstp,
 			  struct nfsd4_compound_state *, clientid_t *clid);
+extern __be32 nfsd4_free_stateid(struct svc_rqst *rqstp,
+		struct nfsd4_compound_state *, struct nfsd4_free_stateid *free_stateid);
 #endif
 
 /*
-- 
1.7.5.2

Re: [PATCH 1/2] NFSD: added FREE_STATEID operation

[Bryan Schumaker]

On 05/20/2011 04:12 PM, bjschuma@netapp.com wrote:
> From: Bryan Schumaker <bjschuma@netapp.com>
> 
> This operation is used by the client to tell the server to free a
> stateid.
> 
> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> ---
>  fs/nfsd/nfs4proc.c  |    5 +++
>  fs/nfsd/nfs4state.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfs4xdr.c   |   32 +++++++++++++++-
>  fs/nfsd/xdr4.h      |    8 ++++
>  4 files changed, 144 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 3a6dbd7..7e00116 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1412,6 +1412,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
>  		.op_flags = OP_HANDLES_WRONGSEC,
>  		.op_name = "OP_SECINFO_NO_NAME",
>  	},
> +	[OP_FREE_STATEID] = {
> +		.op_func = (nfsd4op_func)nfsd4_free_stateid,
> +		.op_flags = ALLOWED_WITHOUT_FH,
> +		.op_name = "OP_FREE_STATEID",
> +	},
>  };
>  
>  static const char *nfsd4_op_name(unsigned opnum)
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a2ea14f..c8c1cad 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -60,9 +60,12 @@ static u64 current_sessionid = 1;
>  
>  /* forward declarations */
>  static struct nfs4_stateid * find_stateid(stateid_t *stid, int flags);
> +static struct nfs4_stateid * search_for_stateid(stateid_t *stid);
> +static struct nfs4_delegation * search_for_delegation(stateid_t *stid);
>  static struct nfs4_delegation * find_delegation_stateid(struct inode *ino, stateid_t *stid);
>  static char user_recovery_dirname[PATH_MAX] = "/var/lib/nfs/v4recovery";
>  static void nfs4_set_recdir(char *recdir);
> +static int check_for_locks(struct nfs4_file *filp, struct nfs4_stateowner *lowner);
>  
>  /* Locking: */
>  
> @@ -3133,6 +3136,11 @@ static int is_delegation_stateid(stateid_t *stateid)
>  	return stateid->si_fileid == 0;
>  }
>  
> +static int is_open_stateid(struct nfs4_stateid *stateid)
> +{
> +	return stateid->st_openstp == NULL;
> +}
> +
>  /*
>  * Checks for stateid operations
>  */
> @@ -3212,6 +3220,53 @@ out:
>  	return status;
>  }
>  
> +static __be32
> +nfsd4_free_delegation_stateid(stateid_t *stateid)
> +{
> +	struct nfs4_delegation *dp = search_for_delegation(stateid);
> +	if (dp)
> +		return nfserr_locks_held;
> +	return nfserr_bad_stateid;
> +}
> +
> +static __be32
> +nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stateid *stp)
> +{
> +	if (check_for_locks(stp->st_file, stp->st_stateowner))

I'm not sure if stateids are unique to (file, stateowner), but I'm not sure how else to check for locks at the moment.

> +		return nfserr_locks_held;
> +	release_lock_stateid(stp);
> +	return nfs_ok;
> +}
> +
> +/*
> + * Free a state id
> + */
> +__be32
> +nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> +		   struct nfsd4_free_stateid *free_stateid)
> +{
> +	stateid_t *stateid = &free_stateid->fr_stateid;
> +	struct nfs4_stateid *stp;
> +
> +	if (is_delegation_stateid(stateid))
> +		return nfsd4_free_delegation_stateid(stateid);
> +
> +	stp = search_for_stateid(stateid);
> +	if (!stp)
> +		return nfserr_bad_stateid;
> +	if (stateid->si_generation != 0) {
> +		if (stateid->si_generation < stp->st_stateid.si_generation)
> +			return nfserr_old_stateid;
> +		if (stateid->si_generation > stp->st_stateid.si_generation)
> +			return nfserr_bad_stateid;
> +	}
> +
> +	if (is_open_stateid(stp))
> +		return nfserr_locks_held;
> +	else
> +		return nfsd4_free_lock_stateid(stateid, stp);
> +}
> +
>  static inline int
>  setlkflg (int type)
>  {
> @@ -3590,6 +3645,14 @@ static struct list_head lock_ownerid_hashtbl[LOCK_HASH_SIZE];
>  static struct list_head	lock_ownerstr_hashtbl[LOCK_HASH_SIZE];
>  static struct list_head lockstateid_hashtbl[STATEID_HASH_SIZE];
>  
> +static int
> +same_stateid(stateid_t *id_one, stateid_t *id_two)
> +{
> +	if (id_one->si_stateownerid != id_two->si_stateownerid)
> +		return 0;
> +	return id_one->si_fileid == id_two->si_fileid;
> +}
> +
>  static struct nfs4_stateid *
>  find_stateid(stateid_t *stid, int flags)
>  {
> @@ -3619,6 +3682,44 @@ find_stateid(stateid_t *stid, int flags)
>  	return NULL;
>  }
>  
> +static struct nfs4_stateid *
> +search_for_stateid(stateid_t *stid)
> +{
> +	struct nfs4_stateid *local;
> +	unsigned int hashval = stateid_hashval(stid->si_stateownerid, stid->si_fileid);
> +
> +	list_for_each_entry(local, &lockstateid_hashtbl[hashval], st_hash) {
> +		if (same_stateid(&local->st_stateid, stid))
> +			return local;
> +	}
> +
> +	list_for_each_entry(local, &stateid_hashtbl[hashval], st_hash) {
> +		if (same_stateid(&local->st_stateid, stid))
> +			return local;
> +	}
> +	return NULL;
> +}
> +
> +static struct nfs4_delegation *
> +search_for_delegation(stateid_t *stid)
> +{
> +	struct nfs4_file *fp;
> +	struct nfs4_delegation *dp;
> +	struct list_head *pos;
> +	int i;
> +
> +	for (i = 0; i < FILE_HASH_SIZE; i++) {
> +		list_for_each_entry(fp, &file_hashtbl[i], fi_hash) {
> +			list_for_each(pos, &fp->fi_delegations) {
> +				dp = list_entry(pos, struct nfs4_delegation, dl_perfile);
> +				if (same_stateid(&dp->dl_stateid, stid))
> +					return dp;
> +			}
> +		}
> +	}
> +	return NULL;
> +}
> +
>  static struct nfs4_delegation *
>  find_delegation_stateid(struct inode *ino, stateid_t *stid)
>  {
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 195a91d..5da6874 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1246,6 +1246,19 @@ nfsd4_decode_destroy_session(struct nfsd4_compoundargs *argp,
>  }
>  
>  static __be32
> +nfsd4_decode_free_stateid(struct nfsd4_compoundargs *argp,
> +			  struct nfsd4_free_stateid *free_stateid)
> +{
> +	DECODE_HEAD;
> +
> +	READ_BUF(sizeof(stateid_t));
> +	READ32(free_stateid->fr_stateid.si_generation);
> +	COPYMEM(&free_stateid->fr_stateid.si_opaque, sizeof(stateid_opaque_t));
> +
> +	DECODE_TAIL;
> +}
> +
> +static __be32
>  nfsd4_decode_sequence(struct nfsd4_compoundargs *argp,
>  		      struct nfsd4_sequence *seq)
>  {
> @@ -1370,7 +1383,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
>  	[OP_EXCHANGE_ID]	= (nfsd4_dec)nfsd4_decode_exchange_id,
>  	[OP_CREATE_SESSION]	= (nfsd4_dec)nfsd4_decode_create_session,
>  	[OP_DESTROY_SESSION]	= (nfsd4_dec)nfsd4_decode_destroy_session,
> -	[OP_FREE_STATEID]	= (nfsd4_dec)nfsd4_decode_notsupp,
> +	[OP_FREE_STATEID]	= (nfsd4_dec)nfsd4_decode_free_stateid,
>  	[OP_GET_DIR_DELEGATION]	= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_GETDEVICEINFO]	= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_GETDEVICELIST]	= (nfsd4_dec)nfsd4_decode_notsupp,
> @@ -3115,6 +3128,21 @@ nfsd4_encode_destroy_session(struct nfsd4_compoundres *resp, int nfserr,
>  	return nfserr;
>  }
>  
> +static __be32
> +nfsd4_encode_free_stateid(struct nfsd4_compoundres *resp, int nfserr,
> +			  struct nfsd4_free_stateid *free_stateid)
> +{
> +	__be32 *p;
> +
> +	if (nfserr)
> +		return nfserr;
> +
> +	RESERVE_SPACE(4);
> +	WRITE32(nfserr);
> +	ADJUST_ARGS();
> +	return nfserr;
> +}
> +
>  __be32
>  nfsd4_encode_sequence(struct nfsd4_compoundres *resp, int nfserr,
>  		      struct nfsd4_sequence *seq)
> @@ -3196,7 +3224,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
>  	[OP_EXCHANGE_ID]	= (nfsd4_enc)nfsd4_encode_exchange_id,
>  	[OP_CREATE_SESSION]	= (nfsd4_enc)nfsd4_encode_create_session,
>  	[OP_DESTROY_SESSION]	= (nfsd4_enc)nfsd4_encode_destroy_session,
> -	[OP_FREE_STATEID]	= (nfsd4_enc)nfsd4_encode_noop,
> +	[OP_FREE_STATEID]	= (nfsd4_enc)nfsd4_encode_free_stateid,
>  	[OP_GET_DIR_DELEGATION]	= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_GETDEVICEINFO]	= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_GETDEVICELIST]	= (nfsd4_enc)nfsd4_encode_noop,
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 366401e..ed1784d 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -342,6 +342,11 @@ struct nfsd4_setclientid_confirm {
>  	nfs4_verifier	sc_confirm;
>  };
>  
> +struct nfsd4_free_stateid {
> +	stateid_t	fr_stateid;         /* request */
> +	__be32		fr_status;          /* response */
> +};
> +
>  /* also used for NVERIFY */
>  struct nfsd4_verify {
>  	u32		ve_bmval[3];        /* request */
> @@ -432,6 +437,7 @@ struct nfsd4_op {
>  		struct nfsd4_destroy_session	destroy_session;
>  		struct nfsd4_sequence		sequence;
>  		struct nfsd4_reclaim_complete	reclaim_complete;
> +		struct nfsd4_free_stateid	free_stateid;
>  	} u;
>  	struct nfs4_replay *			replay;
>  };
> @@ -564,6 +570,8 @@ extern __be32 nfsd4_delegreturn(struct svc_rqst *rqstp,
>  		struct nfsd4_compound_state *, struct nfsd4_delegreturn *dr);
>  extern __be32 nfsd4_renew(struct svc_rqst *rqstp,
>  			  struct nfsd4_compound_state *, clientid_t *clid);
> +extern __be32 nfsd4_free_stateid(struct svc_rqst *rqstp,
> +		struct nfsd4_compound_state *, struct nfsd4_free_stateid *free_stateid);
>  #endif
>  
>  /*

Re: [PATCH 1/2] NFSD: added FREE_STATEID operation

[J. Bruce Fields]

On Fri, May 20, 2011 at 04:16:18PM -0400, Bryan Schumaker wrote:
> On 05/20/2011 04:12 PM, bjschuma@netapp.com wrote:
> > From: Bryan Schumaker <bjschuma@netapp.com>
> > +static __be32
> > +nfsd4_free_delegation_stateid(stateid_t *stateid)
> > +{
> > +	struct nfs4_delegation *dp = search_for_delegation(stateid);
> > +	if (dp)
> > +		return nfserr_locks_held;
> > +	return nfserr_bad_stateid;
> > +}
> > +
> > +static __be32
> > +nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stateid *stp)
> > +{
> > +	if (check_for_locks(stp->st_file, stp->st_stateowner))
> 
> I'm not sure if stateids are unique to (file, stateowner), but I'm not sure how else to check for locks at the moment.

Actually, looking... there's a one-to-one lockowner<->lockstateid
correspondance.  Which is dumb.

The server's handling of lock state needs more serious help than I
realized!

I think your patch is OK now, though.

--b.

Re: [PATCH 1/2] NFSD: added FREE_STATEID operation

[Bryan Schumaker]

On 05/25/2011 11:05 AM, J. Bruce Fields wrote:
> On Fri, May 20, 2011 at 04:16:18PM -0400, Bryan Schumaker wrote:
>> On 05/20/2011 04:12 PM, bjschuma@netapp.com wrote:
>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>> +static __be32
>>> +nfsd4_free_delegation_stateid(stateid_t *stateid)
>>> +{
>>> +	struct nfs4_delegation *dp = search_for_delegation(stateid);
>>> +	if (dp)
>>> +		return nfserr_locks_held;
>>> +	return nfserr_bad_stateid;
>>> +}
>>> +
>>> +static __be32
>>> +nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stateid *stp)
>>> +{
>>> +	if (check_for_locks(stp->st_file, stp->st_stateowner))
>>
>> I'm not sure if stateids are unique to (file, stateowner), but I'm not sure how else to check for locks at the moment.
> 
> Actually, looking... there's a one-to-one lockowner<->lockstateid
> correspondance.  Which is dumb.
> 
> The server's handling of lock state needs more serious help than I
> realized!
> 
> I think your patch is OK now, though.

Thanks for the reviews!

> 
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] NFSD: Added TEST_STATEID operation

[bjschuma]

From: Bryan Schumaker <bjschuma@netapp.com>

This operation is used by the client to check the validity of a list of
stateids.

Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
---
 fs/nfsd/nfs4proc.c  |    5 +++
 fs/nfsd/nfs4state.c |   47 ++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   |   85 +++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/nfsd/state.h     |    1 +
 fs/nfsd/xdr4.h      |   17 ++++++++++
 5 files changed, 152 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7e00116..a856f30 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1412,6 +1412,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_SECINFO_NO_NAME",
 	},
+	[OP_TEST_STATEID] = {
+		.op_func = (nfsd4op_func)nfsd4_test_stateid,
+		.op_flags = ALLOWED_WITHOUT_FH,
+		.op_name = "OP_TEST_STATEID",
+	},
 	[OP_FREE_STATEID] = {
 		.op_func = (nfsd4op_func)nfsd4_free_stateid,
 		.op_flags = ALLOWED_WITHOUT_FH,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c8c1cad..0aa394d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/namei.h>
 #include <linux/swap.h>
+#include <linux/pagemap.h>
 #include <linux/sunrpc/svcauth_gss.h>
 #include <linux/sunrpc/clnt.h>
 #include "xdr4.h"
@@ -3141,6 +3142,32 @@ static int is_open_stateid(struct nfs4_stateid *stateid)
 	return stateid->st_openstp == NULL;
 }
 
+static __be32 nfs4_validate_stateid(stateid_t *stateid, int flags)
+{
+	struct nfs4_stateid *stp = NULL;
+	__be32 status = nfserr_stale_stateid;
+
+	if (STALE_STATEID(stateid))
+		goto out;
+
+	status = nfserr_expired;
+	stp = search_for_stateid(stateid);
+	if (!stp)
+		goto out;
+	status = nfserr_bad_stateid;
+
+	if (!stp->st_stateowner->so_confirmed)
+		goto out;
+
+	status = check_stateid_generation(stateid, &stp->st_stateid, flags);
+	if (status)
+		goto out;
+
+	status = nfs_ok;
+out:
+	return status;
+}
+
 /*
 * Checks for stateid operations
 */
@@ -3239,6 +3266,26 @@ nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stateid *stp)
 }
 
 /*
+ * Test if the stateid is valid
+ */
+__be32
+nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+		   struct nfsd4_test_stateid *test_stateid)
+{
+	test_stateid->ts_has_session = nfsd4_has_session(cstate);
+	return nfs_ok;
+}
+
+__be32
+nfsd4_do_test_stateid(stateid_t *stateid, int flags)
+{
+	__be32 ret = nfs4_validate_stateid(stateid, flags);
+	if (!ret)
+		ret = nfs4_validate_stateid(stateid, flags);
+	return ret;
+}
+
+/*
  * Free a state id
  */
 __be32
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 5da6874..a042174 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -44,13 +44,14 @@
 #include <linux/namei.h>
 #include <linux/statfs.h>
 #include <linux/utsname.h>
+#include <linux/pagemap.h>
 #include <linux/sunrpc/svcauth_gss.h>
 
 #include "idmap.h"
 #include "acl.h"
 #include "xdr4.h"
 #include "vfs.h"
-
+#include "state.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_XDR
 
@@ -131,6 +132,22 @@ xdr_error:					\
 	}					\
 } while (0)
 
+static void save_buf(struct nfsd4_compoundargs *argp, struct nfsd4_saved_compoundargs *savep)
+{
+	savep->p        = argp->p;
+	savep->end      = argp->end;
+	savep->pagelen  = argp->pagelen;
+	savep->pagelist = argp->pagelist;
+}
+
+static void restore_buf(struct nfsd4_compoundargs *argp, struct nfsd4_saved_compoundargs *savep)
+{
+	argp->p        = savep->p;
+	argp->end      = savep->end;
+	argp->pagelen  = savep->pagelen;
+	argp->pagelist = savep->pagelist;
+}
+
 static __be32 *read_buf(struct nfsd4_compoundargs *argp, u32 nbytes)
 {
 	/* We want more bytes than seem to be available.
@@ -1274,6 +1291,40 @@ nfsd4_decode_sequence(struct nfsd4_compoundargs *argp,
 	DECODE_TAIL;
 }
 
+static __be32
+nfsd4_decode_test_stateid(struct nfsd4_compoundargs *argp, struct nfsd4_test_stateid *test_stateid)
+{
+	unsigned int nbytes;
+	stateid_t si;
+	int i;
+	__be32 *p;
+	__be32 status;
+
+	READ_BUF(4);
+	test_stateid->ts_num_ids = ntohl(*p++);
+
+	nbytes = test_stateid->ts_num_ids * sizeof(stateid_t);
+	if (nbytes > (u32)((char *)argp->end - (char *)argp->p))
+		goto xdr_error;
+
+	test_stateid->ts_saved_args = argp;
+	save_buf(argp, &test_stateid->ts_savedp);
+
+	for (i = 0; i < test_stateid->ts_num_ids; i++) {
+		status = nfsd4_decode_stateid(argp, &si);
+		if (status)
+			return status;
+	}
+
+	status = 0;
+out:
+	return status;
+xdr_error:
+	dprintk("NFSD: xdr error (%s:%d)\n", __FILE__, __LINE__);
+	status = nfserr_bad_xdr;
+	goto out;
+}
+
 static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, struct nfsd4_reclaim_complete *rc)
 {
 	DECODE_HEAD;
@@ -1393,7 +1444,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
 	[OP_SECINFO_NO_NAME]	= (nfsd4_dec)nfsd4_decode_secinfo_no_name,
 	[OP_SEQUENCE]		= (nfsd4_dec)nfsd4_decode_sequence,
 	[OP_SET_SSV]		= (nfsd4_dec)nfsd4_decode_notsupp,
-	[OP_TEST_STATEID]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_TEST_STATEID]	= (nfsd4_dec)nfsd4_decode_test_stateid,
 	[OP_WANT_DELEGATION]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_DESTROY_CLIENTID]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
@@ -3166,6 +3217,34 @@ nfsd4_encode_sequence(struct nfsd4_compoundres *resp, int nfserr,
 	return 0;
 }
 
+__be32
+nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, int nfserr,
+			  struct nfsd4_test_stateid *test_stateid)
+{
+	struct nfsd4_compoundargs *argp;
+	stateid_t si;
+	__be32 *p;
+	int i;
+	int valid;
+
+	restore_buf(test_stateid->ts_saved_args, &test_stateid->ts_savedp);
+	argp = test_stateid->ts_saved_args;
+
+	RESERVE_SPACE(4);
+	*p++ = htonl(test_stateid->ts_num_ids);
+	resp->p = p;
+
+	for (i = 0; i < test_stateid->ts_num_ids; i++) {
+		nfsd4_decode_stateid(argp, &si);
+		valid = nfsd4_do_test_stateid(&si, test_stateid->ts_has_session);
+		RESERVE_SPACE(4);
+		*p++ = htonl(valid);
+		resp->p = p;
+	}
+
+	return nfserr;
+}
+
 static __be32
 nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
 {
@@ -3234,7 +3313,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo_no_name,
 	[OP_SEQUENCE]		= (nfsd4_enc)nfsd4_encode_sequence,
 	[OP_SET_SSV]		= (nfsd4_enc)nfsd4_encode_noop,
-	[OP_TEST_STATEID]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_TEST_STATEID]	= (nfsd4_enc)nfsd4_encode_test_stateid,
 	[OP_WANT_DELEGATION]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_DESTROY_CLIENTID]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_enc)nfsd4_encode_noop,
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 6bd2f3c..d26164e 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -482,6 +482,7 @@ extern void nfsd4_recdir_purge_old(void);
 extern int nfsd4_create_clid_dir(struct nfs4_client *clp);
 extern void nfsd4_remove_clid_dir(struct nfs4_client *clp);
 extern void release_session_client(struct nfsd4_session *);
+extern __be32 nfsd4_do_test_stateid(stateid_t *, int);
 
 static inline void
 nfs4_put_stateowner(struct nfs4_stateowner *so)
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index ed1784d..02fb0e0 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -342,6 +342,20 @@ struct nfsd4_setclientid_confirm {
 	nfs4_verifier	sc_confirm;
 };
 
+struct nfsd4_saved_compoundargs {
+	__be32 *p;
+	__be32 *end;
+	int pagelen;
+	struct page **pagelist;
+};
+
+struct nfsd4_test_stateid {
+	__be32		ts_num_ids;
+	__be32		ts_has_session;
+	struct nfsd4_compoundargs *ts_saved_args;
+	struct nfsd4_saved_compoundargs ts_savedp;
+};
+
 struct nfsd4_free_stateid {
 	stateid_t	fr_stateid;         /* request */
 	__be32		fr_status;          /* response */
@@ -437,6 +451,7 @@ struct nfsd4_op {
 		struct nfsd4_destroy_session	destroy_session;
 		struct nfsd4_sequence		sequence;
 		struct nfsd4_reclaim_complete	reclaim_complete;
+		struct nfsd4_test_stateid	test_stateid;
 		struct nfsd4_free_stateid	free_stateid;
 	} u;
 	struct nfs4_replay *			replay;
@@ -570,6 +585,8 @@ extern __be32 nfsd4_delegreturn(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *, struct nfsd4_delegreturn *dr);
 extern __be32 nfsd4_renew(struct svc_rqst *rqstp,
 			  struct nfsd4_compound_state *, clientid_t *clid);
+extern __be32 nfsd4_test_stateid(struct svc_rqst *rqstp,
+		struct nfsd4_compound_state *, struct nfsd4_test_stateid *test_stateid);
 extern __be32 nfsd4_free_stateid(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *, struct nfsd4_free_stateid *free_stateid);
 #endif
-- 
1.7.5.2

Re: [PATCH 2/2] NFSD: Added TEST_STATEID operation

[Bryan Schumaker]

On 05/20/2011 04:13 PM, bjschuma@netapp.com wrote:
> From: Bryan Schumaker <bjschuma@netapp.com>
> 
> This operation is used by the client to check the validity of a list of
> stateids.
> 
> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> ---
>  fs/nfsd/nfs4proc.c  |    5 +++
>  fs/nfsd/nfs4state.c |   47 ++++++++++++++++++++++++++++
>  fs/nfsd/nfs4xdr.c   |   85 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  fs/nfsd/state.h     |    1 +
>  fs/nfsd/xdr4.h      |   17 ++++++++++
>  5 files changed, 152 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 7e00116..a856f30 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1412,6 +1412,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
>  		.op_flags = OP_HANDLES_WRONGSEC,
>  		.op_name = "OP_SECINFO_NO_NAME",
>  	},
> +	[OP_TEST_STATEID] = {
> +		.op_func = (nfsd4op_func)nfsd4_test_stateid,
> +		.op_flags = ALLOWED_WITHOUT_FH,
> +		.op_name = "OP_TEST_STATEID",
> +	},
>  	[OP_FREE_STATEID] = {
>  		.op_func = (nfsd4op_func)nfsd4_free_stateid,
>  		.op_flags = ALLOWED_WITHOUT_FH,
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c8c1cad..0aa394d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -37,6 +37,7 @@
>  #include <linux/slab.h>
>  #include <linux/namei.h>
>  #include <linux/swap.h>
> +#include <linux/pagemap.h>
>  #include <linux/sunrpc/svcauth_gss.h>
>  #include <linux/sunrpc/clnt.h>
>  #include "xdr4.h"
> @@ -3141,6 +3142,32 @@ static int is_open_stateid(struct nfs4_stateid *stateid)
>  	return stateid->st_openstp == NULL;
>  }
>  
> +static __be32 nfs4_validate_stateid(stateid_t *stateid, int flags)
> +{
> +	struct nfs4_stateid *stp = NULL;
> +	__be32 status = nfserr_stale_stateid;
> +
> +	if (STALE_STATEID(stateid))
> +		goto out;
> +
> +	status = nfserr_expired;
> +	stp = search_for_stateid(stateid);
> +	if (!stp)
> +		goto out;
> +	status = nfserr_bad_stateid;
> +
> +	if (!stp->st_stateowner->so_confirmed)
> +		goto out;
> +
> +	status = check_stateid_generation(stateid, &stp->st_stateid, flags);
> +	if (status)
> +		goto out;
> +
> +	status = nfs_ok;
> +out:
> +	return status;
> +}
> +
>  /*
>  * Checks for stateid operations
>  */
> @@ -3239,6 +3266,26 @@ nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stateid *stp)
>  }
>  
>  /*
> + * Test if the stateid is valid
> + */
> +__be32
> +nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> +		   struct nfsd4_test_stateid *test_stateid)
> +{
> +	test_stateid->ts_has_session = nfsd4_has_session(cstate);
> +	return nfs_ok;
> +}
> +
> +__be32
> +nfsd4_do_test_stateid(stateid_t *stateid, int flags)
> +{
> +	__be32 ret = nfs4_validate_stateid(stateid, flags);
> +	if (!ret)
> +		ret = nfs4_validate_stateid(stateid, flags);
> +	return ret;

Looking back at this, I'm not convinced that I need to call nfs4_validate_stateid() twice with the same stateid and flags.  What is the status of these patches?  Should I resubmit everything or send in a small patch to fix this?

- Bryan

> +}
> +
> +/*
>   * Free a state id
>   */
>  __be32
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 5da6874..a042174 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -44,13 +44,14 @@
>  #include <linux/namei.h>
>  #include <linux/statfs.h>
>  #include <linux/utsname.h>
> +#include <linux/pagemap.h>
>  #include <linux/sunrpc/svcauth_gss.h>
>  
>  #include "idmap.h"
>  #include "acl.h"
>  #include "xdr4.h"
>  #include "vfs.h"
> -
> +#include "state.h"
>  
>  #define NFSDDBG_FACILITY		NFSDDBG_XDR
>  
> @@ -131,6 +132,22 @@ xdr_error:					\
>  	}					\
>  } while (0)
>  
> +static void save_buf(struct nfsd4_compoundargs *argp, struct nfsd4_saved_compoundargs *savep)
> +{
> +	savep->p        = argp->p;
> +	savep->end      = argp->end;
> +	savep->pagelen  = argp->pagelen;
> +	savep->pagelist = argp->pagelist;
> +}
> +
> +static void restore_buf(struct nfsd4_compoundargs *argp, struct nfsd4_saved_compoundargs *savep)
> +{
> +	argp->p        = savep->p;
> +	argp->end      = savep->end;
> +	argp->pagelen  = savep->pagelen;
> +	argp->pagelist = savep->pagelist;
> +}
> +
>  static __be32 *read_buf(struct nfsd4_compoundargs *argp, u32 nbytes)
>  {
>  	/* We want more bytes than seem to be available.
> @@ -1274,6 +1291,40 @@ nfsd4_decode_sequence(struct nfsd4_compoundargs *argp,
>  	DECODE_TAIL;
>  }
>  
> +static __be32
> +nfsd4_decode_test_stateid(struct nfsd4_compoundargs *argp, struct nfsd4_test_stateid *test_stateid)
> +{
> +	unsigned int nbytes;
> +	stateid_t si;
> +	int i;
> +	__be32 *p;
> +	__be32 status;
> +
> +	READ_BUF(4);
> +	test_stateid->ts_num_ids = ntohl(*p++);
> +
> +	nbytes = test_stateid->ts_num_ids * sizeof(stateid_t);
> +	if (nbytes > (u32)((char *)argp->end - (char *)argp->p))
> +		goto xdr_error;
> +
> +	test_stateid->ts_saved_args = argp;
> +	save_buf(argp, &test_stateid->ts_savedp);
> +
> +	for (i = 0; i < test_stateid->ts_num_ids; i++) {
> +		status = nfsd4_decode_stateid(argp, &si);
> +		if (status)
> +			return status;
> +	}
> +
> +	status = 0;
> +out:
> +	return status;
> +xdr_error:
> +	dprintk("NFSD: xdr error (%s:%d)\n", __FILE__, __LINE__);
> +	status = nfserr_bad_xdr;
> +	goto out;
> +}
> +
>  static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, struct nfsd4_reclaim_complete *rc)
>  {
>  	DECODE_HEAD;
> @@ -1393,7 +1444,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
>  	[OP_SECINFO_NO_NAME]	= (nfsd4_dec)nfsd4_decode_secinfo_no_name,
>  	[OP_SEQUENCE]		= (nfsd4_dec)nfsd4_decode_sequence,
>  	[OP_SET_SSV]		= (nfsd4_dec)nfsd4_decode_notsupp,
> -	[OP_TEST_STATEID]	= (nfsd4_dec)nfsd4_decode_notsupp,
> +	[OP_TEST_STATEID]	= (nfsd4_dec)nfsd4_decode_test_stateid,
>  	[OP_WANT_DELEGATION]	= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_DESTROY_CLIENTID]	= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
> @@ -3166,6 +3217,34 @@ nfsd4_encode_sequence(struct nfsd4_compoundres *resp, int nfserr,
>  	return 0;
>  }
>  
> +__be32
> +nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, int nfserr,
> +			  struct nfsd4_test_stateid *test_stateid)
> +{
> +	struct nfsd4_compoundargs *argp;
> +	stateid_t si;
> +	__be32 *p;
> +	int i;
> +	int valid;
> +
> +	restore_buf(test_stateid->ts_saved_args, &test_stateid->ts_savedp);
> +	argp = test_stateid->ts_saved_args;
> +
> +	RESERVE_SPACE(4);
> +	*p++ = htonl(test_stateid->ts_num_ids);
> +	resp->p = p;
> +
> +	for (i = 0; i < test_stateid->ts_num_ids; i++) {
> +		nfsd4_decode_stateid(argp, &si);
> +		valid = nfsd4_do_test_stateid(&si, test_stateid->ts_has_session);
> +		RESERVE_SPACE(4);
> +		*p++ = htonl(valid);
> +		resp->p = p;
> +	}
> +
> +	return nfserr;
> +}
> +
>  static __be32
>  nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
>  {
> @@ -3234,7 +3313,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
>  	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo_no_name,
>  	[OP_SEQUENCE]		= (nfsd4_enc)nfsd4_encode_sequence,
>  	[OP_SET_SSV]		= (nfsd4_enc)nfsd4_encode_noop,
> -	[OP_TEST_STATEID]	= (nfsd4_enc)nfsd4_encode_noop,
> +	[OP_TEST_STATEID]	= (nfsd4_enc)nfsd4_encode_test_stateid,
>  	[OP_WANT_DELEGATION]	= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_DESTROY_CLIENTID]	= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_RECLAIM_COMPLETE]	= (nfsd4_enc)nfsd4_encode_noop,
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 6bd2f3c..d26164e 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -482,6 +482,7 @@ extern void nfsd4_recdir_purge_old(void);
>  extern int nfsd4_create_clid_dir(struct nfs4_client *clp);
>  extern void nfsd4_remove_clid_dir(struct nfs4_client *clp);
>  extern void release_session_client(struct nfsd4_session *);
> +extern __be32 nfsd4_do_test_stateid(stateid_t *, int);
>  
>  static inline void
>  nfs4_put_stateowner(struct nfs4_stateowner *so)
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index ed1784d..02fb0e0 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -342,6 +342,20 @@ struct nfsd4_setclientid_confirm {
>  	nfs4_verifier	sc_confirm;
>  };
>  
> +struct nfsd4_saved_compoundargs {
> +	__be32 *p;
> +	__be32 *end;
> +	int pagelen;
> +	struct page **pagelist;
> +};
> +
> +struct nfsd4_test_stateid {
> +	__be32		ts_num_ids;
> +	__be32		ts_has_session;
> +	struct nfsd4_compoundargs *ts_saved_args;
> +	struct nfsd4_saved_compoundargs ts_savedp;
> +};
> +
>  struct nfsd4_free_stateid {
>  	stateid_t	fr_stateid;         /* request */
>  	__be32		fr_status;          /* response */
> @@ -437,6 +451,7 @@ struct nfsd4_op {
>  		struct nfsd4_destroy_session	destroy_session;
>  		struct nfsd4_sequence		sequence;
>  		struct nfsd4_reclaim_complete	reclaim_complete;
> +		struct nfsd4_test_stateid	test_stateid;
>  		struct nfsd4_free_stateid	free_stateid;
>  	} u;
>  	struct nfs4_replay *			replay;
> @@ -570,6 +585,8 @@ extern __be32 nfsd4_delegreturn(struct svc_rqst *rqstp,
>  		struct nfsd4_compound_state *, struct nfsd4_delegreturn *dr);
>  extern __be32 nfsd4_renew(struct svc_rqst *rqstp,
>  			  struct nfsd4_compound_state *, clientid_t *clid);
> +extern __be32 nfsd4_test_stateid(struct svc_rqst *rqstp,
> +		struct nfsd4_compound_state *, struct nfsd4_test_stateid *test_stateid);
>  extern __be32 nfsd4_free_stateid(struct svc_rqst *rqstp,
>  		struct nfsd4_compound_state *, struct nfsd4_free_stateid *free_stateid);
>  #endif

Re: [PATCH 2/2] NFSD: Added TEST_STATEID operation

[J. Bruce Fields]

On Wed, Jun 29, 2011 at 03:49:10PM -0400, Bryan Schumaker wrote:
> On 05/20/2011 04:13 PM, bjschuma@netapp.com wrote:
> > +__be32
> > +nfsd4_do_test_stateid(stateid_t *stateid, int flags)
> > +{
> > +	__be32 ret = nfs4_validate_stateid(stateid, flags);
> > +	if (!ret)
> > +		ret = nfs4_validate_stateid(stateid, flags);
> > +	return ret;
> 
> Looking back at this, I'm not convinced that I need to call nfs4_validate_stateid() twice with the same stateid and flags.  What is the status of these patches?  Should I resubmit everything or send in a small patch to fix this?

Whoops, lazy review on my part....

Looks like I'd applied that locally but not pushed it out to my public
tree yet, so I could still take a replacement.

How are you testing these, by the way?  On second thoughts, I think I
may have already asked you that, and forgotten the answer, so don't
bother answering here--just add some text to the changelog and then I'll
know how to find the answer next time.

--b.

Re: [PATCH 2/2] NFSD: Added TEST_STATEID operation

[Bryan Schumaker]

On 06/29/2011 03:54 PM, J. Bruce Fields wrote:
> On Wed, Jun 29, 2011 at 03:49:10PM -0400, Bryan Schumaker wrote:
>> On 05/20/2011 04:13 PM, bjschuma@netapp.com wrote:
>>> +__be32
>>> +nfsd4_do_test_stateid(stateid_t *stateid, int flags)
>>> +{
>>> +	__be32 ret = nfs4_validate_stateid(stateid, flags);
>>> +	if (!ret)
>>> +		ret = nfs4_validate_stateid(stateid, flags);
>>> +	return ret;
>>
>> Looking back at this, I'm not convinced that I need to call nfs4_validate_stateid() twice with the same stateid and flags.  What is the status of these patches?  Should I resubmit everything or send in a small patch to fix this?
> 
> Whoops, lazy review on my part....
> 
> Looks like I'd applied that locally but not pushed it out to my public
> tree yet, so I could still take a replacement.

Ok, sounds good.
> 
> How are you testing these, by the way?  On second thoughts, I think I
> may have already asked you that, and forgotten the answer, so don't
> bother answering here--just add some text to the changelog and then I'll
> know how to find the answer next time.

I found this as I was going through my fault injection code.  I'm going to double check these with my fault injection changes and then try to send in updates for both next week.

- Bryan

> 
> --b.

Re: [PATCH 2/2] NFSD: Added TEST_STATEID operation

[J. Bruce Fields]

On Wed, Jun 29, 2011 at 04:34:38PM -0400, Bryan Schumaker wrote:
> On 06/29/2011 03:54 PM, J. Bruce Fields wrote:
> > On Wed, Jun 29, 2011 at 03:49:10PM -0400, Bryan Schumaker wrote:
> >> On 05/20/2011 04:13 PM, bjschuma@netapp.com wrote:
> >>> +__be32
> >>> +nfsd4_do_test_stateid(stateid_t *stateid, int flags)
> >>> +{
> >>> +	__be32 ret = nfs4_validate_stateid(stateid, flags);
> >>> +	if (!ret)
> >>> +		ret = nfs4_validate_stateid(stateid, flags);
> >>> +	return ret;
> >>
> >> Looking back at this, I'm not convinced that I need to call nfs4_validate_stateid() twice with the same stateid and flags.  What is the status of these patches?  Should I resubmit everything or send in a small patch to fix this?
> > 
> > Whoops, lazy review on my part....
> > 
> > Looks like I'd applied that locally but not pushed it out to my public
> > tree yet, so I could still take a replacement.
> 
> Ok, sounds good.
> > 
> > How are you testing these, by the way?  On second thoughts, I think I
> > may have already asked you that, and forgotten the answer, so don't
> > bother answering here--just add some text to the changelog and then I'll
> > know how to find the answer next time.
> 
> I found this as I was going through my fault injection code.  I'm going to double check these with my fault injection changes and then try to send in updates for both next week.

Sounds good, thanks Bryan.--b.

Re: [PATCH 2/2] NFSD: Added TEST_STATEID operation

[J. Bruce Fields]

On Wed, Jun 29, 2011 at 04:40:59PM -0400, J. Bruce Fields wrote:
> On Wed, Jun 29, 2011 at 04:34:38PM -0400, Bryan Schumaker wrote:
> > On 06/29/2011 03:54 PM, J. Bruce Fields wrote:
> > > On Wed, Jun 29, 2011 at 03:49:10PM -0400, Bryan Schumaker wrote:
> > >> On 05/20/2011 04:13 PM, bjschuma@netapp.com wrote:
> > >>> +__be32
> > >>> +nfsd4_do_test_stateid(stateid_t *stateid, int flags)
> > >>> +{
> > >>> +	__be32 ret = nfs4_validate_stateid(stateid, flags);
> > >>> +	if (!ret)
> > >>> +		ret = nfs4_validate_stateid(stateid, flags);
> > >>> +	return ret;
> > >>
> > >> Looking back at this, I'm not convinced that I need to call nfs4_validate_stateid() twice with the same stateid and flags.  What is the status of these patches?  Should I resubmit everything or send in a small patch to fix this?
> > > 
> > > Whoops, lazy review on my part....
> > > 
> > > Looks like I'd applied that locally but not pushed it out to my public
> > > tree yet, so I could still take a replacement.
> > 
> > Ok, sounds good.
> > > 
> > > How are you testing these, by the way?  On second thoughts, I think I
> > > may have already asked you that, and forgotten the answer, so don't
> > > bother answering here--just add some text to the changelog and then I'll
> > > know how to find the answer next time.
> > 
> > I found this as I was going through my fault injection code.  I'm going to double check these with my fault injection changes and then try to send in updates for both next week.
> 
> Sounds good, thanks Bryan.--b.

Are those still on the way?  (Or did you send and I missed them?  If so,
if you wouldn't mind resending....)

--b.

Re: [PATCH 2/2] NFSD: Added TEST_STATEID operation

[Bryan Schumaker]

On 07/12/2011 06:59 AM, J. Bruce Fields wrote:
> On Wed, Jun 29, 2011 at 04:40:59PM -0400, J. Bruce Fields wrote:
>> On Wed, Jun 29, 2011 at 04:34:38PM -0400, Bryan Schumaker wrote:
>>> On 06/29/2011 03:54 PM, J. Bruce Fields wrote:
>>>> On Wed, Jun 29, 2011 at 03:49:10PM -0400, Bryan Schumaker wrote:
>>>>> On 05/20/2011 04:13 PM, bjschuma@netapp.com wrote:
>>>>>> +__be32
>>>>>> +nfsd4_do_test_stateid(stateid_t *stateid, int flags)
>>>>>> +{
>>>>>> +	__be32 ret = nfs4_validate_stateid(stateid, flags);
>>>>>> +	if (!ret)
>>>>>> +		ret = nfs4_validate_stateid(stateid, flags);
>>>>>> +	return ret;
>>>>>
>>>>> Looking back at this, I'm not convinced that I need to call nfs4_validate_stateid() twice with the same stateid and flags.  What is the status of these patches?  Should I resubmit everything or send in a small patch to fix this?
>>>>
>>>> Whoops, lazy review on my part....
>>>>
>>>> Looks like I'd applied that locally but not pushed it out to my public
>>>> tree yet, so I could still take a replacement.
>>>
>>> Ok, sounds good.
>>>>
>>>> How are you testing these, by the way?  On second thoughts, I think I
>>>> may have already asked you that, and forgotten the answer, so don't
>>>> bother answering here--just add some text to the changelog and then I'll
>>>> know how to find the answer next time.
>>>
>>> I found this as I was going through my fault injection code.  I'm going to double check these with my fault injection changes and then try to send in updates for both next week.
>>
>> Sounds good, thanks Bryan.--b.
> 
> Are those still on the way?  (Or did you send and I missed them?  If so,
> if you wouldn't mind resending....)

I sent them out last Friday.  I'll resend.

- Bryan
> 
> --b.

Re: [PATCH 2/2] NFSD: Added TEST_STATEID operation

[J. Bruce Fields]

On Tue, Jul 12, 2011 at 08:40:11AM -0400, Bryan Schumaker wrote:
> On 07/12/2011 06:59 AM, J. Bruce Fields wrote:
> > On Wed, Jun 29, 2011 at 04:40:59PM -0400, J. Bruce Fields wrote:
> >> On Wed, Jun 29, 2011 at 04:34:38PM -0400, Bryan Schumaker wrote:
> >>> On 06/29/2011 03:54 PM, J. Bruce Fields wrote:
> >>>> On Wed, Jun 29, 2011 at 03:49:10PM -0400, Bryan Schumaker wrote:
> >>>>> On 05/20/2011 04:13 PM, bjschuma@netapp.com wrote:
> >>>>>> +__be32
> >>>>>> +nfsd4_do_test_stateid(stateid_t *stateid, int flags)
> >>>>>> +{
> >>>>>> +	__be32 ret = nfs4_validate_stateid(stateid, flags);
> >>>>>> +	if (!ret)
> >>>>>> +		ret = nfs4_validate_stateid(stateid, flags);
> >>>>>> +	return ret;
> >>>>>
> >>>>> Looking back at this, I'm not convinced that I need to call nfs4_validate_stateid() twice with the same stateid and flags.  What is the status of these patches?  Should I resubmit everything or send in a small patch to fix this?
> >>>>
> >>>> Whoops, lazy review on my part....
> >>>>
> >>>> Looks like I'd applied that locally but not pushed it out to my public
> >>>> tree yet, so I could still take a replacement.
> >>>
> >>> Ok, sounds good.
> >>>>
> >>>> How are you testing these, by the way?  On second thoughts, I think I
> >>>> may have already asked you that, and forgotten the answer, so don't
> >>>> bother answering here--just add some text to the changelog and then I'll
> >>>> know how to find the answer next time.
> >>>
> >>> I found this as I was going through my fault injection code.  I'm going to double check these with my fault injection changes and then try to send in updates for both next week.
> >>
> >> Sounds good, thanks Bryan.--b.
> > 
> > Are those still on the way?  (Or did you send and I missed them?  If so,
> > if you wouldn't mind resending....)
> 
> I sent them out last Friday.  I'll resend.

Whoops, sorry about that--thanks for resending.--b.

Re: [PATCH v2 0/2] NFSD: add FREE_STATEID and TEST_STATEID operations

[Bryan Schumaker]

The subject should read "v3" here.  Sorry about that.

- Bryan

On 05/20/2011 04:12 PM, bjschuma@netapp.com wrote:
> From: Bryan Schumaker <bjschuma@netapp.com>
> 
> These patches add FREE_STATEID and TEST_STATEID to the NFS server.  I
> appreciate any comments, especially if there is a better way of doing things.
> 
> v2:
> - FREE_STATEID
>   - Free open, lock, and delegation state ids correctly
>   - Use the check_for_locks() function to check for locks
> 
> - TEST_STATEID
>   - Remove openmode check
>   - Sanity check on size of stateid list
>   - Delay decoding stateid list until encoding the reply
> 
> v3:
> - FREE_STATEID
>   - Open stateids and delegations are never freeable (return nfserr_locks_held)
> 
> - TEST_STATEID
>   - Remove unused arguments from a function call
>   - Store and use the HAS_SESSION flag for validating stateids
> 
> Thanks!
> 
> - Bryan