Hi Neel, Please ignore my comment about 'clArrSize' not being zero. I checked the code, it can be zero and in this case, the reply will be send to 'clnt_hdl' (input argument of immnd_evt_proc_ccb_finalize).
BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Hung Nguyen [email protected] Sent: Monday, January 11, 2016 2:59PM To: Neelakanta Reddy, Zoran Milinkovic [email protected], [email protected] Cc: Opensaf-devel [email protected] Subject: Re: [PATCH 1 of 1] imm : Aborted reply for augumentated Ccb should be sent to clients [#1503] Hi Neel, Reviewed and tested the patch. Ack from me with a minor comment below. BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Neelakanta Reddy [email protected] Sent: Friday, January 08, 2016 5:56AM To: Hung Nguyen, Zoran Milinkovic [email protected], [email protected] Cc: Opensaf-devel [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) { + /* 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) { + /* 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*/ } 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); 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) { [Hung] When 'originatedAtThisNd' is true, 'clArrSize' must not be zero. We don't need this if statement. + 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)); [Hung] 'clientArr[ix]' is packed into 'clnt_hdl', then we unpack 'clnt_hdl' to compare with 'clientArr[ix]'. Can you please explain this assert? Is that we don't trust m_IMMSV_PACK_HANDLE/m_IMMSV_UNPACK_HANDLE ? + } + 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); + 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
