Hi Neelakanta, Ack from me. Reviewed and tested the patch with minor inline comments. No need to send the patch for another review (for inline comments).
It would be good if error string is attached to aborted CCB (resource abort). Error string can go in another ticket, or can be fixed in this ticket. I'm leaving for you to decide. -----Original Message----- From: [email protected] [mailto:[email protected]] Sent: Friday, January 08, 2016 5:56 AM To: Hung Duc Nguyen; Zoran Milinkovic Cc: [email protected] Subject: [PATCH 1 of 1] imm : Aborted reply for augumentated Ccb should be sent to clients [#1503] osaf/services/saf/immsv/immnd/ImmModel.cc | 46 ++++++++++--- osaf/services/saf/immsv/immnd/ImmModel.hh | 2 +- osaf/services/saf/immsv/immnd/immnd_evt.c | 97 ++++++++++++++++------------- osaf/services/saf/immsv/immnd/immnd_init.h | 4 +- 4 files changed, 90 insertions(+), 59 deletions(-) When the augumented CCB or the OI for the augumented CCB crahes, then reply has to be sent to the all clients. The reply has to be sent to OM client when augumented OI crashes. The reply has to be sent to both OM and augumneted OI, when OI, for which augumented CCb is waiting crashes. diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc b/osaf/services/saf/immsv/immnd/ImmModel.cc --- a/osaf/services/saf/immsv/immnd/ImmModel.cc +++ b/osaf/services/saf/immsv/immnd/ImmModel.cc @@ -1175,15 +1175,16 @@ immModel_ccbAbort(IMMND_CB *cb, SaUint32T ccbId, SaUint32T* arrSize, SaUint32T** implConnArr, - SaUint32T* client, + SaUint32T** clientArr, + SaUint32T* clArrSize, SaUint32T* nodeId, SaClmNodeIdT* pbeNodeId) { - ConnVector cv; - ConnVector::iterator cvi; + ConnVector cv, cl; + ConnVector::iterator cvi, cli; unsigned int ix=0; - bool aborted = ImmModel::instance(&cb->immModel)->ccbAbort(ccbId, cv, client, nodeId, pbeNodeId); + bool aborted = ImmModel::instance(&cb->immModel)->ccbAbort(ccbId, cv, cl, nodeId, pbeNodeId); *arrSize = (SaUint32T) cv.size(); if(*arrSize) { @@ -1194,6 +1195,16 @@ immModel_ccbAbort(IMMND_CB *cb, } } osafassert(ix==(*arrSize)); + + ix=0; + *clArrSize = (SaUint32T) cl.size(); + if(*clArrSize){ + *clientArr = (SaUint32T *) malloc((*clArrSize)* sizeof(SaUint32T)); + for(cli = cl.begin(); cli!=cl.end(); ++cli, ++ix){ + (*clientArr)[ix] = (*cli); + } + } + osafassert(ix==(*clArrSize)); return aborted; } @@ -5838,7 +5849,7 @@ ImmModel::ccbCommit(SaUint32T ccbId, Con } bool -ImmModel::ccbAbort(SaUint32T ccbId, ConnVector& connVector, SaUint32T* client, +ImmModel::ccbAbort(SaUint32T ccbId, ConnVector& connVector, ConnVector& clVector, SaUint32T* nodeId, unsigned int* pbeNodeIdPtr) { SaUint32T pbeConn=0; @@ -5861,30 +5872,25 @@ ImmModel::ccbAbort(SaUint32T ccbId, Conn case IMM_CCB_CREATE_OP: LOG_NO("Aborting ccb %u while waiting for " "reply from implementer on CREATE-OP", ccbId); - *client = ccb->mOriginatingConn; break; case IMM_CCB_MODIFY_OP: LOG_NO("Aborting ccb %u while waiting for " "reply from implementers on MODIFY-OP", ccbId); - *client = ccb->mOriginatingConn; break; case IMM_CCB_DELETE_OP: LOG_NO("Aborting ccb %u while waiting for " "replies from implementers on DELETE-OP", ccbId); - *client = ccb->mOriginatingConn; break; case IMM_CCB_PREPARE: case IMM_CCB_VALIDATING: LOG_NO("Ccb %u aborted in COMPLETED processing (validation)", ccbId); - *client = ccb->mOriginatingConn; break; case IMM_CCB_VALIDATED: LOG_NO("Ccb %u aborted after explicit validation", ccbId); - *client = ccb->mOriginatingConn; break; case IMM_CCB_CRITICAL: @@ -5894,7 +5900,6 @@ ImmModel::ccbAbort(SaUint32T ccbId, Conn case IMM_CCB_PBE_ABORT: TRACE_5("Aborting ccb %u because PBE decided ABORT", ccbId); - *client = ccb->mOriginatingConn; break; case IMM_CCB_COMMITTED: @@ -5926,8 +5931,25 @@ ImmModel::ccbAbort(SaUint32T ccbId, Conn } } + + clVector.push_back(ccb->mOriginatingConn); *nodeId = ccb->mOriginatingNode; - *client = ccb->mOriginatingConn; + + if(ccb->mAugCcbParent && ccb->mAugCcbParent->mOriginatingConn) { + if(*nodeId && ccb->mOriginatingConn) { [Zoran] 4 lines above: "*nodeId = ccb->mOriginatingNode;" IF statement can be changed to something like: if(*nodeId) or if(ccb->mOriginatingConn) + /* Case where augumented client and Augumented parent + are originated from ths node. Send the client response + to both clients. */ + osafassert(*nodeId == ccb->mAugCcbParent->mOriginatingNode); + } else if (!ccb->mOriginatingConn) { [Zoran] The first IF statement check if ccb->mOriginatingConn is not NULL, then this IF statement can be removed and there can only be ELSE + /* Case where augumented client is not orignated from this node + and Augumented parent is originated from this node. + Send the response to the Augumented parent. */ + *nodeId = ccb->mAugCcbParent->mOriginatingNode; + clVector.pop_back(); + } + clVector.push_back(ccb->mAugCcbParent->mOriginatingConn); + } ccb->mState = IMM_CCB_ABORTED; if(ccb->mVeto == SA_AIS_OK) { diff --git a/osaf/services/saf/immsv/immnd/ImmModel.hh b/osaf/services/saf/immsv/immnd/ImmModel.hh --- a/osaf/services/saf/immsv/immnd/ImmModel.hh +++ b/osaf/services/saf/immsv/immnd/ImmModel.hh @@ -553,7 +553,7 @@ public: bool ccbAbort( SaUint32T ccbId, ConnVector& connVector, - SaUint32T* client, + ConnVector& clientVector, SaUint32T* nodeId, unsigned int* pbeNodeIdPtr); SaUint32T cleanTheBasement( diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c b/osaf/services/saf/immsv/immnd/immnd_evt.c --- a/osaf/services/saf/immsv/immnd/immnd_evt.c +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c @@ -221,7 +221,8 @@ static uint32_t immnd_evt_proc_mds_evt(I /*static uint32_t immnd_evt_immd_new_active(IMMND_CB *cb);*/ -static void immnd_evt_ccb_abort(IMMND_CB *cb, SaUint32T ccbId, SaUint32T *client, SaUint32T *nodeId); +static void immnd_evt_ccb_abort(IMMND_CB *cb, SaUint32T ccbId, SaUint32T **clientArr, + SaUint32T * clArrSize, SaUint32T *nodeId); static uint32_t immnd_evt_proc_reset(IMMND_CB *cb, IMMND_EVT *evt, IMMSV_SEND_INFO *sinfo); @@ -4096,7 +4097,7 @@ static void immnd_evt_proc_ccb_compl_rsp TRACE("Abort in immnd_evt_proc_ccb_compl_rsp reqConn: %u", reqConn); /*err != SA_AIS_OK => generate SaImmOiCcbAbortCallbackT upcall for all local implementers involved in the Ccb */ - immnd_evt_ccb_abort(cb, evt->info.ccbUpcallRsp.ccbId, NULL, NULL); + immnd_evt_ccb_abort(cb, evt->info.ccbUpcallRsp.ccbId, NULL, NULL, NULL); errStrings = immModel_ccbGrabErrStrings(cb, evt->info.ccbUpcallRsp.ccbId); } /* Either commit or abort has been decided. Ccb is now done. @@ -6726,12 +6727,13 @@ static void immnd_evt_proc_rt_object_mod TRACE_LEAVE(); } -static void immnd_evt_ccb_abort(IMMND_CB *cb, SaUint32T ccbId, SaUint32T *client, SaUint32T *nodeId) +static void immnd_evt_ccb_abort(IMMND_CB *cb, SaUint32T ccbId, SaUint32T **clientArr, SaUint32T *clArrsize, SaUint32T *nodeId) { IMMSV_EVT send_evt; SaUint32T *implConnArr = NULL; - SaUint32T arrSize = 0; - SaUint32T dummyClient = 0, dummynodeId = 0; + SaUint32T arrSize = 0, dummyClsize = 0; + SaUint32T dummynodeId = 0; + SaUint32T *dummyClient = NULL; SaImmOiHandleT implHandle = 0LL; NCS_NODE_ID pbeNodeId = 0; NCS_NODE_ID *pbeNodeIdPtr = NULL; @@ -6747,14 +6749,15 @@ static void immnd_evt_ccb_abort(IMMND_CB */ } - if(!immModel_ccbAbort(cb, ccbId, &arrSize, &implConnArr, &dummyClient, &dummynodeId, pbeNodeIdPtr)) { + if(!immModel_ccbAbort(cb, ccbId, &arrSize, &implConnArr, &dummyClient, &dummyClsize, &dummynodeId, pbeNodeIdPtr)) { goto done; } - if (client) { - *client = dummyClient; + if (clientArr) { + *clientArr = dummyClient; + *clArrsize = dummyClsize; } else { - dummyClient = 0; /* dont reply to client here*/ + dummyClient = NULL; /* dont reply to client here*/ [Zoran] Memory leak. Allocated memory for dummyClient must be freed. } if (nodeId && cb->node_id == dummynodeId) { @@ -7494,11 +7497,12 @@ static void immnd_evt_proc_ccb_finalize( SaAisErrorT err = SA_AIS_OK; IMMSV_EVT send_evt; IMMND_IMM_CLIENT_NODE *cl_node = NULL; - SaUint32T client = 0, nodeId = 0; + SaUint32T clArrSize = 0, nodeId = 0, ix = 0; + SaUint32T *clientArr = NULL; TRACE_ENTER(); osafassert(evt); - immnd_evt_ccb_abort(cb, evt->info.ccbId, &client, &nodeId); + immnd_evt_ccb_abort(cb, evt->info.ccbId, &clientArr, &clArrSize, &nodeId); [Zoran] Memory leak. Allocated memory for clientArr must be freed if (nodeId && err == SA_AIS_OK && !originatedAtThisNd) { /* nodeId will be set when CCB is originated from this node.The CCB is aborted, @@ -7506,41 +7510,46 @@ static void immnd_evt_proc_ccb_finalize( */ originatedAtThisNd = SA_TRUE; - clnt_hdl = m_IMMSV_PACK_HANDLE(client, nodeId); err = SA_AIS_ERR_FAILED_OPERATION; } else if(err == SA_AIS_OK) { TRACE_2("ccb aborted and finalized"); } if (originatedAtThisNd) { /*Send reply to client from this ND. */ - TRACE_2("ccbFinalize originated at this node => Send reply"); - /* Perhaps the following osafassert is a bit strong. A corrupt client/agent - could "accidentally" use the wrong ccbId if its heap was thrashed. - This wrong ccbid could accidentally be an existing used ccbId. - But in this case it would also have to originate at this node. - */ - TRACE_2("client == m_IMMSV_UNPACK_HANDLE_HIGH(clnt_hdl))" - "??: %u == %u", client, (SaUint32T)m_IMMSV_UNPACK_HANDLE_HIGH(clnt_hdl)); - osafassert(!client || client == m_IMMSV_UNPACK_HANDLE_HIGH(clnt_hdl)); - immnd_client_node_get(cb, clnt_hdl, &cl_node); - if (cl_node == NULL || cl_node->mIsStale) { - LOG_WA("IMMND - Client went down so no response"); - goto done; - } - - memset(&send_evt, '\0', sizeof(IMMSV_EVT)); - - send_evt.type = IMMSV_EVT_TYPE_IMMA; - send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; - send_evt.info.imma.info.errRsp.error = err; - - TRACE_2("SENDRSP %u", err); - - if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != NCSCC_RC_SUCCESS) { - LOG_WA("Failed to send response to agent/client over MDS"); - } - } - done: + do { + TRACE_2("ccbFinalize originated at this node => Send reply"); + /* Perhaps the following osafassert is a bit strong. A corrupt client/agent + could "accidentally" use the wrong ccbId if its heap was thrashed. + This wrong ccbid could accidentally be an existing used ccbId. + But in this case it would also have to originate at this node. + */ + if(clArrSize) { + clnt_hdl = m_IMMSV_PACK_HANDLE(clientArr[ix], nodeId); + TRACE_2("client == m_IMMSV_UNPACK_HANDLE_HIGH(clnt_hdl))" + "??: %u == %u", clientArr[ix], (SaUint32T)m_IMMSV_UNPACK_HANDLE_HIGH(clnt_hdl)); + osafassert(!clArrSize || clientArr[ix] == m_IMMSV_UNPACK_HANDLE_HIGH(clnt_hdl)); + } + immnd_client_node_get(cb, clnt_hdl, &cl_node); + if (cl_node == NULL || cl_node->mIsStale) { + LOG_WA("IMMND - Client went down so no response"); + } else { + + memset(&send_evt, '\0', sizeof(IMMSV_EVT)); + + send_evt.type = IMMSV_EVT_TYPE_IMMA; + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; + send_evt.info.imma.info.errRsp.error = err; + + TRACE_2("SENDRSP %u", err); + + if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != NCSCC_RC_SUCCESS) { + LOG_WA("Failed to send response to agent/client over MDS"); + } + } + ix++; + } while(clArrSize && (ix < clArrSize)); + } + err = immModel_ccbFinalize(cb, evt->info.ccbId); TRACE_LEAVE(); } @@ -7866,7 +7875,7 @@ static void immnd_evt_proc_ccb_apply(IMM applConnArr = NULL; } } else { - SaUint32T client = 0; + SaUint32T clArrSize = 0, *clArr = NULL; if(err == SA_AIS_ERR_ACCESS_DENIED) { LOG_WA("Spurious and redundant ccb-apply request ignored ccbId:%u", evt->info.ccbId); @@ -7875,8 +7884,8 @@ static void immnd_evt_proc_ccb_apply(IMM } /*err != SA_AIS_OK => generate SaImmOiCcbAbortCallbackT upcalls */ - immnd_evt_ccb_abort(cb, evt->info.ccbId, &client, NULL); - osafassert(!client || originatedAtThisNd); + immnd_evt_ccb_abort(cb, evt->info.ccbId, &clArr, &clArrSize, NULL); [Zoran] Memory leak. Allocated memory for clArr must be freed. Best regards,, Zoran + osafassert(!clArrSize || originatedAtThisNd); } TRACE_2("CCB APPLY TERMINATING CCB: %u", evt->info.ccbId); bCcbFinalize = 1; @@ -9021,7 +9030,7 @@ static void immnd_evt_proc_discard_node( SaUint32T ix; for (ix = 0; ix < arrSize; ++ix) { LOG_WA("Detected crash at node %x, abort ccbId %u", evt->info.ctrl.nodeId, idArr[ix]); - immnd_evt_ccb_abort(cb, idArr[ix], NULL, NULL); + immnd_evt_ccb_abort(cb, idArr[ix], NULL, NULL, NULL); err = immModel_ccbFinalize(cb, idArr[ix]); if (err != SA_AIS_OK) { LOG_WA("Failed to remove Ccb %u - ignoring", idArr[ix]); diff --git a/osaf/services/saf/immsv/immnd/immnd_init.h b/osaf/services/saf/immsv/immnd/immnd_init.h --- a/osaf/services/saf/immsv/immnd/immnd_init.h +++ b/osaf/services/saf/immsv/immnd/immnd_init.h @@ -145,8 +145,8 @@ extern "C" { SaUint32T **implIdArr, SaUint32T **ctnArr, bool validateOnly); bool immModel_ccbAbort(IMMND_CB *cb, - SaUint32T ccbId, SaUint32T *arrSize, SaUint32T **implConnArr, SaUint32T *client, - SaUint32T* nodeId, SaClmNodeIdT *pbeNodeId); + SaUint32T ccbId, SaUint32T *arrSize, SaUint32T **implConnArr, SaUint32T **clientArr, + SaUint32T *clArrSize, SaUint32T* nodeId, SaClmNodeIdT *pbeNodeId); void immModel_getCcbIdsForOrigCon(IMMND_CB *cb, SaUint32T origConn, SaUint32T *arrSize, SaUint32T **ccbIdArr); ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
