Ack with inline comments. I think we should allow changing autoRepair with requiring the SG to be locked. That means service outage and it is not acceptable and we might just drop the support completely instead. Thanks, Hans
On 28 June 2013 08:21, <[email protected]> wrote: > osaf/services/saf/avsv/avd/avd_sg.cc | 16 +++- > osaf/services/saf/avsv/avd/avd_sgtype.cc | 106 > +++++++++++++++++++++++++++- > osaf/services/saf/avsv/avd/include/avd_sg.h | 3 +- > 3 files changed, 119 insertions(+), 6 deletions(-) > > > Refloating the patch after incorporating comments. > > diff --git a/osaf/services/saf/avsv/avd/avd_sg.cc > b/osaf/services/saf/avsv/avd/avd_sg.cc > --- a/osaf/services/saf/avsv/avd/avd_sg.cc > +++ b/osaf/services/saf/avsv/avd/avd_sg.cc > @@ -293,7 +293,10 @@ static AVD_SG *sg_create(const SaNameT * > > if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGAutoRepair"), > attributes, 0, &sg->saAmfSGAutoRepair) != SA_AIS_OK) { > sg->saAmfSGAutoRepair = sgt->saAmfSgtDefAutoRepair; > + sg->saAmfSGAutoRepair_configured = false; > } > + else > + sg->saAmfSGAutoRepair_configured = true; > > if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGAutoAdjust"), > attributes, 0, &sg->saAmfSGAutoAdjust) != SA_AIS_OK) { > sg->saAmfSGAutoAdjust = sgt->saAmfSgtDefAutoAdjust; > @@ -527,6 +530,12 @@ static SaAisErrorT ccb_completed_modify_ > rc = SA_AIS_ERR_BAD_OPERATION; > goto done; > } else if (!strcmp(attribute->attrName, > "saAmfSGAutoRepair")) { > + uint32_t sg_autorepair = *((SaUint32T > *)attribute->attrValues[0]); > + if (sg_autorepair > SA_TRUE) { > + LOG_ER("Invalid saAmfSGAutoRepair > SG:'%s'", avd_sg->name.value); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > } else if (!strcmp(attribute->attrName, > "saAmfSGAutoAdjust")) { > } else if (!strcmp(attribute->attrName, > "saAmfSGNumPrefActiveSUs")) { > } else if (!strcmp(attribute->attrName, > "saAmfSGNumPrefStandbySUs")) { > @@ -744,10 +753,13 @@ static void ccb_apply_modify_hdlr(CcbUti > sg_nd_attribute_update(sg, > saAmfSGCompRestartProb_ID); > sg_nd_attribute_update(sg, > saAmfSGCompRestartMax_ID); > } else if (!strcmp(attribute->attrName, > "saAmfSGAutoRepair")) { > - if (value_is_deleted) > + if (value_is_deleted) { > sg->saAmfSGAutoRepair = > sg_type->saAmfSgtDefAutoRepair; > - else > + sg->saAmfSGAutoRepair_configured = > false; > + } else { > sg->saAmfSGAutoRepair = > static_cast<SaBoolT>(*((SaUint32T *)value)); > + sg->saAmfSGAutoRepair_configured = > true; > + } > } else if (!strcmp(attribute->attrName, > "saAmfSGAutoAdjust")) { > if (value_is_deleted) > sg->saAmfSGAutoAdjust = > sg_type->saAmfSgtDefAutoAdjust; > diff --git a/osaf/services/saf/avsv/avd/avd_sgtype.cc > b/osaf/services/saf/avsv/avd/avd_sgtype.cc > --- a/osaf/services/saf/avsv/avd/avd_sgtype.cc > +++ b/osaf/services/saf/avsv/avd/avd_sgtype.cc > @@ -221,7 +221,10 @@ static AVD_AMF_SG_TYPE *sgtype_create(Sa > > if > (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSgtDefAutoRepair"), > attributes, 0, &sgt->saAmfSgtDefAutoRepair) != SA_AIS_OK) { > sgt->saAmfSgtDefAutoRepair = SA_TRUE; > + sgt->saAmfSgtDefAutoRepair_configured = false; > } > + else > + sgt->saAmfSgtDefAutoRepair_configured = true; > > if > (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSgtDefAutoAdjust"), > attributes, 0, &sgt->saAmfSgtDefAutoAdjust) != SA_AIS_OK) { > sgt->saAmfSgtDefAutoAdjust = SA_FALSE; > @@ -314,6 +317,56 @@ SaAisErrorT avd_sgtype_config_get(void) > return error; > } > > + > +/** > + * This routine validates modify CCB operations on SaAmfSGType objects. > + * @param Ccb Util Oper Data > + * @return > + */ > + > +static SaAisErrorT sgtype_ccb_completed_modify_hdlr(CcbUtilOperationData_t > *opdata) > +{ > + SaAisErrorT rc = SA_AIS_OK; > + const SaImmAttrModificationT_2 *attr_mod; > + int i = 0; > + AVD_AMF_SG_TYPE *sgt = avd_sgtype_get(&opdata->objectName); > + > + TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, > opdata->objectName.value); > + while ((attr_mod = opdata->param.modify.attrMods[i++]) != NULL) { > + > + if ((attr_mod->modType == SA_IMM_ATTR_VALUES_DELETE) || > (attr_mod->modAttr.attrValues == NULL)) > + continue; > + > + if (!strcmp(attr_mod->modAttr.attrName, > "saAmfSgtDefAutoRepair")) { > + > + uint32_t sgt_autorepair = *((SaUint32T > *)attr_mod->modAttr.attrValues[0]); > + if (sgt_autorepair > SA_TRUE) { > + LOG_ER("invalid saAmfSgtDefAutoRepair > in:'%s'", sgt->name.value); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + AVD_SG *sg; > + for (sg = sgt->list_of_sg; sg; sg = > sg->sg_list_sg_type_next) { same comment as before, move sg definition inside AND move the whole validation outside the LOCK check. > + if (sg->saAmfSGAutoRepair_configured) > + continue; > + if (sg->saAmfSGAdminState != > SA_AMF_ADMIN_LOCKED_INSTANTIATION) { > + rc = SA_AIS_ERR_BAD_OPERATION; > + LOG_WA("SG state is not in Lock-in > state:'%s'",sg->name.value); > + goto done; > + } > + } > + } else { > + LOG_ER("Modification of attribute is Not > Supported:'%s' ", attr_mod->modAttr.attrName); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + } > + > +done: > + return rc; > +} > + > /** > * Validate SaAmfSGType CCB completed > * @param opdata > @@ -336,8 +389,7 @@ static SaAisErrorT sgtype_ccb_completed_ > rc = SA_AIS_OK; > break; > case CCBUTIL_MODIFY: > - LOG_ER("Modification of SaAmfSGType not supported"); > - goto done; > + rc = sgtype_ccb_completed_modify_hdlr(opdata); > break; > case CCBUTIL_DELETE: > sgt = avd_sgtype_get(&opdata->objectName); > @@ -373,6 +425,52 @@ done: > } > > /** > + * This function handles modify operations on SaAmfSGType objects. > + * @param ptr to opdata > + */ > +static void sgtype_ccb_apply_modify_hdlr(struct CcbUtilOperationData *opdata) > +{ > + const SaImmAttrModificationT_2 *attr_mod; > + int i = 0; > + > + TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, > opdata->objectName.value); > + > + AVD_AMF_SG_TYPE *sgt = avd_sgtype_get(&opdata->objectName); > + > + while ((attr_mod = opdata->param.modify.attrMods[i++]) != NULL) { > + bool value_is_deleted; > + if ((attr_mod->modType == SA_IMM_ATTR_VALUES_DELETE) || > (attr_mod->modAttr.attrValues == NULL)) > + /* Attribute value is deleted, revert to default > value */ > + value_is_deleted = true; > + else > + /* Attribute value is modified */ > + value_is_deleted = false; > + > + if (!strcmp(attr_mod->modAttr.attrName, > "saAmfSgtDefAutoRepair")) { > + SaBoolT old_value = sgt->saAmfSgtDefAutoRepair; > + if (value_is_deleted) { > + sgt->saAmfSgtDefAutoRepair = > static_cast<SaBoolT>(true); > + sgt->saAmfSgtDefAutoRepair_configured = false; > + } else { > + sgt->saAmfSgtDefAutoRepair = > static_cast<SaBoolT>(*((SaUint32T *)attr_mod->modAttr.attrValues[0])); > + sgt->saAmfSgtDefAutoRepair_configured = true; > + } > + /* Modify saAmfSGAutoRepair for SGs which had > inherited saAmfSgtDefAutoRepair.*/ > + if (old_value != sgt->saAmfSgtDefAutoRepair) { > + AVD_SG *sg; > + for (sg = sgt->list_of_sg; sg; sg = > sg->sg_list_sg_type_next) { move sg definition inside > + if (!sg->saAmfSGAutoRepair_configured) > + sg->saAmfSGAutoRepair = > static_cast<SaBoolT>(sgt->saAmfSgtDefAutoRepair); > + } > + } > + } > + } > + > + TRACE_LEAVE(); > +} > + > + > +/** > * Apply SaAmfSGType CCB changes > * @param opdata > * > @@ -390,6 +488,9 @@ static void sgtype_ccb_apply_cb(CcbUtilO > osafassert(sgt); > sgtype_add_to_model(sgt); > break; > + case CCBUTIL_MODIFY: > + sgtype_ccb_apply_modify_hdlr(opdata); > + break; > case CCBUTIL_DELETE: > > sgtype_delete(static_cast<AVD_AMF_SG_TYPE*>(opdata->userData)); > break; > @@ -410,4 +511,3 @@ void avd_sgtype_constructor(void) > avd_class_impl_set(const_cast<SaImmClassNameT>("SaAmfSGType"), NULL, > NULL, sgtype_ccb_completed_cb, sgtype_ccb_apply_cb); > avd_class_impl_set(const_cast<SaImmClassNameT>("SaAmfSGBaseType"), > NULL, NULL, avd_imm_default_OK_completed_cb, NULL); > } > - > diff --git a/osaf/services/saf/avsv/avd/include/avd_sg.h > b/osaf/services/saf/avsv/avd/include/avd_sg.h > --- a/osaf/services/saf/avsv/avd/include/avd_sg.h > +++ b/osaf/services/saf/avsv/avd/include/avd_sg.h > @@ -69,6 +69,7 @@ typedef struct avd_sg_tag { > * Checkpointing - Sent as a one time update. > */ > > + bool saAmfSGAutoRepair_configured; /* True when user configures > saAmfSGAutoRepair else false */ > /******************** B.04 model > *************************************************/ > SaNameT saAmfSGType; /* Network order. */ > SaNameT saAmfSGSuHostNodeGroup; /* Network order. */ > @@ -242,7 +243,7 @@ typedef struct avd_sg_tag { > typedef struct avd_amf_sg_type_tag { > NCS_PATRICIA_NODE tree_node; /* key will be sg type name */ > SaNameT name; > - > + bool saAmfSgtDefAutoRepair_configured; /* True when user configures > saAmfSGDefAutoRepair else false */ > /******************** B.04 model > *************************************************/ > SaNameT *saAmfSGtValidSuTypes; /* array of DNs, size in > number_su_type */ > SaAmfRedundancyModelT saAmfSgtRedundancyModel; > > ------------------------------------------------------------------------------ > This SF.net email is sponsored by Windows: > > Build for Windows Store. > > http://p.sf.net/sfu/windows-dev2dev > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
