nfsv4 compound status

PATCH In review Benny Halevy 2 participants 24 messages Mar 31, 2008 – Jul 21, 2008 1 message not in the archive

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Benny Halevy

author

2008-03-31 14:35 UTC #0 lore raw

cover nfsv4 compound status

Two patches in this patchset implement what we discussed last month:
http://linux-nfs.org/pipermail/nfsv4/2008-February/007945.html

[PATCH 1/2] nfs: return negative error value from nfs{,4}_stat_to_errno

All use sites for nfs{,4}_stat_to_errno negate their return value. It's more efficient to return a negative error from the stat_to_errno convertors rather than negating its return value everywhere. This also produces slightly smaller code.

[PATCH 2/2] nfs: use compound hdr.status to override op status.

The compound header status must be equivalent to the status of the last operation in the compound results. In certain cases like lack of resources or xdr decoding error, the nfs server may return a non-zero status in the compound header which is not returned by any operation. In this case we would notice that today when looking for the respective operations code in the results and we return -EIO when we cannot find it. This patch fixes that by returning the status available in the comound header instead.

This patch also fixes 3 call sites where we looked at the comound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}

Benny

Benny Halevy

author

2008-03-31 14:39 UTC #1 lore raw

PATCH 1/2 nfs: return negative error value from nfs{,4}_stat_to_errno

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs2xdr.c |   76 +++++++++++++++++++++++++-------------------------
 fs/nfs/nfs3xdr.c |   34 +++++++++++-----------
 fs/nfs/nfs4xdr.c |   80 +++++++++++++++++++++++++++---------------------------
 3 files changed, 95 insertions(+), 95 deletions(-)

fs/nfs/nfs2xdr.c +38 ~~-38~~

index 1f7ea67..9930178 100644
@@ -267,7 +267,7 @@ nfs_xdr_readres(struct rpc_rqst *req, __be32 *p, struct nfs_readres *res)
 	int status;
 
 	if ((status = ntohl(*p++)))
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	p = xdr_decode_fattr(p, res->fattr);
 
 	count = ntohl(*p++);

@@ -432,7 +432,7 @@ nfs_xdr_readdirres(struct rpc_rqst *req, __be32 *p, void *dummy)
 	__be32 *end, *entry, *kaddr;
 
 	if ((status = ntohl(*p++)))
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	hdrlen = (u8 *) p - (u8 *) iov->iov_base;
 	if (iov->iov_len < hdrlen) {

@@ -518,7 +518,7 @@ nfs_xdr_stat(struct rpc_rqst *req, __be32 *p, void *dummy)
 	int	status;
 
 	if ((status = ntohl(*p++)) != 0)
-		status = -nfs_stat_to_errno(status);
+		status = nfs_stat_to_errno(status);
 	return status;
 }

@@ -532,7 +532,7 @@ nfs_xdr_attrstat(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 	int	status;
 
 	if ((status = ntohl(*p++)))
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	xdr_decode_fattr(p, fattr);
 	return 0;
 }

@@ -547,7 +547,7 @@ nfs_xdr_diropres(struct rpc_rqst *req, __be32 *p, struct nfs_diropok *res)
 	int	status;
 
 	if ((status = ntohl(*p++)))
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	p = xdr_decode_fhandle(p, res->fh);
 	xdr_decode_fattr(p, res->fattr);
 	return 0;

