Hi Minh, Please responses inline with [Praveen]
Thanks, Praveen On 24-Aug-16 2:01 PM, minh chau wrote: > Hi Praveen, > > I have tested the patches in case all SUs of NpM/Nway belong to one > nodegroup, it works for me. Will try the case that SUs are partially > in/out of nodegroup tomorrow. > But please, if you have time, can you try to make NpM's ng_admin under > FSM_SG_ADMIN. Addition to my previous comment, and also as you knew, in > #1725 it's hard to deduce the states (sg fsm state is one of those) > because different SG is using FSM differently in some cases. This > inconsistency causes a difficult, instead of applying a generic solution > for all SG, then each SG has to be treated different way. I think we > could also see this kind of problem in future. [Praveen] In 5.1, #1725 is targeting only 2N model.In future when #1725 will support other red models (upcoming releases), I would do any changes that is required in other red models for NG operations. > Thanks, > Minh > > On 23/08/16 16:58, praveen malviya wrote: >> Hi Minh, >> >> I have attached patches for #1454 and #1608 in the ticket #1454. >> Please apply them in order. >> >> Thanks, >> Praveen >> >> On 23-Aug-16 11:56 AM, minh chau wrote: >>> 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