Hi AndersBj,
Reviewed and tested the patch.
Ack when pushed with below inline comments:
/Neel
On Wednesday 24 September 2014 04:47 PM, Anders Bjornerstedt wrote:
> osaf/services/saf/immsv/immnd/immnd_evt.c | 63
> ++++++++++++++++++++----------
> 1 files changed, 42 insertions(+), 21 deletions(-)
>
>
> The fix is mainly in immnd_evt_proc_ccb_apply, but also some cleanup in
> immnd_evt_proc_ccb_compl_rsp where part of the same problem was addressed
> by the earlier ticket: #1096.
>
> The general case is that the ccb has just gone critical in ImmModel
> (at all IMMNDs) and one IMMND is just about to send the completed
> callback to the PBE. The completed callback can be sent to PBE either
> in 'proc_ccb_apply' when there are no regular OIs involved with the ccb;
> or in 'proc_ccb_compl_rsp' if there are regular OIs and the reply from
> all of them has been received.
>
> Solution:
>
> The impossible case of the PBE-OI being known to ImmModel yet the
> client_node not existing is handled by osafassert.
>
> The rare but possible case of the client_node existing but being stale
> is handled by skipping over the send-attempt (which would fail) and
> letting ccb-recovery sort things out.
>
> The rare but possible case of the client_node being ok but the send
> over MDS resulting in an error return from MDS is also handled by letting
> ccb-recovery sort it out. Note that the handling an error code from MDS
> for this case as the send having been processed is safer, since the message
> could possibly have reached the PBE despite that MDS reported error.
>
> The client node stale case could in principle have been handled by aborting
> the CCB, but the ImmModel has entered the critical state for this ccb at
> all nodes and so such a solution would require a new message type:
> revert-critical-and-abort-ccb being broadcast. Just broadcasting the
> currenttly supported ccb-abort would not work because ccbs in critical
> will (correctly) discard such a request.
>
> 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
> @@ -3733,19 +3733,21 @@ static void immnd_evt_proc_ccb_compl_rsp
> SaImmOiHandleT implHandle =
> m_IMMSV_PACK_HANDLE(pbeConn, pbeNodeId);
> /*Fetch client node for OI ! */
> immnd_client_node_get(cb, implHandle,
> &oi_cl_node);
> - if (oi_cl_node == NULL || oi_cl_node->mIsStale)
> {
> + osafassert(cl_node);
osafassert(oi_cl_node)
> + if (oi_cl_node->mIsStale) {
> LOG_WA("PBE went down");
> - /* ###TODO need to ABORT ccb in
> CRITICAL. or set CRITICAL below*/
> /* This is a bad case. The immnds have
> just delegated the decision
> - to commit or abort this ccb to the
> PBE, yet it has just crashed.
> - I should mark the ccb being in limbo
> until a new PBE has attached,
> - then resend the completed upcall,
> which should generate an aborted
> - reply to all IMMNDs.
> - err = SA_AIS_ERR_FAILED_OPERATION;
> - But actually we know here that we
> have not sent the completed call to
> - the pbe. This means it should be
> easy to abort!
> + to commit or abort this ccb to the
> PBE, yet it has just detached.
> + We know here that we have not sent
> the completed call to the pbe,
> + so in principle the abort should be
> simple. But the Ccb has just
> + entered the critical state in
> ImmModel, so we would need to undo
> + that critical state. Problem is it
> would have to be undone at all
> + IMMNDs which is complicated. Instead
> of optimizing this error case
> + with complex logic and messaging, we
> unify it with the case of
> + having sent completed to the PBE,
> not getting any response and
> + let ccb-recovery sort it out.
> */
> - goto done;
> + goto skip_send;
> }
>
> memset(&send_evt, '\0', sizeof(IMMSV_EVT));
> @@ -3759,13 +3761,17 @@ static void immnd_evt_proc_ccb_compl_rsp
> TRACE_2("MAKING PBE-IMPLEMENTER CCB COMPLETED
> upcall");
> if(immnd_mds_msg_send(cb, NCSMDS_SVC_ID_IMMA_OI,
> oi_cl_node->agent_mds_dest,
> &send_evt) != NCSCC_RC_SUCCESS) {
> - LOG_ER("CCB COMPLETED UPCALL SEND TO
> PBE FAILED");
> + LOG_WA("CCB COMPLETED UPCALL SEND TO
> PBE FAILED");
> + /* If the send to PBE fails in Mds then
> we *dont* know for sure
> + that the message did not reach the
> PBE. Therefore act as if
> + send succceeded and let ccb-recovery
> sort it out.
> + */
> } else {
> TRACE_5("IMMND UPCALL TO PBE for ccb
> %u, SEND SUCCEEDED",
> evt->info.ccbUpcallRsp.ccbId);
> }
> -
> - }
> + }
> + skip_send:
> reqConn = 0; /* Ensure we dont reply to OM client yet.
> */
> }
> } else {
> @@ -7358,7 +7364,7 @@ static void immnd_evt_proc_ccb_apply(IMM
> delayedReply = SA_TRUE;
> if(pbeNodeId) {
> /* There is be a PBE. */
> - osafassert(err == SA_AIS_OK); /* I not OK then
> we should not be waiting. */
> + osafassert(err == SA_AIS_OK); /* If not OK then
> we should not be waiting. */
> TRACE_5("Wait for PBE commit decision for ccb
> %u", evt->info.ccbId);
> if(pbeConn) {
> TRACE_5("PBE is LOCAL - send completed
> upcall for %u", evt->info.ccbId);
> @@ -7369,11 +7375,21 @@ static void immnd_evt_proc_ccb_apply(IMM
> SaImmOiHandleT implHandle =
> m_IMMSV_PACK_HANDLE(pbeConn, pbeNodeId);
> /*Fetch client node for OI ! */
> immnd_client_node_get(cb, implHandle,
> &oi_cl_node);
> - if (oi_cl_node == NULL ||
> oi_cl_node->mIsStale) {
> - LOG_WA("PBE went down");
> - /* TODO need to ABORT ccb in
> CRITICAL. or set CRITICAL below*/
> - err =
> SA_AIS_ERR_FAILED_OPERATION;
> - abort();
> + osafassert(cl_node);
osafassert(oi_cl_node)
> + if (oi_cl_node->mIsStale) {
> + LOG_WA("PBE-OI has detached");
> + /* This is a bad case. The
> immnds have just delegated the decision
> + to commit or abort this ccb
> to the PBE, yet it has just detached.
> + We know here that we have
> not sent the completed call to the pbe,
> + so in principle the abort
> should be simple. But the Ccb has just
> + entered the critical state
> in ImmModel, so we would need to undo
> + that critical state. Problem
> is it would have to be undone at all
> + IMMNDs which is complicated.
> Instead of optimizing this error case
> + with complex logic and
> messaging, we unify it with the case of
> + having sent completed to the
> PBE, not getting any response and
> + let ccb-recovery sort it out.
> + */
> + goto skip_send;
> }
>
> memset(&send_evt, '\0',
> sizeof(IMMSV_EVT));
> @@ -7387,8 +7403,11 @@ static void immnd_evt_proc_ccb_apply(IMM
> TRACE_2("MAKING PBE-IMPLEMENTER CCB
> COMPLETED upcall");
> if(immnd_mds_msg_send(cb,
> NCSMDS_SVC_ID_IMMA_OI,
> oi_cl_node->agent_mds_dest,
> &send_evt) != NCSCC_RC_SUCCESS) {
> - LOG_ER("CCB COMPLETED UPCALL
> SEND TO PBE FAILED");
> - abort();
> + LOG_WA("CCB COMPLETED UPCALL
> SEND TO PBE FAILED");
> + /* If the send to PBE fails in
> Mds then we *dont* know for sure
> + that the message did not
> reach the PBE. Therefore act as if
> + send succceeded and let
> ccb-recovery sort it out.
> + */
> } else {
> TRACE_5("IMMND UPCALL TO PBE
> for ccb %u, SEND SUCCEEDED",
> evt->info.ccbId);
> @@ -7400,6 +7419,8 @@ static void immnd_evt_proc_ccb_apply(IMM
> TRACE("delayedReply:%u", delayedReply);
> }
> }
> +
> + skip_send:
> /* err != SA_AIS_OK or no implementers => immediate reply. */
> if (arrSize) {
> free(implConnArr);
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
