Ack. Code review only. /AndersBj
[email protected] wrote: > osaf/services/saf/amf/amfd/compcstype.cc | 13 ++++++++++--- > osaf/services/saf/amf/amfd/imm.cc | 3 ++- > osaf/services/saf/amf/amfd/include/imm.h | 2 +- > osaf/services/saf/amf/amfd/su.cc | 18 ++++++++++++------ > 4 files changed, 25 insertions(+), 11 deletions(-) > > > As of now, if saImmOiRtObjectUpdate_2 in context of > SaImmOiRtAttrUpdateCallbackT, > amf still returns OK to imm. > Amf should return FAILED_OP to SaImmOiRtAttrUpdateCallbackT in case, it > couldn't update the attributes successfully. > This case is hitting when object is being deleted and Amf > is trying to update the some of the attributes of that object to imm. > So, if saImmOiRtObjectUpdate_2 fails, then Amf need to abort to > update the other attributes and immediately return FAILED_OP > to imm. Anyway the object is getting deleted. > > 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 > @@ -394,13 +394,14 @@ static SaAisErrorT compcstype_rt_attr_ca > AVD_COMPCS_TYPE *cst = compcstype_db->find(Amf::to_string(objectName)); > SaImmAttrNameT attributeName; > int i = 0; > + SaAisErrorT rc = SA_AIS_OK; > > - TRACE("%s", objectName->value); > + TRACE_ENTER2("%s", objectName->value); > osafassert(cst != NULL); > > while ((attributeName = attributeNames[i++]) != NULL) { > if (!strcmp("saAmfCompNumCurrActiveCSIs", attributeName)) { > - avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > + rc = avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > SA_IMM_ATTR_SAUINT32T, > &cst->saAmfCompNumCurrActiveCSIs); > } else if (!strcmp("saAmfCompNumCurrStandbyCSIs", > attributeName)) { > avd_saImmOiRtObjectUpdate(objectName, attributeName, > @@ -412,7 +413,13 @@ static SaAisErrorT compcstype_rt_attr_ca > } > } > > - return SA_AIS_OK; > + if (rc != SA_AIS_OK) { > + /* For any failures of update, return FAILED_OP. */ > + rc = SA_AIS_ERR_FAILED_OPERATION; > + } > + > + TRACE_LEAVE2("%u", rc); > + return rc; > } > > void avd_compcstype_constructor(void) > diff --git a/osaf/services/saf/amf/amfd/imm.cc > b/osaf/services/saf/amf/amfd/imm.cc > --- a/osaf/services/saf/amf/amfd/imm.cc > +++ b/osaf/services/saf/amf/amfd/imm.cc > @@ -1430,7 +1430,7 @@ done: > * @param attrValueType > * @param value > */ > -void avd_saImmOiRtObjectUpdate_sync(const SaNameT *dn, SaImmAttrNameT > attributeName, > +SaAisErrorT avd_saImmOiRtObjectUpdate_sync(const SaNameT *dn, SaImmAttrNameT > attributeName, > SaImmValueTypeT attrValueType, void *value) > { > SaAisErrorT rc; > @@ -1451,6 +1451,7 @@ void avd_saImmOiRtObjectUpdate_sync(cons > LOG_WA("saImmOiRtObjectUpdate of '%s' %s failed with %u", > dn->value, attributeName, rc); > } > + return rc; > } > > /** > diff --git a/osaf/services/saf/amf/amfd/include/imm.h > b/osaf/services/saf/amf/amfd/include/imm.h > --- a/osaf/services/saf/amf/amfd/include/imm.h > +++ b/osaf/services/saf/amf/amfd/include/imm.h > @@ -148,7 +148,7 @@ void avd_class_impl_set(const char *clas > SaAisErrorT avd_imm_default_OK_completed_cb(CcbUtilOperationData_t *opdata); > > extern unsigned int avd_imm_config_get(void); > -extern void avd_saImmOiRtObjectUpdate_sync(const SaNameT *dn, > +extern SaAisErrorT avd_saImmOiRtObjectUpdate_sync(const SaNameT *dn, > SaImmAttrNameT attributeName, SaImmValueTypeT attrValueType, void > *value); > extern void avd_saImmOiRtObjectUpdate(const SaNameT* dn, const char > *attributeName, > SaImmValueTypeT attrValueType, void* value); > 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 > @@ -1222,8 +1222,9 @@ static SaAisErrorT su_rt_attr_cb(SaImmOi > AVD_SU *su = su_db->find(Amf::to_string(objectName)); > SaImmAttrNameT attributeName; > int i = 0; > + SaAisErrorT rc = SA_AIS_OK; > > - TRACE("%s", objectName->value); > + TRACE_ENTER2("%s", objectName->value); > > while ((attributeName = attributeNames[i++]) != NULL) { > if (!strcmp("saAmfSUAssignedSIs", attributeName)) { > @@ -1234,20 +1235,25 @@ static SaAisErrorT su_rt_attr_cb(SaImmOi > attributeName, SA_IMM_ATTR_SAUINT32T, > &saAmfSUAssignedSIs); > #endif > } else if (!strcmp("saAmfSUNumCurrActiveSIs", attributeName)) { > - avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > + rc = avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > SA_IMM_ATTR_SAUINT32T, > &su->saAmfSUNumCurrActiveSIs); > } else if (!strcmp("saAmfSUNumCurrStandbySIs", attributeName)) { > - avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > + rc = avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > SA_IMM_ATTR_SAUINT32T, > &su->saAmfSUNumCurrStandbySIs); > } else if (!strcmp("saAmfSURestartCount", attributeName)) { > - avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > + rc = avd_saImmOiRtObjectUpdate_sync(objectName, > attributeName, > SA_IMM_ATTR_SAUINT32T, > &su->saAmfSURestartCount); > } else { > LOG_ER("Ignoring unknown attribute '%s'", > attributeName); > } > + if (rc != SA_AIS_OK) { > + /* For any failures of update, return FAILED_OP. */ > + rc = SA_AIS_ERR_FAILED_OPERATION; > + break; > + } > } > - > - return SA_AIS_OK; > + TRACE_LEAVE2("%u", rc); > + return rc; > } > > > /***************************************************************************** > ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
