Skip to contentSkip to conversation

nfs41: implement DESTROY_CLIENTID operation

PATCH Superseded Mi Jinlong 2 participants 5 messages Oct 20, 2011

Subsystems (2)
lore
Share & tools
lore

Mi Jinlong

author
#0 lore raw

PATCH 1/1 nfs41: implement DESTROY_CLIENTID operation

According to rfc5661 18.50, implement DESTROY_CLIENTID operation.

---
 fs/nfsd/nfs4proc.c  |    2 +-
 fs/nfsd/nfs4state.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   |   12 +++++++++++-
 fs/nfsd/xdr4.h      |    5 +++++
 4 files changed, 68 insertions(+), 2 deletions(-)
fs/nfsd/nfs4proc.c +1 -1
index e807776..8412a22 100644
@@ -1433,7 +1433,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_name = "OP_SEQUENCE",
 	},
 	[OP_DESTROY_CLIENTID] = {
-		.op_func = NULL,
+		.op_func = (nfsd4op_func)nfsd4_destroy_clientid,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP,
 		.op_name = "OP_DESTROY_CLIENTID",
 	},
fs/nfsd/nfs4state.c +51 -0

57 lines not on this page · raw message

fs/nfsd/nfs4xdr.c +11 -1
index c8bf405..5d9fff5 100644
@@ -1326,6 +1326,16 @@ xdr_error:
 	goto out;
 }
 
+static __be32 nfsd4_decode_destroy_clientid(struct nfsd4_compoundargs *argp, struct nfsd4_destroy_clientid *dc)
+{
+	DECODE_HEAD;
+
+	READ_BUF(8);
+	COPYMEM(&dc->clientid, 8);
+
+	DECODE_TAIL;
+}
+
 static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, struct nfsd4_reclaim_complete *rc)
 {
 	DECODE_HEAD;
@@ -1447,7 +1457,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
 	[OP_SET_SSV]		= (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_DESTROY_CLIENTID]	= (nfsd4_dec)nfsd4_decode_destroy_clientid,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
 };
 
fs/nfsd/xdr4.h +5 -0
index d2a8d044..d67d680 100644
@@ -405,6 +405,10 @@ struct nfsd4_destroy_session {
 	struct nfs4_sessionid	sessionid;
 };
 
+struct nfsd4_destroy_clientid {
+	clientid_t clientid;
+};
+
 struct nfsd4_reclaim_complete {
 	u32 rca_one_fs;
 };
@@ -558,6 +562,7 @@ extern __be32 nfsd4_sequence(struct svc_rqst *,
 extern __be32 nfsd4_destroy_session(struct svc_rqst *,
 		struct nfsd4_compound_state *,
 		struct nfsd4_destroy_session *);
+extern __be32 nfsd4_destroy_clientid(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_destroy_clientid *);
 __be32 nfsd4_reclaim_complete(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_reclaim_complete *);
 extern __be32 nfsd4_process_open1(struct nfsd4_compound_state *,
 		struct nfsd4_open *open);
Signature · 1 line
1.7.6
#1 lore raw
Quoted 28 lines · Mi Jinlong

According to rfc5661 18.50, implement DESTROY_CLIENTID operation.

---
 fs/nfsd/nfs4proc.c  |    2 +-
 fs/nfsd/nfs4state.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   |   12 +++++++++++-
 fs/nfsd/xdr4.h      |    5 +++++
 4 files changed, 68 insertions(+), 2 deletions(-)
fs/nfsd/nfs4proc.c +1 -1
index e807776..8412a22 100644
@@ -1433,7 +1433,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_name = "OP_SEQUENCE",
 	},
 	[OP_DESTROY_CLIENTID] = {
-		.op_func = NULL,
+		.op_func = (nfsd4op_func)nfsd4_destroy_clientid,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP,
 		.op_name = "OP_DESTROY_CLIENTID",
 	},
fs/nfsd/nfs4state.c +0 -0
index 3787ec1..d46cbbe 100644

fs/nfsd/nfs4state.c out: · line 1866

Quoted 9 lines
fs/nfsd/nfs4state.c +6 -0
@@ -1858,6 +1858,57 @@ out:
 	return status;
 }
 
