minor fix for 2.6.38

minor fix for 2.6.38

[J. Bruce Fields]

I'm queuing up the following small fixes for 2.6.38 absent any
objections.--b.

[PATCH 1/3] nfsd4: replace unintuitive match_clientid_establishment

[J. Bruce Fields]

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index afa7525..febb283 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1135,19 +1135,13 @@ find_unconfirmed_client(clientid_t *clid)
 }
 
 /*
- * Return 1 iff clp's clientid establishment method matches the use_exchange_id
- * parameter. Matching is based on the fact the at least one of the
- * EXCHGID4_FLAG_USE_{NON_PNFS,PNFS_MDS,PNFS_DS} flags must be set for v4.1
- *
  * FIXME: we need to unify the clientid namespaces for nfsv4.x
  * and correctly deal with client upgrade/downgrade in EXCHANGE_ID
  * and SET_CLIENTID{,_CONFIRM}
  */
-static inline int
-match_clientid_establishment(struct nfs4_client *clp, bool use_exchange_id)
+static bool clp_used_exchangeid(struct nfs4_client *clp)
 {
-	bool has_exchange_flags = (clp->cl_exchange_flags != 0);
-	return use_exchange_id == has_exchange_flags;
+	return clp->cl_exchange_flags != 0;
 }
 
 static struct nfs4_client *
@@ -1158,7 +1152,7 @@ find_confirmed_client_by_str(const char *dname, unsigned int hashval,
 
 	list_for_each_entry(clp, &conf_str_hashtbl[hashval], cl_strhash) {
 		if (same_name(clp->cl_recdir, dname) &&
-		    match_clientid_establishment(clp, use_exchange_id))
+		    clp_used_exchangeid(clp) == use_exchange_id)
 			return clp;
 	}
 	return NULL;
@@ -1172,7 +1166,7 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval,
 
 	list_for_each_entry(clp, &unconf_str_hashtbl[hashval], cl_strhash) {
 		if (same_name(clp->cl_recdir, dname) &&
-		    match_clientid_establishment(clp, use_exchange_id))
+		    clp_used_exchangeid(clp) == use_exchange_id)
 			return clp;
 	}
 	return NULL;
-- 
1.7.1

[PATCH 2/3] nfsd4: fix mixed 4.0/4.1 handling, 4.1 reboot

[J. Bruce Fields]

Instead of failing to find client entries which don't match the
minorversion, we should be finding them, then either erroring out or
expiring them as appropriate.

This also fixes a problem which would cause the 4.1 server to fail to
recognize clients after a second reboot.

Reported-by: Casey Bodley <cbodley@citi.umich.edu>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4recover.c |    1 -
 fs/nfsd/nfs4state.c   |   37 +++++++++++++++++--------------------
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 7e26caa..ffb59ef 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -302,7 +302,6 @@ purge_old(struct dentry *parent, struct dentry *child)
 {
 	int status;
 
-	/* note: we currently use this path only for minorversion 0 */
 	if (nfs4_has_reclaimed_state(child->d_name.name, false))
 		return 0;
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index febb283..d1e37ba 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1134,39 +1134,30 @@ find_unconfirmed_client(clientid_t *clid)
 	return NULL;
 }
 
-/*
- * FIXME: we need to unify the clientid namespaces for nfsv4.x
- * and correctly deal with client upgrade/downgrade in EXCHANGE_ID
- * and SET_CLIENTID{,_CONFIRM}
- */
 static bool clp_used_exchangeid(struct nfs4_client *clp)
 {
 	return clp->cl_exchange_flags != 0;
-}
+} 
 
 static struct nfs4_client *
-find_confirmed_client_by_str(const char *dname, unsigned int hashval,
-			     bool use_exchange_id)
+find_confirmed_client_by_str(const char *dname, unsigned int hashval)
 {
 	struct nfs4_client *clp;
 
 	list_for_each_entry(clp, &conf_str_hashtbl[hashval], cl_strhash) {
-		if (same_name(clp->cl_recdir, dname) &&
-		    clp_used_exchangeid(clp) == use_exchange_id)
+		if (same_name(clp->cl_recdir, dname))
 			return clp;
 	}
 	return NULL;
 }
 
 static struct nfs4_client *
