Hi Nagu,

you mean the patch is acked and it can be pushed?

  /Thanks HansN

On 09/10/2015 01:40 PM, Nagendra Kumar wrote:
> Hi Hans N,
>               Comments inlined.
>
> Thanks
> -Nagu
>
>> -----Original Message-----
>> From: Hans Nordebäck [mailto:[email protected]]
>> Sent: 10 September 2015 16:54
>> To: Nagendra Kumar; Praveen Malviya; Gary Lee
>> Cc: [email protected]
>> Subject: RE: [devel] [PATCH 1 of 1] amfd: Don't send alarm SI has no current
>> active assignments if node is locked V2 [#1465]
>>
>> Hi Nagu,
>>
>> Please see comment inlined/Thanks HansN
>>
>> -----Original Message-----
>> From: Nagendra Kumar [mailto:[email protected]]
>> Sent: den 10 september 2015 13:04
>> To: Hans Nordebäck; Praveen Malviya; Gary Lee
>> Cc: [email protected]
>> Subject: RE: [devel] [PATCH 1 of 1] amfd: Don't send alarm SI has no current
>> active assignments if node is locked V2 [#1465]
>>
>> Hi Hans N,
>>      Please find comment inlined.
>>
>> Thanks
>> -Nagu
>>> -----Original Message-----
>>> From: Hans Nordebäck [mailto:[email protected]]
>>> Sent: 10 September 2015 16:17
>>> To: Nagendra Kumar; Praveen Malviya; [email protected]
>>> Cc: [email protected]
>>> Subject: Re: [devel] [PATCH 1 of 1] amfd: Don't send alarm SI has no
>>> current active assignments if node is locked V2 [#1465]
>>>
>>> Hi Nagu,
>>>
>>> please see comment below. /Thanks HansN
>>>
>>> On 09/10/2015 12:30 PM, Nagendra Kumar wrote:
>>>> Forgot to provide comments in the previous email. Please find
>>>> comments
>>> inlined with [Nagu].
>>>> Thanks
>>>> -Nagu
>>>>> -----Original Message-----
>>>>> From: Nagendra Kumar
>>>>> Sent: 10 September 2015 15:58
>>>>> To: Hans Nordeback; Praveen Malviya; [email protected]
>>>>> Cc: [email protected]
>>>>> Subject: Re: [devel] [PATCH 1 of 1] amfd: Don't send alarm SI has
>>>>> no current active assignments if node is locked V2 [#1465]
>>>>>
>>>>> Hi Hans N,
>>>>>
>>>>> I tested with combination of SU/Node lock/locked-in with pres state
>>>>> inst fail/term fail.
>>>>> So, when node is locked/locked-in, even if su goes into term fail,
>>>>> the alarm
>>> is
>>>>> not sent because the pres state becomes uninst before
>>>>> update_ass_state
>>> gets
>>>>> called. Hope this is ok.
>>>>>
>>>>> Ack with the comments inlined with [Nagu] and the documentation
>>>>> need
>>> to
>>>>> be updated as well.
>>>>>
>>>>>
>>>>> Thanks
>>>>> -Nagu
>>>>>> -----Original Message-----
>>>>>> From: Hans Nordeback [mailto:[email protected]]
>>>>>> Sent: 04 September 2015 19:20
>>>>>> To: Praveen Malviya; Nagendra Kumar; [email protected]
>>>>>> Cc: [email protected]
>>>>>> Subject: [PATCH 1 of 1] amfd: Don't send alarm SI has no current
>>>>>> active assignments if node is locked V2 [#1465]
>>>>>>
>>>>>>    osaf/services/saf/amf/amfd/include/si.h |   2 ++
>>>>>>    osaf/services/saf/amf/amfd/si.cc        |  23 +++++++++++++++++++----
>>>>>>    osaf/services/saf/amf/amfd/siass.cc     |   3 +++
>>>>>>    3 files changed, 24 insertions(+), 4 deletions(-)
>>>>>>
>>>>>>
>>>>>> diff --git a/osaf/services/saf/amf/amfd/include/si.h
>>>>>> b/osaf/services/saf/amf/amfd/include/si.h
>>>>>> --- a/osaf/services/saf/amf/amfd/include/si.h
>>>>>> +++ b/osaf/services/saf/amf/amfd/include/si.h
>>>>>> @@ -109,6 +109,8 @@ public:
>>>>>>
>>>>>>          bool alarm_sent; /* SI unassigned alarm has been sent */
>>>>>>
>>>>>> +        struct avd_avnd_tag *si_on_node;        /*  the node on which 
>>>>>> this SI
>>>>>> resides */
>>>>>> +        AVD_SU *curr_su;                        /* The SU with the 
>>>>>> latest
>>>>> assignment*/
>>>>>>          void inc_curr_act_ass();
>>>>>>          void dec_curr_act_ass();
>>>>>>          void inc_curr_stdby_ass();
>>>>>> diff --git a/osaf/services/saf/amf/amfd/si.cc
>>>>>> b/osaf/services/saf/amf/amfd/si.cc
>>>>>> --- a/osaf/services/saf/amf/amfd/si.cc
>>>>>> +++ b/osaf/services/saf/amf/amfd/si.cc
>>>>>> @@ -26,6 +26,7 @@
>>>>>>    #include <csi.h>
>>>>>>    #include <proc.h>
>>>>>>    #include <si_dep.h>
>>>>>> +#include <node.h>
>>>>>>
>>>>>>    AmfDb<std::string, AVD_SI> *si_db = NULL;
>>>>>>
>>>>>> @@ -320,7 +321,9 @@ AVD_SI::AVD_SI() :
>>>>>>          list_of_sus_per_si_rank(NULL),
>>>>>>          rankedsu_list_head(NULL),
>>>>>>          invocation(0),
>>>>>> -        alarm_sent(false)
>>>>>> +        alarm_sent(false),
>>>>>> +        si_on_node{nullptr},
>>>>>> +        curr_su{nullptr}
>>>>>>    {
>>>>>>          memset(&name, 0, sizeof(SaNameT));
>>>>>>          memset(&saAmfSvcType, 0, sizeof(SaNameT)); @@ -1315,9 +1318,21
>>>>> @@
>>>>>> void AVD_SI::update_ass_state()
>>>>>>
>>>>>>                  /* alarm & notifications */
>>>>>>                  if (saAmfSIAssignmentState ==
>>>>>> SA_AMF_ASSIGNMENT_UNASSIGNED) {
>>>>>> -                        avd_send_si_unassigned_alarm(&name);
>>>>>> -                        alarm_sent = true;
>>>>>> -                        m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(avd_cb,
>>>>>> this, AVSV_CKPT_SI_ALARM_SENT);
>>>>>> +                        osafassert(si_on_node != nullptr);
>>>>>> +                        osafassert(curr_su != nullptr);
>>>>>> +                        if ((sg_of_si->sg_ncs_spec == true) &&
>>>>>> +                            (sg_of_si->sg_type->saAmfSgtRedundancyModel 
>>>>>> ==
>>>>>> SA_AMF_NO_REDUNDANCY_MODEL) &&
>>>>>> +                            (curr_su->saAmfSUPresenceState !=
>>>>>> SA_AMF_PRESENCE_INSTANTIATION_FAILED) &&
>>>>>> +                            (curr_su->saAmfSUPresenceState !=
>>>>>> SA_AMF_PRESENCE_TERMINATION_FAILED) &&
>>>>>> +                            ((si_on_node->saAmfNodeAdminState ==
>>>>>> SA_AMF_ADMIN_SHUTTING_DOWN) ||
>>>>>> +                             (si_on_node->saAmfNodeAdminState ==
>>>>>> SA_AMF_ADMIN_LOCKED) ||
>>>>>> +                             (si_on_node->saAmfNodeAdminState ==
>>>>>> SA_AMF_ADMIN_LOCKED_INSTANTIATION))) {
>>>>>> +                                LOG_NO("Node is locked, no SI unassigned
>>>>>> alarm will be sent");
>>>>>> +                        } else {
>>>>>> +                                avd_send_si_unassigned_alarm(&name);
>>>>>> +                                alarm_sent = true;
>>>>>> +
>>>>>>  m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(avd_cb, this,
>>>>>> AVSV_CKPT_SI_ALARM_SENT);
>>>>>> +                        }
>>>>>>                  }
>>>>>>                  else {
>>>>>>                          avd_send_si_assigned_ntf(&name, oldState,
>>>>> saAmfSIAssignmentState);
>>>>>> diff --git a/osaf/services/saf/amf/amfd/siass.cc
>>>>>> b/osaf/services/saf/amf/amfd/siass.cc
>>>>>> --- a/osaf/services/saf/amf/amfd/siass.cc
>>>>>> +++ b/osaf/services/saf/amf/amfd/siass.cc
>>>>>> @@ -166,6 +166,9 @@ AVD_SU_SI_REL *avd_susi_create(AVD_CL_CB
>>>>>>          su_si->si = si;
>>>>>>          su_si->su = su;
>>>>>>
>>>>>> +        si->si_on_node = su->su_on_node;
>>>>>> +        si->curr_su = su;
>>>> [Nagu]: This need to be reset when susi is deleted.
>>> [HansN] The curr_su reflects the latest assigned su and it will be
>>> changed when a new susi is created. It is needed to keep the curr_su
>>> as "update_ass_state" is called after susi has been deleted. The
>>> curr_su presence state is checked in "update_ass_state", so curr_su
>>> can not be null but point to the latest su, before susi delete.
>> [Nagu]: After deleting SUSI of the SI, the information of si_on_node and
>> curr_su is of no use.
>> [HansN] I thought that the information si_on_node and curr_su is of no use
>> after, at least, "update_ass_state" has finished, now It is removed in
>> avd_susi_delete before calling "dec_curr_...", so it should be valid in
>> "update_ass_state" as I see it.
> [Nagu]: I got it, so you mean first use in "update_ass_state" and then
> delete the information later on after calling "update_ass_state".
>
> But I think safer is do these assignments(si->si_on_node = su->su_on_node; 
> si->curr_su = su;) in the if condition for Mw NoRed.
> Anyway, it gets overwritten when next SUSI is created with same SI. Also, No 
> Red SI is not assigned to more than one
> SI, so no problem.
>
>>      So, as per balancing act, when assigned during creation of association
>> of SU-SI, delete should happen during deletion of association of     SU-SI.
>> Because SI can get assigned to another node and another SU when creating
>> SU-SI and the older information is not valid.
>>      Also, since this information is only used for m/w No Red SI, the check
>> is need here for the same and comment could be added.
>>      The another reason is SI can be assigned to another multiple SUs for
>> other Red Models and the values of node and SU will get overridden.
>>>>
>>>>>> +
>>>>>>          /*
>>>>>>           * Add the susi rel rec to the ordered si-list
>>>>>>           */
>>>>> -------------------------------------------------------------------
>>>>> ----------- Monitor Your Dynamic Infrastructure at Any Scale With
>>>>> Datadog!
>>>>> Get real-time metrics from all of your servers, apps and tools in one
>> place.
>>>>> SourceForge users - Click here to start your Free Trial of Datadog now!
>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
>>>>> _______________________________________________
>>>>> Opensaf-devel mailing list
>>>>> [email protected]
>>>>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel



------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to