diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index 3517f6791deb5f03ebfb5005301b54f2e12d603d..d4f37e213087c13ec8db46ab66f67280b2c8c2d6 100644
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -14,6 +14,11 @@ tbd Cluster File Systems, Inc. <info@clusterfs.com>
* Recommended e2fsprogs version: 1.40.2-cfs4
* Note that reiserfs quotas are disabled on SLES 10 in this kernel.
+Severity : normal
+Bugzilla : 11791
+Description: Inconsistent usage of lustre_pack_reply()
+Details : Standardize the usage of lustre_pack_reply() such that it
+ always generate a CERROR on failure.
Severity : normal
Frequency : very rare
diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c
index fa45e044150c1bf9c310535ede2ef495e89653a2..57a5cee30fcdebf687cac811df5d46eb34e140ae 100644
--- a/lustre/ldlm/ldlm_lib.c
+++ b/lustre/ldlm/ldlm_lib.c
@@ -994,12 +994,11 @@ static void abort_recovery_queue(struct obd_device *obd)
req->rq_status = -ENOTCONN;
req->rq_type = PTL_RPC_MSG_ERR;
rc = lustre_pack_reply(req, 1, NULL, NULL);
- if (rc == 0) {
+ if (rc == 0)
ptlrpc_reply(req);
- } else {
+ else
DEBUG_REQ(D_ERROR, req,
"packing failed for abort-reply; skipping");
- }
target_release_saved_req(req);
}
}
@@ -1605,10 +1604,9 @@ int target_handle_dqacq_callback(struct ptlrpc_request *req)
ENTRY;
rc = lustre_pack_reply(req, 2, repsize, NULL);
- if (rc) {
- CERROR("packing reply failed!: rc = %d\n", rc);
+ if (rc)
RETURN(rc);
- }
+
LASSERT(req->rq_export);
/* fixed for bug10707 */
diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c
index aac218c5e28a99cb964cf610d946fbaa567fd248..a8dd851980fd837ecd91ddb9c08554944c748f58 100644
--- a/lustre/ldlm/ldlm_lockd.c
+++ b/lustre/ldlm/ldlm_lockd.c
@@ -1091,10 +1091,9 @@ int ldlm_handle_convert(struct ptlrpc_request *req)
LDLM_CONVERT - LDLM_FIRST_OPC);
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc) {
- CERROR("out of memory\n");
- RETURN(-ENOMEM);
- }
+ if (rc)
+ RETURN(rc);
+
dlm_rep = lustre_msg_buf(req->rq_repmsg, DLM_LOCKREPLY_OFF,
sizeof(*dlm_rep));
dlm_rep->lock_flags = dlm_req->lock_flags;
@@ -1201,10 +1200,8 @@ int ldlm_handle_cancel(struct ptlrpc_request *req)
LDLM_CANCEL - LDLM_FIRST_OPC);
rc = lustre_pack_reply(req, 1, NULL, NULL);
- if (rc) {
- CERROR("out of memory\n");
- RETURN(-ENOMEM);
- }
+ if (rc)
+ RETURN(rc);
if (!ldlm_request_cancel(req, dlm_req, 0))
req->rq_status = ESTALE;
diff --git a/lustre/liblustre/rw.c b/lustre/liblustre/rw.c
index 16146f9549ed28a63871a664b0d2ce55e378f637..76d6c3de35ac86df1fdf57978053525795d0319f 100644
--- a/lustre/liblustre/rw.c
+++ b/lustre/liblustre/rw.c
@@ -196,10 +196,8 @@ static int llu_glimpse_callback(struct ldlm_lock *lock, void *reqp)
stripe = llu_lock_to_stripe_offset(inode, lock);
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc) {
- CERROR("lustre_pack_reply: %d\n", rc);
+ if (rc)
GOTO(iput, rc);
- }
lvb = lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF, sizeof(*lvb));
lvb->lvb_size = lli->lli_smd->lsm_oinfo[stripe]->loi_kms;
diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index e90f1c188907b95c4cb3e7af95c9eaf141e8640a..c0d6f51192f44b87db976333b5b8c7477a52d65a 100644
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -934,10 +934,8 @@ static int ll_glimpse_callback(struct ldlm_lock *lock, void *reqp)
GOTO(iput, rc = -ELDLM_NO_LOCK_DATA);
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc) {
- CERROR("lustre_pack_reply: %d\n", rc);
+ if (rc)
GOTO(iput, rc);
- }
lvb = lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF, sizeof(*lvb));
lvb->lvb_size = lli->lli_smd->lsm_oinfo[stripe]->loi_kms;
diff --git a/lustre/mds/handler.c b/lustre/mds/handler.c
index edaa77f1bce92cd087148b3d4649e992a908a7b6..fba1806132e45cca1624dbe5bc4342df69b7552c 100644
--- a/lustre/mds/handler.c
+++ b/lustre/mds/handler.c
@@ -542,12 +542,10 @@ static int mds_getstatus(struct ptlrpc_request *req)
int rc, size[2] = { sizeof(struct ptlrpc_body), sizeof(*body) };
ENTRY;
+ OBD_FAIL_RETURN(OBD_FAIL_MDS_GETSTATUS_PACK, req->rq_status = -ENOMEM);
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc || OBD_FAIL_CHECK(OBD_FAIL_MDS_GETSTATUS_PACK)) {
- CERROR("mds: out of memory for message\n");
- req->rq_status = -ENOMEM; /* superfluous? */
- RETURN(-ENOMEM);
- }
+ if (rc)
+ RETURN(req->rq_status = rc);
body = lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF, sizeof(*body));
memcpy(&body->fid1, &mds->mds_rootfid, sizeof(body->fid1));
@@ -873,7 +871,6 @@ static int mds_getattr_pack_msg(struct ptlrpc_request *req, struct inode *inode,
rc = lustre_pack_reply(req, bufcount, size, NULL);
if (rc) {
- CERROR("lustre_pack_reply failed: rc %d\n", rc);
req->rq_status = rc;
RETURN(rc);
}
@@ -1043,8 +1040,10 @@ static int mds_getattr_lock(struct ptlrpc_request *req, int offset,
default:
mds_exit_ucred(&uc, mds);
if (!req->rq_packed_final) {
+ int rc2 = lustre_pack_reply(req, 1, NULL, NULL);
+ if (rc == 0)
+ rc = rc2;
req->rq_status = rc;
- lustre_pack_reply(req, 1, NULL, NULL);
}
}
return rc;
@@ -1093,8 +1092,10 @@ out_pop:
pop_ctxt(&saved, &obd->obd_lvfs_ctxt, &uc);
out_ucred:
if (!req->rq_packed_final) {
+ int rc2 = lustre_pack_reply(req, 1, NULL, NULL);
+ if (rc == 0)
+ rc = rc2;
req->rq_status = rc;
- lustre_pack_reply(req, 1, NULL, NULL);
}
mds_exit_ucred(&uc, mds);
return rc;
@@ -1128,11 +1129,11 @@ static int mds_statfs(struct ptlrpc_request *req)
at_get(&svc->srv_at_estimate) / 1000) + 1);
OBD_COUNTER_INCREMENT(obd, statfs);
+ if (OBD_FAIL_CHECK(OBD_FAIL_MDS_STATFS_PACK))
+ GOTO(out, rc = -ENOMEM);
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc || OBD_FAIL_CHECK(OBD_FAIL_MDS_STATFS_PACK)) {
- CERROR("mds: statfs lustre_pack_reply failed: rc = %d\n", rc);
+ if (rc)
GOTO(out, rc);
- }
/* We call this so that we can cache a bit - 1 jiffie worth */
rc = mds_obd_statfs(obd, lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF,
@@ -1162,11 +1163,11 @@ static int mds_sync(struct ptlrpc_request *req, int offset)
if (body == NULL)
GOTO(out, rc = -EFAULT);
+ if (OBD_FAIL_CHECK(OBD_FAIL_MDS_SYNC_PACK))
+ GOTO(out, rc = -ENOMEM);
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc || OBD_FAIL_CHECK(OBD_FAIL_MDS_SYNC_PACK)) {
- CERROR("fsync lustre_pack_reply failed: rc = %d\n", rc);
+ if (rc)
GOTO(out, rc);
- }
if (body->fid1.id == 0) {
/* a fid of zero is taken to mean "sync whole filesystem" */
@@ -1215,14 +1216,10 @@ static int mds_readpage(struct ptlrpc_request *req, int offset)
struct lvfs_ucred uc = {NULL,};
ENTRY;
- if (OBD_FAIL_CHECK(OBD_FAIL_MDS_READPAGE_PACK))
- RETURN(-ENOMEM);
-
+ OBD_FAIL_RETURN(OBD_FAIL_MDS_READPAGE_PACK, -ENOMEM);
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc) {
- CERROR("error packing readpage reply: rc %d\n", rc);
+ if (rc)
GOTO(out, rc);
- }
body = lustre_swab_reqbuf(req, offset, sizeof(*body),
lustre_swab_mds_body);
@@ -1359,6 +1356,7 @@ static int mds_set_info_rpc(struct obd_export *exp, struct ptlrpc_request *req)
rc = lustre_pack_reply(req, 1, NULL, NULL);
if (rc)
RETURN(rc);
+
lustre_msg_set_status(req->rq_repmsg, 0);
if (KEY_IS("read-only")) {
@@ -1390,10 +1388,8 @@ static int mds_handle_quotacheck(struct ptlrpc_request *req)
RETURN(-EPROTO);
rc = lustre_pack_reply(req, 1, NULL, NULL);
- if (rc) {
- CERROR("mds: out of memory while packing quotacheck reply\n");
+ if (rc)
RETURN(rc);
- }
req->rq_status = obd_quotacheck(req->rq_export, oqctl);
RETURN(0);
@@ -2415,7 +2411,8 @@ static int mds_intent_policy(struct ldlm_namespace *ns,
if (lustre_msg_bufcount(req->rq_reqmsg) <= DLM_INTENT_IT_OFF) {
/* No intent was provided */
rc = lustre_pack_reply(req, 2, repsize, NULL);
- LASSERT(rc == 0);
+ if (rc)
+ RETURN(rc);
RETURN(0);
}
diff --git a/lustre/mds/mds_open.c b/lustre/mds/mds_open.c
index f287664fcd87e0534544d04798885f53c0f35e60..b2aed1631cd912f1202c02de5ace837eb7dfe3ab 100644
--- a/lustre/mds/mds_open.c
+++ b/lustre/mds/mds_open.c
@@ -817,6 +817,7 @@ int mds_pin(struct ptlrpc_request *req, int offset)
rc = lustre_pack_reply(req, 2, size, NULL);
if (rc)
RETURN(rc);
+
repbody = lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF,
sizeof(*repbody));
@@ -1442,13 +1443,11 @@ int mds_close(struct ptlrpc_request *req, int offset)
ENTRY;
rc = lustre_pack_reply(req, 4, repsize, NULL);
- if (rc) {
- CERROR("lustre_pack_reply: rc = %d\n", rc);
+ if (rc)
req->rq_status = rc;
/* continue on to drop local open even if we can't send reply */
- } else {
+ else
MDS_CHECK_RESENT(req, mds_reconstruct_generic(req));
- }
CDEBUG(D_INODE, "close req->rep_len %d mdsize %d cookiesize %d\n",
req->rq_replen,
@@ -1533,10 +1532,8 @@ int mds_done_writing(struct ptlrpc_request *req, int offset)
}
rc = lustre_pack_reply(req, 2, size, NULL);
- if (rc) {
- CERROR("lustre_pack_reply: rc = %d\n", rc);
+ if (rc)
req->rq_status = rc;
- }
RETURN(0);
}
diff --git a/lustre/ost/ost_handler.c b/lustre/ost/ost_handler.c
index 69e12f9be19314cb764f7c54a19345eed27c9a3a..5a5bd2e73c4cc2b2b61a7639d587149ddf7467cd 100644
--- a/lustre/ost/ost_handler.c
+++ b/lustre/ost/ost_handler.c
@@ -991,6 +991,7 @@ static int ost_brw_write(struct ptlrpc_request *req, struct obd_trans_info *oti)
rc = lustre_pack_reply(req, 3, size, NULL);
if (rc != 0)
GOTO(out, rc);
+
OBD_FAIL_TIMEOUT(OBD_FAIL_OST_BRW_PAUSE_PACK, obd_fail_val);
rcs = lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF + 1,
niocount * sizeof(*rcs));
@@ -1334,10 +1335,8 @@ static int ost_handle_quotacheck(struct ptlrpc_request *req)
RETURN(-EPROTO);
rc = lustre_pack_reply(req, 1, NULL, NULL);
- if (rc) {
- CERROR("ost: out of memory while packing quotacheck reply\n");
- RETURN(-ENOMEM);
- }
+ if (rc)
+ RETURN(rc);
req->rq_status = obd_quotacheck(req->rq_export, oqctl);
RETURN(0);
diff --git a/lustre/ptlrpc/pack_generic.c b/lustre/ptlrpc/pack_generic.c
index dc31d08a3eb3079e41a0f6b1c1ed47f0a836f003..d27188ecbda9de006d8b9c7c740e5b70c9b1f250 100644
--- a/lustre/ptlrpc/pack_generic.c
+++ b/lustre/ptlrpc/pack_generic.c
@@ -454,7 +454,11 @@ int lustre_pack_reply_flags(struct ptlrpc_request *req, int count, int *lens,
int lustre_pack_reply(struct ptlrpc_request *req, int count, int *lens,
char **bufs)
{
- return lustre_pack_reply_flags(req, count, lens, bufs, 0);
+ int rc = lustre_pack_reply_flags(req, count, lens, bufs, 0);
+ if (rc != 0)
+ CERROR("lustre_pack_reply failed: rc=%d size=%d\n", rc,
+ lustre_msg_size(req->rq_reqmsg->lm_magic, count, lens));
+ return rc;
}