If the recovery of failed Su doesn't happen i.e. goes into instantiatin failed state/termination failed state, then are you going to rerun failed operation ?
Thanks -Nagu > -----Original Message----- > From: Hans Feldt [mailto:[email protected]] > Sent: 02 April 2014 12:14 > To: praveen malviya > Cc: [email protected] > Subject: Re: [devel] [PATCH 1 of 1] amfd: return AIS_OK instead of BAD_OP for > si-swap admin op [#823] > > Well my view point still stands. By separating the su_switch from > invocation_id > we introduce even more sub-states and we already have enough of those! > Basically the same patch as you have with a small addition causes the admin > response to be sent after recovery of the failed SU. Will send a patch. > /Hans > > On 04/02/2014 08:28 AM, praveen malviya wrote: > > Any update/comment on this. > > > > Thanks, > > Praveen > > On 01-Apr-14 2:27 PM, praveen malviya wrote: > >> On 01-Apr-14 1:27 PM, Hans Feldt wrote: > >>> > >>> On 03/28/2014 02:14 PM, [email protected] wrote: > >>>> 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); > >>> Why isn't the above line removed also? > >>> > >>> This state should be kept until the admin op has been responded, > >>> then we forget all about it. > >>> > >> In the AMF code nowhere we are checking su->su_switch and > >> si->invocation simultaneously which means they are kept very much > >> independent. So responding for the admin operation is not very much > >> dependent on the toggling of su_switch. So we can keep the above > >> line. > >>>> } 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) { > >>> if the above line is removed, this code won't get triggered. The > >>> same needs to be placed somewhere else. > >>> > >> I think if "if statement" is removed, the code inside it will always > >> be executed which we do not want, just to not break existing functionality. > >> I did not get what do you mean by removing and placing it somewhere else. > >> > >> Thanks > >> Praveen > >> > >>>> + 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 > > > > > > > > ------------------------------------------------------------------------------ > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