-find_unconfirmed_client_by_str(const char *dname, unsigned int hashval,
-			       bool use_exchange_id)
+find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 {
 	struct nfs4_client *clp;
 
 	list_for_each_entry(clp, &unconf_str_hashtbl[hashval], cl_strhash) {
-		if (same_name(clp->cl_recdir, dname) &&
-		    clp_used_exchangeid(clp) == use_exchange_id)
+		if (same_name(clp->cl_recdir, dname))
 			return clp;
 	}
 	return NULL;
@@ -1357,8 +1348,12 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 	nfs4_lock_state();
 	status = nfs_ok;
 
-	conf = find_confirmed_client_by_str(dname, strhashval, true);
+	conf = find_confirmed_client_by_str(dname, strhashval);
 	if (conf) {
+		if (!clp_used_exchangeid(conf)) {
+			status = nfserr_clid_inuse; /* XXX: ? */
+			goto out;
+		}
 		if (!same_verf(&verf, &conf->cl_verifier)) {
 			/* 18.35.4 case 8 */
 			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
@@ -1399,7 +1394,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 		goto out;
 	}
 
-	unconf  = find_unconfirmed_client_by_str(dname, strhashval, true);
+	unconf  = find_unconfirmed_client_by_str(dname, strhashval);
 	if (unconf) {
 		/*
 		 * Possible retry or client restart.  Per 18.35.4 case 4,
@@ -1799,10 +1794,12 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	strhashval = clientstr_hashval(dname);
 
 	nfs4_lock_state();
-	conf = find_confirmed_client_by_str(dname, strhashval, false);
+	conf = find_confirmed_client_by_str(dname, strhashval);
 	if (conf) {
 		/* RFC 3530 14.2.33 CASE 0: */
 		status = nfserr_clid_inuse;
+		if (clp_used_exchangeid(conf))
+			goto out;
 		if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
 			char addr_str[INET6_ADDRSTRLEN];
 			rpc_ntop((struct sockaddr *) &conf->cl_addr, addr_str,
@@ -1817,7 +1814,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * has a description of SETCLIENTID request processing consisting
 	 * of 5 bullet points, labeled as CASE0 - CASE4 below.
 	 */
-	unconf = find_unconfirmed_client_by_str(dname, strhashval, false);
+	unconf = find_unconfirmed_client_by_str(dname, strhashval);
 	status = nfserr_resource;
 	if (!conf) {
 		/*
@@ -1962,7 +1959,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
 			unsigned int hash =
 				clientstr_hashval(unconf->cl_recdir);
 			conf = find_confirmed_client_by_str(unconf->cl_recdir,
-							    hash, false);
+							    hash);
 			if (conf) {
 				nfsd4_remove_clid_dir(conf);
 				expire_client(conf);
@@ -4106,7 +4103,7 @@ nfs4_has_reclaimed_state(const char *name, bool use_exchange_id)
 	unsigned int strhashval = clientstr_hashval(name);
 	struct nfs4_client *clp;
 
-	clp = find_confirmed_client_by_str(name, strhashval, use_exchange_id);
+	clp = find_confirmed_client_by_str(name, strhashval);
 	return clp ? 1 : 0;
 }
 
-- 
1.7.1

[PATCH 3/3] nfsd: fix offset printk's in nfsd3 read/write

[J. Bruce Fields]

Thanks to dysbr01@ca.com for noticing that the debugging printk in
the v3 write procedure can print >2GB offsets as negative numbers:
	https://bugzilla.kernel.org/show_bug.cgi?id=23342

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs3proc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 5b7e302..2247fc9 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -151,10 +151,10 @@ nfsd3_proc_read(struct svc_rqst *rqstp, struct nfsd3_readargs *argp,
 	__be32	nfserr;
 	u32	max_blocksize = svc_max_payload(rqstp);
 
-	dprintk("nfsd: READ(3) %s %lu bytes at %lu\n",
+	dprintk("nfsd: READ(3) %s %lu bytes at %Lu\n",
 				SVCFH_fmt(&argp->fh),
 				(unsigned long) argp->count,
-				(unsigned long) argp->offset);
+				(unsigned long long) argp->offset);
 
 	/* Obtain buffer pointer for payload.
 	 * 1 (status) + 22 (post_op_attr) + 1 (count) + 1 (eof)
@@ -191,10 +191,10 @@ nfsd3_proc_write(struct svc_rqst *rqstp, struct nfsd3_writeargs *argp,
 	__be32	nfserr;
 	unsigned long cnt = argp->len;
 
-	dprintk("nfsd: WRITE(3)    %s %d bytes at %ld%s\n",
+	dprintk("nfsd: WRITE(3)    %s %d bytes at %Lu%s\n",
 				SVCFH_fmt(&argp->fh),
 				argp->len,
-				(unsigned long) argp->offset,
+				(unsigned long long) argp->offset,
 				argp->stable? " stable" : "");
 
 	fh_copy(&resp->fh, &argp->fh);
-- 
1.7.1

Re: [PATCH 3/3] nfsd: fix offset printk's in nfsd3 read/write

[Jim Rees]

J. Bruce Fields wrote:

  Thanks to dysbr01@ca.com for noticing that the debugging printk in
  the v3 write procedure can print >2GB offsets as negative numbers:
  	https://bugzilla.kernel.org/show_bug.cgi?id=23342
  
  Signed-off-by: J. Bruce Fields <bfields@redhat.com>
  ---
   fs/nfsd/nfs3proc.c |    8 ++++----
   1 files changed, 4 insertions(+), 4 deletions(-)
  
  diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
  index 5b7e302..2247fc9 100644
  --- a/fs/nfsd/nfs3proc.c
  +++ b/fs/nfsd/nfs3proc.c
  @@ -151,10 +151,10 @@ nfsd3_proc_read(struct svc_rqst *rqstp, struct nfsd3_readargs *argp,
   	__be32	nfserr;
   	u32	max_blocksize = svc_max_payload(rqstp);
   
  -	dprintk("nfsd: READ(3) %s %lu bytes at %lu\n",
  +	dprintk("nfsd: READ(3) %s %lu bytes at %Lu\n",

Isn't "llu" preferred over "Lu" these days?

Re: [PATCH 3/3] nfsd: fix offset printk's in nfsd3 read/write

[J. Bruce Fields]

On Mon, Dec 06, 2010 at 07:31:04PM -0500, Jim Rees wrote:
> J. Bruce Fields wrote:
> 
>   Thanks to dysbr01@ca.com for noticing that the debugging printk in
>   the v3 write procedure can print >2GB offsets as negative numbers:
>   	https://bugzilla.kernel.org/show_bug.cgi?id=23342
>   
>   Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>   ---
>    fs/nfsd/nfs3proc.c |    8 ++++----
>    1 files changed, 4 insertions(+), 4 deletions(-)
>   
>   diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
>   index 5b7e302..2247fc9 100644
>   --- a/fs/nfsd/nfs3proc.c
>   +++ b/fs/nfsd/nfs3proc.c
>   @@ -151,10 +151,10 @@ nfsd3_proc_read(struct svc_rqst *rqstp, struct nfsd3_readargs *argp,
>    	__be32	nfserr;
>    	u32	max_blocksize = svc_max_payload(rqstp);
>    
>   -	dprintk("nfsd: READ(3) %s %lu bytes at %lu\n",
>   +	dprintk("nfsd: READ(3) %s %lu bytes at %Lu\n",
> 
> Isn't "llu" preferred over "Lu" these days?

Uh, OK, reading sprintf(3), "L" seems to only be meaningful for floating
point, and "ll" a long long.

Looking at the kernel's printk(), "L" seems to be a synomym for "ll".

Both seem to be used all over.

I'm leaving it as is out of laziness and consistency with the one
preexisting use in this file, unless someone wants to argue otherwise.

--b.

Re: [PATCH 1/3] nfsd4: replace unintuitive match_clientid_establishment

[Benny Halevy]

On 2010-12-07 02:09, J. Bruce Fields wrote:
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>

ACK

> ---
>  fs/nfsd/nfs4state.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index afa7525..febb283 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1135,19 +1135,13 @@ find_unconfirmed_client(clientid_t *clid)
>  }
>  
>  /*
> - * Return 1 iff clp's clientid establishment method matches the use_exchange_id
> - * parameter. Matching is based on the fact the at least one of the
> - * EXCHGID4_FLAG_USE_{NON_PNFS,PNFS_MDS,PNFS_DS} flags must be set for v4.1
> - *
>   * FIXME: we need to unify the clientid namespaces for nfsv4.x
>   * and correctly deal with client upgrade/downgrade in EXCHANGE_ID
>   * and SET_CLIENTID{,_CONFIRM}
>   */
> -static inline int
> -match_clientid_establishment(struct nfs4_client *clp, bool use_exchange_id)
> +static bool clp_used_exchangeid(struct nfs4_client *clp)
>  {
> -	bool has_exchange_flags = (clp->cl_exchange_flags != 0);
> -	return use_exchange_id == has_exchange_flags;
> +	return clp->cl_exchange_flags != 0;
>  }
>  
>  static struct nfs4_client *
> @@ -1158,7 +1152,7 @@ find_confirmed_client_by_str(const char *dname, unsigned int hashval,
>  
>  	list_for_each_entry(clp, &conf_str_hashtbl[hashval], cl_strhash) {
>  		if (same_name(clp->cl_recdir, dname) &&
> -		    match_clientid_establishment(clp, use_exchange_id))
> +		    clp_used_exchangeid(clp) == use_exchange_id)
>  			return clp;
>  	}
>  	return NULL;
> @@ -1172,7 +1166,7 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval,
>  
>  	list_for_each_entry(clp, &unconf_str_hashtbl[hashval], cl_strhash) {
>  		if (same_name(clp->cl_recdir, dname) &&
> -		    match_clientid_establishment(clp, use_exchange_id))
> +		    clp_used_exchangeid(clp) == use_exchange_id)
>  			return clp;
>  	}
>  	return NULL;

Re: [PATCH 2/3] nfsd4: fix mixed 4.0/4.1 handling, 4.1 reboot

[Benny Halevy]

On 2010-12-07 02:09, J. Bruce Fields wrote:
> Instead of failing to find client entries which don't match the
> minorversion, we should be finding them, then either erroring out or
> expiring them as appropriate.
> 
> This also fixes a problem which would cause the 4.1 server to fail to
> recognize clients after a second reboot.
> 
> Reported-by: Casey Bodley <cbodley@citi.umich.edu>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/nfsd/nfs4recover.c |    1 -
>  fs/nfsd/nfs4state.c   |   37 +++++++++++++++++--------------------
>  2 files changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 7e26caa..ffb59ef 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -302,7 +302,6 @@ purge_old(struct dentry *parent, struct dentry *child)
>  {
>  	int status;
>  
> -	/* note: we currently use this path only for minorversion 0 */
>  	if (nfs4_has_reclaimed_state(child->d_name.name, false))
>  		return 0;
>  
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index febb283..d1e37ba 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1134,39 +1134,30 @@ find_unconfirmed_client(clientid_t *clid)
>  	return NULL;
>  }
>  
> -/*
> - * FIXME: we need to unify the clientid namespaces for nfsv4.x
> - * and correctly deal with client upgrade/downgrade in EXCHANGE_ID
> - * and SET_CLIENTID{,_CONFIRM}
> - */
>  static bool clp_used_exchangeid(struct nfs4_client *clp)
>  {
>  	return clp->cl_exchange_flags != 0;
> -}
> +} 

nit: looks like this line adds a trailing space character...

>  
>  static struct nfs4_client *
> -find_confirmed_client_by_str(const char *dname, unsigned int hashval,
> -			     bool use_exchange_id)
> +find_confirmed_client_by_str(const char *dname, unsigned int hashval)
>  {
>  	struct nfs4_client *clp;
>  
>  	list_for_each_entry(clp, &conf_str_hashtbl[hashval], cl_strhash) {
> -		if (same_name(clp->cl_recdir, dname) &&
> -		    clp_used_exchangeid(clp) == use_exchange_id)
> +		if (same_name(clp->cl_recdir, dname))
>  			return clp;
>  	}
>  	return NULL;
>  }
>  
>  static struct nfs4_client *
> -find_unconfirmed_client_by_str(const char *dname, unsigned int hashval,
> -			       bool use_exchange_id)
> +find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
>  {
>  	struct nfs4_client *clp;
>  
>  	list_for_each_entry(clp, &unconf_str_hashtbl[hashval], cl_strhash) {
> -		if (same_name(clp->cl_recdir, dname) &&
> -		    clp_used_exchangeid(clp) == use_exchange_id)
> +		if (same_name(clp->cl_recdir, dname))
>  			return clp;
>  	}
>  	return NULL;
> @@ -1357,8 +1348,12 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
>  	nfs4_lock_state();
>  	status = nfs_ok;
>  
> -	conf = find_confirmed_client_by_str(dname, strhashval, true);
> +	conf = find_confirmed_client_by_str(dname, strhashval);
>  	if (conf) {
> +		if (!clp_used_exchangeid(conf)) {
> +			status = nfserr_clid_inuse; /* XXX: ? */
> +			goto out;
> +		}

So if a client host wants to mount a server both over v4.0 and v4.1
it should use different client names?
Just wondering, is that required by RFC5661?

Benny

>  		if (!same_verf(&verf, &conf->cl_verifier)) {
>  			/* 18.35.4 case 8 */
>  			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
> @@ -1399,7 +1394,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
>  		goto out;
>  	}
>  
> -	unconf  = find_unconfirmed_client_by_str(dname, strhashval, true);
> +	unconf  = find_unconfirmed_client_by_str(dname, strhashval);
>  	if (unconf) {
>  		/*
>  		 * Possible retry or client restart.  Per 18.35.4 case 4,
> @@ -1799,10 +1794,12 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	strhashval = clientstr_hashval(dname);
>  
>  	nfs4_lock_state();
> -	conf = find_confirmed_client_by_str(dname, strhashval, false);
> +	conf = find_confirmed_client_by_str(dname, strhashval);
>  	if (conf) {
>  		/* RFC 3530 14.2.33 CASE 0: */
>  		status = nfserr_clid_inuse;
> +		if (clp_used_exchangeid(conf))
> +			goto out;
>  		if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
>  			char addr_str[INET6_ADDRSTRLEN];
>  			rpc_ntop((struct sockaddr *) &conf->cl_addr, addr_str,
> @@ -1817,7 +1814,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	 * has a description of SETCLIENTID request processing consisting
>  	 * of 5 bullet points, labeled as CASE0 - CASE4 below.
>  	 */
> -	unconf = find_unconfirmed_client_by_str(dname, strhashval, false);
> +	unconf = find_unconfirmed_client_by_str(dname, strhashval);
>  	status = nfserr_resource;
>  	if (!conf) {
>  		/*
> @@ -1962,7 +1959,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
>  			unsigned int hash =
>  				clientstr_hashval(unconf->cl_recdir);
>  			conf = find_confirmed_client_by_str(unconf->cl_recdir,
> -							    hash, false);
> +							    hash);
>  			if (conf) {
>  				nfsd4_remove_clid_dir(conf);
>  				expire_client(conf);
> @@ -4106,7 +4103,7 @@ nfs4_has_reclaimed_state(const char *name, bool use_exchange_id)
>  	unsigned int strhashval = clientstr_hashval(name);
>  	struct nfs4_client *clp;
>  
> -	clp = find_confirmed_client_by_str(name, strhashval, use_exchange_id);
> +	clp = find_confirmed_client_by_str(name, strhashval);
>  	return clp ? 1 : 0;
>  }
>  

Re: [PATCH 2/3] nfsd4: fix mixed 4.0/4.1 handling, 4.1 reboot

[J. Bruce Fields]

On Thu, Dec 09, 2010 at 03:37:43PM +0200, Benny Halevy wrote:
> On 2010-12-07 02:09, J. Bruce Fields wrote:
> > Instead of failing to find client entries which don't match the
> > minorversion, we should be finding them, then either erroring out or
> > expiring them as appropriate.
> > 
> > This also fixes a problem which would cause the 4.1 server to fail to
> > recognize clients after a second reboot.
> > 
> > Reported-by: Casey Bodley <cbodley@citi.umich.edu>
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  fs/nfsd/nfs4recover.c |    1 -
> >  fs/nfsd/nfs4state.c   |   37 +++++++++++++++++--------------------
> >  2 files changed, 17 insertions(+), 21 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> > index 7e26caa..ffb59ef 100644
> > --- a/fs/nfsd/nfs4recover.c
> > +++ b/fs/nfsd/nfs4recover.c
> > @@ -302,7 +302,6 @@ purge_old(struct dentry *parent, struct dentry *child)
> >  {
> >  	int status;
> >  
> > -	/* note: we currently use this path only for minorversion 0 */
> >  	if (nfs4_has_reclaimed_state(child->d_name.name, false))
> >  		return 0;
> >  
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index febb283..d1e37ba 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1134,39 +1134,30 @@ find_unconfirmed_client(clientid_t *clid)
> >  	return NULL;
> >  }
> >  
> > -/*
> > - * FIXME: we need to unify the clientid namespaces for nfsv4.x
> > - * and correctly deal with client upgrade/downgrade in EXCHANGE_ID
> > - * and SET_CLIENTID{,_CONFIRM}
> > - */
> >  static bool clp_used_exchangeid(struct nfs4_client *clp)
> >  {
> >  	return clp->cl_exchange_flags != 0;
> > -}
> > +} 
> 
> nit: looks like this line adds a trailing space character...

Whoops, thanks.

> >  static struct nfs4_client *
> > -find_confirmed_client_by_str(const char *dname, unsigned int hashval,
> > -			     bool use_exchange_id)
> > +find_confirmed_client_by_str(const char *dname, unsigned int hashval)
> >  {
> >  	struct nfs4_client *clp;
> >  
> >  	list_for_each_entry(clp, &conf_str_hashtbl[hashval], cl_strhash) {
> > -		if (same_name(clp->cl_recdir, dname) &&
> > -		    clp_used_exchangeid(clp) == use_exchange_id)
> > +		if (same_name(clp->cl_recdir, dname))
> >  			return clp;
> >  	}
> >  	return NULL;
> >  }
> >  
> >  static struct nfs4_client *
> > -find_unconfirmed_client_by_str(const char *dname, unsigned int hashval,
> > -			       bool use_exchange_id)
> > +find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
> >  {
> >  	struct nfs4_client *clp;
> >  
> >  	list_for_each_entry(clp, &unconf_str_hashtbl[hashval], cl_strhash) {
> > -		if (same_name(clp->cl_recdir, dname) &&
> > -		    clp_used_exchangeid(clp) == use_exchange_id)
> > +		if (same_name(clp->cl_recdir, dname))
> >  			return clp;
> >  	}
> >  	return NULL;
> > @@ -1357,8 +1348,12 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
> >  	nfs4_lock_state();
> >  	status = nfs_ok;
> >  
> > -	conf = find_confirmed_client_by_str(dname, strhashval, true);
> > +	conf = find_confirmed_client_by_str(dname, strhashval);
> >  	if (conf) {
> > +		if (!clp_used_exchangeid(conf)) {
> > +			status = nfserr_clid_inuse; /* XXX: ? */
> > +			goto out;
> > +		}
> 
> So if a client host wants to mount a server both over v4.0 and v4.1
> it should use different client names?

I believe so.

> Just wondering, is that required by RFC5661?

Section 2.4.1 permits a server to compare the 4.0 nfs_clientid_4 with
the 4.1 client_owner4 in an exchanged_id.  That wouldn't work if a
client could expect the server to treat the two as independent clients.

Thanks for the review!

--b.

> 
> Benny
> 
> >  		if (!same_verf(&verf, &conf->cl_verifier)) {
> >  			/* 18.35.4 case 8 */
> >  			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
> > @@ -1399,7 +1394,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
> >  		goto out;
> >  	}
> >  
> > -	unconf  = find_unconfirmed_client_by_str(dname, strhashval, true);
> > +	unconf  = find_unconfirmed_client_by_str(dname, strhashval);
> >  	if (unconf) {
> >  		/*
> >  		 * Possible retry or client restart.  Per 18.35.4 case 4,
> > @@ -1799,10 +1794,12 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> >  	strhashval = clientstr_hashval(dname);
> >  
> >  	nfs4_lock_state();
> > -	conf = find_confirmed_client_by_str(dname, strhashval, false);
> > +	conf = find_confirmed_client_by_str(dname, strhashval);
> >  	if (conf) {
> >  		/* RFC 3530 14.2.33 CASE 0: */
> >  		status = nfserr_clid_inuse;
> > +		if (clp_used_exchangeid(conf))
> > +			goto out;
> >  		if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
> >  			char addr_str[INET6_ADDRSTRLEN];
> >  			rpc_ntop((struct sockaddr *) &conf->cl_addr, addr_str,
> > @@ -1817,7 +1814,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> >  	 * has a description of SETCLIENTID request processing consisting
> >  	 * of 5 bullet points, labeled as CASE0 - CASE4 below.
> >  	 */
> > -	unconf = find_unconfirmed_client_by_str(dname, strhashval, false);
> > +	unconf = find_unconfirmed_client_by_str(dname, strhashval);
> >  	status = nfserr_resource;
> >  	if (!conf) {
> >  		/*
> > @@ -1962,7 +1959,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
> >  			unsigned int hash =
> >  				clientstr_hashval(unconf->cl_recdir);
> >  			conf = find_confirmed_client_by_str(unconf->cl_recdir,
> > -							    hash, false);
> > +							    hash);
> >  			if (conf) {
> >  				nfsd4_remove_clid_dir(conf);
> >  				expire_client(conf);
> > @@ -4106,7 +4103,7 @@ nfs4_has_reclaimed_state(const char *name, bool use_exchange_id)
> >  	unsigned int strhashval = clientstr_hashval(name);
> >  	struct nfs4_client *clp;
> >  
> > -	clp = find_confirmed_client_by_str(name, strhashval, use_exchange_id);
> > +	clp = find_confirmed_client_by_str(name, strhashval);
> >  	return clp ? 1 : 0;
> >  }
> >