+static inline bool has_resources(struct nfs4_client *clp)
+{
+	return !list_empty(&clp->cl_openowners)
+		|| !list_empty(&clp->cl_delegations)
+		|| !list_empty(&clp->cl_sessions)
+		|| !list_empty(&clp->cl_callbacks);

I don't think the existance of callbacks should prevent destroying the client.

Quoted 17 lines
fs/nfsd/nfs4state.c +17 -0
+}
+
+__be32
+nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_destroy_clientid *dc)
+{
+	struct nfs4_client *conf, *unconf, *clp;
+	int status = 0;
+
+	nfs4_lock_state();
+	unconf = find_unconfirmed_client(&dc->clientid);
+	conf = find_confirmed_client(&dc->clientid);
+
+	if (conf) {
+		clp = conf;
+
+		if (!is_client_expired(conf) && has_resources(conf)) {
+			status = nfserr_clientid_busy;

fs/nfsd/nfs4state.c out: · line 1893

Quoted 10 lines
fs/nfsd/nfs4state.c +10 -0
+			goto out;
+		}
+
+		/*
+		 * DESTROY_CLIENTID MAY be preceded with a SEQUENCE operation as
+		 * long as the client ID derived from the session ID of SEQUENCE
+		 * is not the same as the client ID to be destroyed. 
+		 * If the client IDs are the same, then the server MUST return
+		 * NFS4ERR_CLIENTID_BUSY.
+		 */

Let's just replace that comment by:

		/* rfc 551 18.50.3: */

(I agree that it's useful to have a reference to explain where this requirement came from, but I don't think it's necessary to write out the whole thing, as the behavior is obvious enough from the following code.)

Looks fine to me otherwise, thanks!

--b.

Quoted 78 lines
fs/nfsd/nfs4state.c +18 -0
+		if (cstate->session && conf == cstate->session->se_client) {
+			status = nfserr_clientid_busy;
+			goto out;
+		}
+	} else if (unconf)
+		clp = unconf;
+	else {
+		status = nfserr_stale_clientid;
+		goto out;
+	}
+
+	expire_client(clp);
+out:
+	nfs4_unlock_state();
+	dprintk("%s return %d\n", __func__, ntohl(status));
+	return status;
+}
+
 __be32
 nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_reclaim_complete *rc)
 {
fs/nfsd/nfs4xdr.c +11 -1
index c8bf405..5d9fff5 100644
@@ -1326,6 +1326,16 @@ xdr_error:
 	goto out;
 }
 
+static __be32 nfsd4_decode_destroy_clientid(struct nfsd4_compoundargs *argp, struct nfsd4_destroy_clientid *dc)
+{
+	DECODE_HEAD;
+
+	READ_BUF(8);
+	COPYMEM(&dc->clientid, 8);
+
+	DECODE_TAIL;
+}
+
 static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, struct nfsd4_reclaim_complete *rc)
 {
 	DECODE_HEAD;
@@ -1447,7 +1457,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
 	[OP_SET_SSV]		= (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_DESTROY_CLIENTID]	= (nfsd4_dec)nfsd4_decode_destroy_clientid,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
 };
 
fs/nfsd/xdr4.h +5 -0
index d2a8d044..d67d680 100644
@@ -405,6 +405,10 @@ struct nfsd4_destroy_session {
 	struct nfs4_sessionid	sessionid;
 };
 
+struct nfsd4_destroy_clientid {
+	clientid_t clientid;
+};
+
 struct nfsd4_reclaim_complete {
 	u32 rca_one_fs;
 };
@@ -558,6 +562,7 @@ extern __be32 nfsd4_sequence(struct svc_rqst *,
 extern __be32 nfsd4_destroy_session(struct svc_rqst *,
 		struct nfsd4_compound_state *,
 		struct nfsd4_destroy_session *);
+extern __be32 nfsd4_destroy_clientid(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_destroy_clientid *);
 __be32 nfsd4_reclaim_complete(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_reclaim_complete *);
 extern __be32 nfsd4_process_open1(struct nfsd4_compound_state *,
 		struct nfsd4_open *open);
Signature · 1 line
1.7.6
author
#2 lore raw

J. Bruce Fields:

Quoted 32 lines · Mi Jinlong
Quoted 31 lines

According to rfc5661 18.50, implement DESTROY_CLIENTID operation.

---
 fs/nfsd/nfs4proc.c  |    2 +-
 fs/nfsd/nfs4state.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   |   12 +++++++++++-
 fs/nfsd/xdr4.h      |    5 +++++
 4 files changed, 68 insertions(+), 2 deletions(-)
fs/nfsd/nfs4proc.c +1 -1
index e807776..8412a22 100644
@@ -1433,7 +1433,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_name = "OP_SEQUENCE",
 	},
 	[OP_DESTROY_CLIENTID] = {
-		.op_func = NULL,
+		.op_func = (nfsd4op_func)nfsd4_destroy_clientid,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP,
 		.op_name = "OP_DESTROY_CLIENTID",
 	},
