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

Reply via email to