From 27dd14f3dfdbcad29b29270c33b69c1e28cdecfa Mon Sep 17 00:00:00 2001 From: bobijam Date: Thu, 22 Nov 2007 04:03:14 +0000 Subject: [PATCH] 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. --- lustre/ChangeLog | 6 ++++++ lustre/ldlm/ldlm_lib.c | 5 ++--- lustre/ldlm/ldlm_lockd.c | 13 +++++-------- lustre/liblustre/rw.c | 4 +--- lustre/llite/file.c | 4 +--- lustre/mds/handler.c | 45 +++++++++++++++++++++----------------------- lustre/mds/mds_open.c | 11 ++++------- lustre/ost/ost_handler.c | 6 ++---- lustre/ptlrpc/pack_generic.c | 15 +++++++++++---- 9 files changed, 53 insertions(+), 56 deletions(-) diff --git a/lustre/ChangeLog b/lustre/ChangeLog index 84d35a0..b514c22 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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) -- 1.8.3.1