fs/nfsd/nfs4state.c +0 -0
index 3787ec1..d46cbbe 100644
@@ -1858,6 +1858,57 @@ out:
 	return status;
 }
 
Quoted 9 lines

fs/nfsd/nfs4state.c out: · line 1866

Quoted 6 lines
fs/nfsd/nfs4state.c +6 -0
+static inline bool has_resources(struct nfs4_client *clp)
+{
+	return !list_empty(&clp->cl_openowners)
+		|| !list_empty(&clp->cl_delegations)
+		|| !list_empty(&clp->cl_sessions)
+		|| !list_empty(&clp->cl_callbacks);

I don't think the existance of callbacks should prevent destroying the client.

It's my fault.

Quoted 31 lines
Quoted 18 lines
fs/nfsd/nfs4state.c +17 -0
+}
+
+__be32
+nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_destroy_clientid *dc)
+{
+	struct nfs4_client *conf, *unconf, *clp;
+	int status = 0;
+
+	nfs4_lock_state();
+	unconf = find_unconfirmed_client(&dc->clientid);
+	conf = find_confirmed_client(&dc->clientid);
+
+	if (conf) {
+		clp = conf;
+
+		if (!is_client_expired(conf) && has_resources(conf)) {
+			status = nfserr_clientid_busy;

fs/nfsd/nfs4state.c out: · line 1893

Quoted 10 lines
fs/nfsd/nfs4state.c +10 -0
+			goto out;
+		}
+
+		/*
+		 * DESTROY_CLIENTID MAY be preceded with a SEQUENCE operation as
+		 * long as the client ID derived from the session ID of SEQUENCE
+		 * is not the same as the client ID to be destroyed. 
+		 * If the client IDs are the same, then the server MUST return
+		 * NFS4ERR_CLIENTID_BUSY.
+		 */

Let's just replace that comment by:

Quoted 5 lines
		/* rfc 551 18.50.3: */

(I agree that it's useful to have a reference to explain where this requirement came from, but I don't think it's necessary to write out the whole thing, as the behavior is obvious enough from the following code.)

Yes.

A new version will be post.

thanks,
Mi Jinlong

author
#3 lore raw

PATCH v2 1/1 nfs41: implement DESTROY_CLIENTID operation

According to rfc5661 18.50, implement DESTROY_CLIENTID operation.

---
 fs/nfsd/nfs4proc.c  |    2 +-
 fs/nfsd/nfs4state.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   |   12 +++++++++++-
 fs/nfsd/xdr4.h      |    5 +++++
 4 files changed, 61 insertions(+), 2 deletions(-)
fs/nfsd/nfs4proc.c +1 -1
index e807776..8412a22 100644
@@ -1433,7 +1433,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_name = "OP_SEQUENCE",
 	},
 	[OP_DESTROY_CLIENTID] = {
-		.op_func = NULL,
+		.op_func = (nfsd4op_func)nfsd4_destroy_clientid,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP,
 		.op_name = "OP_DESTROY_CLIENTID",
 	},
fs/nfsd/nfs4state.c +44 -0

50 lines not on this page · raw message

fs/nfsd/nfs4xdr.c +11 -1
index c8bf405..5d9fff5 100644
@@ -1326,6 +1326,16 @@ xdr_error:
 	goto out;
 }
 
+static __be32 nfsd4_decode_destroy_clientid(struct nfsd4_compoundargs *argp, struct nfsd4_destroy_clientid *dc)
+{
+	DECODE_HEAD;
+
+	READ_BUF(8);
+	COPYMEM(&dc->clientid, 8);
+
+	DECODE_TAIL;
+}
+
 static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, struct nfsd4_reclaim_complete *rc)
 {
 	DECODE_HEAD;
@@ -1447,7 +1457,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
 	[OP_SET_SSV]		= (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_DESTROY_CLIENTID]	= (nfsd4_dec)nfsd4_decode_destroy_clientid,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
 };
 
fs/nfsd/xdr4.h +5 -0
index d2a8d044..d67d680 100644
@@ -405,6 +405,10 @@ struct nfsd4_destroy_session {
 	struct nfs4_sessionid	sessionid;
 };
 
+struct nfsd4_destroy_clientid {
+	clientid_t clientid;
+};
+
 struct nfsd4_reclaim_complete {
 	u32 rca_one_fs;
 };
