> -----Original Message----- > From: praveen malviya [mailto:[email protected]] > Sent: den 26 juni 2013 11:32 > To: Hans Feldt > Cc: Hans Feldt; Mathivanan Naickan Palanivelu; [email protected]; > [email protected] > Subject: Re: [devel] [PATCH 6 of 6] amf: handle sufailover in SU FSM and > Comp FSM at amfnd [#98] > > >>Please move the code where it belongs and we get less and cleaner code. > I will refactor node failover and su failover together(may be a ticket) after > i > finish testing this feature, as of now, i am in middle of testing.
I don't get this reply. You want review comments and you get them. But you plan to address comments later in another ticket? That is not the way it works. Patches are continuously refactored until accepted. I assume "testing" is automated so that does not matter or affected anything. Thanks, Hans > > Thanks > -Praveen > > On 26-Jun-13 2:36 PM, Hans Feldt wrote: > >> -----Original Message----- > >> From: praveen malviya [mailto:[email protected]] > >> Sent: den 26 juni 2013 09:36 > >> To: Hans Feldt > >> Cc: Hans Feldt; Mathivanan Naickan Palanivelu; [email protected]; > >> [email protected] > >> Subject: Re: [devel] [PATCH 6 of 6] amf: handle sufailover in SU FSM > >> and Comp FSM at amfnd [#98] > >> > >> Please see inline. > >> > >> Thanks > >> Praveen > >> On 24-Jun-13 7:34 PM, Hans Feldt wrote: > >>> On 7 June 2013 08:39, <[email protected]> wrote: > >>>> osaf/services/saf/avsv/avnd/avnd_clc.c | 130 > >> ++++++++++++++++++++++--------- > >>>> osaf/services/saf/avsv/avnd/avnd_comp.c | 2 +- > >>>> osaf/services/saf/avsv/avnd/avnd_su.c | 1 + > >>>> osaf/services/saf/avsv/avnd/avnd_susm.c | 81 +++++++++++++----- > - > >>>> 4 files changed, 150 insertions(+), 64 deletions(-) > >>>> > >>>> > >>>> With this patch amfnd informs amfd for failover of assignments > >>>> when all components are terminated successfully during suFailover. > >>>> If some > >> component faults and SU moves to inst/term failed state, compFailover > >> will be performed. > >>>> diff --git a/osaf/services/saf/avsv/avnd/avnd_clc.c > >>>> b/osaf/services/saf/avsv/avnd/avnd_clc.c > >>>> --- a/osaf/services/saf/avsv/avnd/avnd_clc.c > >>>> +++ b/osaf/services/saf/avsv/avnd/avnd_clc.c > >>>> @@ -72,6 +72,7 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_ > >>>> > >>>> static uint32_t > >> avnd_comp_clc_resp(NCS_OS_PROC_EXECUTE_TIMED_CB_INFO *); > >>>> static uint32_t avnd_instfail_su_failover(AVND_CB *, AVND_SU *, > >>>> AVND_COMP *); > >>>> +static bool all_comps_terminated_in_su(AVND_SU *su); > >>>> > >>>> > >> > /********************************************************** > >> ***************** > >>>> ** C O M P O N E N T C L C F S M M A T R I X D E F I N I T I > >>>> O N ** > >>>> @@ -840,9 +841,29 @@ uint32_t avnd_comp_clc_fsm_run(AVND_CB * > >>>> break; > >>>> } > >>>> } > >>>> - > >>>> /* get the prv presence state */ > >>>> prv_st = comp->pres; > >>>> + > >>>> + if (avnd_sufailover_in_progress(comp->su) && > >>>> + ((ev == > >>>> + AVND_COMP_CLC_PRES_FSM_EV_CLEANUP_SUCC) > >> || > >>>> + (ev == > AVND_COMP_CLC_PRES_FSM_EV_CLEANUP_FAIL))) > >>>> + { > >>>> + TRACE("SU_FAILOVER is in progress, event '%s'", > >> pres_state_evt[ev]); > >>>> + if (ev == AVND_COMP_CLC_PRES_FSM_EV_CLEANUP_FAIL) > >>>> + avnd_comp_pres_state_set(comp, > >> SA_AMF_PRESENCE_TERMINATION_FAILED); > >>>> + else > >>>> + avnd_comp_pres_state_set(comp, > >> SA_AMF_PRESENCE_UNINSTANTIATED); > >>>> + /* If all components terminated, finish sufailover at > >>>> amfnd. */ > >>>> + if (all_comps_terminated_in_su(comp->su)) > >>>> + { > >>>> + LOG_NO("Terminated all components in '%s'", > >>>> + comp->su- > >>> name.value); > >>>> + LOG_NO("Informing director of sufailover"); > >>>> + rc = avnd_di_oper_send(cb, comp->su, > >> AVSV_ERR_RCVR_SU_FAILOVER); > >>>> + osafassert(NCSCC_RC_SUCCESS == rc); > >>>> + avnd_su_si_del(cb, &comp->su->name); > >>>> + } > >>>> + } > >>>> + > >>> I definitively think the above belongs to the SU presence state > >>> machine. I did the change and it works the same but gets less > >>> changes and much cleaner. > >> It is the same way we are dealing with node failover and in the same > >> function. When last component is cleaned up, component fsm will be > > Then maybe node failover should be refactored too? > > > >> triggered in clc response event.So this handling will take care if > >> termination of all components is completed, which means completion of > >> su failover at amfnd and ask amfd for its role for su-failover. > >>> all_comps_terminated_in_su() means the same as > >>> su->pres==UNINSTANTIATED > >>> > >>> Comments? > >> In case a components moves to inst/term failed state, presence state > >> of su will not be UNINSTANTIATED. > > During SU failover, if any comp moves to TERM-FAILED state, SU failover > can anyway not proceed. So this is not an argument for putting SU state > machine related code into the Comp state machine. > > > > Please move the code where it belongs and we get less and cleaner code. > > > > Thanks, > > Hans > > > >>> Thanks, > >>> Hans ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
