Hi, AVD_SU saAmfSUFailover should be changed to bool (instead of SaBoolT), that will avoid a number of casts.
See inline. Thanks, Hans On 7 June 2013 08:39, <[email protected]> wrote: > osaf/services/saf/avsv/avd/avd_comp.cc | 9 +- > osaf/services/saf/avsv/avd/avd_sg.cc | 10 ++ > osaf/services/saf/avsv/avd/avd_su.cc | 79 +++++++++++++++++++- > osaf/services/saf/avsv/avd/avd_sutype.cc | 104 > +++++++++++++++++++++++++++- > osaf/services/saf/avsv/avd/include/avd_su.h | 2 + > 5 files changed, 196 insertions(+), 8 deletions(-) > > > The patch allows modify operation on attribute saAmfSutDefSUFailover of > class SaAmfSUType. > Modificaion of saAmfSUFailover in SaAmfSU is allowed only when SG is stable. > > 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 == false) { do not check for == false, negate the expression instead (using !) > + comp->su->saAmfSUFailover = > static_cast<SaBoolT>(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 > @@ -1386,6 +1386,8 @@ static void avd_verify_equal_ranked_su(A > */ > void avd_sg_adjust_config(AVD_SG *sg) > { > + AVD_SU *su; move down to block where needed > + > // SUs in an SG are equal, only need to look at the first one > if ((sg->list_of_su != NULL) && > !sg->list_of_su->saAmfSUPreInstantiable) { > // NPI SUs can only be assigned one SI, see 3.2.1.1 > @@ -1400,6 +1402,14 @@ void avd_sg_adjust_config(AVD_SG *sg) > sg->saAmfSGMaxStandbySIsperSU, > sg->name.value); > } > sg->saAmfSGMaxStandbySIsperSU = 1; > + /* 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) { > + if (!su->saAmfSUFailover) { > + su->saAmfSUFailover = > static_cast<SaBoolT>(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 > @@ -481,6 +481,8 @@ 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); > } > + else > + su->sufailover_conf = true; "sufailover_conf": I think we need to establish a pattern here since we have the problem in different scenarios. First, spell out "configured". Second I think we should use the same prefix as the variable it is associated with. This gives us: "saAmfSUFailover_configured" > > > (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSUMaintenanceCampaign"), > attributes, 0, &su->saAmfSUMaintenanceCampaign); > > @@ -1228,9 +1230,16 @@ 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; > + uint32_t su_failover = *((SaUint32T > *)attr_mod->modAttr.attrValues[0]); > + su = avd_su_get(&opdata->objectName); define and initialize su variable in one go. > + 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"); change log to trace or remove > + goto done; > + } Can't we just remove this "STABLE" check? > 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 +1419,15 @@ static void su_ccb_apply_modify_hdlr(str > value_is_deleted = false; > > if (!strcmp(attr_mod->modAttr.attrName, "saAmfSUFailover")) { > - if (value_is_deleted) > + if (value_is_deleted) { > su->saAmfSUFailover = > static_cast<SaBoolT>(su->su_type->saAmfSutDefSUFailover); > - else > + su->sufailover_conf = false; > + } > + else { > su->saAmfSUFailover = > static_cast<SaBoolT>(*((SaUint32T *)attr_mod->modAttr.attrValues[0])); > + su->sufailover_conf = 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 +1446,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<SaBoolT>(sut->saAmfSutDefSUFailover); > + su->sufailover_conf = false; > + } else { > + su->saAmfSUFailover = > static_cast<SaBoolT>(true); > + su->sufailover_conf = true; > + } > + su_nd_attribute_update(su, saAmfSUFailOver_ID); > su->su_is_external = sut->saAmfSutIsExternal; > } else > osafassert(0); > @@ -1565,3 +1586,51 @@ 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); > } > > +/** > + * update these attributes on the nd rather send attribute update to node director > + * @param su > + * @param attrib_id > + * @param value no value! > + */ > +void su_nd_attribute_update(AVD_SU *su, uint32_t attrib_id) add const to su for attrib_id use enum AVSV_AMF_SU_ATTR_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 > @@ -28,6 +28,8 @@ > #include <avd_proc.h> > > static NCS_PATRICIA_TREE sutype_db; > +static void sutype_ccb_apply_modify_hdlr(struct CcbUtilOperationData > *opdata); > +static SaAisErrorT sutype_ccb_completed_modify_hdlr(CcbUtilOperationData_t > *opdata); move the source for the above functions to before they are used and remove the above forward declarations > > static struct avd_sutype *sutype_new(const SaNameT *dn) > { > @@ -249,6 +251,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); > @@ -278,7 +283,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); > @@ -356,3 +361,100 @@ void avd_sutype_constructor(void) > avd_class_impl_set(const_cast<SaImmClassNameT>("SaAmfSUBaseType"), > NULL, NULL, avd_imm_default_OK_completed_cb, NULL); > avd_class_impl_set(const_cast<SaImmClassNameT>("SaAmfSUType"), NULL, > NULL, sutype_ccb_completed_cb, sutype_ccb_apply_cb); > } > + > +/** > + * 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; > + avd_sutype *sut; > + AVD_SU *su; move to block where needed > + uint32_t old_value; > + > + TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, > opdata->objectName.value); > + > + sut = avd_sutype_get(&opdata->objectName); > + > + while ((attr_mod = opdata->param.modify.attrMods[i++]) != NULL) { > + if (!strcmp(attr_mod->modAttr.attrName, > "saAmfSutDefSUFailover")) { > + 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) { > + su = sut->list_of_su; define and initialize su in one go > + while (su != NULL) { use for loop > + if ((!su->sufailover_conf) && > (su->saAmfSUPreInstantiable)) { > + su->saAmfSUFailover = > static_cast<SaBoolT>(sut->saAmfSutDefSUFailover); > + su_nd_attribute_update(su, > saAmfSUFailOver_ID); > + } > + su = su->su_list_su_type_next; > + } > + } > + } else > + osafassert(0); > + } > + > + 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; > + > + 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 = sut->list_of_su; > + uint32_t sut_failover = *((SaUint32T > *)attr_mod->modAttr.attrValues[0]); > + > + if (sut_failover > SA_TRUE) { > + LOG_ER("invalid saAmfSutDefSUFailover > in:'%s'", su->name.value); > + rc = SA_AIS_ERR_BAD_OPERATION; > + goto done; > + } > + > + while (su != NULL) { use for loop > + if (su->sufailover_conf) { > + su = su->su_list_su_type_next; can be remove with for loop > + 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; > + } > + su = su->su_list_su_type_next; > + } > + } 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; > +} > 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 > @@ -64,6 +64,7 @@ typedef struct avd_su_tag { > uint32_t saAmfSURank; > SaNameT saAmfSUHostNodeOrNodeGroup; > SaBoolT saAmfSUFailover; > + bool sufailover_conf; /* 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(AVD_SU *su, uint32_t attrib_id); > > #endif > > ------------------------------------------------------------------------------ > How ServiceNow helps IT people transform IT departments: > 1. A cloud service to automate IT design, transition and operations > 2. Dashboards that offer high-level views of enterprise services > 3. A single system of record for all IT processes > http://p.sf.net/sfu/servicenow-d2d-j > _______________________________________________ > 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
