Branch b1_6

b=11791
i=adilger, nathan.rutman

Description: Inconsistent usage of lustre_pack_reply()
Details    : Standardize the usage of lustre_pack_reply() such that it
             always generate a CERROR on failure.

diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index 3517f67..d4f37e2 100644 (file)
--- 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 fa45e04..57a5cee 100644 (file)
--- 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 aac218c..a8dd851 100644 (file)
--- 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 16146f9..76d6c3d 100644 (file)
--- 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 e90f1c1..c0d6f51 100644 (file)
--- 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 edaa77f..fba1806 100644 (file)
--- 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 f287664..b2aed16 100644 (file)
--- 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 69e12f9..5a5bd2e 100644 (file)
--- 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 dc31d08..d27188e 100644 (file)
--- 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;
 }