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
