Branch HEAD

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 84d35a0..b514c22 100644 (file)
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -12,6 +12,12 @@
        * 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   : major
 Frequency  : occasional
 Bugzilla   : 13917

diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c
index 18306b2..41d241f 100644 (file)
--- a/lustre/ldlm/ldlm_lib.c
+++ b/lustre/ldlm/ldlm_lib.c
@@ -1983,10 +1983,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 35b08ca..268c705 100644 (file)
--- a/lustre/ldlm/ldlm_lockd.c
+++ b/lustre/ldlm/ldlm_lockd.c
@@ -1112,10 +1112,9 @@ int ldlm_handle_convert0(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;
@@ -1240,10 +1239,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 3475670..d7afd79 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 f381fb8..e6c8d39 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -1035,10 +1035,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 450626e..2a9c066 100644 (file)
--- a/lustre/mds/handler.c
+++ b/lustre/mds/handler.c
@@ -515,12 +515,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));
@@ -831,7 +829,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);
         }
@@ -1001,8 +998,10 @@ static int mds_getattr_lock(struct ptlrpc_request *req, int offset,
         default:
                 mds_exit_ucred(&uc, mds);
                 if (req->rq_reply_state == NULL) {
+                        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;
@@ -1052,8 +1051,10 @@ out_pop:
         pop_ctxt(&saved, &obd->obd_lvfs_ctxt, &uc);
 out_ucred:
         if (req->rq_reply_state == NULL) {
+                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;
@@ -1085,11 +1086,11 @@ static int mds_statfs(struct ptlrpc_request *req)
                          (MDS_SERVICE_WATCHDOG_TIMEOUT / 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,
@@ -1119,11 +1120,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" */
@@ -1172,14 +1173,10 @@ static int mds_readpage(struct ptlrpc_request *req, int offset)
         struct lvfs_ucred uc = {0,};
         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);
@@ -1318,6 +1315,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")) {
@@ -1349,10 +1347,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);
@@ -2360,7 +2356,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 e21b1c5..0bce9e6 100644 (file)
--- a/lustre/mds/mds_open.c
+++ b/lustre/mds/mds_open.c
@@ -813,6 +813,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));
 
@@ -1444,13 +1445,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,
@@ -1532,10 +1531,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 4bb2054..733c8b9 100644 (file)
--- a/lustre/ost/ost_handler.c
+++ b/lustre/ost/ost_handler.c
@@ -1332,10 +1332,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 e4ff115..b561523 100644 (file)
--- a/lustre/ptlrpc/pack_generic.c
+++ b/lustre/ptlrpc/pack_generic.c
@@ -387,6 +387,7 @@ EXPORT_SYMBOL(lustre_pack_reply_v2);
 int lustre_pack_reply(struct ptlrpc_request *req, int count, int *lens,
                       char **bufs)
 {
+        int rc = 0;
         int size[] = { sizeof(struct ptlrpc_body) };
 
         if (!lens) {
@@ -400,16 +401,22 @@ int lustre_pack_reply(struct ptlrpc_request *req, int count, int *lens,
         switch (req->rq_reqmsg->lm_magic) {
         case LUSTRE_MSG_MAGIC_V1:
         case LUSTRE_MSG_MAGIC_V1_SWABBED:
-                return lustre_pack_reply_v1(req, count - 1, lens + 1,
-                                            bufs ? bufs + 1 : NULL);
+                rc = lustre_pack_reply_v1(req, count - 1, lens + 1,
+                                          bufs ? bufs + 1 : NULL);
+                break;
         case LUSTRE_MSG_MAGIC_V2:
         case LUSTRE_MSG_MAGIC_V2_SWABBED:
-                return lustre_pack_reply_v2(req, count, lens, bufs);
+                rc = lustre_pack_reply_v2(req, count, lens, bufs);
+                break;
         default:
                 LASSERTF(0, "incorrect message magic: %08x\n",
                          req->rq_reqmsg->lm_magic);
-                return -EINVAL;
+                rc = -EINVAL;
         }
+        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;
 }
 
 void *lustre_msg_buf_v1(void *msg, int n, int min_size)