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
