Ack code review only, one minor comment.
Thanks
Praveen
On 12-Nov-15 11:01 AM, Gary Lee wrote:
> osaf/services/saf/amf/amfd/include/sg.h | 2 +
> osaf/services/saf/amf/amfd/sg_npm_fsm.cc | 230
> ++++--------------------------
> 2 files changed, 39 insertions(+), 193 deletions(-)
>
>
> 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
> @@ -510,6 +510,8 @@
> uint32_t su_fault_sg_relgn(AVD_CL_CB *cb, AVD_SU *su);
> uint32_t susi_sucss_sg_reln(AVD_CL_CB *cb, AVD_SU *su, struct
> avd_su_si_rel_tag *susi,
> AVSV_SUSI_ACT act, SaAmfHAStateT state);
> + bool sg_stable(AVD_CL_CB* cb, AVD_SU* su);
> + void node_fail_sg_relgn(AVD_CL_CB* cb, AVD_SU* su);
> };
>
> /**
> 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
> @@ -3213,7 +3213,7 @@
> *
> **************************************************************************/
>
> -static void avd_sg_npm_node_fail_sg_relgn(AVD_CL_CB *cb, AVD_SU *su)
> +void SG_NPM::node_fail_sg_relgn(AVD_CL_CB *cb, AVD_SU *su)
> {
> AVD_SU_SI_REL *l_susi, *o_susi, *ot_susi;
> bool l_flag = false;
> @@ -3329,26 +3329,7 @@
>
> su->sg_of_su->admin_si->set_si_switch(cb, AVSV_SI_TOGGLE_STABLE);
> m_AVD_CLEAR_SG_ADMIN_SI(cb,
> (su->sg_of_su));
>
> - if (su->sg_of_su->su_oper_list.empty()
> == true) {
> - /* both the SI admin pointer
> and SU oper list are empty.
> - * Do the functionality as in
> stable state to verify if
> - * new assignments can be done.
> If yes stay in the same state.
> - * If no new assignments change
> state to stable.
> - */
> - if
> (avd_sg_npm_su_chose_asgn(cb, su->sg_of_su) == NULL) {
> - /* all the assignments
> have already been done in the SG. */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_STABLE);
> -
> avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - }
> - }
> -
> + sg_stable(cb, su);
> } /* if ((l_susi->state ==
> SA_AMF_HA_QUIESCED) &&
> (l_susi->fsm ==
> AVD_SU_SI_STATE_MODIFY)) */
> else if ((l_susi->state == SA_AMF_HA_QUIESCED)
> &&
> @@ -3367,25 +3348,7 @@
> /* Free all the SI assignments to this
> SU. */
> su->delete_all_susis();
>
> - if (su->sg_of_su->su_oper_list.empty()
> == true) {
> - /* both the SI admin pointer
> and SU oper list are empty.
> - * Do the functionality as in
> stable state to verify if
> - * new assignments can be done.
> If yes stay in the same state.
> - * If no new assignments change
> state to stable.
> - */
> - if
> (avd_sg_npm_su_chose_asgn(cb, su->sg_of_su) == NULL) {
> - /* all the assignments
> have already been done in the SG. */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_STABLE);
> -
> avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - }
> - }
> + sg_stable(cb, su);
> } /* else if ((l_susi->state ==
> SA_AMF_HA_QUIESCED) &&
> (l_susi->fsm ==
> AVD_SU_SI_STATE_ASGND)) */
> else if (l_susi->state == SA_AMF_HA_STANDBY) {
> @@ -3427,25 +3390,7 @@
> /* Free all the SI assignments to this
> SU. */
> su->delete_all_susis();
>
> - if (su->sg_of_su->su_oper_list.empty()
> == true) {
> - /* both the SI admin pointer
> and SU oper list are empty.
> - * Do the functionality as in
> stable state to verify if
> - * new assignments can be done.
> If yes stay in the same state.
> - * If no new assignments change
> state to stable.
> - */
> - if
> (avd_sg_npm_su_chose_asgn(cb, su->sg_of_su) == NULL) {
> - /* all the assignments
> have already been done in the SG. */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_STABLE);
> -
> avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - }
> - }
> + sg_stable(cb, su);
> }
> /* else if (l_susi->state == SA_AMF_HA_ACTIVE)
> */
> } else { /* if
> (su->sg_of_su->admin_si->si_switch == AVSV_SI_TOGGLE_SWITCH) */
> @@ -3481,26 +3426,7 @@
> /* Remove the SI from the SI admin
> pointer. */
> m_AVD_CLEAR_SG_ADMIN_SI(cb,
> (su->sg_of_su));
>
> - if (su->sg_of_su->su_oper_list.empty()
> == true) {
> - /* both the SI admin pointer
> and SU oper list are empty.
> - * Do the functionality as in
> stable state to verify if
> - * new assignments can be done.
> If yes stay in the same state.
> - * If no new assignments change
> state to stable.
> - */
> - if
> (avd_sg_npm_su_chose_asgn(cb, su->sg_of_su) == NULL) {
> - /* all the assignments
> have already been done in the SG. */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_STABLE);
> -
> avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - }
> - }
> -
> + sg_stable(cb, su);
> } /* if ((l_susi->state ==
> SA_AMF_HA_QUIESCING) ||
> (l_susi->state ==
> SA_AMF_HA_QUIESCED)) */
> else if (l_susi->state == SA_AMF_HA_STANDBY) {
> @@ -3595,26 +3521,7 @@
> (su->list_of_susi->state ==
> SA_AMF_HA_QUIESCING) ||
> (su->list_of_susi->state ==
> SA_AMF_HA_ACTIVE)) */
>
> - if (su->sg_of_su->su_oper_list.empty() == true) {
> - /* both the SI admin pointer and SU oper list
> are empty.
> - * Do the functionality as in stable state to
> verify if
> - * new assignments can be done. If yes stay in
> the same state.
> - * If no new assignments change state to stable.
> - */
> - if (avd_sg_npm_su_chose_asgn(cb, su->sg_of_su)
> == NULL) {
> - /* all the assignments have already
> been done in the SG. */
> - m_AVD_SET_SG_FSM(cb, (su->sg_of_su),
> AVD_SG_FSM_STABLE);
> - avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - }
> - }
> -
> + sg_stable(cb, su);
> } else { /*if (l_flag == true) */
>
> /* SU not in the SU oper list */
> @@ -3722,31 +3629,9 @@
> }
> }
>
> - if (su_oper_list.empty() == true) {
> - /* SU oper list are empty.
> - * Do the functionality as in stable state to verify if
> - * new assignments can be done. If yes change state to
> SG realign.
> - * If no new assignments change state to stable.
> - */
> - if (avd_sg_npm_su_chose_asgn(cb, su->sg_of_su) == NULL)
> {
> - /* all the assignments have already been done
> in the SG. */
> - m_AVD_SET_SG_FSM(cb, (su->sg_of_su),
> AVD_SG_FSM_STABLE);
> - avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - } else {
> - /* new assignments need to be done in the SG. */
> - m_AVD_SET_SG_FSM(cb, (su->sg_of_su),
> AVD_SG_FSM_SG_REALIGN);
> - }
> - } else {
> - /* Change state to SG realign. */
> + if (sg_stable(cb, su) == false) {
> m_AVD_SET_SG_FSM(cb, (su->sg_of_su),
> AVD_SG_FSM_SG_REALIGN);
> - }
> + }
> } else { /* if(su->sg_of_su->su_oper_list.su == su) */
>
> /* the SU is not the same as the SU in the list */
> @@ -3876,29 +3761,7 @@
> su->sg_of_su->admin_si->set_si_switch(cb,
> AVSV_SI_TOGGLE_STABLE);
> m_AVD_CLEAR_SG_ADMIN_SI(cb, (su->sg_of_su));
>
> - if (su_oper_list.empty() == true) {
> - /* both the SI admin pointer and SU
> oper list are empty.
> - * Do the functionality as in stable
> state to verify if
> - * new assignments can be done. If yes
> stay in the same state.
> - * If no new assignments change state
> to stable.
> - */
> - if (avd_sg_npm_su_chose_asgn(cb,
> su->sg_of_su) == NULL) {
> - /* all the assignments have
> already been done in the SG. */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_STABLE);
> -
> avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - } else {
> - /* Change state to SG realign.
> */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_SG_REALIGN);
> - }
> - } else {
> - /* Change state to SG realign. */
> + if (sg_stable(cb, su) == false) {
> m_AVD_SET_SG_FSM(cb, (su->sg_of_su),
> AVD_SG_FSM_SG_REALIGN);
> }
>
> @@ -3977,29 +3840,7 @@
> /* Free all the SI assignments to this SU. */
> su->delete_all_susis();
>
> - if (su_oper_list.empty() == true) {
> - /* both the SI admin pointer and SU
> oper list are empty.
> - * Do the functionality as in stable
> state to verify if
> - * new assignments can be done. If yes
> stay in the same state.
> - * If no new assignments change state
> to stable.
> - */
> - if (avd_sg_npm_su_chose_asgn(cb,
> su->sg_of_su) == NULL) {
> - /* all the assignments have
> already been done in the SG. */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_STABLE);
> -
> avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - } else {
> - /* Change state to SG realign.
> */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_SG_REALIGN);
> - }
> - } else {
> - /* Change state to SG realign. */
> + if (sg_stable(cb, su) == false) {
> m_AVD_SET_SG_FSM(cb, (su->sg_of_su),
> AVD_SG_FSM_SG_REALIGN);
> }
> }
> @@ -4034,29 +3875,7 @@
> /* Remove the SI from the SI admin pointer. */
> m_AVD_CLEAR_SG_ADMIN_SI(cb, (su->sg_of_su));
>
> - if (su_oper_list.empty() == true) {
> - /* both the SI admin pointer and SU
> oper list are empty.
> - * Do the functionality as in stable
> state to verify if
> - * new assignments can be done. If yes
> stay in the same state.
> - * If no new assignments change state
> to stable.
> - */
> - if (avd_sg_npm_su_chose_asgn(cb,
> su->sg_of_su) == NULL) {
> - /* all the assignments have
> already been done in the SG. */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_STABLE);
> -
> avd_sidep_sg_take_action(su->sg_of_su);
> - if ((AVD_SG_FSM_STABLE
> == su->sg_of_su->sg_fsm_state) &&
> - (true
> == su->sg_of_su->equal_ranked_su) &&
> -
> (SA_TRUE == su->sg_of_su->saAmfSGAutoAdjust)) {
> - /* SG fsm is
> stable, screen for possibility of
> -
> redistributing SI to achieve equal distribution */
> -
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> - }
> - } else {
> - /* Change state to SG realign.
> */
> - m_AVD_SET_SG_FSM(cb,
> (su->sg_of_su), AVD_SG_FSM_SG_REALIGN);
> - }
> - } else {
> - /* Change state to SG realign. */
> + if (sg_stable(cb, su) == false) {
> m_AVD_SET_SG_FSM(cb, (su->sg_of_su),
> AVD_SG_FSM_SG_REALIGN);
> }
>
> @@ -4132,7 +3951,7 @@
> break; /* case AVD_SG_FSM_STABLE: */
> case AVD_SG_FSM_SG_REALIGN:
>
> - avd_sg_npm_node_fail_sg_relgn(cb, su);
> + node_fail_sg_relgn(cb, su);
>
> break; /* case AVD_SG_FSM_SG_REALIGN: */
> case AVD_SG_FSM_SU_OPER:
> @@ -4452,3 +4271,28 @@
> SG_NPM::~SG_NPM() {
> }
>
> +bool SG_NPM::sg_stable(AVD_CL_CB* cb, AVD_SU* su) {
I think the function should contain some header to explain its use. This
function is not meant to check if SG is stable. Even if SG is unstable
it will return true in some cases.
Thanks,
Praveen
> + if (su->sg_of_su->su_oper_list.empty() == true) {
> + /* both the SI admin pointer and SU oper list are empty.
> + * Do the functionality as in stable state to verify if
> + * new assignments can be done. If yes stay in the same state.
> + * If no new assignments change state to stable.
> + */
> + if (avd_sg_npm_su_chose_asgn(cb, su->sg_of_su) == NULL) {
> + /* all the assignments have already been done in the
> SG. */
> + m_AVD_SET_SG_FSM(cb, (su->sg_of_su), AVD_SG_FSM_STABLE);
> + avd_sidep_sg_take_action(su->sg_of_su);
> + if ((AVD_SG_FSM_STABLE == su->sg_of_su->sg_fsm_state) &&
> + (true == su->sg_of_su->equal_ranked_su)
> &&
> + (SA_TRUE ==
> su->sg_of_su->saAmfSGAutoAdjust)) {
> + /* SG fsm is stable, screen for possibility of
> + redistributing SI to achieve equal
> distribution */
> +
> avd_sg_npm_screening_for_si_redistr(su->sg_of_su);
> + }
> +
> + return true;
> + }
> + }
> +
> + return false;
> +}
> \ No newline at end of file
>
------------------------------------------------------------------------------
Presto, an open source distributed SQL query engine for big data, initially
developed by Facebook, enables you to easily query your data on Hadoop in a
more interactive manner. Teradata is also now providing full enterprise
support for Presto. Download a free open source copy now.
http://pubads.g.doubleclick.net/gampad/clk?id=250295911&iu=/4140
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
