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
