[LU-7715] out_handle() misuses GOTO() Created: 26/Jan/16  Updated: 04/Feb/16  Resolved: 04/Feb/16

Status: Resolved
Project: Lustre
Component/s: None
Affects Version/s: None
Fix Version/s: Lustre 2.8.0

Type: Bug Priority: Blocker
Reporter: John Hammond Assignee: Di Wang
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Related
is related to LU-7039 llog_osd.c:778:llog_osd_next_block())... Resolved
Severity: 3
Rank (Obsolete): 9223372036854775807

 Description   

In out_handle() we have two GOTO(out_free, err_serious(rc)) that should be GOTO(out_free, rc = err_serious(rc)). If followed these will cause an LBUG since the reply has not been packed yet.



 Comments   
Comment by Joseph Gmitter (Inactive) [ 27/Jan/16 ]

Hi Di,

Can you have a look to see if this is related to the LU-7039 landing? Discussion at triage today seemed to lead down that path.

Thanks.
Joe

Comment by Di Wang [ 27/Jan/16 ]

Hmm, according to the comments in tgt_handle_request0(), all of requests handler should return serious error if the reply is not packed yet. so out_handle() should all return err_serious() before req_capsule_server_pack(). I do not think this is related with the landing of LU-7039.

Comment by John Hammond [ 27/Jan/16 ]

> I do not think this is related with the landing of LU-7039.

http://review.whamcloud.com/#/c/16969/34/lustre/target/out_handler.c

Comment by Di Wang [ 27/Jan/16 ]

do you mean this? So if it return with non-serious error, and did not pack reply, then it will panic.

               /*
                 * Process request, there can be two types of rc:
                 * 1) errors with msg unpack/pack, other failures outside the
                 * operation itself. This is counted as serious errors;
                 * 2) errors during fs operation, should be placed in rq_status
                 * only
                 */
                rc = h->th_act(tsi);
                if (!is_serious(rc) &&
                    !req->rq_no_reply && req->rq_reply_state == NULL) {
                        DEBUG_REQ(D_ERROR, req, "%s \"handler\" %s did not "
                                  "pack reply and returned 0 error\n",
                                  tgt_name(tsi->tsi_tgt), h->th_name);
                        LBUG();
                }

So the patch for LU-7039 turns a few normal error to serious error, though it did not convert all of them. Why this is related with the fix of LU-7039? or I miss sth?

Comment by Di Wang [ 27/Jan/16 ]

ah, sorry, I missed the comments.

Comment by Gerrit Updater [ 27/Jan/16 ]

wangdi (di.wang@intel.com) uploaded a new patch: http://review.whamcloud.com/18187
Subject: LU-7715 out: fix err_serious in out_handle
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 3c11d3024df04989c7499004560a8358cdce1e11

Comment by Gerrit Updater [ 04/Feb/16 ]

Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/18187/
Subject: LU-7715 out: fix err_serious in out_handle
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 7ab18ff6ffef4f5267e3893210a5fbc6cb46efb0

Comment by Joseph Gmitter (Inactive) [ 04/Feb/16 ]

Landed for 2.8

Generated at Sat Feb 10 02:11:18 UTC 2024 using Jira 9.4.14#940014-sha1:734e6822bbf0d45eff9af51f82432957f73aa32c.