On 04-Dec-14 2:16 PM, Hans Nordebäck wrote:
> Hi Praveen,
>
> thanks, but my question was related to the if statement in the patch
> below, see inline/Thanks HansN
> On 12/04/2014 09:34 AM, praveen malviya wrote:
>>
>>
>> On 03-Dec-14 5:48 PM, Hans Nordebäck wrote:
>>> ack, code review only. One question, why is a non-configured
>>> saAmfSIPrefActiveAssignments set to 1, and not to
>>> the value of saAmfSGNumPrefAssignedSUs?
>>
>> In section 8.11 SI class diagram says that non configured value of
>> saAmfSIPrefActiveAssignments is 1. But spec redefines it to
>> aAmfSGNumPrefAssignedSUs in section 3.6.5.3.
>> We are keeping the default value 1 in when SI is being created.
>> When assignments are going on saAmfSGNumPrefAssignedSUs will be used if
>> saAmfSIPrefActiveAssignments is not configured.
>>
>> Thanks,
>> Praveen
>>
>> /Regards HansN
>>>
>>> On 12/03/2014 12:37 PM, praveen.malv...@oracle.com wrote:
>>>> osaf/services/saf/amf/amfd/include/ckpt.h | 3 ++-
>>>> osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc | 12 ++++++++++++
>>>> osaf/services/saf/amf/amfd/si.cc | 9 +++++----
>>>> 3 files changed, 19 insertions(+), 5 deletions(-)
>>>>
>>>>
>>>> Note:Please apply this patch on top of already floated patch for #1190.
>>>>
>>>> A patch is floated on opensaf-devel for fixing the default value of
>>>> saAmfSIPrefActiveAssignments against #1190.
>>>> Floated patch had following problems:
>>>> 1)If a configuration sets saAmfSIPrefActiveAssignments =
>>>> saAmfSGNumPrefAssignedSUs.
>>>> In this case SI will be marked Fully Assigned as soon as it is
>>>> assigned to first SU
>>>> and other SUs are still in lock-in state.
>>>>
>>>> 2)If peer AMFD is with old version than old AMFD will asset in some
>>>> cases like:
>>>> a)configuration: 3 SUs in nway active SG with following params
>>>> saAmfSGNumPrefInserviceSUs=3,
>>>> saAmfSGNumPrefAssignedSUs=3, and
>>>> saAmfSIPrefActiveAssignments=1.
>>>>
>>>> b)Bring SC-1 with this patch and SC-2 without this patch.
>>>> Unlock-in and unlock all the SUs.
>>>> c)controller swap.
>>>> d) lock SU3. AMFD will crash.
>>>> si.cc:1260: update_ass_state: Assertion
>>>> 'saAmfSINumCurrActiveAssignments <= saAmfSIPrefActiveAssignments'
>>>> failed.
>>>>
>>>> This is because SC-1 had check pointed
>>>> saAmfSINumCurrActiveAssignments
>>>> =3 which is not accordance with old amfd.
>>>>
>>>> 3)If user himself wants to configure saAmfSIPrefActiveAssignments=1.
>>>> AMF cannot distinguish if value 1 is set by the user or it is the
>>>> default value given by IMM.
>>>> This is AMF schema related issue and AMF PR doc needs to be updated
>>>> for this.
>>>>
>>>> Patch fixes the problem by incrementing the AVD_MBCSV_SUB_PART_VERSION
>>>> and
>>>> adjusting saAmfSIPrefActiveAssignments using peer amfd version.
>>>>
>>>> Since AMFD version is being upgraded, this ticket needs to be pushed as
>>>> enhancement.
>>>>
>>>> diff --git a/osaf/services/saf/amf/amfd/include/ckpt.h
>>>> b/osaf/services/saf/amf/amfd/include/ckpt.h
>>>> --- a/osaf/services/saf/amf/amfd/include/ckpt.h
>>>> +++ b/osaf/services/saf/amf/amfd/include/ckpt.h
>>>> @@ -34,9 +34,10 @@
>>>> #define AVD_CKP_H
>>>> // current version
>>>> -#define AVD_MBCSV_SUB_PART_VERSION 6
>>>> +#define AVD_MBCSV_SUB_PART_VERSION 7
>>>> // supported versions
>>>> +#define AVD_MBCSV_SUB_PART_VERSION_7 7
>>>> #define AVD_MBCSV_SUB_PART_VERSION_6 6
>>>> #define AVD_MBCSV_SUB_PART_VERSION_5 5
>>>> #define AVD_MBCSV_SUB_PART_VERSION_4 4
>>>> diff --git a/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc
>>>> b/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc
>>>> --- a/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc
>>>> +++ b/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc
>>>> @@ -518,6 +518,18 @@ uint32_t SG_NACV::su_insvc(AVD_CL_CB *cb
>>>> return NCSCC_RC_SUCCESS;
>>>> }
>>>> + for (AVD_SI *si = su->sg_of_su->list_of_si; si != NULL; si =
>>>> si->sg_list_of_si_next) {
>>>> + if ((si->saAmfSIPrefActiveAssignments == 1) &&
>>>> + (avd_cb->avd_peer_ver <=
>>>> AVD_MBCSV_SUB_PART_VERSION_6) &&
>>>> + (si->saAmfSIPrefActiveAssignments <
>>>> si->saAmfSINumCurrActiveAssignments)) {
>>>> + si->saAmfSIPrefActiveAssignments =
>>>> si->saAmfSINumCurrActiveAssignments;
>>>> + si->adjust_si_assignments(1);
>>>> + si->saAmfSIPrefActiveAssignments = 1;
>>>> + }
>>>> + }
>>>> + if (su->sg_of_su->sg_fsm_state != AVD_SG_FSM_STABLE)
>>>> + return NCSCC_RC_SUCCESS;
>>>> +
>>>> if (avd_sg_nacvred_su_chose_asgn(cb, su->sg_of_su) == NULL) {
>>>> avd_sg_app_su_inst_func(cb, su->sg_of_su);
>>>> if (AVD_SG_FSM_STABLE == su->sg_of_su->sg_fsm_state) {
>>>> 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
>>>> @@ -1260,8 +1260,7 @@ void AVD_SI::update_ass_state()
>>>> osafassert(saAmfSINumCurrActiveAssignments <=
>>>> pref_active_assignments());
>>>> // TODO sg-num_pref_assigned_sus() should return number
>>>> of in-service
>>>> // SUs then the second statement can be removed!
>>>> - if ((saAmfSINumCurrActiveAssignments ==
>>>> pref_active_assignments()) ||
>>>> - (pref_active_assignments() ==
>>>> sg_of_si->num_pref_assigned_sus()))
>>>> + if (saAmfSINumCurrActiveAssignments ==
>>>> pref_active_assignments())
>>>> newState = SA_AMF_ASSIGNMENT_FULLY_ASSIGNED;
>>>> else
>>>> newState = SA_AMF_ASSIGNMENT_PARTIALLY_ASSIGNED;
>>>> @@ -1408,10 +1407,12 @@ void AVD_SI::set_si_switch(AVD_CL_CB *cb
>>>> uint32_t AVD_SI::pref_active_assignments() const
>>>> {
>>>> - if (saAmfSIPrefActiveAssignments == 1) {
>>>> + if ((saAmfSIPrefActiveAssignments == 1) &&
> [HansN] what does == 1 mean here?
If user does not configure this attribute then its value will be 1 as
per schema and its value is set to 1 when SI was created. In this if
condition, 1 means we have to return default value which means
saAmfSGNumPrefAssignedSUs.
If not 1 it means user has configured the attribute and else part will
return the configured value.
At the time of SI creation it cannot be set to saAmfSGNumPrefAssignedSUs
because Sg may not be available to read this attribute. If this SG
attribute is also not configured then we have to count no of SUs.
Because of this it is better to read and adjust it when needed.
Thanks,
Praveen
>>>> + (avd_cb->avd_peer_ver > AVD_MBCSV_SUB_PART_VERSION_6)) {
>>>> + TRACE("Peer AVD has higher AVD_MBCSV_SUB_PART_VERSION than
>>>> AVD_MBCSV_SUB_PART_VERSION_6");
>>>> // Default value: the preferred number of assigned service
>>>> units.
>>>> return sg_of_si->num_pref_assigned_sus();
>>>> - } else
>>>> + } else
>>>> return saAmfSIPrefActiveAssignments;
>>>> }
>>>
>
>
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
