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);
--- Begin Message ---On 31-Mar-14 4:24 PM, Hans Feldt wrote: > Hi, > > Nack for this version, I think we need todo some iterations of this. > > I have a slightly similar patch. I think this one is strange in that the > response I sent if the su_switch state is STABLE. No logic in that... > > Also the response code should be refactored into its own function e.g. > "complete_siswap" > > Besides the patch does not handle SUFailover during SI-swap. The floated patch is applicable for all the branches. For default and 4.4 branch, this extra patch to handle su-failover is also required: 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,7 +334,14 @@ 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); } With this patch and floated one for #823, I have tested si-swap with sufailover in default branch. Thanks, Praveen > I would also prefer to fix 822 first since they are somewhat related. Will > share a patch for it. > > Thanks, > Hans > >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] >> Sent: den 28 mars 2014 14:14 >> To: Hans Feldt; [email protected]; Hans Nordebäck >> Cc: [email protected] >> Subject: [PATCH 1 of 1] amfd: return AIS_OK instead of BAD_OP for si-swap >> admin op [#823] >> >> osaf/services/saf/amf/amfd/sg_2n_fsm.cc | 23 +++++++++++++---------- >> 1 files changed, 13 insertions(+), 10 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 >> @@ -888,16 +888,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 +2085,19 @@ static uint32_t avd_sg_2n_susi_sucss_su_ >> } >> >> m_AVD_SET_SG_FSM(cb, (su->sg_of_su), >> AVD_SG_FSM_SG_REALIGN); >> + >> + if (su->su_switch == AVSV_SI_TOGGLE_STABLE) { >> + for (AVD_SU_SI_REL *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_OK); >> + temp_susi->si->invocation = 0; >> + } >> + } >> + } >> + >> } >> } else if ((act == AVSV_SUSI_ACT_MOD) && (state == SA_AMF_HA_STANDBY) && >> (su->sg_of_su->su_oper_list.su == su)) { ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
--- End Message ---
------------------------------------------------------------------------------ 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
