Ack, but please add some comment in the sg_su_failover_func() what the code does. Thanks, Hans
> -----Original Message----- > From: praveen malviya [mailto:[email protected]] > Sent: den 7 april 2014 14:29 > To: Hans Feldt > Cc: [email protected]; [email protected] > Subject: Re: [PATCH 1 of 1] amfd: return AIS_OK instead of BAD_OP for si-swap > admin op V2 [#823] > > > On 07-Apr-14 4:46 PM, Hans Feldt wrote: > > What is the addition in sg_su_failover_func() there for? > I had posted the change in the reply for the following comment given by you: > >> "Besides the patch does not handle SUFailover during SI-swap." > I had posted the diff. Please see the attached mail > (sufailover_change.eml) of 1st April. > So for suFailover it is included in V2. > > > It is new since V1 and uncommented. > > > > In all places complete_siswap() is called, do we know for sure that an > > SI swap is in progress or could it be any SI admin operation? > > > But only si-swap operation is responded after completion. > si.cc :----> si_admin_op_cb(): > if ((operationId != SA_AMF_ADMIN_SI_SWAP)) > avd_saImmOiAdminOperationResult(immOiHandle, > invocation, rc); > Already a ticket is there for it #505. > > With these explanations, can I consider this as an ack? > > Thanks, > Praveen > > Thanks, > > Hans > > > > On 04/07/2014 11:46 AM, [email protected] wrote: > >> osaf/services/saf/amf/amfd/sg_2n_fsm.cc | 70 > >> ++++++++++++++++++-------------- > >> osaf/services/saf/amf/amfd/sgproc.cc | 6 ++ > >> 2 files changed, 45 insertions(+), 31 deletions(-) > >> > >> > >> Problem: During si-swap if quiesced assignment faults, AMF returns > >> BAD_OPERATION even though si-swap completes successfuly after recovery. > >> > >> Reason: During si-swap, AMF sends quiesced assignment to active SU. > >> During quiesced assignments, one of the components faults. > >> AMF performs cleanup of this failed component and perfroms the > >> failover of > >> assignments to standby SU. For the faulted SU assignments are deleted > >> and > >> when it gets sucessfully repaired, AMF assigns it with standby > >> assignments. > >> Thus si-swap operation eventually gets successful, but AMF returns > >> BAD_OPERATION for the operation. > >> > >> Fix: Since AMF performs si-swap successfuly despite fault in quiesced > >> state, > >> this patch ensures that AMF returns SA_AIS_OK for the operation. > >> > >> diff --git a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > >> b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > >> --- a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > >> +++ b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc > >> @@ -31,6 +31,41 @@ > >> #include <imm.h> > >> > >> /** > >> + * @brief Respond to IMM for si-swap operation. > >> + * > >> + * @param [in] su > >> + * @param [in] status > >> + * > >> + */ > >> + > >> +static void complete_siswap(AVD_SU *su, SaAisErrorT status) > >> +{ > >> + AVD_SU_SI_REL *l_susi; > >> + > >> + TRACE_ENTER(); > >> + > >> + /* find the SI on which SWAP admin operation is pending */ > >> + for (l_susi = su->list_of_susi; l_susi != NULL; l_susi = > >> l_susi->su_next) { > >> + if (l_susi->si->invocation != 0) > >> + break; > >> + } > >> + > >> + if (l_susi != NULL) { > >> + avd_saImmOiAdminOperationResult(avd_cb->immOiHandle, > >> l_susi->si->invocation, status); > >> + l_susi->si->invocation = 0; > >> + LOG_NO("%s Swap done", l_susi->si->name.value); > >> + saflog(LOG_NOTICE, amfSvcUsrName, "%s Swap done", > >> l_susi->si->name.value); > >> + } else { > >> + /* si->invocation field is not check pointed. If controller > >> failovers when si-swap > >> + operation is in progress, si->invocation will be zero on > >> the new active controller. > >> + Log an error when si-swap operation completes.*/ > >> + LOG_ER("Operation done, but invocationId for the operation > >> on SI not found '%s'", su->name.value); > >> + } > >> + TRACE_LEAVE(); > >> +} > >> + > >> + > >> +/** > >> * @brief Determine fsm state of an SU. > >> * > >> * @param [in] su > >> @@ -888,16 +923,6 @@ static uint32_t avd_sg_2n_su_fault_su_op > >> if (su->sg_of_su->su_oper_list.su == su) { > >> su_ha_state = avd_su_state_determine(su); > >> if (su_ha_state == SA_AMF_HA_QUIESCED) { > >> - if (su->su_switch == AVSV_SI_TOGGLE_SWITCH) { > >> - AVD_SU_SI_REL *temp_susi; > >> - for (temp_susi = su->list_of_susi; temp_susi != > >> NULL; temp_susi = temp_susi->su_next) { > >> - if (temp_susi->si->invocation != 0) { > >> - avd_saImmOiAdminOperationResult(cb->immOiHandle, > >> - temp_susi->si->invocation, SA_AIS_ERR_BAD_OPERATION); > >> - temp_susi->si->invocation = 0; > >> - } > >> - } > >> - } > >> m_AVD_SET_SU_SWITCH(cb, su, AVSV_SI_TOGGLE_STABLE); > >> } else if (su_ha_state == SA_AMF_HA_QUIESCING) { > >> if (avd_sidep_si_dependency_exists_within_su(su)) { > >> @@ -2095,6 +2120,8 @@ static uint32_t avd_sg_2n_susi_sucss_su_ > >> } > >> > >> m_AVD_SET_SG_FSM(cb, (su->sg_of_su), > >> AVD_SG_FSM_SG_REALIGN); > >> + complete_siswap(su, SA_AIS_OK); > >> + > >> } > >> } else if ((act == AVSV_SUSI_ACT_MOD) && (state == > >> SA_AMF_HA_STANDBY) && > >> (su->sg_of_su->su_oper_list.su == su)) { > >> @@ -2112,20 +2139,7 @@ static uint32_t avd_sg_2n_susi_sucss_su_ > >> /*As sg is stable, screen for si dependencies and take > >> action on whole sg*/ > >> avd_sidep_update_si_dep_state_for_all_sis(su->sg_of_su); > >> avd_sidep_sg_take_action(su->sg_of_su); > >> - /* find the SI on which SWAP admin operation is pending */ > >> - for (l_susi = su->list_of_susi; l_susi != NULL && > >> l_susi->si->invocation == 0; l_susi = l_susi->su_next); > >> - if (l_susi != NULL){ > >> - avd_saImmOiAdminOperationResult(cb->immOiHandle, > >> l_susi->si->invocation, SA_AIS_OK); > >> - l_susi->si->invocation = 0; > >> - LOG_NO("%s Swap done", l_susi->si->name.value); > >> - saflog(LOG_NOTICE, amfSvcUsrName, "%s Swap done", > >> l_susi->si->name.value); > >> - } > >> - else { > >> - /* si->invocation field is not check pointed. If > >> controller failovers when si-swap > >> - operation is in progress, si->invocation will be zero > >> on the new active controller. > >> - Log an error when si-swap operation completes.*/ > >> - LOG_ER("Swap done, but invocationId for the swap > >> operation not found '%s'", su->name.value); > >> - } > >> + complete_siswap(su, SA_AIS_OK); > >> > >> if (su->sg_of_su->sg_ncs_spec) > >> amfd_switch(avd_cb); > >> @@ -2708,13 +2722,7 @@ uint32_t avd_sg_2n_susi_fail_func(AVD_CL > >> > >> m_AVD_SET_SU_SWITCH(cb, su, AVSV_SI_TOGGLE_STABLE); > >> m_AVD_SET_SG_FSM(cb, (su->sg_of_su), > >> AVD_SG_FSM_SG_REALIGN); > >> - for (l_susi = su->list_of_susi; l_susi != NULL; l_susi = > >> l_susi->su_next) { > >> - if (l_susi->si->invocation != 0) { > >> - avd_saImmOiAdminOperationResult(cb->immOiHandle, > >> - l_susi->si->invocation, > >> SA_AIS_ERR_BAD_OPERATION); > >> - l_susi->si->invocation = 0; > >> - } > >> - } > >> + complete_siswap(su, SA_AIS_ERR_BAD_OPERATION); > >> > >> } else if ((act == AVSV_SUSI_ACT_MOD) && > >> ((state == SA_AMF_HA_QUIESCED) || (state == > >> SA_AMF_HA_QUIESCING)) && > >> diff --git a/osaf/services/saf/amf/amfd/sgproc.cc > >> b/osaf/services/saf/amf/amfd/sgproc.cc > >> --- a/osaf/services/saf/amf/amfd/sgproc.cc > >> +++ b/osaf/services/saf/amf/amfd/sgproc.cc > >> @@ -334,6 +334,12 @@ static uint32_t sg_su_failover_func(AVD_ > >> deleting the SUSI. */ > >> avd_si_dec_curr_act_ass(susi->si); > >> } > >> + > >> + if (susi->si->invocation != 0) { > >> + avd_saImmOiAdminOperationResult(avd_cb->immOiHandle, > >> + susi->si->invocation, SA_AIS_OK); > >> + susi->si->invocation = 0; > >> + } > >> } > >> su->sg_of_su->node_fail(avd_cb, su); > >> avd_sg_su_asgn_del_util(avd_cb, su, true, false); > >> > >> ------------------------------------------------------------------------------ Put Bad Developers to Shame Dominate Development with Jenkins Continuous Integration Continuously Automate Build, Test & Deployment Start a new project now. Try Jenkins in the cloud. http://p.sf.net/sfu/13600_Cloudbees_APR _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