@@ -558,6 +562,7 @@ extern __be32 nfsd4_sequence(struct svc_rqst *,
 extern __be32 nfsd4_destroy_session(struct svc_rqst *,
 		struct nfsd4_compound_state *,
 		struct nfsd4_destroy_session *);
+extern __be32 nfsd4_destroy_clientid(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_destroy_clientid *);
 __be32 nfsd4_reclaim_complete(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_reclaim_complete *);
 extern __be32 nfsd4_process_open1(struct nfsd4_compound_state *,
 		struct nfsd4_open *open);
Signature · 1 line
1.7.6
#4 lore raw
Quoted 1 line · Mi Jinlong

According to rfc5661 18.50, implement DESTROY_CLIENTID operation.

This looks good, thanks, queueing up for 3.2.--b.

Quoted 135 lines
---
 fs/nfsd/nfs4proc.c  |    2 +-
 fs/nfsd/nfs4state.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   |   12 +++++++++++-
 fs/nfsd/xdr4.h      |    5 +++++
 4 files changed, 61 insertions(+), 2 deletions(-)
fs/nfsd/nfs4proc.c +1 -1
index e807776..8412a22 100644
@@ -1433,7 +1433,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_name = "OP_SEQUENCE",
 	},
 	[OP_DESTROY_CLIENTID] = {
-		.op_func = NULL,
+		.op_func = (nfsd4op_func)nfsd4_destroy_clientid,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP,
 		.op_name = "OP_DESTROY_CLIENTID",
 	},
fs/nfsd/nfs4state.c +44 -0
index 3787ec1..d94ad19 100644
@@ -1858,6 +1858,50 @@ out:
 	return status;
 }
 
+static inline bool has_resources(struct nfs4_client *clp)
+{
+	return !list_empty(&clp->cl_openowners)
+		|| !list_empty(&clp->cl_delegations)
+		|| !list_empty(&clp->cl_sessions);
+}
+
+__be32
+nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_destroy_clientid *dc)
+{
+	struct nfs4_client *conf, *unconf, *clp;
+	int status = 0;
+
+	nfs4_lock_state();
+	unconf = find_unconfirmed_client(&dc->clientid);
+	conf = find_confirmed_client(&dc->clientid);
+
+	if (conf) {
+		clp = conf;
+
+		if (!is_client_expired(conf) && has_resources(conf)) {
+			status = nfserr_clientid_busy;
+			goto out;
+		}
+
+		/* rfc5661 18.50.3 */
+		if (cstate->session && conf == cstate->session->se_client) {
+			status = nfserr_clientid_busy;
+			goto out;
+		}
+	} else if (unconf)
+		clp = unconf;
+	else {
+		status = nfserr_stale_clientid;
+		goto out;
+	}
+
+	expire_client(clp);
+out:
+	nfs4_unlock_state();
+	dprintk("%s return %d\n", __func__, ntohl(status));
+	return status;
+}
+
 __be32
 nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_reclaim_complete *rc)
 {
fs/nfsd/nfs4xdr.c +11 -1
index c8bf405..5d9fff5 100644
@@ -1326,6 +1326,16 @@ xdr_error:
 	goto out;
 }
 
+static __be32 nfsd4_decode_destroy_clientid(struct nfsd4_compoundargs *argp, struct nfsd4_destroy_clientid *dc)
+{
+	DECODE_HEAD;
+
+	READ_BUF(8);
+	COPYMEM(&dc->clientid, 8);
+
+	DECODE_TAIL;
+}
+
 static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, struct nfsd4_reclaim_complete *rc)
 {
 	DECODE_HEAD;
@@ -1447,7 +1457,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
 	[OP_SET_SSV]		= (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_DESTROY_CLIENTID]	= (nfsd4_dec)nfsd4_decode_destroy_clientid,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
 };
 
fs/nfsd/xdr4.h +5 -0
index d2a8d044..d67d680 100644
@@ -405,6 +405,10 @@ struct nfsd4_destroy_session {
 	struct nfs4_sessionid	sessionid;
 };
 
+struct nfsd4_destroy_clientid {
+	clientid_t clientid;
+};
+
 struct nfsd4_reclaim_complete {
 	u32 rca_one_fs;
 };
@@ -558,6 +562,7 @@ extern __be32 nfsd4_sequence(struct svc_rqst *,
 extern __be32 nfsd4_destroy_session(struct svc_rqst *,
 		struct nfsd4_compound_state *,
 		struct nfsd4_destroy_session *);
+extern __be32 nfsd4_destroy_clientid(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_destroy_clientid *);
 __be32 nfsd4_reclaim_complete(struct svc_rqst *, struct nfsd4_compound_state *, struct nfsd4_reclaim_complete *);
 extern __be32 nfsd4_process_open1(struct nfsd4_compound_state *,
 		struct nfsd4_open *open);
Signature · 1 line
1.7.6