Hi,
instead of the complex if statements you can add member functions to
AVD_SI,
example, if (dep_si->is_tol_timer_running() etc.
/Thanks HansN
On 08/26/2015 09:05 AM, Nagendra Kumar wrote:
> Hi Praveen,
> Thanks for your comment.
> I think we can have two approaches:
> 1. Remove the code itself as suggested by you. OR
> 2. Correct the logic in the intended way. For example, please check below
> correction:
>> - if((dep_si->si_dep_state != AVD_SI_TOL_TIMER_RUNNING) ||
>> - (dep_si->si_dep_state !=
>> AVD_SI_READY_TO_UNASSIGN)) {
>> + /* Don't start tol timer if dep state are either in
>> running or unassigned. */
>> + if(!((dep_si->si_dep_state == AVD_SI_TOL_TIMER_RUNNING)
>> ||
>> + (dep_si->si_dep_state ==
>> AVD_SI_READY_TO_UNASSIGN))) {
>>
>> avd_sidep_start_tolerance_timer_for_dependant(dep_si, si);
> The intention looks that if si_dep_state is in either tol_running or
> ready_to_unassigned, then don't start the timer, else start the timer.
> So, I corrected the logic in the intended way. Now, the code will have the
> same sense as thought while coding.
> And if the logic written is wrong in the first place and since the check was
> always passing, now after
> correction of code, the test case may fail. But then we can correct the fault
> in si dep flow and add logic on top.
>
> Let me know everybody opinion.
>
> Thanks
> -Nagu
>
>> -----Original Message-----
>> From: praveen malviya
>> Sent: 26 August 2015 12:15
>> To: Nagendra Kumar; hans.nordeb...@ericsson.com;
>> minh.c...@dektech.com.au; gary....@dektech.com.au
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] amfd: correct logic in si dep flow [#276]
>>
>> Code inside the if block was always executing, which means if condition is
>> not
>> needed.
>> Why can't simply remove the if condition.
>>
>> Thanks,
>> Praveen
>> On 26-Aug-15 11:51 AM, nagendr...@oracle.com wrote:
>>> osaf/services/saf/amf/amfd/si_dep.cc | 25 ++++++++++++-------------
>>> 1 files changed, 12 insertions(+), 13 deletions(-)
>>>
>>>
>>> At many places, there has been tautological errors in si dep flow.
>>> The fix corrects them
>>>
>>> diff --git a/osaf/services/saf/amf/amfd/si_dep.cc
>>> b/osaf/services/saf/amf/amfd/si_dep.cc
>>> --- a/osaf/services/saf/amf/amfd/si_dep.cc
>>> +++ b/osaf/services/saf/amf/amfd/si_dep.cc
>>> @@ -1468,10 +1468,10 @@ void avd_sidep_unassign_dependents(AVD_S
>>> if (m_NCS_MDS_DEST_EQUAL(&sisu->su->su_on_node-
>>> adest,&su->su_on_node->adest)) {
>>> avd_si_unassign(dep_si);
>>> } else {
>>> - if((dep_si->si_dep_state !=
>> AVD_SI_TOL_TIMER_RUNNING) ||
>>> - (dep_si->si_dep_state !=
>> AVD_SI_READY_TO_UNASSIGN)) {
>>> + /* Don't start tol timer if dep state are either in
>> running or unassigned. */
>>> + if(!((dep_si->si_dep_state ==
>> AVD_SI_TOL_TIMER_RUNNING) ||
>>> + (dep_si->si_dep_state ==
>> AVD_SI_READY_TO_UNASSIGN))) {
>> avd_sidep_start_tolerance_timer_for_dependant(dep_si, si);
>>> -
>>> }
>>> }
>>> /* If this dependent SI is sponsor too, then unassign its
>>> dependents also */ @@ -1788,9 +1788,9 @@ void
>>> avd_sidep_update_depstate_si_failov
>>>
>>> if(su->su_on_node->saAmfNodeOperState ==
>> SA_AMF_OPERATIONAL_DISABLED) {
>>> if ((m_NCS_MDS_DEST_EQUAL(&sisu-
>>> su->su_on_node->adest,&su->su_on_node->adest))) {
>>> - if(((dep_si->si_dep_state !=
>> AVD_SI_TOL_TIMER_RUNNING) ||
>>> - (dep_si->si_dep_state
>> != AVD_SI_READY_TO_UNASSIGN) ||
>>> - (dep_si->si_dep_state
>> != AVD_SI_FAILOVER_UNDER_PROGRESS)) &&
>>> + if((!((dep_si->si_dep_state ==
>> AVD_SI_TOL_TIMER_RUNNING) ||
>>> + (dep_si->si_dep_state
>> == AVD_SI_READY_TO_UNASSIGN) ||
>>> + (dep_si->si_dep_state
>> == AVD_SI_FAILOVER_UNDER_PROGRESS))) &&
>> (avd_sidep_sponsors_assignment_states(dep_si))) {
>>>
>> avd_sidep_si_dep_state_set(dep_si,
>>> AVD_SI_FAILOVER_UNDER_PROGRESS); @@ -1801,10 +1801,9 @@ void
>> avd_sidep_update_depstate_si_failov
>>> }
>>> }
>>> } else if (dep_si->sg_of_si == si->sg_of_si) {
>>> - if((dep_si->si_dep_state !=
>> AVD_SI_TOL_TIMER_RUNNING) ||
>>> - (dep_si->si_dep_state !=
>> AVD_SI_READY_TO_UNASSIGN) ||
>>> - (dep_si->si_dep_state !=
>> AVD_SI_FAILOVER_UNDER_PROGRESS)) {
>>> -
>>> + if(!((dep_si->si_dep_state ==
>> AVD_SI_TOL_TIMER_RUNNING) ||
>>> + (dep_si->si_dep_state ==
>> AVD_SI_READY_TO_UNASSIGN) ||
>>> + (dep_si->si_dep_state ==
>> AVD_SI_FAILOVER_UNDER_PROGRESS))) {
>> avd_sidep_si_dep_state_set(dep_si,
>> AVD_SI_FAILOVER_UNDER_PROGRESS);
>>> if (dep_si->num_dependents
>>> 0) {
>>> /* This SI also has
>> dependents under it, update their state
>>> also */ @@ -1842,9 +1841,9 @@ void avd_sidep_update_depstate_si_failov
>>> }
>>> if (sponsor_assignments_state
>> == true) {
>>> - if((dep_si-
>>> si_dep_state != AVD_SI_TOL_TIMER_RUNNING) ||
>>> - (dep_si-
>>> si_dep_state != AVD_SI_READY_TO_UNASSIGN) ||
>>> - (dep_si-
>>> si_dep_state != AVD_SI_FAILOVER_UNDER_PROGRESS)) {
>>> + if(!((dep_si-
>>> si_dep_state == AVD_SI_TOL_TIMER_RUNNING) ||
>>> + (dep_si-
>>> si_dep_state == AVD_SI_READY_TO_UNASSIGN) ||
>>> + (dep_si-
>>> si_dep_state == AVD_SI_FAILOVER_UNDER_PROGRESS)))
>>> +{
>>>
>>>
>> avd_sidep_si_dep_state_set(dep_si,
>> AVD_SI_FAILOVER_UNDER_PROGRESS);
>>> if (dep_si-
>>> num_dependents > 0) {
>>>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
