Hi Nagu, Please let me know what particular thing that concerns you with (1), or do you agree if I do like (2)?
Thanks, Minh On 23/09/16 16:27, minh chau wrote: > Hi Nagu, > > The macro m_AVD_CLINIT_TMR_START has the setting *is_active = false*, > so if this macro is called again without checking timer status, then > amf_init_tmr will be started again while the same instance of this > timer could be running. > In the fix of #1988, I could not reuse this macro and had to manually > start/stop this timer. But #1988 is in context of headless feature > only, so it would not cause any problem because m_AVD_CLINIT_TMR_START > is always called first. > Now in #2063 which has roaming feature enabled, m_AVD_CLINIT_TMR_START > is not always called first. The first instance of amf_init_tmr is > started in avd_node_up_evh(), because leds state of amfnd in active > promoted SC is green (true). Then, m_AVD_CLINIT_TMR_START get hit > after 2N Opensaf SU (in promoted SC) is assigned ACTIVE. > So, to fix the problem in #2063, I do either (1) or (2): > (1). As in this patch, remove *is_active = false* in > m_AVD_CLINIT_TMR_START > (2). Before call m_AVD_CLINIT_TMR_START, need to stop this timer if > it's running, likely: > diff --git a/osaf/services/saf/amf/amfd/ndfsm.cc > b/osaf/services/saf/amf/amfd/ndfsm.cc > --- a/osaf/services/saf/amf/amfd/ndfsm.cc > +++ b/osaf/services/saf/amf/amfd/ndfsm.cc > @@ -570,6 +570,8 @@ void avd_nd_ncs_su_assigned(AVD_CL_CB *c > cb->init_state = AVD_INIT_DONE; > > /* start the cluster init timer. */ > + if (cb->amf_init_tmr.is_active == true) > + avd_stop_tmr(cb, &cb->amf_init_tmr); > m_AVD_CLINIT_TMR_START(cb); > } > > I think (1) is reasonable, the is_active could not be forced to be > false before starting timer regardless actual status of timer. The > current *is_active = false* in macro, I think it was meant as an > initial setting, so it'd better be done somewhere in initialize(void), > similarly as > cb->heartbeat_tmr.is_active = false; > > Do you agree if I also add *cb->amf_init_tmr.is_active = false* in > initialize(void)? > > Thanks, > Minh > > On 23/09/16 15:46, Nagendra Kumar wrote: >> Please don't change anything in this function. Please stop and start >> the timer or write a function if needed. >> >> Thanks >> -Nagu >>> -----Original Message----- >>> From: Minh Hon Chau [mailto:minh.c...@dektech.com.au] >>> Sent: 23 September 2016 05:33 >>> To: hans.nordeb...@ericsson.com; Nagendra Kumar; Praveen Malviya; >>> gary....@dektech.com.au; long.hb.ngu...@dektech.com.au; >>> minh.c...@dektech.com.au >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: [PATCH 2 of 2] AMFD: Fix double start timer of AVD_TMR_CL_INIT >>> [#2036] >>> >>> osaf/services/saf/amf/amfd/include/timer.h | 1 - >>> 1 files changed, 0 insertions(+), 1 deletions(-) >>> >>> >>> Since the AVD_TMR_CL_INIT can be started/restarted to wait for all SU >>> presence state >>> synchronization, so m_AVD_CLINIT_TMR_START could not always be called >>> as the first start. As >>> a result, due to @is_active is set to false before start timer, >>> therefore the >>> timer can be >>> restarted without stop in advance. It appears a warning as below >>> >>> Sep 22 22:08:22.640710 osafamfd [477:timer.cc:0066] TEST >> >>> avd_start_tmr: 1 >>> Sep 22 22:08:22.640717 osafamfd [477:sysf_tmr.c:0690] TR IN >>> LEAP_DBG_SINK >>> Sep 22 22:08:22.640758 osafamfd [477:timer.cc:0096] << avd_start_tmr >>> >>> Patch removes setting @is_active to false in m_AVD_CLINIT_TMR_START >>> >>> diff --git a/osaf/services/saf/amf/amfd/include/timer.h >>> b/osaf/services/saf/amf/amfd/include/timer.h >>> --- a/osaf/services/saf/amf/amfd/include/timer.h >>> +++ b/osaf/services/saf/amf/amfd/include/timer.h >>> @@ -65,7 +65,6 @@ typedef struct avd_tmr_tag { >>> #define m_AVD_CLINIT_TMR_START(cb) \ >>> {\ >>> saflog(LOG_NOTICE, amfSvcUsrName, "Starting cluster startup >>> timer"); \ >>> - cb->amf_init_tmr.is_active = false; \ >>> cb->amf_init_tmr.type = AVD_TMR_CL_INIT; \ >>> avd_start_tmr(cb,&(cb->amf_init_tmr), avd_cluster- >>>> saAmfClusterStartupTimeout); \ >>> } > ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
