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
