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
