Hi Praveen, Since AMF longDn has been pushed, can you please attach a longDn rebased version to ticket (both #1454 + #1608) so we can do some test?
Thanks, Minh On 23/08/16 15:56, praveen malviya wrote: > Hi Minh, > > Thanks for reviewing the patch. > Please see inline with [Praveen] > > Thanks, > Praveen > > > > On 23-Aug-16 5:53 AM, minh chau wrote: >> Hi Praveen, >> >> One comment in line with [Minh] >> >> Thanks >> Minh >> >> On 20/07/16 18:57, praveen.malv...@oracle.com wrote: >>> osaf/services/saf/amf/amfd/include/sg.h | 1 + >>> osaf/services/saf/amf/amfd/nodegroup.cc | 4 +- >>> osaf/services/saf/amf/amfd/sg_npm_fsm.cc | 62 >>> ++++++++++++++++++++++++++++++- >>> 3 files changed, 62 insertions(+), 5 deletions(-) >>> >>> >>> Currently 2N, N-Way Active and NoRed models are supported for lock, >>> shutdown, >>> lock-in and unlock-in admin operations on NGs. >>> >>> This patch supports NplusM model also. >>> >>> diff --git a/osaf/services/saf/amf/amfd/include/sg.h >>> b/osaf/services/saf/amf/amfd/include/sg.h >>> --- a/osaf/services/saf/amf/amfd/include/sg.h >>> +++ b/osaf/services/saf/amf/amfd/include/sg.h >>> @@ -507,6 +507,7 @@ public: >>> uint32_t susi_failed(AVD_CL_CB *cb, AVD_SU *su, >>> struct avd_su_si_rel_tag *susi, AVSV_SUSI_ACT act, >>> SaAmfHAStateT state); >>> void node_fail_si_oper(AVD_CL_CB *cb, AVD_SU *su); >>> + void ng_admin(AVD_SU *su, AVD_AMF_NG *ng); >>> private: >>> uint32_t su_fault_su_oper(AVD_CL_CB *cb, AVD_SU *su); >>> diff --git a/osaf/services/saf/amf/amfd/nodegroup.cc >>> b/osaf/services/saf/amf/amfd/nodegroup.cc >>> --- a/osaf/services/saf/amf/amfd/nodegroup.cc >>> +++ b/osaf/services/saf/amf/amfd/nodegroup.cc >>> @@ -687,6 +687,7 @@ void avd_ng_admin_state_set(AVD_AMF_NG* >>> avd_send_admin_state_chg_ntf(&ng->name, >>> (SaAmfNotificationMinorIdT)SA_AMF_NTFID_NG_ADMIN_STATE, >>> old_state, ng->saAmfNGAdminState); >>> + TRACE_LEAVE(); >>> } >>> /** >>> * @brief Verify if Node is stable for admin operation on Nodegroup >>> etc. >>> @@ -749,8 +750,7 @@ static SaAisErrorT check_red_model_servi >>> LOG_NO("service outage for '%s' because of >>> shutdown/lock " >>> "on '%s'",sg->name.value,ng->name.value); >>> - if ((sg->sg_redundancy_model == >>> SA_AMF_N_WAY_REDUNDANCY_MODEL) || >>> - (sg->sg_redundancy_model == >>> SA_AMF_NPM_REDUNDANCY_MODEL)) { >>> + if (sg->sg_redundancy_model == >>> SA_AMF_N_WAY_REDUNDANCY_MODEL) { >>> LOG_NO("Admin op on '%s' hosting SUs of '%s' with >>> redundancy '%u' " >>> "is not supported",ng->name.value, >>> sg->name.value, >>> sg->sg_redundancy_model); >>> diff --git a/osaf/services/saf/amf/amfd/sg_npm_fsm.cc >>> b/osaf/services/saf/amf/amfd/sg_npm_fsm.cc >>> --- a/osaf/services/saf/amf/amfd/sg_npm_fsm.cc >>> +++ b/osaf/services/saf/amf/amfd/sg_npm_fsm.cc >>> @@ -120,16 +120,16 @@ static AVD_SU_SI_REL *avd_sg_npm_su_othr >>> if (i_susi->si->list_of_sisu != i_susi) { >>> o_susi = i_susi->si->list_of_sisu; >>> if (o_susi->fsm != AVD_SU_SI_STATE_UNASGN) >>> - return o_susi; >>> + break; >>> } else if (i_susi->si->list_of_sisu->si_next != >>> AVD_SU_SI_REL_NULL) { >>> o_susi = i_susi->si->list_of_sisu->si_next; >>> if (o_susi->fsm != AVD_SU_SI_STATE_UNASGN) >>> - return o_susi; >>> + break; >>> } >>> i_susi = i_susi->su_next; >>> } >>> - >>> + TRACE_LEAVE2("o_susi:'%p'",o_susi); >>> return o_susi; >>> } >>> @@ -4452,6 +4452,62 @@ uint32_t SG_NPM::sg_admin_down(AVD_CL_CB >>> return NCSCC_RC_SUCCESS; >>> } >>> +/* >>> + * @brief Handles modification of assignments in SU of NpM SG >>> + * because of lock or shutdown operation on Node group. >>> + * If SU does not have any SIs assigned to it, AMF will >>> try >>> + * to instantiate new SUs in the SG. If SU has >>> assignments, >>> + * then depending upon lock or shutdown operation, >>> quiesced >>> + * or quiescing state will be sent for active SIs in SU. >>> + * If SU has only standby assignments then remove the >>> assignments. >>> + * >>> + * @param[in] ptr to SU >>> + * @param[in] ptr to nodegroup AVD_AMF_NG. >>> + */ >>> +void SG_NPM::ng_admin(AVD_SU *su, AVD_AMF_NG *ng) >>> +{ >>> + SaAmfHAStateT ha_state; >>> + >>> + TRACE_ENTER2("'%s', sg_fsm_state:%u",su->name.value, >>> + su->sg_of_su->sg_fsm_state); >>> + >>> + if (su->list_of_susi == nullptr) { >>> + avd_sg_app_su_inst_func(avd_cb, su->sg_of_su); >>> + return; >>> + } >>> + >>> + if (ng->saAmfNGAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) >>> + ha_state = SA_AMF_HA_QUIESCING; >>> + else >>> + ha_state = SA_AMF_HA_QUIESCED; >>> + >>> + if (su->list_of_susi->state == SA_AMF_HA_ACTIVE) { >>> + if (avd_sg_su_si_mod_snd(avd_cb, su, ha_state) == >>> NCSCC_RC_FAILURE) { >>> + LOG_ER("quiescing/quiesced state transtion failed for >>> '%s'",su->name.value); >>> + goto done; >>> + } >>> + } else { >>> + if (avd_sg_su_si_del_snd(avd_cb, su) == NCSCC_RC_FAILURE) { >>> + LOG_ER("removal of standby assignment failed for >>> '%s'",su->name.value); >>> + goto done; >>> + } >>> + } >>> + >>> + avd_sg_su_oper_list_add(avd_cb, su, false); >>> + su->sg_of_su->set_fsm_state(AVD_SG_FSM_SG_REALIGN); >> [Minh]: For ng_admin operation, 2N is using fsm AVD_SG_FSM_SG_ADMIN, the >> NoRed, NwayActive are using AVD_SG_FSM_SG_REALIGN. Now the support for >> NpM and Nway are using AVD_SG_FSM_SG_REALIGN also. >> Quickly browsing through ng_admin() of NoRed, NwayActive, NpM, Nway, >> they are very similar to the existing of those sg_admin_down(), >> respectively. I think the NoRed, Nway, NwayACtive, NpM should be using >> AVD_SG_FSM_SG_ADMIN in the same way as 2N. The reason is the same admin >> operation should be treated in the same SG FSM state/code between SG >> types, and if sg_admin_down() works, we should reuse them. > > In 2N model, there are only two cases: > A) Whole SG is mapped in NG or > B) SG is partially mapped in NG. One of the SU is having active or > standby assignments in one of the nodes of NG. > > So in 2N model, AVD_SG_FSM_SG_ADMIN is used in case A) because 2N > model supports SI dep within SU. Because of this quiesced/quiescing > assignments must be given honoring si dep. AVD_SG_FSM_SG_ADMIN honors > that while giving quiesced/quiescing assignments. > Case B) becomes the case of either lock of standby Node/su or active > node/su which still needs to be handled honoring SI dep in > su_admin_down(). > > Other red models do not support, SI deps within SU. Once it is > supported we will have to use internal SG FSM state. But without SI > deps, other red models can still use AVD_SG_FSM_SG_ADMIN when whole SG > is mapped in NG. But possibility of such a case is more in 2N model > where only two SUs can be assigned anytime. In other red models, there > can be many assigned SUs so possibility of whole SG is mapped in NG is > very less. So in other models AVD_SG_FSM_SG_REALIGN states is used by > keeping multiple SUs in oper list because these red models are > handling switchover situation in REALIGN state (removal of standby is > always handled in realign state). But when SI deps, is completely > supported within SU in these models, then we cannot use realign state > and we will have to use internal FSM code because internal FSM code > will be enhanced for that. > In 2N model, switchover is being done honoring SI dep but that is in > some bug fix cases. > > Thanks, > Praveen >>> + >>> + //Increment node counter for tracking status of ng operation. >>> + if ((su->any_susi_fsm_in_modify() == true) || >>> + (su->any_susi_fsm_in_unasgn() == true)) { >>> + su->su_on_node->su_cnt_admin_oper++; >>> + TRACE("node:%s, su_cnt_admin_oper:%u", su->su_on_node->name.value, >>> + su->su_on_node->su_cnt_admin_oper); >>> + } >>> +done: >>> + TRACE_LEAVE(); >>> + return; >>> +} >>> + >>> SG_NPM::~SG_NPM() { >>> } >>> >> > ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel