Ack, code review only.
Thanks,
Praveen
On 06-Feb-17 1:32 PM, Nguyen Luu wrote:
> Hi Praveen,
>
> Thank you for your comments.
>
> New patch has been sent out for review. Document update will be sent for
> review soon.
>
> Best Regards,
> Nguyen
>
> On 1/31/2017 1:41 PM, praveen malviya wrote:
>> Hi,
>>
>> Patch looks fine but some minor corrections are needed and some
>> documentation is also required. Please find them inline with [Praveen]
>>
>> Thanks,
>> Praveen
>>
>> On 24-Jan-17 2:53 PM, Nguyen TK Luu wrote:
>>> src/amf/amfd/comp.cc | 15 +++++++++++++++
>>> src/amf/amfnd/compdb.cc | 2 ++
>>> src/amf/common/amf_defs.h | 2 ++
>>> src/amf/config/amf_classes.xml | 1 +
>>> 4 files changed, 20 insertions(+), 0 deletions(-)
>>>
>>>
>>> Current OpenSAF implementation defines the attribute saAmfCompCmdEnv
>>> in the SaAmfComp class as non-writable according to last AMF
>>> specification.
>>> This restriction doesn't allow the upgrade of environment attributes
>>> defined
>>> at component instance, or to remove it.
>>>
>>> The attribute was made writable in an errata to the AMF model
>>> (www.saforum.org/HOA/assn16627/images/SAI-IM-XMI-A.04.02.errata.xml.zip).
>>>
>>> OpenSAF should comply to this change in the errata to enable the
>>> upgrade of
>>> environment attributes defined at component instance level.
>>>
>>> diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc
>>> --- a/src/amf/amfd/comp.cc
>>> +++ b/src/amf/amfd/comp.cc
>>> @@ -1,6 +1,7 @@
>>> /* -*- OpenSAF -*-
>>> *
>>> * (C) Copyright 2008 The OpenSAF Foundation
>>> + * (C) Copyright Ericsson AB 2017 - All Rights Reserved.
>>> *
>>> * This program is distributed in the hope that it will be useful, but
>>> * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> MERCHANTABILITY
>>> @@ -963,6 +964,15 @@
>>> }
>>> }
>>> }
>>> + } else if (!strcmp(attribute->attrName,
>>> "saAmfCompCmdEnv")) {
>>> + if (value_is_deleted == true)
>>> + continue;
>>> + char *param_val = *(static_cast<char
>>> **>(value));
>>> + if (nullptr == param_val) {
>>> + report_ccb_validation_error(opdata,
>>> + "Modification of
>>> saAmfCompCmdEnv Fail, nullptr arg");
>>> + goto done;
>>> + }
>> [Praveen] Since this attribute is becoming writable, it needs to be
>> documented in AMF PR doc. Please also mention in PR doc that
>> modification should be done after complete upgrade to 5.2 release.
>> This is needed because a) during upgrade if modification is attempted
>> and standby AMFD is old it will assert in this function and b) old
>> payloads will ignore message with notice " NO avnd_comp_oper_req:
>> Unsupported attribute 41" and will return error to amfd.
>>
>> In ndproc.cc LOG_ER can be converted to warning.
>>
>>> } else if (!strcmp(attribute->attrName,
>>> "saAmfCompInstantiateCmdArgv")) {
>>> if (value_is_deleted == true)
>>> continue;
>>> @@ -1314,6 +1324,11 @@
>>> param.attr_id = saAmfCompType_ID;
>>> param.name_sec = *dn;
>>>
>>> + } else if (!strcmp(attribute->attrName,
>>> "saAmfCompCmdEnv")) {
>>> + /* Node director will reread configuration
>>> from IMM */
>>> + param.attr_id = saAmfCompCmdEnv_ID;
>>> + TRACE("saAmfCompCmdEnv modified.");
>>> +
>>> } else if (!strcmp(attribute->attrName,
>>> "saAmfCompInstantiateCmdArgv")) {
>>>
>>> /* Node director will reread configuration from IMM */
>>> diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc
>>> --- a/src/amf/amfnd/compdb.cc
>>> +++ b/src/amf/amfnd/compdb.cc
>>> @@ -1,6 +1,7 @@
>>> /* -*- OpenSAF -*-
>>> *
>>> * (C) Copyright 2008 The OpenSAF Foundation
>>> + * (C) Copyright Ericsson AB 2017 - All Rights Reserved.
>>> *
>>> * This program is distributed in the hope that it will be useful, but
>>> * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> MERCHANTABILITY
>>> @@ -366,6 +367,7 @@
>>> case saAmfCompCleanupCmd_ID:
>>> case saAmfCompAmStartCmd_ID:
>>> case saAmfCompAmStopCmd_ID:
>>> + case saAmfCompCmdEnv_ID:
>>> comp->config_is_valid = 0;
>>> break;
>>> case saAmfCompInstantiateTimeout_ID:
>>> diff --git a/src/amf/common/amf_defs.h b/src/amf/common/amf_defs.h
>>> --- a/src/amf/common/amf_defs.h
>>> +++ b/src/amf/common/amf_defs.h
>>> @@ -1,6 +1,7 @@
>>> /* -*- OpenSAF -*-
>>> *
>>> * (C) Copyright 2008 The OpenSAF Foundation
>>> + * (C) Copyright Ericsson AB 2017 - All Rights Reserved.
>>> *
>>> * This program is distributed in the hope that it will be useful, but
>>> * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> MERCHANTABILITY
>>> @@ -247,6 +248,7 @@
>>> saAmfCompCurrProxyName_ID = 37,
>>> saAmfCompAMEnable_ID = 38,
>>> saAmfCompProxyStatus_ID = 39,
>>> + saAmfCompCmdEnv_ID = 40,
>>> saAmfCompType_ID,
>>> } AVSV_AMF_COMP_ATTR_ID;
>>>
>> [Praveen] Above change is backward incompatible. keep saAmfCompType_ID
>> 40.
>>> diff --git a/src/amf/config/amf_classes.xml
>>> b/src/amf/config/amf_classes.xml
>>> --- a/src/amf/config/amf_classes.xml
>>> +++ b/src/amf/config/amf_classes.xml
>>> @@ -1056,6 +1056,7 @@
>>> <name>saAmfCompCmdEnv</name>
>>> <type>SA_STRING_T</type>
>>> <category>SA_CONFIG</category>
>>> + <flag>SA_WRITABLE</flag>
>>> <flag>SA_MULTI_VALUE</flag>
>>> </attr>
>>> <attr>
>>>
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
