Hi Nagu I decided to be less restrictive and ignore the unknown attributes, in case the AMF model is extended in the future.
Eg. an attribute may be added that does not conflict with the current version, so an application designer may deploy the same configuration on multiple versions of OpenSAF. But perhaps that is not a valid use case. Thanks Gary On 15/05/14 22:10, Nagendra Kumar wrote: > Some comments inlined with Nagu .... > > Thanks > -Nagu >> -----Original Message----- >> From: Gary Lee [mailto:gary....@dektech.com.au] >> Sent: 08 May 2014 12:14 >> To: hans.fe...@ericsson.com; hans.nordeb...@ericsson.com; Nagendra >> Kumar; Praveen Malviya >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: [PATCH 1 of 1] amfd: Remove asserts from validation routines [#849] >> >> osaf/services/saf/amf/amfd/app.cc | 18 +++++++++++------- >> osaf/services/saf/amf/amfd/comp.cc | 5 +++-- >> osaf/services/saf/amf/amfd/compcstype.cc | 5 +++-- >> osaf/services/saf/amf/amfd/hlt.cc | 10 ++++++---- >> osaf/services/saf/amf/amfd/sg.cc | 9 +++++---- >> osaf/services/saf/amf/amfd/si.cc | 5 +++-- >> osaf/services/saf/amf/amfd/su.cc | 5 +++-- >> 7 files changed, 34 insertions(+), 23 deletions(-) >> >> >> When an unknown attribute in encountered in various callbacks, sometimes an >> assert is called. >> In other cases, the operation is rejected. >> >> This may create forward compatibility issues if new attributes are added in >> the >> future. >> >> In this patch, the asserts are replaced with a warning and the corresponding >> attribute change >> will be permitted but ignored. >> >> diff --git a/osaf/services/saf/amf/amfd/app.cc >> b/osaf/services/saf/amf/amfd/app.cc >> --- a/osaf/services/saf/amf/amfd/app.cc >> +++ b/osaf/services/saf/amf/amfd/app.cc >> @@ -243,12 +243,15 @@ static SaAisErrorT app_ccb_completed_cb( >> SaNameT dn = *((SaNameT*)attribute- >>> attrValues[0]); >> if (NULL == avd_apptype_get(&dn)) { >> report_ccb_validation_error(opdata, >> "saAmfAppType '%s' not found", dn.value); >> + rc = SA_AIS_ERR_BAD_OPERATION; >> goto done; >> } >> rc = SA_AIS_OK; >> break; >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", >> attribute->attrName); >> + rc = SA_AIS_OK; >> + } > [Nagu]:: Returning ok for bad attribute may not be appropriate.BAD_OP can be > returned. > >> } >> break; >> case CCBUTIL_DELETE: >> @@ -294,10 +297,10 @@ static void app_ccb_apply_cb(CcbUtilOper >> app->saAmfAppType = >> *((SaNameT*)attribute->attrValues[0]); >> app->app_type = avd_apptype_get(&app- >>> saAmfAppType); >> avd_apptype_add_app(app); >> - break; >> } >> - else >> - osafassert(0); >> + else { >> + TRACE("Ignoring unknown attribute '%s'", >> attribute->attrName); >> + } > [Nagu]: If above comments are accepted, then this check is not required. >> } >> break; >> } >> @@ -396,8 +399,9 @@ static SaAisErrorT app_rt_attr_cb(SaImmO >> if (!strcmp(attributeName, "saAmfApplicationCurrNumSGs")) { >> avd_saImmOiRtObjectUpdate_sync(objectName, >> attributeName, >> SA_IMM_ATTR_SAUINT32T, &app- >>> saAmfApplicationCurrNumSGs); >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", >> attributeName); >> + } >> } >> >> return SA_AIS_OK; >> diff --git a/osaf/services/saf/amf/amfd/comp.cc >> b/osaf/services/saf/amf/amfd/comp.cc >> --- a/osaf/services/saf/amf/amfd/comp.cc >> +++ b/osaf/services/saf/amf/amfd/comp.cc >> @@ -830,8 +830,9 @@ static SaAisErrorT comp_rt_attr_cb(SaImm >> /* TODO */ >> } else if (!strcmp("saAmfCompCurrProxiedNames", >> attributeName)) { >> /* TODO */ >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", >> attributeName); >> + } >> } >> >> return SA_AIS_OK; >> diff --git a/osaf/services/saf/amf/amfd/compcstype.cc >> b/osaf/services/saf/amf/amfd/compcstype.cc >> --- a/osaf/services/saf/amf/amfd/compcstype.cc >> +++ b/osaf/services/saf/amf/amfd/compcstype.cc >> @@ -435,8 +435,9 @@ static SaAisErrorT compcstype_rt_attr_ca >> SA_IMM_ATTR_SAUINT32T, &cst- >>> saAmfCompNumCurrStandbyCSIs); >> } else if (!strcmp("saAmfCompAssignedCsi", attributeName)) { >> /* TODO */ >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", >> attributeName); >> + } > [Nagu]: Same comment here. > >> } >> >> return SA_AIS_OK; >> diff --git a/osaf/services/saf/amf/amfd/hlt.cc >> b/osaf/services/saf/amf/amfd/hlt.cc >> --- a/osaf/services/saf/amf/amfd/hlt.cc >> +++ b/osaf/services/saf/amf/amfd/hlt.cc >> @@ -48,8 +48,9 @@ static SaAisErrorT ccb_completed_modify_ >> *value, opdata- >>> objectName.value); >> return SA_AIS_ERR_BAD_OPERATION; >> } >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", attribute- >>> attrName); >> + } > [Nagu]: Same comment here. >> } >> >> return SA_AIS_OK; >> @@ -144,8 +145,9 @@ static void ccb_apply_modify_hdlr(CcbUti >> TRACE("saAmfHealthcheckMaxDuration modified to >> '%llu' for Comp'%s'", *param_val, >> comp->comp_info.name.value); >> param.attr_id = saAmfHealthcheckMaxDuration_ID; >> - } else >> - osafassert(0); >> + } else { >> + TRACE("Ignoring unknown attribute '%s'", attribute- >>> attrName); >> + } > [Nagu]: Same comment here. > >> avd_snd_op_req_msg(avd_cb, comp->su->su_on_node, >> ¶m); >> } >> diff --git a/osaf/services/saf/amf/amfd/sg.cc >> b/osaf/services/saf/amf/amfd/sg.cc >> --- a/osaf/services/saf/amf/amfd/sg.cc >> +++ b/osaf/services/saf/amf/amfd/sg.cc >> @@ -627,7 +627,7 @@ static SaAisErrorT ccb_completed_modify_ >> goto done; >> } >> } else { >> - osafassert(0); >> + LOG_WA("Ignoring unknown attribute '%s'", >> attribute->attrName); > [Nagu]: Same comment here. > >> } >> } /* while (attr_mod != NULL) */ >> >> @@ -933,7 +933,7 @@ static void ccb_apply_modify_hdlr(CcbUti >> } else if (!strcmp(attribute->attrName, >> "saAmfSGSuHostNodeGroup")) { >> sg->saAmfSGSuHostNodeGroup = *((SaNameT >> *)value); >> } else { >> - osafassert(0); >> + TRACE("Ignoring unknown attribute '%s'", >> attribute->attrName); >> } >> >> attr_mod = opdata->param.modify.attrMods[i++]; >> @@ -1322,8 +1322,9 @@ static SaAisErrorT sg_rt_attr_cb(SaImmOi >> } else if (!strcmp("saAmfSGNumCurrInstantiatedSpareSUs", >> attributeName)) { >> avd_saImmOiRtObjectUpdate_sync(objectName, >> attributeName, >> SA_IMM_ATTR_SAUINT32T, &sg- >>> saAmfSGNumCurrInstantiatedSpareSUs); >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", >> attributeName); >> + } >> } >> >> return SA_AIS_OK; >> 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 >> @@ -973,8 +973,9 @@ static SaAisErrorT si_rt_attr_cb(SaImmOi >> } else if (!strcmp("saAmfSINumCurrStandbyAssignments", >> attributeName)) { >> avd_saImmOiRtObjectUpdate_sync(objectName, >> attributeName, >> SA_IMM_ATTR_SAUINT32T, &si- >>> saAmfSINumCurrStandbyAssignments); >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", >> attributeName); >> + } >> } >> >> return SA_AIS_OK; >> diff --git a/osaf/services/saf/amf/amfd/su.cc >> b/osaf/services/saf/amf/amfd/su.cc >> --- a/osaf/services/saf/amf/amfd/su.cc >> +++ b/osaf/services/saf/amf/amfd/su.cc >> @@ -1197,8 +1197,9 @@ static SaAisErrorT su_rt_attr_cb(SaImmOi >> } else if (!strcmp("saAmfSURestartCount", attributeName)) { >> avd_saImmOiRtObjectUpdate_sync(objectName, >> attributeName, >> SA_IMM_ATTR_SAUINT32T, &su- >>> saAmfSURestartCount); >> - } else >> - osafassert(0); >> + } else { >> + LOG_WA("Ignoring unknown attribute '%s'", >> attributeName); >> + } >> } >> >> return SA_AIS_OK; ------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
