Ack with minor comments inline Btw it was not easy to understand that this was not a complete patch series. Please always send a complete series. /Hans
On 28 June 2013 08:21, <[email protected]> wrote: > osaf/services/saf/avsv/avd/avd_comp.cc | 9 ++- > osaf/services/saf/avsv/avd/avd_sg.cc | 9 ++ > osaf/services/saf/avsv/avd/avd_su.cc | 87 ++++++++++++++++++++++++-- > osaf/services/saf/avsv/avd/avd_sutype.cc | 92 > ++++++++++++++++++++++++++++- > osaf/services/saf/avsv/avd/include/avd_su.h | 4 +- > 5 files changed, 188 insertions(+), 13 deletions(-) > > > Refloating the patch after incorporating comments. > > diff --git a/osaf/services/saf/avsv/avd/avd_comp.cc > b/osaf/services/saf/avsv/avd/avd_comp.cc > --- a/osaf/services/saf/avsv/avd/avd_comp.cc > +++ b/osaf/services/saf/avsv/avd/avd_comp.cc > @@ -1449,8 +1449,13 @@ static void comp_ccb_apply_delete_hdlr(s > /* if SU is not being deleted and the PreInstantiable state has > changed > * then update the IMM with the new value for saAmfSUPreInstantiable > */ > if (su_delete == SA_FALSE && old_val != > comp->su->saAmfSUPreInstantiable) { > - avd_saImmOiRtObjectUpdate(&comp->su->name, > const_cast<SaImmAttrNameT>("saAmfSUPreInstantiable"), SA_IMM_ATTR_SAUINT32T, > - &comp->su->saAmfSUPreInstantiable); > + avd_saImmOiRtObjectUpdate(&comp->su->name, > const_cast<SaImmAttrNameT>("saAmfSUPreInstantiable"), > + SA_IMM_ATTR_SAUINT32T, > &comp->su->saAmfSUPreInstantiable); > + /* If SU becomes NPI then enable saAmfSUFailover flag Sec > 3.11.1.3.2 AMF-B.04.01 spec */ > + if (!comp->su->saAmfSUPreInstantiable) { > + comp->su->saAmfSUFailover = true; > + su_nd_attribute_update(comp->su, saAmfSUFailOver_ID); > + } > } > > /* send a message to the AVND deleting the > 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 > @@ -1400,6 +1400,15 @@ void avd_sg_adjust_config(AVD_SG *sg) > sg->saAmfSGMaxStandbySIsperSU, > sg->name.value); > } > sg->saAmfSGMaxStandbySIsperSU = 1; > + AVD_SU *su; > + /* saAmfSUFailover must be true for a NPI SU sec 3.11.1.3.2 > AMF-B.04.01 spec */ > + for (su = sg->list_of_su; su != NULL; su = > su->sg_list_su_next) { this is now C++ and you could move the su variable definition inside the for() like for (AVD_SU *su = ...) we should do this a lot more after the C++ change to avoid bugs > + if (!su->saAmfSUFailover) { > + su->saAmfSUFailover = true; > + su_nd_attribute_update(su, > saAmfSUFailOver_ID); > + } > + } > + > } > > /* adjust saAmfSGNumPrefInserviceSUs if not configured */ > diff --git a/osaf/services/saf/avsv/avd/avd_su.cc > b/osaf/services/saf/avsv/avd/avd_su.cc > --- a/osaf/services/saf/avsv/avd/avd_su.cc > +++ b/osaf/services/saf/avsv/avd/avd_su.cc > @@ -58,7 +58,7 @@ AVD_SU *avd_su_new(const SaNameT *dn) > su->name.length = dn->length; > su->tree_node.key_info = (uint8_t *)&(su->name); > avsv_sanamet_init(dn, &sg_name, "safSg"); > - su->saAmfSUFailover = static_cast<SaBoolT>(false); > + su->saAmfSUFailover = false; > su->term_state = static_cast<SaBoolT>(false); > su->su_switch = AVSV_SI_TOGGLE_STABLE; > su->saAmfSUPreInstantiable = static_cast<SaBoolT>(false); > @@ -479,8 +479,10 @@ static AVD_SU *su_create(const SaNameT * > } > > if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSUFailover"), > attributes, 0, &su->saAmfSUFailover) != SA_AIS_OK) { > - su->saAmfSUFailover = > static_cast<SaBoolT>(sut->saAmfSutDefSUFailover); > + su->saAmfSUFailover = > static_cast<bool>(sut->saAmfSutDefSUFailover); > } > + else > + su->saAmfSUFailover_configured = true; > > > (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSUMaintenanceCampaign"), > attributes, 0, &su->saAmfSUMaintenanceCampaign); > > @@ -1228,9 +1230,17 @@ static SaAisErrorT su_ccb_completed_modi > continue; > > if (!strcmp(attr_mod->modAttr.attrName, "saAmfSUFailover")) { > - bool su_failover = *((SaTimeT > *)attr_mod->modAttr.attrValues[0]); > + AVD_SU *su = avd_su_get(&opdata->objectName); > + uint32_t su_failover = *((SaUint32T > *)attr_mod->modAttr.attrValues[0]); > + > + if (su->sg_of_su->sg_fsm_state != AVD_SG_FSM_STABLE) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + TRACE("'%s' is not > stable",su->sg_of_su->name.value); > + goto done; > + } > + > if (su_failover > SA_TRUE) { > - LOG_ER("Invalid saAmfSUFailover %u", > su_failover); > + LOG_ER("Invalid saAmfSUFailover SU:'%s'", > su->name.value); > rc = SA_AIS_ERR_BAD_OPERATION; > goto done; > } > @@ -1410,10 +1420,15 @@ static void su_ccb_apply_modify_hdlr(str > value_is_deleted = false; > > if (!strcmp(attr_mod->modAttr.attrName, "saAmfSUFailover")) { > - if (value_is_deleted) > - su->saAmfSUFailover = > static_cast<SaBoolT>(su->su_type->saAmfSutDefSUFailover); > - else > - su->saAmfSUFailover = > static_cast<SaBoolT>(*((SaUint32T *)attr_mod->modAttr.attrValues[0])); > + if (value_is_deleted) { > + su->saAmfSUFailover = > static_cast<bool>(su->su_type->saAmfSutDefSUFailover); > + su->saAmfSUFailover_configured = false; > + } > + else { > + su->saAmfSUFailover = > static_cast<bool>(*((SaUint32T *)attr_mod->modAttr.attrValues[0])); > + su->saAmfSUFailover_configured = true; > + } > + su_nd_attribute_update(su, saAmfSUFailOver_ID); > } else if (!strcmp(attr_mod->modAttr.attrName, > "saAmfSUMaintenanceCampaign")) { > if (value_is_deleted) { > su->saAmfSUMaintenanceCampaign.length = 0; > @@ -1432,7 +1447,14 @@ static void su_ccb_apply_modify_hdlr(str > su->saAmfSUType = sutype_name; > su->su_type = sut; > avd_sutype_add_su(su); > - su->saAmfSUFailover = > static_cast<SaBoolT>(sut->saAmfSutDefSUFailover); > + if (su->saAmfSUPreInstantiable) { > + su->saAmfSUFailover = > static_cast<bool>(sut->saAmfSutDefSUFailover); > + su->saAmfSUFailover_configured = false; > + } else { > + su->saAmfSUFailover = true; > + su->saAmfSUFailover_configured = true; > + } > + su_nd_attribute_update(su, saAmfSUFailOver_ID); > su->su_is_external = sut->saAmfSutIsExternal; > } else > osafassert(0); > @@ -1565,3 +1587,50 @@ void avd_su_constructor(void) > avd_class_impl_set(const_cast<SaImmClassNameT>("SaAmfSU"), > su_rt_attr_cb, su_admin_op_cb, su_ccb_completed_cb, su_ccb_apply_cb); > } > > +/** > + * send update of SU attribute to node director > + * @param su > + * @param attrib_id > + */ > +void su_nd_attribute_update(const AVD_SU *su, AVSV_AMF_SU_ATTR_ID attrib_id) > +{ > + AVD_AVND *su_node_ptr = NULL; > + AVSV_PARAM_INFO param; > + memset(((uint8_t *)¶m), '\0', sizeof(AVSV_PARAM_INFO)); > + > + TRACE_ENTER(); > + > + if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) { > + TRACE_LEAVE2("avd is not in active state"); > + return; > + } > + m_AVD_GET_SU_NODE_PTR(avd_cb, su, su_node_ptr); > + param.class_id = AVSV_SA_AMF_SU; > + param.act = AVSV_OBJ_OPR_MOD; > + param.name = su->name; > + > + switch (attrib_id) { > + case saAmfSUFailOver_ID: > + { > + uint32_t sufailover; > + param.attr_id = saAmfSUFailOver_ID; > + param.value_len = sizeof(uint32_t); > + sufailover = htonl(su->saAmfSUFailover); > + memcpy(¶m.value[0], &sufailover, param.value_len); > + break; > + } > + default: > + osafassert(0); > + } > + > + /*Update this value on the node hosting this SU*/ > + if ((su_node_ptr) && ((su_node_ptr->node_state == > AVD_AVND_STATE_PRESENT) || > + (su_node_ptr->node_state == AVD_AVND_STATE_NO_CONFIG) > || > + (su_node_ptr->node_state == > AVD_AVND_STATE_NCS_INIT))) { > + if (avd_snd_op_req_msg(avd_cb, su_node_ptr, ¶m) != > NCSCC_RC_SUCCESS) { > + LOG_ER("%s:failed for %s",__FUNCTION__, > su_node_ptr->name.value); > + } > + } > + > + TRACE_LEAVE(); > +} > diff --git a/osaf/services/saf/avsv/avd/avd_sutype.cc > b/osaf/services/saf/avsv/avd/avd_sutype.cc > --- a/osaf/services/saf/avsv/avd/avd_sutype.cc > +++ b/osaf/services/saf/avsv/avd/avd_sutype.cc > @@ -237,6 +237,40 @@ done1: > return error; > } > > +/** > + * This function handles modify operations on SaAmfSUType objects. > + * @param ptr to opdata > + */ > +static void sutype_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_sutype *sut = avd_sutype_get(&opdata->objectName); > + > + while ((attr_mod = opdata->param.modify.attrMods[i++]) != NULL) { > + if (!strcmp(attr_mod->modAttr.attrName, > "saAmfSutDefSUFailover")) { > + uint32_t old_value = sut->saAmfSutDefSUFailover; > + sut->saAmfSutDefSUFailover = *((SaUint32T > *)attr_mod->modAttr.attrValues[0]); > + /* Modify saAmfSUFailover for SUs which had inherited > saAmfSutDefSUFailover. > + Modification will not be done for the NPI SU > */ > + if (old_value != sut->saAmfSutDefSUFailover) { > + AVD_SU *su; > + for (su = sut->list_of_su; su; su = > su->su_list_su_type_next) { > + if ((!su->saAmfSUFailover_configured) > && (su->saAmfSUPreInstantiable)) { > + su->saAmfSUFailover = > static_cast<bool>(sut->saAmfSutDefSUFailover); > + su_nd_attribute_update(su, > saAmfSUFailOver_ID); > + } > + } > + } > + } else > + osafassert(0); > + } > + > + TRACE_LEAVE(); > +} > + > static void sutype_ccb_apply_cb(CcbUtilOperationData_t *opdata) > { > struct avd_sutype *sut; > @@ -249,6 +283,9 @@ static void sutype_ccb_apply_cb(CcbUtilO > osafassert(sut); > sutype_db_add(sut); > break; > + case CCBUTIL_MODIFY: > + sutype_ccb_apply_modify_hdlr(opdata); > + break; > case CCBUTIL_DELETE: > sut = avd_sutype_get(&opdata->objectName); > sutype_db_delete(sut); > @@ -262,6 +299,59 @@ static void sutype_ccb_apply_cb(CcbUtilO > TRACE_LEAVE(); > } > > +/** > + * This routine validates modify CCB operations on SaAmfSUType objects. > + * @param Ccb Util Oper Data > + */ > + > +static SaAisErrorT sutype_ccb_completed_modify_hdlr(CcbUtilOperationData_t > *opdata) > +{ > + SaAisErrorT rc = SA_AIS_OK; > + const SaImmAttrModificationT_2 *attr_mod; > + int i = 0; > + avd_sutype *sut = avd_sutype_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)) { > + LOG_ER("Attributes cannot be deleted in SaAmfSUType"); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + if (!strcmp(attr_mod->modAttr.attrName, > "saAmfSutDefSUFailover")) { > + AVD_SU *su; > + uint32_t sut_failover = *((SaUint32T > *)attr_mod->modAttr.attrValues[0]); > + > + if (sut_failover > SA_TRUE) { > + LOG_ER("invalid saAmfSutDefSUFailover > in:'%s'", sut->name.value); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + for (su = sut->list_of_su; su; su = > su->su_list_su_type_next) { > + if (su->saAmfSUFailover_configured) > + continue; > + > + if (su->sg_of_su->sg_fsm_state != > AVD_SG_FSM_STABLE) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + LOG_WA("SG state is not stable"); > + goto done; > + } > + } > + } else { > + LOG_ER("attribute is non writable:'%s' ", > attr_mod->modAttr.attrName); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + } > + > +done: > + TRACE_LEAVE(); > + return rc; > +} > + > static SaAisErrorT sutype_ccb_completed_cb(CcbUtilOperationData_t *opdata) > { > SaAisErrorT rc = SA_AIS_ERR_BAD_OPERATION; > @@ -278,7 +368,7 @@ static SaAisErrorT sutype_ccb_completed_ > rc = SA_AIS_OK; > break; > case CCBUTIL_MODIFY: > - LOG_ER("Modification of SaAmfSUType not supported"); > + rc = sutype_ccb_completed_modify_hdlr(opdata); > break; > case CCBUTIL_DELETE: > sut = avd_sutype_get(&opdata->objectName); > diff --git a/osaf/services/saf/avsv/avd/include/avd_su.h > b/osaf/services/saf/avsv/avd/include/avd_su.h > --- a/osaf/services/saf/avsv/avd/include/avd_su.h > +++ b/osaf/services/saf/avsv/avd/include/avd_su.h > @@ -63,7 +63,8 @@ typedef struct avd_su_tag { > SaNameT saAmfSUType; > uint32_t saAmfSURank; > SaNameT saAmfSUHostNodeOrNodeGroup; > - SaBoolT saAmfSUFailover; > + bool saAmfSUFailover; > + bool saAmfSUFailover_configured; /* True when user configures > saAmfSUFailover else false */ > SaNameT saAmfSUMaintenanceCampaign; > > /* runtime attributes */ > @@ -239,5 +240,6 @@ extern void avd_su_inc_curr_stdby_si(AVD > extern void avd_su_dec_curr_stdby_si(AVD_SU *su); > extern uint32_t avd_su_get_current_no_of_assignments(AVD_SU *su, > SaAmfHAStateT ha_state); > extern AVD_SU *avd_su_get_or_create(const SaNameT *dn); > +extern void su_nd_attribute_update(const AVD_SU *su, AVSV_AMF_SU_ATTR_ID > attrib_id); > > #endif > > ------------------------------------------------------------------------------ > 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
