Gentle reminder for review !! Thanks -Nagu > -----Original Message----- > From: Nagendra Kumar > Sent: 14 October 2015 14:17 > To: [email protected]; Praveen Malviya > Cc: [email protected] > Subject: [devel] [PATCH 1 of 1] amfd: return BAD OP in fault cases during si > swap [#1294] > > osaf/services/saf/amf/amfd/include/si.h | 1 + > osaf/services/saf/amf/amfd/sg_2n_fsm.cc | 67 +---------- > osaf/services/saf/amf/amfd/sg_nway_fsm.cc | 31 +---- > osaf/services/saf/amf/amfd/si.cc | 191 > ++++++++++++++++++++++++++++++ > 4 files changed, 194 insertions(+), 96 deletions(-) > > > Return SA_AIS_ERR_BAD_OPERATION for SI SWAP admin op when 1. SI is > locked. > 2. Only one SU is configured. > 3. None of other SUs or its hosting nodes are in unlocked state. > 4. All other nodes hosting SUs are down. > 5. Other SUs are in inst/term failed state or Out of service. > > Return SA_AIS_ERR_TRY_AGAIN for SI SWAP admin op when 1. Only one > assignment for M/w SI. This is in assumption that the another controller will > come and join shortly. > 2. If nodes hosting other unlocked SUs are joining. > 3. If other SUs are in INSTANTIATING/TERMINATING/RESTARTING state. > > diff --git a/osaf/services/saf/amf/amfd/include/si.h > b/osaf/services/saf/amf/amfd/include/si.h > --- a/osaf/services/saf/amf/amfd/include/si.h > +++ b/osaf/services/saf/amf/amfd/include/si.h > @@ -141,6 +141,7 @@ public: > bool is_sirank_valid(uint32_t newSiRank) const; > void update_sirank(uint32_t newSiRank); > bool si_dep_states_check(); > + SaAisErrorT si_swap_validate(); > private: > AVD_SI(const AVD_SI&); > AVD_SI& operator=(const AVD_SI&); > diff --git a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > --- a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > +++ b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > @@ -736,67 +736,11 @@ done: > SaAisErrorT SG_2N::si_swap(AVD_SI *si, SaInvocationT invocation) { > AVD_SU_SI_REL *susi; > SaAisErrorT rc = SA_AIS_OK; > - AVD_AVND *node; > > TRACE_ENTER2("'%s' sg_fsm_state=%u", si->name.value, si- > >sg_of_si->sg_fsm_state); > > - if (si->saAmfSIAdminState != SA_AMF_ADMIN_UNLOCKED) { > - LOG_ER("%s SWAP failed - wrong admin state=%u", si- > >name.value, > - si->saAmfSIAdminState); > - rc = SA_AIS_ERR_TRY_AGAIN; > + if ((rc = si->si_swap_validate()) != SA_AIS_OK) > goto done; > - } > - > - if (avd_cb->init_state != AVD_APP_STATE) { > - LOG_ER("%s SWAP failed - not in app state (%u)", si- > >name.value, > - avd_cb->init_state); > - rc = SA_AIS_ERR_TRY_AGAIN; > - goto done; > - } > - > - if (si->sg_of_si->sg_fsm_state != AVD_SG_FSM_STABLE) { > - LOG_ER("%s SWAP failed - SG not stable (%u)", si- > >name.value, > - si->sg_of_si->sg_fsm_state); > - rc = SA_AIS_ERR_TRY_AGAIN; > - goto done; > - } > - > - if (si->list_of_sisu == NULL) { > - LOG_ER("%s SWAP failed - no assignments to swap", si- > >name.value); > - rc = SA_AIS_ERR_BAD_OPERATION; > - goto done; > - } > - > - if (si->sg_of_si->sg_ncs_spec) { > - if (avd_cb->stby_sync_state == AVD_STBY_OUT_OF_SYNC) { > - LOG_ER("%s SWAP failed - Cold sync in progress", si- > >name.value); > - rc = SA_AIS_ERR_TRY_AGAIN; > - goto done; > - } > - } > - > - if (si->list_of_sisu->si_next == NULL) { > - LOG_ER("%s SWAP failed - only one assignment", si- > >name.value); > - rc = SA_AIS_ERR_TRY_AGAIN; > - goto done; > - } > - > - /* Since middleware components can still have si->list_of_sisu- > >si_next as not NULL, but we need to check > - whether it is unlocked. We need to reject si_swap on controllers > when stdby controller is locked. */ > - if (si->sg_of_si->sg_ncs_spec) { > - /* Check if the Standby is there in unlocked state. */ > - node = avd_node_find_nodeid(avd_cb->node_id_avd_other); > - if (node == NULL) { > - LOG_NO("SI Swap not possible, node %x is not > available", avd_cb->node_id_avd_other); > - rc = SA_AIS_ERR_BAD_OPERATION; > - goto done; > - } > - if (SA_FALSE == node->node_info.member) { > - LOG_NO("SI Swap not possible, node %x is locked", > avd_cb->node_id_avd_other); > - rc = SA_AIS_ERR_BAD_OPERATION; > - goto done; > - } > - } > > /* Identify the active susi rel */ > if (si->list_of_sisu->state == SA_AMF_HA_ACTIVE) { @@ -809,15 > +753,6 @@ SaAisErrorT SG_2N::si_swap(AVD_SI *si, S > goto done; > } > > - /* If the swap is on m/w si, then check whether any ccb was going > on. */ > - if (si->sg_of_si->sg_ncs_spec) { > - if (ccbutil_EmptyCcbExists() == false) { > - rc = SA_AIS_ERR_TRY_AGAIN; > - LOG_NO("%s SWAP failed - Ccb going on", si- > >name.value); > - goto done; > - } > - } > - > /* Check if there is dependency between SI's within SU */ > if (avd_sidep_si_dependency_exists_within_su(susi->su)) { > if (avd_sg_susi_mod_snd_honouring_si_dependency(susi- > >su, SA_AMF_HA_QUIESCED) == NCSCC_RC_FAILURE) { diff --git > a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > --- a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > +++ b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > @@ -3532,32 +3532,8 @@ SaAisErrorT SG_NWAY::si_swap(AVD_SI *si, > > TRACE_ENTER2("'%s' sg_fsm_state=%u", si->name.value, si- > >sg_of_si->sg_fsm_state); > > - if (si->saAmfSIAdminState != SA_AMF_ADMIN_UNLOCKED) { > - LOG_NO("%s SWAP failed - wrong admin state=%u", si- > >name.value, > - si->saAmfSIAdminState); > - rc = SA_AIS_ERR_TRY_AGAIN; > + if ((rc = si->si_swap_validate()) != SA_AIS_OK) > goto done; > - } > - > - if (avd_cb->init_state != AVD_APP_STATE) { > - LOG_NO("%s SWAP failed - not in app state (%u)", si- > >name.value, > - avd_cb->init_state); > - rc = SA_AIS_ERR_TRY_AGAIN; > - goto done; > - } > - > - if (si->sg_of_si->sg_fsm_state != AVD_SG_FSM_STABLE) { > - LOG_NO("%s SWAP failed - SG not stable (%u)", si- > >name.value, > - si->sg_of_si->sg_fsm_state); > - rc = SA_AIS_ERR_TRY_AGAIN; > - goto done; > - } > - > - if (si->list_of_sisu == NULL) { > - LOG_NO("%s SWAP failed - no assignments to swap", si- > >name.value); > - rc = SA_AIS_ERR_BAD_OPERATION; > - goto done; > - } > > if ((sg->equal_ranked_su == true) && (sg->saAmfSGAutoAdjust == > SA_TRUE)) { > LOG_NO("%s Equal distribution is enabled, si-swap not > allowed", si->name.value); @@ -3569,11 +3545,6 @@ SaAisErrorT > SG_NWAY::si_swap(AVD_SI *si, > rc = SA_AIS_ERR_BAD_OPERATION; > goto done; > } > - if (si->list_of_sisu->si_next == NULL) { > - LOG_NO("%s SWAP failed - only one assignment", si- > >name.value); > - rc = SA_AIS_ERR_BAD_OPERATION; > - goto done; > - } > > /* > After SI swap, there should not be violation of > saAmfSGMaxActiveSIsperSU. > diff --git a/osaf/services/saf/amf/amfd/si.cc > b/osaf/services/saf/amf/amfd/si.cc > --- a/osaf/services/saf/amf/amfd/si.cc > +++ b/osaf/services/saf/amf/amfd/si.cc > @@ -1500,3 +1500,194 @@ void AVD_SI::update_sirank(uint32_t newS > saAmfSIRank = (newSiRank == 0) ? ~0U : newSiRank; > sg->add_si(this); > } > + > +/* > + * @brief This function does generic validation for admin op SI_SWAP. > + * @return int > + */ > +SaAisErrorT AVD_SI::si_swap_validate() > +{ > + AVD_AVND *node; > + SaAisErrorT rc = SA_AIS_OK; > + if (saAmfSIAdminState != SA_AMF_ADMIN_UNLOCKED) { > + LOG_ER("%s SWAP failed - wrong admin state=%u", > name.value, > + saAmfSIAdminState); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + if (avd_cb->init_state != AVD_APP_STATE) { > + LOG_ER("%s SWAP failed - not in app state (%u)", > name.value, > + avd_cb->init_state); > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto done; > + } > + > + if (sg_of_si->sg_fsm_state != AVD_SG_FSM_STABLE) { > + LOG_ER("%s SWAP failed - SG not stable (%u)", name.value, > + sg_of_si->sg_fsm_state); > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto done; > + } > + > + if (list_of_sisu == NULL) { > + LOG_ER("%s SWAP failed - no assignments to swap", > name.value); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + /* First check for another node for M/w. */ > + /* Since middleware components can still have si->list_of_sisu- > >si_next as not NULL, but we need to check > + whether it is unlocked. We need to reject si_swap on controllers > when stdby controller is locked. */ > + if (sg_of_si->sg_ncs_spec) { > + /* Check if the Standby is there in unlocked state. */ > + node = avd_node_find_nodeid(avd_cb->node_id_avd_other); > + if (node == NULL) { > + LOG_NO("SI Swap not possible, node %x is not > available", avd_cb->node_id_avd_other); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + if (SA_FALSE == node->node_info.member) { > + LOG_NO("SI Swap not possible, node %x is locked", > avd_cb->node_id_avd_other); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + } > + > + /* Check whether Amfd is in sync if node is illigible to join. */ > + if (sg_of_si->sg_ncs_spec) { > + if (avd_cb->stby_sync_state == AVD_STBY_OUT_OF_SYNC) { > + LOG_ER("%s SWAP failed - Cold sync in progress", > name.value); > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto done; > + } > + } > + > + if (list_of_sisu->si_next == NULL) { > + LOG_ER("%s SWAP failed - only one assignment", > name.value); > + if (sg_of_si->sg_ncs_spec) { > + /* Another M/w SU will come anyway because of > two controller, > + so wait to come back.*/ > + TRACE("M/w SI"); > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto done; > + } > + > + /* Check whether one assignment is because of > configuration. */ > + if (sg_of_si->list_of_su.size() == 1) { > + LOG_ER("Only once SU configured"); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } else { > + /* The validation logic has been written into many > sub-logics of 'for loop', > + because of > + a. keeping the code simple. If we combine them, > then the code > + becomes complex to understand and dificult to fix > any further bug. > + b. these code not being executed frequently as > these scenarios are rare.*/ > + bool any_su_unlocked = false; > + /* 1. If more than one SU configured, check if any > other are unlocked.*/ > + for (const auto& su : sg_of_si->list_of_su) { > + if ((su->saAmfSUAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (su->su_on_node- > >saAmfNodeAdminState == SA_AMF_ADMIN_UNLOCKED) && > + (list_of_sisu->su != su)) { > + any_su_unlocked = true; > + break; > + } > + } > + /* All other SUs are unlocked, so return BAD OP. */ > + if (any_su_unlocked == false) { > + LOG_ER("All other SUs and their hosting > nodes are not in UNLOCKED state"); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + /* 2. If the containing nodes (hosting all other SUs) > absent: > + return BAD OP.*/ > + bool all_nodes_absent = true; > + for (const auto& su : sg_of_si->list_of_su) { > + if ((su->su_on_node->node_state != > AVD_AVND_STATE_ABSENT) && > + (list_of_sisu->su != su)) { > + all_nodes_absent = false; > + break; > + } > + } > + /* all other nodes absent, so return BAD OP. */ > + if (all_nodes_absent == true) { > + LOG_ER("All other nodes hosting SUs are > down"); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + /* 3. If node (hosting any other SUs) is joining(A case > of Temporary degeneration): > + return TRY_AGAIN. */ > + bool any_nodes_joining = false; > + for (const auto& su : sg_of_si->list_of_su) { > + if ((su->saAmfSUAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + ((su->su_on_node- > >node_state == AVD_AVND_STATE_NO_CONFIG) || > + (su->su_on_node- > >node_state == AVD_AVND_STATE_NCS_INIT)) && > + (list_of_sisu->su != su)) { > + any_nodes_joining = true; > + break; > + } > + } > + /* Any other node hosting unlocked SUs are joining, > so return TRY AGAIN. */ > + if (any_nodes_joining == true) { > + LOG_ER("All other nodes hosting SUs are > joining."); > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto done; > + } > + > + /* If the containing nodes have joined: If any one of > the SUs is in > + instantiating/restarting/terminating state(A case of > Temporary degeneration): > + return TRY_AGAIN.*/ > + bool any_su_on_nodes_in_trans = false, > any_su_on_nodes_failed = false; > + for (const auto& su : sg_of_si->list_of_su) { > + if ((su->su_on_node->node_state == > AVD_AVND_STATE_PRESENT) && > + (list_of_sisu->su != su)) { > + /* */ > + if ((su->saAmfSUPresenceState == > SA_AMF_PRESENCE_INSTANTIATING) || > + (su- > >saAmfSUPresenceState == SA_AMF_PRESENCE_TERMINATING) || > + (su- > >saAmfSUPresenceState == SA_AMF_PRESENCE_RESTARTING)) { > + any_su_on_nodes_in_trans = > true; > + } else if ((su->saAmfSUPresenceState > == SA_AMF_PRESENCE_INSTANTIATION_FAILED) || > + (su- > >saAmfSUPresenceState == SA_AMF_PRESENCE_TERMINATION_FAILED) || > + (su- > >saAmfSuReadinessState == SA_AMF_READINESS_OUT_OF_SERVICE)) { > + /* If any one of the SUs is in > Disabled/OutOfService/ > + Instantiation > failed/Termination failed: > + return BAD OP.*/ > + any_su_on_nodes_failed = > true; > + } > + } > + } > + /* Any other SUs are in > instantiating/restarting/terminating, so return TRY AGAIN. */ > + if (any_su_on_nodes_in_trans == true) { > + LOG_ER("Other SUs are in > INSTANTIATING/TERMINATING/RESTARTING state."); > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto done; > + } else if (any_su_on_nodes_failed == true) { > + /* Nothing in inst/rest/term and others are in > failed state, so reutrn BAD OP.*/ > + LOG_ER("Other SUs are in > INSTANTIATION/TERMINATION failed state/ Out of Service."); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + } > + /* Still the SUs may be in Uninstantiated or Instantiated, but > might not have got > + assignment because of other configurations like > PrefInServiceSUs, etc, so return > + BAD_OP for them. */ > + LOG_ER("Other SUs are not instantiated or asigned because > of configurations."); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + /* If the swap is on m/w si, then check whether any ccb was going > on. */ > + if (sg_of_si->sg_ncs_spec) { > + if (ccbutil_EmptyCcbExists() == false) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + LOG_NO("%s SWAP failed - Ccb going on", > name.value); > + goto done; > + } > + } > + > +done: > + return rc; > +} > > ------------------------------------------------------------------------------ > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741551&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