@@ -585,7 +585,7 @@ nfs_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, void *dummy)
 	int	status;
 
 	if ((status = ntohl(*p++)))
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	/* Convert length of symlink */
 	len = ntohl(*p++);
 	if (len >= rcvbuf->page_len) {

@@ -634,7 +634,7 @@ nfs_xdr_statfsres(struct rpc_rqst *req, __be32 *p, struct nfs2_fsstat *res)
 	int	status;
 
 	if ((status = ntohl(*p++)))
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	res->tsize  = ntohl(*p++);
 	res->bsize  = ntohl(*p++);

@@ -653,39 +653,39 @@ static struct {
 	int errno;
 } nfs_errtbl[] = {
 	{ NFS_OK,		0		},
-	{ NFSERR_PERM,		EPERM		},
-	{ NFSERR_NOENT,		ENOENT		},
-	{ NFSERR_IO,		errno_NFSERR_IO	},
-	{ NFSERR_NXIO,		ENXIO		},
-/*	{ NFSERR_EAGAIN,	EAGAIN		}, */
-	{ NFSERR_ACCES,		EACCES		},
-	{ NFSERR_EXIST,		EEXIST		},
-	{ NFSERR_XDEV,		EXDEV		},
-	{ NFSERR_NODEV,		ENODEV		},
-	{ NFSERR_NOTDIR,	ENOTDIR		},
-	{ NFSERR_ISDIR,		EISDIR		},
-	{ NFSERR_INVAL,		EINVAL		},
-	{ NFSERR_FBIG,		EFBIG		},
-	{ NFSERR_NOSPC,		ENOSPC		},
-	{ NFSERR_ROFS,		EROFS		},
-	{ NFSERR_MLINK,		EMLINK		},
-	{ NFSERR_NAMETOOLONG,	ENAMETOOLONG	},
-	{ NFSERR_NOTEMPTY,	ENOTEMPTY	},
-	{ NFSERR_DQUOT,		EDQUOT		},
-	{ NFSERR_STALE,		ESTALE		},
-	{ NFSERR_REMOTE,	EREMOTE		},
+	{ NFSERR_PERM,		-EPERM		},
+	{ NFSERR_NOENT,		-ENOENT		},
+	{ NFSERR_IO,		-errno_NFSERR_IO},
+	{ NFSERR_NXIO,		-ENXIO		},
+/*	{ NFSERR_EAGAIN,	-EAGAIN		}, */
+	{ NFSERR_ACCES,		-EACCES		},
+	{ NFSERR_EXIST,		-EEXIST		},
+	{ NFSERR_XDEV,		-EXDEV		},
+	{ NFSERR_NODEV,		-ENODEV		},
+	{ NFSERR_NOTDIR,	-ENOTDIR	},
+	{ NFSERR_ISDIR,		-EISDIR		},
+	{ NFSERR_INVAL,		-EINVAL		},
+	{ NFSERR_FBIG,		-EFBIG		},
+	{ NFSERR_NOSPC,		-ENOSPC		},
+	{ NFSERR_ROFS,		-EROFS		},
+	{ NFSERR_MLINK,		-EMLINK		},
+	{ NFSERR_NAMETOOLONG,	-ENAMETOOLONG	},
+	{ NFSERR_NOTEMPTY,	-ENOTEMPTY	},
+	{ NFSERR_DQUOT,		-EDQUOT		},
+	{ NFSERR_STALE,		-ESTALE		},
+	{ NFSERR_REMOTE,	-EREMOTE	},
 #ifdef EWFLUSH
-	{ NFSERR_WFLUSH,	EWFLUSH		},
+	{ NFSERR_WFLUSH,	-EWFLUSH	},
 #endif
-	{ NFSERR_BADHANDLE,	EBADHANDLE	},
-	{ NFSERR_NOT_SYNC,	ENOTSYNC	},
-	{ NFSERR_BAD_COOKIE,	EBADCOOKIE	},
-	{ NFSERR_NOTSUPP,	ENOTSUPP	},
-	{ NFSERR_TOOSMALL,	ETOOSMALL	},
-	{ NFSERR_SERVERFAULT,	ESERVERFAULT	},
-	{ NFSERR_BADTYPE,	EBADTYPE	},
-	{ NFSERR_JUKEBOX,	EJUKEBOX	},
-	{ -1,			EIO		}
+	{ NFSERR_BADHANDLE,	-EBADHANDLE	},
+	{ NFSERR_NOT_SYNC,	-ENOTSYNC	},
+	{ NFSERR_BAD_COOKIE,	-EBADCOOKIE	},
+	{ NFSERR_NOTSUPP,	-ENOTSUPP	},
+	{ NFSERR_TOOSMALL,	-ETOOSMALL	},
+	{ NFSERR_SERVERFAULT,	-ESERVERFAULT	},
+	{ NFSERR_BADTYPE,	-EBADTYPE	},
+	{ NFSERR_JUKEBOX,	-EJUKEBOX	},
+	{ -1,			-EIO		}
 };
 
 /*

fs/nfs/nfs3xdr.c +17 ~~-17~~

index 3917e2f..feeab62 100644
@@ -515,7 +515,7 @@ nfs3_xdr_readdirres(struct rpc_rqst *req, __be32 *p, struct nfs3_readdirres *res
 	/* Decode post_op_attrs */
 	p = xdr_decode_post_op_attr(p, res->dir_attr);
 	if (status)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	/* Decode verifier cookie */
 	if (res->verf) {
 		res->verf[0] = *p++;

@@ -732,7 +732,7 @@ nfs3_xdr_attrstat(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 	int	status;
 
 	if ((status = ntohl(*p++)))
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	xdr_decode_fattr(p, fattr);
 	return 0;
 }

@@ -747,7 +747,7 @@ nfs3_xdr_wccstat(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 	int	status;
 
 	if ((status = ntohl(*p++)))
-		status = -nfs_stat_to_errno(status);
+		status = nfs_stat_to_errno(status);
 	xdr_decode_wcc_data(p, fattr);
 	return status;
 }

@@ -767,7 +767,7 @@ nfs3_xdr_lookupres(struct rpc_rqst *req, __be32 *p, struct nfs3_diropres *res)
 	int	status;
 
 	if ((status = ntohl(*p++))) {
-		status = -nfs_stat_to_errno(status);
+		status = nfs_stat_to_errno(status);
 	} else {
 		if (!(p = xdr_decode_fhandle(p, res->fh)))
 			return -errno_NFSERR_IO;

@@ -787,7 +787,7 @@ nfs3_xdr_accessres(struct rpc_rqst *req, __be32 *p, struct nfs3_accessres *res)
 
 	p = xdr_decode_post_op_attr(p, res->fattr);
 	if (status)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	res->access = ntohl(*p++);
 	return 0;
 }

@@ -824,7 +824,7 @@ nfs3_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 	p = xdr_decode_post_op_attr(p, fattr);
 
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	/* Convert length of symlink */
 	len = ntohl(*p++);

@@ -872,7 +872,7 @@ nfs3_xdr_readres(struct rpc_rqst *req, __be32 *p, struct nfs_readres *res)
 	p = xdr_decode_post_op_attr(p, res->fattr);
 
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	/* Decode reply count and EOF flag. NFSv3 is somewhat redundant
 	 * in that it puts the count both in the res struct and in the

@@ -922,7 +922,7 @@ nfs3_xdr_writeres(struct rpc_rqst *req, __be32 *p, struct nfs_writeres *res)
 	p = xdr_decode_wcc_data(p, res->fattr);
 
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	res->count = ntohl(*p++);
 	res->verf->committed = (enum nfs3_stable_how)ntohl(*p++);

@@ -953,7 +953,7 @@ nfs3_xdr_createres(struct rpc_rqst *req, __be32 *p, struct nfs3_diropres *res)
 			res->fattr->valid = 0;
 		}
 	} else {
-		status = -nfs_stat_to_errno(status);
+		status = nfs_stat_to_errno(status);
 	}
 	p = xdr_decode_wcc_data(p, res->dir_attr);
 	return status;

@@ -968,7 +968,7 @@ nfs3_xdr_renameres(struct rpc_rqst *req, __be32 *p, struct nfs3_renameres *res)
 	int	status;
 
 	if ((status = ntohl(*p++)) != 0)
-		status = -nfs_stat_to_errno(status);
+		status = nfs_stat_to_errno(status);
 	p = xdr_decode_wcc_data(p, res->fromattr);
 	p = xdr_decode_wcc_data(p, res->toattr);
 	return status;

@@ -983,7 +983,7 @@ nfs3_xdr_linkres(struct rpc_rqst *req, __be32 *p, struct nfs3_linkres *res)
 	int	status;
 
 	if ((status = ntohl(*p++)) != 0)
-		status = -nfs_stat_to_errno(status);
+		status = nfs_stat_to_errno(status);
 	p = xdr_decode_post_op_attr(p, res->fattr);
 	p = xdr_decode_wcc_data(p, res->dir_attr);
 	return status;

@@ -1001,7 +1001,7 @@ nfs3_xdr_fsstatres(struct rpc_rqst *req, __be32 *p, struct nfs_fsstat *res)
 
 	p = xdr_decode_post_op_attr(p, res->fattr);
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	p = xdr_decode_hyper(p, &res->tbytes);
 	p = xdr_decode_hyper(p, &res->fbytes);

@@ -1026,7 +1026,7 @@ nfs3_xdr_fsinfores(struct rpc_rqst *req, __be32 *p, struct nfs_fsinfo *res)
 
 	p = xdr_decode_post_op_attr(p, res->fattr);
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	res->rtmax  = ntohl(*p++);
 	res->rtpref = ntohl(*p++);

@@ -1054,7 +1054,7 @@ nfs3_xdr_pathconfres(struct rpc_rqst *req, __be32 *p, struct nfs_pathconf *res)
 
 	p = xdr_decode_post_op_attr(p, res->fattr);
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	res->max_link = ntohl(*p++);
 	res->max_namelen = ntohl(*p++);

@@ -1073,7 +1073,7 @@ nfs3_xdr_commitres(struct rpc_rqst *req, __be32 *p, struct nfs_writeres *res)
 	status = ntohl(*p++);
 	p = xdr_decode_wcc_data(p, res->fattr);
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 
 	res->verf->verifier[0] = *p++;
 	res->verf->verifier[1] = *p++;

@@ -1095,7 +1095,7 @@ nfs3_xdr_getaclres(struct rpc_rqst *req, __be32 *p,
 	int err, base;
 
 	if (status != 0)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	p = xdr_decode_post_op_attr(p, res->fattr);
 	res->mask = ntohl(*p++);
 	if (res->mask & ~(NFS_ACL|NFS_ACLCNT|NFS_DFACL|NFS_DFACLCNT))

@@ -1122,7 +1122,7 @@ nfs3_xdr_setaclres(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 	int status = ntohl(*p++);
 
 	if (status)
-		return -nfs_stat_to_errno(status);
+		return nfs_stat_to_errno(status);
 	xdr_decode_post_op_attr(p, fattr);
 	return 0;
 }

fs/nfs/nfs4xdr.c +40 ~~-40~~

131 lines not on this page · raw message

Signature · 1 line

1.5.3.3

Benny Halevy

author

2008-03-31 14:48 UTC #2 lore raw

PATCH 2/2 nfs: use compound hdr.status to override op status.

In reply to Benny Halevy

This patch also fixes 3 call sites where we looked at the comound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs4xdr.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 68 insertions(+), 6 deletions(-)

fs/nfs/nfs4xdr.c +68 -6

index bb95b7c..edaa2fe 100644
@@ -3779,6 +3779,8 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

@@ -3806,6 +3808,8 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3829,6 +3833,8 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3849,6 +3855,8 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	if ((status = decode_getfh(&xdr, res->fh)) == 0)
 		status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3870,6 +3878,8 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem
 		goto out;
 	decode_getfattr(&xdr, &res->dir_attr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3900,6 +3910,8 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re
 		goto out;
 	decode_getfattr(&xdr, res->old_fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3933,6 +3945,8 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3962,6 +3976,8 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr
 		goto out;
 	decode_getfattr(&xdr, res->dir_fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3991,6 +4007,8 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 
 }

@@ -4014,6 +4032,8 @@ nfs4_xdr_enc_setacl(struct rpc_rqst *req, __be32 *p, struct nfs_setaclargs *args
                 goto out;
         status = encode_setacl(&xdr, args);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }
 /*

@@ -4035,6 +4055,8 @@ nfs4_xdr_dec_setacl(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_setattr(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4058,6 +4080,8 @@ nfs4_xdr_dec_getacl(struct rpc_rqst *rqstp, __be32 *p, size_t *acl_len)
 	status = decode_getacl(&xdr, rqstp, acl_len);
 
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4088,6 +4112,8 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
 	 */
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

@@ -4121,6 +4147,8 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr
 		goto out;
 	decode_getfattr(&xdr, res->dir_attr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

@@ -4142,6 +4170,8 @@ static int nfs4_xdr_dec_open_confirm(struct rpc_rqst *rqstp, __be32 *p, struct n
                 goto out;
         status = decode_open_confirm(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

@@ -4166,6 +4196,8 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf
                 goto out;
 	decode_getfattr(&xdr, res->f_attr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

@@ -4190,8 +4222,10 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se
                 goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 	if (status == NFS4ERR_DELAY)
-		status = 0;
+		return 0;
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

@@ -4213,6 +4247,8 @@ static int nfs4_xdr_dec_lock(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock_
 		goto out;
 	status = decode_lock(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4234,6 +4270,8 @@ static int nfs4_xdr_dec_lockt(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_lockt(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4255,6 +4293,8 @@ static int nfs4_xdr_dec_locku(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_locku(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4276,6 +4316,8 @@ static int nfs4_xdr_dec_readlink(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_readlink(&xdr, rqstp);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4297,6 +4339,8 @@ static int nfs4_xdr_dec_readdir(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_r
 		goto out;
 	status = decode_readdir(&xdr, rqstp, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4318,8 +4362,10 @@ static int nfs4_xdr_dec_read(struct rpc_rqst *rqstp, __be32 *p, struct nfs_readr
 		goto out;
 	status = decode_read(&xdr, rqstp, res);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4344,8 +4390,10 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4370,6 +4418,8 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4388,7 +4438,7 @@ static int nfs4_xdr_dec_fsinfo(struct rpc_rqst *req, __be32 *p, struct nfs_fsinf
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
+	if (hdr.status)
 		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4408,6 +4458,8 @@ static int nfs4_xdr_dec_pathconf(struct rpc_rqst *req, __be32 *p, struct nfs_pat
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_pathconf(&xdr, pathconf);
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4426,6 +4478,8 @@ static int nfs4_xdr_dec_statfs(struct rpc_rqst *req, __be32 *p, struct nfs_fssta
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_statfs(&xdr, fsstat);
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4445,6 +4499,8 @@ static int nfs4_xdr_dec_server_caps(struct rpc_rqst *req, __be32 *p, struct nfs4
 		goto out;
 	status = decode_server_caps(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4461,6 +4517,8 @@ static int nfs4_xdr_dec_renew(struct rpc_rqst *rqstp, __be32 *p, void *dummy)
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_renew(&xdr);
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4478,7 +4536,7 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_setclientid(&xdr, clp);
-	if (!status)
+	if (hdr.status)
 		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4500,7 +4558,7 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, str
 		status = decode_putrootfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
+	if (hdr.status)
 		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4524,6 +4582,8 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	status = decode_delegreturn(&xdr);
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4547,6 +4607,8 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, struct nfs
 	xdr_enter_page(&xdr, PAGE_SIZE);
 	status = decode_getfattr(&xdr, &res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

Signature · 1 line

1.5.3.3

Trond Myklebust

2008-03-31 22:25 UTC #3 lore raw

Re: [PATCH 2/2] nfs: use compound hdr.status to override op status.

Quoted 24 lines · Benny Halevy

The compound header status must be equivalent to the status of the last operation in the compound results. In certain cases like lack of resources or xdr decoding error, the nfs server may return a non-zero status in the compound header which is not returned by any operation. In this case we would notice that today when looking for the respective operations code in the results and we return -EIO when we cannot find it. This patch fixes that by returning the status available in the comound header instead.
This patch also fixes 3 call sites where we looked at the comound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}
Signed-off-by Benny Halevy <[email protected]>
---
 fs/nfs/nfs4xdr.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 68 insertions(+), 6 deletions(-)
fs/nfs/nfs4xdr.c +0 -0
index bb95b7c..edaa2fe 100644

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct · line 3783

Quoted 5 lines

fs/nfs/nfs4xdr.c +2 -0

@@ -3779,6 +3779,8 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

This changes the return value so that the outcome of the RPC call now depends on the success of a previously optional post-op GETATTR operation. For non-idempotent RPC calls, that's not acceptable.

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac · line 3812

Quoted 9 lines

fs/nfs/nfs4xdr.c +2 -0

         return status;
 }

@@ -3806,6 +3808,8 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

Quoted 22 lines

fs/nfs/nfs4xdr.c +4 -0

 	return status;
 }

@@ -3829,6 +3833,8 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -3849,6 +3855,8 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	if ((status = decode_getfh(&xdr, res->fh)) == 0)
 		status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem · line 3882

Quoted 5 lines

fs/nfs/nfs4xdr.c +2 -0

@@ -3870,6 +3878,8 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem
 		goto out;
 	decode_getfattr(&xdr, &res->dir_attr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re · line 3914

Quoted 9 lines

fs/nfs/nfs4xdr.c +2 -0

 	return status;
 }

@@ -3900,6 +3910,8 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re
 		goto out;
 	decode_getfattr(&xdr, res->old_fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link · line 3949

Quoted 9 lines

fs/nfs/nfs4xdr.c +2 -0

 	return status;
 }

@@ -3933,6 +3945,8 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr · line 3980

Quoted 9 lines

fs/nfs/nfs4xdr.c +2 -0

 	return status;
 }

@@ -3962,6 +3976,8 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr
 		goto out;
 	decode_getfattr(&xdr, res->dir_fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

Quoted 40 lines

fs/nfs/nfs4xdr.c +8 -0

 	return status;
 }

@@ -3991,6 +4007,8 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 
 }

@@ -4014,6 +4032,8 @@ nfs4_xdr_enc_setacl(struct rpc_rqst *req, __be32 *p, struct nfs_setaclargs *args
                 goto out;
         status = encode_setacl(&xdr, args);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }
 /*

@@ -4035,6 +4055,8 @@ nfs4_xdr_dec_setacl(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_setattr(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4058,6 +4080,8 @@ nfs4_xdr_dec_getacl(struct rpc_rqst *rqstp, __be32 *p, size_t *acl_len)
 	status = decode_getacl(&xdr, rqstp, acl_len);
 
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos · line 4116

Quoted 5 lines

fs/nfs/nfs4xdr.c +2 -0

@@ -4088,6 +4112,8 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
 	 */
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr · line 4151

Quoted 9 lines

fs/nfs/nfs4xdr.c +2 -0

         return status;
 }

@@ -4121,6 +4147,8 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr
 		goto out;
 	decode_getfattr(&xdr, res->dir_attr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

Quoted 13 lines

fs/nfs/nfs4xdr.c +2 -0

         return status;
 }

@@ -4142,6 +4170,8 @@ static int nfs4_xdr_dec_open_confirm(struct rpc_rqst *rqstp, __be32 *p, struct n
                 goto out;
         status = decode_open_confirm(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf · line 4200

Quoted 5 lines

fs/nfs/nfs4xdr.c +2 -0

@@ -4166,6 +4196,8 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf
                 goto out;
 	decode_getfattr(&xdr, res->f_attr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

Quoted 85 lines

fs/nfs/nfs4xdr.c +19 -3

         return status;
 }

@@ -4190,8 +4222,10 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se
                 goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 	if (status == NFS4ERR_DELAY)
-		status = 0;
+		return 0;
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
         return status;
 }

@@ -4213,6 +4247,8 @@ static int nfs4_xdr_dec_lock(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock_
 		goto out;
 	status = decode_lock(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4234,6 +4270,8 @@ static int nfs4_xdr_dec_lockt(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_lockt(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4255,6 +4293,8 @@ static int nfs4_xdr_dec_locku(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_locku(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4276,6 +4316,8 @@ static int nfs4_xdr_dec_readlink(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_readlink(&xdr, rqstp);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4297,6 +4339,8 @@ static int nfs4_xdr_dec_readdir(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_r
 		goto out;
 	status = decode_readdir(&xdr, rqstp, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4318,8 +4362,10 @@ static int nfs4_xdr_dec_read(struct rpc_rqst *rqstp, __be32 *p, struct nfs_readr
 		goto out;
 	status = decode_read(&xdr, rqstp, res);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4344,8 +4390,10 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri · line 4422

Quoted 5 lines

fs/nfs/nfs4xdr.c +2 -0

@@ -4370,6 +4418,8 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

Quoted 67 lines

fs/nfs/nfs4xdr.c +11 -3

 	return status;
 }

@@ -4388,7 +4438,7 @@ static int nfs4_xdr_dec_fsinfo(struct rpc_rqst *req, __be32 *p, struct nfs_fsinf
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
+	if (hdr.status)
 		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4408,6 +4458,8 @@ static int nfs4_xdr_dec_pathconf(struct rpc_rqst *req, __be32 *p, struct nfs_pat
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_pathconf(&xdr, pathconf);
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4426,6 +4478,8 @@ static int nfs4_xdr_dec_statfs(struct rpc_rqst *req, __be32 *p, struct nfs_fssta
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_statfs(&xdr, fsstat);
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4445,6 +4499,8 @@ static int nfs4_xdr_dec_server_caps(struct rpc_rqst *req, __be32 *p, struct nfs4
 		goto out;
 	status = decode_server_caps(&xdr, res);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4461,6 +4517,8 @@ static int nfs4_xdr_dec_renew(struct rpc_rqst *rqstp, __be32 *p, void *dummy)
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_renew(&xdr);
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4478,7 +4536,7 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_setclientid(&xdr, clp);
-	if (!status)
+	if (hdr.status)
 		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4500,7 +4558,7 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, str
 		status = decode_putrootfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
+	if (hdr.status)
 		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf · line 4586

Quoted 5 lines

fs/nfs/nfs4xdr.c +2 -0

@@ -4524,6 +4582,8 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	status = decode_delegreturn(&xdr);
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);

ditto

Quoted 12 lines

fs/nfs/nfs4xdr.c +2 -0

 	return status;
 }

@@ -4547,6 +4607,8 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, struct nfs
 	xdr_enter_page(&xdr, PAGE_SIZE);
 	status = decode_getfattr(&xdr, &res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

Benny Halevy

author

2008-04-01 09:37 UTC #5 lore raw

Re: [PATCH 2/2] nfs: use compound hdr.status to override op status.

In reply to a message not in the archive

Quoted 20 lines · Trond Myklebust

Quoted 19 lines · Benny Halevy
The compound header status must be equivalent to the status of the last operation in the compound results. In certain cases like lack of resources or xdr decoding error, the nfs server may return a non-zero status in the compound header which is not returned by any operation. In this case we would notice that today when looking for the respective operations code in the results and we return -EIO when we cannot find it. This patch fixes that by returning the status available in the comound header instead.
This patch also fixes 3 call sites where we looked at the comound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}
Signed-off-by Benny Halevy <[email protected]>
---
 fs/nfs/nfs4xdr.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 68 insertions(+), 6 deletions(-)

Quoted 14 lines

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct · line 3783
Quoted 10 lines
fs/nfs/nfs4xdr.c +2 -0
index bb95b7c..edaa2fe 100644
@@ -3779,6 +3779,8 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
+	if (hdr.status)
+		status = nfs4_stat_to_errno(hdr.status);
This changes the return value so that the outcome of the RPC call now depends on the success of a previously optional post-op GETATTR operation. For non-idempotent RPC calls, that's not acceptable.

Point taken.

Please see the patch in reply to this message. It takes a different approach which overrides the status only in the error case. For example:

+#define nfs4_fixup_status(status, hdr_status) \
+ ( likely(!status) ? 0 : nfs4_stat_to_errno(hdr_status) )
+
/*
* Decode OPEN_DOWNGRADE response
*/
@@ -3779,7 +3782,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
goto out;
decode_getfattr(&xdr, res->fattr, res->server);
out:
- return status;
+ return nfs4_fixup_status(status, hdr.status);
}

Benny

P.S. The reason I defined this macro rather than a static inline function is that it produced the smallest machine code (for x86_64, as reported by objdump -h).

Benny Halevy

author

2008-04-01 09:41 UTC #6 lore raw

PATCH 2/2 nfs: use compound hdr.status to override op status.

This patch also fixes 3 call sites where we looked at the compound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs4xdr.c |   84 +++++++++++++++++++++++++----------------------------
 1 files changed, 40 insertions(+), 44 deletions(-)

fs/nfs/nfs4xdr.c +40 ~~-44~~

index bb95b7c..85fae46 100644
@@ -3758,6 +3758,9 @@ static int decode_delegreturn(struct xdr_stream *xdr)
 	return decode_op_hdr(xdr, OP_DELEGRETURN);
 }
 
+#define nfs4_fixup_status(status, hdr_status) \
+	( likely(!status) ? 0 : nfs4_stat_to_errno(hdr_status) )
+
 /*
  * Decode OPEN_DOWNGRADE response
  */

@@ -3779,7 +3782,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3806,7 +3809,7 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3829,7 +3832,7 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3849,7 +3852,7 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	if ((status = decode_getfh(&xdr, res->fh)) == 0)
 		status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3870,7 +3873,7 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem
 		goto out;
 	decode_getfattr(&xdr, &res->dir_attr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3900,7 +3903,7 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re
 		goto out;
 	decode_getfattr(&xdr, res->old_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3933,7 +3936,7 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3962,7 +3965,7 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr
 		goto out;
 	decode_getfattr(&xdr, res->dir_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3991,8 +3994,7 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
-
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4014,7 +4016,7 @@ nfs4_xdr_enc_setacl(struct rpc_rqst *req, __be32 *p, struct nfs_setaclargs *args
                 goto out;
         status = encode_setacl(&xdr, args);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 /*
  * Decode SETACL response

@@ -4035,7 +4037,7 @@ nfs4_xdr_dec_setacl(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_setattr(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4058,7 +4060,7 @@ nfs4_xdr_dec_getacl(struct rpc_rqst *rqstp, __be32 *p, size_t *acl_len)
 	status = decode_getacl(&xdr, rqstp, acl_len);
 
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4088,7 +4090,7 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
 	 */
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4121,7 +4123,7 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr
 		goto out;
 	decode_getfattr(&xdr, res->dir_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4142,7 +4144,7 @@ static int nfs4_xdr_dec_open_confirm(struct rpc_rqst *rqstp, __be32 *p, struct n
                 goto out;
         status = decode_open_confirm(&xdr, res);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4166,7 +4168,7 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf
                 goto out;
 	decode_getfattr(&xdr, res->f_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4190,9 +4192,9 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se
                 goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 	if (status == NFS4ERR_DELAY)
-		status = 0;
+		return 0;
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4213,7 +4215,7 @@ static int nfs4_xdr_dec_lock(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock_
 		goto out;
 	status = decode_lock(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4234,7 +4236,7 @@ static int nfs4_xdr_dec_lockt(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_lockt(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4255,7 +4257,7 @@ static int nfs4_xdr_dec_locku(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_locku(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4276,7 +4278,7 @@ static int nfs4_xdr_dec_readlink(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_readlink(&xdr, rqstp);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4297,7 +4299,7 @@ static int nfs4_xdr_dec_readdir(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_r
 		goto out;
 	status = decode_readdir(&xdr, rqstp, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4318,9 +4320,9 @@ static int nfs4_xdr_dec_read(struct rpc_rqst *rqstp, __be32 *p, struct nfs_readr
 		goto out;
 	status = decode_read(&xdr, rqstp, res);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4344,9 +4346,9 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4370,7 +4372,7 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4388,9 +4390,7 @@ static int nfs4_xdr_dec_fsinfo(struct rpc_rqst *req, __be32 *p, struct nfs_fsinf
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4408,7 +4408,7 @@ static int nfs4_xdr_dec_pathconf(struct rpc_rqst *req, __be32 *p, struct nfs_pat
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_pathconf(&xdr, pathconf);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4426,7 +4426,7 @@ static int nfs4_xdr_dec_statfs(struct rpc_rqst *req, __be32 *p, struct nfs_fssta
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_statfs(&xdr, fsstat);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4445,7 +4445,7 @@ static int nfs4_xdr_dec_server_caps(struct rpc_rqst *req, __be32 *p, struct nfs4
 		goto out;
 	status = decode_server_caps(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4461,7 +4461,7 @@ static int nfs4_xdr_dec_renew(struct rpc_rqst *rqstp, __be32 *p, void *dummy)
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_renew(&xdr);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4478,9 +4478,7 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_setclientid(&xdr, clp);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4500,9 +4498,7 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, str
 		status = decode_putrootfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4524,7 +4520,7 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	status = decode_delegreturn(&xdr);
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4547,7 +4543,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, struct nfs
 	xdr_enter_page(&xdr, PAGE_SIZE);
 	status = decode_getfattr(&xdr, &res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 __be32 *nfs4_decode_dirent(__be32 *p, struct nfs_entry *entry, int plus)

Signature · 1 line

1.5.3.3

Benny Halevy

author

2008-05-09 23:38 UTC #7 lore raw

PATCH v2 1/1 nfs: use compound hdr.status to override op status.

Quoted 1 line

From 513dd2b4ebab280343db4bf6071b75c4820089fc Mon Sep 17 00:00:00 2001

From: Benny Halevy <[email protected]>
Date: Tue, 1 Apr 2008 12:41:08 +0300
Subject: [PATCH 1/1 v2] nfs: use compound hdr.status to override op status.

This patch also fixes 3 call sites where we looked at the compound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs4xdr.c |   84 +++++++++++++++++++++++++----------------------------
 1 files changed, 40 insertions(+), 44 deletions(-)

Changes in PATCH v2:
* rebased onto 2.6.26 (off of linux-2.6 28a4acb4)
* fixed checkpatch.pl nits
* do not fixup status in nfs4_xdr_enc_setacl

fs/nfs/nfs4xdr.c +40 ~~-44~~

index 5a2d649..57b512e 100644
@@ -3791,6 +3791,9 @@ static int decode_delegreturn(struct xdr_stream *xdr)
 	return decode_op_hdr(xdr, OP_DELEGRETURN);
 }
 
+#define nfs4_fixup_status(status, hdr_status) \
+	(likely(!status) ? 0 : nfs4_stat_to_errno(hdr_status))
+
 /*
  * Decode OPEN_DOWNGRADE response
  */

@@ -3812,7 +3815,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3839,7 +3842,7 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3862,7 +3865,7 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3882,7 +3885,7 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	if ((status = decode_getfh(&xdr, res->fh)) == 0)
 		status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3903,7 +3906,7 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem
 		goto out;
 	decode_getfattr(&xdr, &res->dir_attr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3933,7 +3936,7 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re
 		goto out;
 	decode_getfattr(&xdr, res->old_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3966,7 +3969,7 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3995,7 +3998,7 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr
 		goto out;
 	decode_getfattr(&xdr, res->dir_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4024,8 +4027,7 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
-
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4047,7 +4049,7 @@ nfs4_xdr_enc_setacl(struct rpc_rqst *req, __be32 *p, struct nfs_setaclargs *args
                 goto out;
         status = encode_setacl(&xdr, args);
 out:
-        return status;
+	return status;
 }
 /*
  * Decode SETACL response

@@ -4068,7 +4070,7 @@ nfs4_xdr_dec_setacl(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_setattr(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4091,7 +4093,7 @@ nfs4_xdr_dec_getacl(struct rpc_rqst *rqstp, __be32 *p, size_t *acl_len)
 	status = decode_getacl(&xdr, rqstp, acl_len);
 
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4121,7 +4123,7 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
 	 */
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4154,7 +4156,7 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr
 		goto out;
 	decode_getfattr(&xdr, res->dir_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4175,7 +4177,7 @@ static int nfs4_xdr_dec_open_confirm(struct rpc_rqst *rqstp, __be32 *p, struct n
                 goto out;
         status = decode_open_confirm(&xdr, res);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4199,7 +4201,7 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf
                 goto out;
 	decode_getfattr(&xdr, res->f_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4223,9 +4225,9 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se
                 goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 	if (status == NFS4ERR_DELAY)
-		status = 0;
+		return 0;
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4246,7 +4248,7 @@ static int nfs4_xdr_dec_lock(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock_
 		goto out;
 	status = decode_lock(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4267,7 +4269,7 @@ static int nfs4_xdr_dec_lockt(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_lockt(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4288,7 +4290,7 @@ static int nfs4_xdr_dec_locku(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_locku(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4309,7 +4311,7 @@ static int nfs4_xdr_dec_readlink(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_readlink(&xdr, rqstp);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4330,7 +4332,7 @@ static int nfs4_xdr_dec_readdir(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_r
 		goto out;
 	status = decode_readdir(&xdr, rqstp, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4351,9 +4353,9 @@ static int nfs4_xdr_dec_read(struct rpc_rqst *rqstp, __be32 *p, struct nfs_readr
 		goto out;
 	status = decode_read(&xdr, rqstp, res);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4377,9 +4379,9 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4403,7 +4405,7 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4421,9 +4423,7 @@ static int nfs4_xdr_dec_fsinfo(struct rpc_rqst *req, __be32 *p, struct nfs_fsinf
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4441,7 +4441,7 @@ static int nfs4_xdr_dec_pathconf(struct rpc_rqst *req, __be32 *p, struct nfs_pat
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_pathconf(&xdr, pathconf);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4459,7 +4459,7 @@ static int nfs4_xdr_dec_statfs(struct rpc_rqst *req, __be32 *p, struct nfs_fssta
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_statfs(&xdr, fsstat);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4478,7 +4478,7 @@ static int nfs4_xdr_dec_server_caps(struct rpc_rqst *req, __be32 *p, struct nfs4
 		goto out;
 	status = decode_server_caps(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4494,7 +4494,7 @@ static int nfs4_xdr_dec_renew(struct rpc_rqst *rqstp, __be32 *p, void *dummy)
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_renew(&xdr);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4511,9 +4511,7 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_setclientid(&xdr, clp);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4533,9 +4531,7 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, str
 		status = decode_putrootfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4557,7 +4553,7 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	status = decode_delegreturn(&xdr);
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4580,7 +4576,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, struct nfs
 	xdr_enter_page(&xdr, PAGE_SIZE);
 	status = decode_getfattr(&xdr, &res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 __be32 *nfs4_decode_dirent(__be32 *p, struct nfs_entry *entry, int plus)

Signature · 1 line

1.5.3.3

Benny Halevy

author

2008-05-10 05:58 UTC #22 lore raw

PATCH v2 1/1 nfs: use compound hdr.status to override op status.

In reply to Benny Halevy

Benny Halevy

author

2008-05-10 06:12 UTC #23 lore raw

Re: [PATCH 1/1 v2] nfs: use compound hdr.status to override op status.

Please ignore the set of empty messages.
[problem with sendmail from inside the connectathon network...]

Benny

Quoted 4 lines · Benny Halevy

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

Benny Halevy

author

2008-05-10 06:45 UTC #24 lore raw

PATCH v2 1/1 nfs: use compound hdr.status to override op status.

In reply to Benny Halevy

Trond Myklebust

2008-05-10 17:24 UTC #8 lore raw

Re: [PATCH v2] nfs: use compound hdr.status to override op status.

In reply to Benny Halevy

Quoted 34 lines · Benny Halevy

Quoted 1 line
From 513dd2b4ebab280343db4bf6071b75c4820089fc Mon Sep 17 00:00:00 2001
From: Benny Halevy <[email protected]>
Date: Tue, 1 Apr 2008 12:41:08 +0300
Subject: [PATCH 1/1 v2] nfs: use compound hdr.status to override op status.
The compound header status must be equivalent to the status of the last operation in the compound results. In certain cases like lack of resources or xdr decoding error, the nfs server may return a non-zero status in the compound header which is not returned by any operation. In this case we would notice that today when looking for the respective operations code in the results and we return -EIO when we cannot find it. This patch fixes that by returning the status available in the compound header instead.
This patch also fixes 3 call sites where we looked at the compound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}
Signed-off-by Benny Halevy <[email protected]>
---
 fs/nfs/nfs4xdr.c |   84 +++++++++++++++++++++++++----------------------------
 1 files changed, 40 insertions(+), 44 deletions(-)
Changes in PATCH v2:
* rebased onto 2.6.26 (off of linux-2.6 28a4acb4)
* fixed checkpatch.pl nits
* do not fixup status in nfs4_xdr_enc_setacl
fs/nfs/nfs4xdr.c +0 -0
index 5a2d649..57b512e 100644

fs/nfs/nfs4xdr.c static int decode_delegreturn(struct xdr_stream *xdr) · line 3796

Quoted 6 lines

fs/nfs/nfs4xdr.c +3 -0

@@ -3791,6 +3791,9 @@ static int decode_delegreturn(struct xdr_stream *xdr)
 	return decode_op_hdr(xdr, OP_DELEGRETURN);
 }
 
+#define nfs4_fixup_status(status, hdr_status) \
+	(likely(!status) ? 0 : nfs4_stat_to_errno(hdr_status))
+

static function please!

Trond

Benny Halevy

author

2008-05-11 23:21 UTC #9 lore raw

PATCH v3 1/1 nfs: use compound hdr.status to override op status.

This patch also fixes 3 call sites where we looked at the compound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs4xdr.c |   88 +++++++++++++++++++++++++++---------------------------
 1 files changed, 44 insertions(+), 44 deletions(-)

Changes in PATCH v3:
* nfs4_fixup_status made static inline (with very minor increase in code size over macro)

Changes in PATCH v2:
* rebased onto 2.6.26 (off of linux-2.6 28a4acb4)
* fixed checkpatch.pl nits
* do not fixup status in nfs4_xdr_enc_setacl

fs/nfs/nfs4xdr.c +44 ~~-44~~

index 5a2d649..e6f7f0b 100644
@@ -3791,6 +3791,13 @@ static int decode_delegreturn(struct xdr_stream *xdr)
 	return decode_op_hdr(xdr, OP_DELEGRETURN);
 }
 
+static inline int nfs4_fixup_status(int status, int hdr_status)
+{
+	if (likely(!status))
+		return 0;
+	return nfs4_stat_to_errno(hdr_status);
+}
+
 /*
  * Decode OPEN_DOWNGRADE response
  */

@@ -3812,7 +3819,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3839,7 +3846,7 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3862,7 +3869,7 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3882,7 +3889,7 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	if ((status = decode_getfh(&xdr, res->fh)) == 0)
 		status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3903,7 +3910,7 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem
 		goto out;
 	decode_getfattr(&xdr, &res->dir_attr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3933,7 +3940,7 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re
 		goto out;
 	decode_getfattr(&xdr, res->old_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3966,7 +3973,7 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3995,7 +4002,7 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr
 		goto out;
 	decode_getfattr(&xdr, res->dir_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4024,8 +4031,7 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
-
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4047,7 +4053,7 @@ nfs4_xdr_enc_setacl(struct rpc_rqst *req, __be32 *p, struct nfs_setaclargs *args
                 goto out;
         status = encode_setacl(&xdr, args);
 out:
-        return status;
+	return status;
 }
 /*
  * Decode SETACL response

@@ -4068,7 +4074,7 @@ nfs4_xdr_dec_setacl(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_setattr(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4091,7 +4097,7 @@ nfs4_xdr_dec_getacl(struct rpc_rqst *rqstp, __be32 *p, size_t *acl_len)
 	status = decode_getacl(&xdr, rqstp, acl_len);
 
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4121,7 +4127,7 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
 	 */
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4154,7 +4160,7 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr
 		goto out;
 	decode_getfattr(&xdr, res->dir_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4175,7 +4181,7 @@ static int nfs4_xdr_dec_open_confirm(struct rpc_rqst *rqstp, __be32 *p, struct n
                 goto out;
         status = decode_open_confirm(&xdr, res);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4199,7 +4205,7 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf
                 goto out;
 	decode_getfattr(&xdr, res->f_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4223,9 +4229,9 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se
                 goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 	if (status == NFS4ERR_DELAY)
-		status = 0;
+		return 0;
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4246,7 +4252,7 @@ static int nfs4_xdr_dec_lock(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock_
 		goto out;
 	status = decode_lock(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4267,7 +4273,7 @@ static int nfs4_xdr_dec_lockt(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_lockt(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4288,7 +4294,7 @@ static int nfs4_xdr_dec_locku(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_locku(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4309,7 +4315,7 @@ static int nfs4_xdr_dec_readlink(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_readlink(&xdr, rqstp);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4330,7 +4336,7 @@ static int nfs4_xdr_dec_readdir(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_r
 		goto out;
 	status = decode_readdir(&xdr, rqstp, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4351,9 +4357,9 @@ static int nfs4_xdr_dec_read(struct rpc_rqst *rqstp, __be32 *p, struct nfs_readr
 		goto out;
 	status = decode_read(&xdr, rqstp, res);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4377,9 +4383,9 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4403,7 +4409,7 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4421,9 +4427,7 @@ static int nfs4_xdr_dec_fsinfo(struct rpc_rqst *req, __be32 *p, struct nfs_fsinf
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4441,7 +4445,7 @@ static int nfs4_xdr_dec_pathconf(struct rpc_rqst *req, __be32 *p, struct nfs_pat
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_pathconf(&xdr, pathconf);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4459,7 +4463,7 @@ static int nfs4_xdr_dec_statfs(struct rpc_rqst *req, __be32 *p, struct nfs_fssta
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_statfs(&xdr, fsstat);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4478,7 +4482,7 @@ static int nfs4_xdr_dec_server_caps(struct rpc_rqst *req, __be32 *p, struct nfs4
 		goto out;
 	status = decode_server_caps(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4494,7 +4498,7 @@ static int nfs4_xdr_dec_renew(struct rpc_rqst *rqstp, __be32 *p, void *dummy)
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_renew(&xdr);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4511,9 +4515,7 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_setclientid(&xdr, clp);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4533,9 +4535,7 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, str
 		status = decode_putrootfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4557,7 +4557,7 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	status = decode_delegreturn(&xdr);
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4580,7 +4580,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, struct nfs
 	xdr_enter_page(&xdr, PAGE_SIZE);
 	status = decode_getfattr(&xdr, &res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 __be32 *nfs4_decode_dirent(__be32 *p, struct nfs_entry *entry, int plus)

Signature · 1 line

1.5.3.3

Trond Myklebust

NAK

2008-05-11 23:26 UTC #10 lore raw

Re: [PATCH v3] nfs: use compound hdr.status to override op status.

Quoted 48 lines · Benny Halevy

The compound header status must be equivalent to the status of the last operation in the compound results. In certain cases like lack of resources or xdr decoding error, the nfs server may return a non-zero status in the compound header which is not returned by any operation. In this case we would notice that today when looking for the respective operations code in the results and we return -EIO when we cannot find it. This patch fixes that by returning the status available in the compound header instead.
This patch also fixes 3 call sites where we looked at the compound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}
Signed-off-by Benny Halevy <[email protected]>
---
 fs/nfs/nfs4xdr.c |   88 +++++++++++++++++++++++++++---------------------------
 1 files changed, 44 insertions(+), 44 deletions(-)
Changes in PATCH v3:
* nfs4_fixup_status made static inline (with very minor increase in code size over macro)
Changes in PATCH v2:
* rebased onto 2.6.26 (off of linux-2.6 28a4acb4)
* fixed checkpatch.pl nits
* do not fixup status in nfs4_xdr_enc_setacl
fs/nfs/nfs4xdr.c +7 -0
index 5a2d649..e6f7f0b 100644
@@ -3791,6 +3791,13 @@ static int decode_delegreturn(struct xdr_stream *xdr)
 	return decode_op_hdr(xdr, OP_DELEGRETURN);
 }
 
+static inline int nfs4_fixup_status(int status, int hdr_status)
+{
+	if (likely(!status))
+		return 0;
+	return nfs4_stat_to_errno(hdr_status);
+}
+
 /*
  * Decode OPEN_DOWNGRADE response
  */

fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct · line 3823

Quoted 6 lines

fs/nfs/nfs4xdr.c +1 -1

@@ -3812,7 +3819,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }

NACK.

This is still screwed up: if the getattr above fails, then your change will propagate that error despite the fact that we don't care. Please see the same comments on earlier drafts of this patch.

Quoted 315 lines

fs/nfs/nfs4xdr.c +36 ~~-43~~

/*

@@ -3839,7 +3846,7 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3862,7 +3869,7 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3882,7 +3889,7 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	if ((status = decode_getfh(&xdr, res->fh)) == 0)
 		status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3903,7 +3910,7 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem
 		goto out;
 	decode_getfattr(&xdr, &res->dir_attr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3933,7 +3940,7 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re
 		goto out;
 	decode_getfattr(&xdr, res->old_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3966,7 +3973,7 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -3995,7 +4002,7 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr
 		goto out;
 	decode_getfattr(&xdr, res->dir_fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4024,8 +4031,7 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g
 		goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
-
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4047,7 +4053,7 @@ nfs4_xdr_enc_setacl(struct rpc_rqst *req, __be32 *p, struct nfs_setaclargs *args
                 goto out;
         status = encode_setacl(&xdr, args);
 out:
-        return status;
+	return status;
 }
 /*
  * Decode SETACL response

@@ -4068,7 +4074,7 @@ nfs4_xdr_dec_setacl(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_setattr(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4091,7 +4097,7 @@ nfs4_xdr_dec_getacl(struct rpc_rqst *rqstp, __be32 *p, size_t *acl_len)
 	status = decode_getacl(&xdr, rqstp, acl_len);
 
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4121,7 +4127,7 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
 	 */
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4154,7 +4160,7 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr
 		goto out;
 	decode_getfattr(&xdr, res->dir_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4175,7 +4181,7 @@ static int nfs4_xdr_dec_open_confirm(struct rpc_rqst *rqstp, __be32 *p, struct n
                 goto out;
         status = decode_open_confirm(&xdr, res);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4199,7 +4205,7 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf
                 goto out;
 	decode_getfattr(&xdr, res->f_attr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4223,9 +4229,9 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se
                 goto out;
 	status = decode_getfattr(&xdr, res->fattr, res->server);
 	if (status == NFS4ERR_DELAY)
-		status = 0;
+		return 0;
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4246,7 +4252,7 @@ static int nfs4_xdr_dec_lock(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock_
 		goto out;
 	status = decode_lock(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4267,7 +4273,7 @@ static int nfs4_xdr_dec_lockt(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_lockt(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4288,7 +4294,7 @@ static int nfs4_xdr_dec_locku(struct rpc_rqst *rqstp, __be32 *p, struct nfs_lock
 		goto out;
 	status = decode_locku(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4309,7 +4315,7 @@ static int nfs4_xdr_dec_readlink(struct rpc_rqst *rqstp, __be32 *p, void *res)
 		goto out;
 	status = decode_readlink(&xdr, rqstp);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4330,7 +4336,7 @@ static int nfs4_xdr_dec_readdir(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_r
 		goto out;
 	status = decode_readdir(&xdr, rqstp, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4351,9 +4357,9 @@ static int nfs4_xdr_dec_read(struct rpc_rqst *rqstp, __be32 *p, struct nfs_readr
 		goto out;
 	status = decode_read(&xdr, rqstp, res);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4377,9 +4383,9 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 	if (!status)
-		status = res->count;
+		return res->count;
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4403,7 +4409,7 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4421,9 +4427,7 @@ static int nfs4_xdr_dec_fsinfo(struct rpc_rqst *req, __be32 *p, struct nfs_fsinf
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4441,7 +4445,7 @@ static int nfs4_xdr_dec_pathconf(struct rpc_rqst *req, __be32 *p, struct nfs_pat
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_pathconf(&xdr, pathconf);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4459,7 +4463,7 @@ static int nfs4_xdr_dec_statfs(struct rpc_rqst *req, __be32 *p, struct nfs_fssta
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_statfs(&xdr, fsstat);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4478,7 +4482,7 @@ static int nfs4_xdr_dec_server_caps(struct rpc_rqst *req, __be32 *p, struct nfs4
 		goto out;
 	status = decode_server_caps(&xdr, res);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4494,7 +4498,7 @@ static int nfs4_xdr_dec_renew(struct rpc_rqst *rqstp, __be32 *p, void *dummy)
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_renew(&xdr);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4511,9 +4515,7 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_setclientid(&xdr, clp);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4533,9 +4535,7 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, str
 		status = decode_putrootfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4557,7 +4557,7 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf
 	status = decode_delegreturn(&xdr);
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 /*

@@ -4580,7 +4580,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, struct nfs
 	xdr_enter_page(&xdr, PAGE_SIZE);
 	status = decode_getfattr(&xdr, &res->fattr, res->server);
 out:
-	return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
 
 __be32 *nfs4_decode_dirent(__be32 *p, struct nfs_entry *entry, int plus)

Benny Halevy

author

2008-05-12 01:55 UTC #11 lore raw

Re: [PATCH v3] nfs: use compound hdr.status to override op status.

Quoted 58 lines · Trond Myklebust

Quoted 48 lines · Benny Halevy
The compound header status must be equivalent to the status of the last operation in the compound results. In certain cases like lack of resources or xdr decoding error, the nfs server may return a non-zero status in the compound header which is not returned by any operation. In this case we would notice that today when looking for the respective operations code in the results and we return -EIO when we cannot find it. This patch fixes that by returning the status available in the compound header instead.
This patch also fixes 3 call sites where we looked at the compound hdr.status in the success case which is useless (yet benign). These are nfs4_xdr_dec_{fsinfo,setclientid,setclientid_confirm}
Signed-off-by Benny Halevy <[email protected]>
---
 fs/nfs/nfs4xdr.c |   88 +++++++++++++++++++++++++++---------------------------
 1 files changed, 44 insertions(+), 44 deletions(-)
Changes in PATCH v3:
* nfs4_fixup_status made static inline (with very minor increase in code size over macro)
Changes in PATCH v2:
* rebased onto 2.6.26 (off of linux-2.6 28a4acb4)
* fixed checkpatch.pl nits
* do not fixup status in nfs4_xdr_enc_setacl
fs/nfs/nfs4xdr.c +7 -0
index 5a2d649..e6f7f0b 100644
@@ -3791,6 +3791,13 @@ static int decode_delegreturn(struct xdr_stream *xdr)
 	return decode_op_hdr(xdr, OP_DELEGRETURN);
 }
 
+static inline int nfs4_fixup_status(int status, int hdr_status)
+{
+	if (likely(!status))
+		return 0;
+	return nfs4_stat_to_errno(hdr_status);
+}
+
 /*
  * Decode OPEN_DOWNGRADE response
  */
fs/nfs/nfs4xdr.c static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct · line 3823
Quoted 6 lines
fs/nfs/nfs4xdr.c +1 -1
@@ -3812,7 +3819,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct
 		goto out;
 	decode_getfattr(&xdr, res->fattr, res->server);
 out:
-        return status;
+	return nfs4_fixup_status(status, hdr.status);
 }
NACK.

Quoted 4 lines

This is still screwed up: if the getattr above fails, then your change will propagate that error despite the fact that we don't care. Please see the same comments on earlier drafts of this patch.

Trond, sorry, but I don't see the problem.

If all ops before the getattr succeeded then "status" will be zero regardless if getattr failed or not and nfs4_fixup_status will return success.

If any other op before the getattr failed then getfattr will not get processed and hdr.status will contain the latest failure.

Benny

Benny Halevy

author

2008-07-03 17:49 UTC #12 lore raw

v4 cover nfs: return nfs4 compound header status on op header decoding error

In reply to Benny Halevy

Trond, following our conversation during the Connectathon I reduced this patch as much as possible and restricted the use of the compound header status to cases where op_hdr decoding hit an error.

This is needed for nfs41 for graceful fallback when trying to mount a 4.0 server with 4.1. In this case the server returns no ops and the hdr status is set to NFS4ERR_MINOR_VERS_MISMATCH.

Please consider these patches:

[PATCH 1/2] nfs: return nfs4 compound header status on op header decoding error [PATCH 2/2] nfs: remove incorrect usage of nfs4 compound response hdr.status

Thanks,

Benny

Benny Halevy

author

2008-07-03 17:52 UTC #13 lore raw

PATCH 1/2 nfs: return nfs4 compound header status on op header decoding error

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs4xdr.c           |   30 +++++++++++++++++++++++-------
 include/linux/sunrpc/xdr.h |    1 +
 2 files changed, 24 insertions(+), 7 deletions(-)

fs/nfs/nfs4xdr.c +23 -7

index b916297..2cf724d 100644
@@ -2214,8 +2214,10 @@ static int decode_compound_hdr(struct xdr_stream *xdr, struct compound_hdr *hdr)
 {
 	__be32 *p;
 
+	xdr->status = -EIO;
 	READ_BUF(8);
 	READ32(hdr->status);
+	xdr->status = hdr->status;
 	READ32(hdr->taglen);
 	
 	READ_BUF(hdr->taglen + 4);

@@ -2225,24 +2227,38 @@ static int decode_compound_hdr(struct xdr_stream *xdr, struct compound_hdr *hdr)
 	return 0;
 }
 
-static int decode_op_hdr(struct xdr_stream *xdr, enum nfs_opnum4 expected)
+static int
+decode_op_hdr(struct xdr_stream *xdr, enum nfs_opnum4 expected)
 {
 	__be32 *p;
 	uint32_t opnum;
 	int32_t nfserr;
 
-	READ_BUF(8);
+	p = xdr_inline_decode(xdr, 8);
+	if (unlikely(!p)) {
+		dprintk("nfs: %s: prematurely hit end of receive"
+				" buffer\n", __func__);
+		dprintk("nfs: %s: xdr->p=%p, bytes=%u, xdr->end=%p\n",
+				__func__, xdr->p, 8, xdr->end);
+		goto err;
+	}
 	READ32(opnum);
-	if (opnum != expected) {
+	if (unlikely(opnum != expected)) {
 		dprintk("nfs: Server returned operation"
 			" %d but we issued a request for %d\n",
 				opnum, expected);
-		return -EIO;
+		goto err;
 	}
 	READ32(nfserr);
-	if (nfserr != NFS_OK)
-		return nfs4_stat_to_errno(nfserr);
-	return 0;
+	if (likely(nfserr == NFS_OK))
+		return 0;
+	xdr->status = nfserr;
+err:
+	if (xdr->status != NFS_OK)
+		return nfs4_stat_to_errno(xdr->status);
+	dprintk("nfs: %s: xdr status %d, returning %d\n",
+		__func__, xdr->status, -EIO);
+	return -EIO;
 }
 
 /* Dummy routine */

include/linux/sunrpc/xdr.h +1 -0

index e4057d7..0c09ae5 100644
@@ -208,6 +208,7 @@ struct xdr_stream {
 
 	__be32 *end;		/* end of available buffer space */
 	struct kvec *iov;	/* pointer to the current kvec */
+	int status;
 };
 
 extern void xdr_init_encode(struct xdr_stream *xdr, struct xdr_buf *buf, __be32 *p);

Signature · 1 line

1.5.6.GIT

Benny Halevy

author

2008-07-03 18:42 UTC #14 lore raw

PATCH 2/2 nfs: remove incorrect usage of nfs4 compound response hdr.status

In reply to Benny Halevy

3 call sites look at hdr.status before returning success. hdr.status must be zero in this case so there's no point in this.

Currently, hdr.status is correctly processed at decode_op_hdr time if the op status cannot be decoded.

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs4xdr.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

fs/nfs/nfs4xdr.c +0 -6

index 2cf724d..22df89f 100644
@@ -4437,8 +4437,6 @@ static int nfs4_xdr_dec_fsinfo(struct rpc_rqst *req, __be32 *p, struct nfs_fsinf
 		status = decode_putfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4527,8 +4525,6 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
 	status = decode_compound_hdr(&xdr, &hdr);
 	if (!status)
 		status = decode_setclientid(&xdr, clp);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

@@ -4549,8 +4545,6 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, str
 		status = decode_putrootfh(&xdr);
 	if (!status)
 		status = decode_fsinfo(&xdr, fsinfo);
-	if (!status)
-		status = nfs4_stat_to_errno(hdr.status);
 	return status;
 }

Signature · 1 line

1.5.6.GIT

Trond Myklebust

2008-07-15 21:57 UTC #15 lore raw

Re: [PATCH 0/2 v4] nfs: return nfs4 compound header status on op header decoding error

In reply to Benny Halevy

Quoted 5 lines · Benny Halevy

Trond, following our conversation during the Connectathon I reduced this patch as much as possible and restricted the use of the compound header status to cases where op_hdr decoding hit an error.

Quoted 8 lines

This is needed for nfs41 for graceful fallback when trying to mount a 4.0 server with 4.1. In this case the server returns no ops and the hdr status is set to NFS4ERR_MINOR_VERS_MISMATCH.
Please consider these patches:
[PATCH 1/2] nfs: return nfs4 compound header status on op header decoding error

No, this patch doesn't look right either. If we overrun the end of the reply buffer, then xdr_inline_decode() will return a NULL pointer, so you should never hit the your (opnum != expected) case.

So given that your concern is primarily the case where nops==0, why don't you just add that particular case to decode_compound_hdr?

IOW: something like

@@static int decode_compound_hdr(

	p += XDR_QUADLEN(hdr->taglen);
	READ32(hdr->nops);
+	if (hdr->nops < 1)
+		return nfs4_stat_to_errno(hdr->status);
	return 0;
}

Benny Halevy

author

2008-07-16 08:21 UTC #16 lore raw

Re: [PATCH 0/2 v4] nfs: return nfs4 compound header status on op header decoding error

Quoted 11 lines · Trond Myklebust

Quoted 10 lines · Benny Halevy
Trond, following our conversation during the Connectathon I reduced this patch as much as possible and restricted the use of the compound header status to cases where op_hdr decoding hit an error.
This is needed for nfs41 for graceful fallback when trying to mount a 4.0 server with 4.1. In this case the server returns no ops and the hdr status is set to NFS4ERR_MINOR_VERS_MISMATCH.

Quoted 7 lines

Quoted 3 lines
Please consider these patches:
[PATCH 1/2] nfs: return nfs4 compound header status on op header decoding error
No, this patch doesn't look right either. If we overrun the end of the reply buffer, then xdr_inline_decode() will return a NULL pointer, so you should never hit the your (opnum != expected) case.

In patch 1, this is supposed to be handled like this:

decode_compound_hdr:
+ xdr->status = hdr->status;

decode_op_hdr:
+ p = xdr_inline_decode(xdr, 8);
+ if (unlikely(!p)) {
...
+ goto err;
+ }

+err:
+ if (xdr->status != NFS_OK)
+ return nfs4_stat_to_errno(xdr->status);

Quoted 12 lines

So given that your concern is primarily the case where nops==0, why don't you just add that particular case to decode_compound_hdr?
IOW: something like
@@static int decode_compound_hdr(
	p += XDR_QUADLEN(hdr->taglen);
	READ32(hdr->nops);
(quoted diff) +2 -0
+	if (hdr->nops < 1)
+		return nfs4_stat_to_errno(hdr->status);

Quoted 3 lines

	return 0;
}

This certainly provides a shortcut for the nops==0 case. However, it doesn't solve the OP_ILLEGAL case all other cases where xdr_inline_decode failed or opnum != expected, we can easily handle the OP_ILLEGAL case explicitly in decode_op_hdr. Are you ok with the approach of carrying the hdr.status in xdr->status for decode_op_hdr use, or do you rather prefer to leave things as they are for the invalid cases and explicitly handle only the nops==0 and OP_ILLEGAL cases?

Benny

Trond Myklebust

2008-07-16 12:57 UTC #17 lore raw

Re: [PATCH 0/2 v4] nfs: return nfs4 compound header status on op header decoding error

Quoted 14 lines · Benny Halevy

Quoted 11 lines · Trond Myklebust
IOW: something like
@@static int decode_compound_hdr(
	p += XDR_QUADLEN(hdr->taglen);
	READ32(hdr->nops);
(quoted diff) +2 -0
+	if (hdr->nops < 1)
+		return nfs4_stat_to_errno(hdr->status);
	return 0;
}
This certainly provides a shortcut for the nops==0 case.

Quoted 8 lines

However, it doesn't solve the OP_ILLEGAL case all other cases where xdr_inline_decode failed or opnum != expected, we can easily handle the OP_ILLEGAL case explicitly in decode_op_hdr. Are you ok with the approach of carrying the hdr.status in xdr->status for decode_op_hdr use, or do you rather prefer to leave things as they are for the invalid cases and explicitly handle only the nops==0 and OP_ILLEGAL cases?

Why do we need to handle OP_ILLEGAL in the first place? This is the client; it isn't supposed to send illegal operations...

Trond

Benny Halevy

author

2008-07-16 13:22 UTC #18 lore raw

Re: [PATCH 0/2 v4] nfs: return nfs4 compound header status on op header decoding error

Quoted 23 lines · Trond Myklebust

Quoted 13 lines · Benny Halevy
Quoted 11 lines · Trond Myklebust
IOW: something like
@@static int decode_compound_hdr(
	p += XDR_QUADLEN(hdr->taglen);
	READ32(hdr->nops);
(quoted diff) +2 -0
+	if (hdr->nops < 1)
+		return nfs4_stat_to_errno(hdr->status);
	return 0;
}
This certainly provides a shortcut for the nops==0 case.
Quoted 8 lines
However, it doesn't solve the OP_ILLEGAL case all other cases where xdr_inline_decode failed or opnum != expected, we can easily handle the OP_ILLEGAL case explicitly in decode_op_hdr. Are you ok with the approach of carrying the hdr.status in xdr->status for decode_op_hdr use, or do you rather prefer to leave things as they are for the invalid cases and explicitly handle only the nops==0 and OP_ILLEGAL cases?

Quoted 2 lines

Why do we need to handle OP_ILLEGAL in the first place? This is the client; it isn't supposed to send illegal operations...

Right, but it helps in the development process when dealing with a broken version of the server or the client to pass a less generic error (-EOPNOTSUPP) up the stack rather than -EIO.

Benny

Quoted 3 lines

Trond

Trond Myklebust

2008-07-17 12:09 UTC #19 lore raw

Re: [PATCH 0/2 v4] nfs: return nfs4 compound header status on op header decoding error

Quoted 7 lines · Benny Halevy

Quoted 2 lines · Trond Myklebust
Why do we need to handle OP_ILLEGAL in the first place? This is the client; it isn't supposed to send illegal operations...
Right, but it helps in the development process when dealing with a broken version of the server or the client to pass a less generic error (-EOPNOTSUPP) up the stack rather than -EIO.

NFS4ERR_OP_ILLEGAL literally means "this operation isn't even listed in the 4.0/4.1 RFC". That's out in EYOUUTTERLYINSANECLIENT territory, and so the current mapping to EOPNOTSUPP is just wrong.

NFS4ERR_NOTSUPP is the correct return value if a server doesn't (yet) support an operation.

Benny Halevy

author

2008-07-17 13:20 UTC #20 lore raw

Re: [PATCH 0/2 v4] nfs: return nfs4 compound header status on op header decoding error

Quoted 11 lines · Trond Myklebust

Quoted 6 lines · Benny Halevy
Quoted 2 lines · Trond Myklebust
Why do we need to handle OP_ILLEGAL in the first place? This is the client; it isn't supposed to send illegal operations...
Right, but it helps in the development process when dealing with a broken version of the server or the client to pass a less generic error (-EOPNOTSUPP) up the stack rather than -EIO.
NFS4ERR_OP_ILLEGAL literally means "this operation isn't even listed in the 4.0/4.1 RFC". That's out in EYOUUTTERLYINSANECLIENT territory, and so the current mapping to EOPNOTSUPP is just wrong.

FWIW, we currently map NFS4ERR_NOTSUPP to -ENOTSUPP and NFS4ERR_OP_ILLEGAL to -EOPNOTSUPP so one can distinct between the two cases, even if the latter mapping is wrong.

I'm not sure if there's an available error code that would describe this NFS4ERR_OP_ILLEGAL perfectly, -EINVAL might be reasonable but I it's not very distinctive. Maybe we should add one to include/linux/errno.h?

Quoted 6 lines

NFS4ERR_NOTSUPP is the correct return value if a server doesn't (yet) support an operation.

Agreed.

Just that we had a bug in the server that caused us to return NFS4ERR_OP_ILLEGAL for unimplemented ops rather than NFS4ERR_NOTSUPP. That's a condition I really want to be aware of while developing the server.

Benny

Benny Halevy

author

2008-07-21 16:59 UTC #21 lore raw

PATCH 1/1 nfs: return compound hdr.status when there are no op replies

In reply to Trond Myklebust

When there are no op replies encoded in the comound reply hdr.status still contains the overall status of the compound rpc. This can happen, e.g., when the server returns an NFS4ERR_MINOR_VERS_MISMATCH error.

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/nfs4xdr.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

fs/nfs/nfs4xdr.c +2 -0

index b916297..ac2386b 100644
@@ -2222,6 +2222,8 @@ static int decode_compound_hdr(struct xdr_stream *xdr, struct compound_hdr *hdr)
 	hdr->tag = (char *)p;
 	p += XDR_QUADLEN(hdr->taglen);
 	READ32(hdr->nops);
+	if (unlikely(hdr->nops < 1))
+		return nfs4_stat_to_errno(hdr->status);
 	return 0;
 }

Signature · 1 line

1.5.6.4