On 01/24/2014 05:06 AM, Nagendra Kumar wrote: >>> + (su_ptr->saAmfSUPresenceState == >>> + SA_AMF_PRESENCE_INSTANTIATED) && >> >> keep above expression on same line > > But the line is exceeding 80 lines when the line break is kept in the same > line.
That should not be the case with proper code structure and naming. I think the above is an exemption to the rule because it makes the code very hard to read and understand if you break lines like that. /Hans > > Thanks > -Nagu > >> -----Original Message----- >> From: Hans Feldt [mailto:hans.fe...@ericsson.com] >> Sent: 23 January 2014 19:23 >> To: Nagendra Kumar; hans.nordeb...@ericsson.com; Praveen Malviya; >> Mathivanan Naickan Palanivelu; Hrishikesh Chenna >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: Re: [PATCH 1 of 1] amf: support for automatic comp instantiation and >> deletion [#597] >> >> Ack with minor comments inline. >> Thanks, >> Hans >> >> >> On 01/23/2014 07:20 AM, nagendr...@oracle.com wrote: >>> osaf/services/saf/amf/amfd/comp.cc | 37 ++++++++++++++++--- >>> osaf/services/saf/amf/amfd/ndproc.cc | 10 +++- >>> osaf/services/saf/amf/amfnd/clc.cc | 10 +++++ >>> osaf/services/saf/amf/amfnd/compdb.cc | 47 >> +++++++++++++++++++++--- >>> osaf/services/saf/amf/amfnd/include/avnd_comp.h | 2 + >>> osaf/services/saf/amf/amfnd/su.cc | 27 ++++++++++++- >>> 6 files changed, 115 insertions(+), 18 deletions(-) >>> >>> >>> When any component is added in the instantiated SU, Amf will try to >> instantiate the component. >>> When any component is deleted from instantiated SU, Amf will try to >> terminate the component. >>> >>> 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 >>> @@ -173,17 +173,18 @@ void avd_comp_delete(AVD_COMP *comp) >>> static void comp_add_to_model(AVD_COMP *comp) >>> { >>> SaNameT dn; >>> + AVD_SU *su_ptr = comp->su; >> >> better with just "su" >> >>> >>> TRACE_ENTER2("%s", comp->comp_info.name.value); >>> >>> /* Check parent link to see if it has been added already */ >>> - if (comp->su != NULL) { >>> + if (su_ptr != NULL) { >>> TRACE("already added"); >>> goto done; >>> } >>> >>> avsv_sanamet_init(&comp->comp_info.name, &dn, "safSu"); >>> - comp->su = avd_su_get(&dn); >>> + su_ptr = avd_su_get(&dn); >>> >>> avd_comp_db_add(comp); >>> comp->comp_type = avd_comptype_get(&comp->saAmfCompType); >>> @@ -195,7 +196,7 @@ static void comp_add_to_model(AVD_COMP * >>> * corresponding node is UP send the component information >>> * to the Node. >>> */ >>> - if (comp->su->su_is_external) { >>> + if (su_ptr->su_is_external) { >>> /* This is an external SU, so there is no node assigned to it. >>> For some purpose of validations and sending SU/Comps info >> to >>> hosting node (Controllers), we can take use of the hosting >>> @@ -219,12 +220,36 @@ static void comp_add_to_model(AVD_COMP * >>> if ((comp->comp_info.category == AVSV_COMP_TYPE_SA_AWARE) || >>> (comp->comp_info.category == >> AVSV_COMP_TYPE_PROXIED_LOCAL_PRE_INSTANTIABLE) || >>> (comp->comp_info.category == >> AVSV_COMP_TYPE_EXTERNAL_PRE_INSTANTIABLE)) { >>> - comp->su->saAmfSUPreInstantiable = >> static_cast<SaBoolT>(true); >>> + su_ptr->saAmfSUPreInstantiable = static_cast<SaBoolT>(true); >>> + } >>> + >>> + /* This is a case of adding a component in SU which is instantiated >>> + state. This could be used in upgrade scenarios. When components >>> + are added, it is sent to Amfnd for instantiation and Amfnd >>> + instantiates it. Amfnd has capability for finding which component >>> + has been added newly. */ >>> + if ((su_ptr->saAmfSUAdminState != >> SA_AMF_ADMIN_LOCKED_INSTANTIATION) && >>> + (su_ptr->saAmfSUPresenceState == >>> + SA_AMF_PRESENCE_INSTANTIATED) && >> >> keep above expression on same line >> >>> + (avd_cb->avail_state_avd == SA_AMF_HA_ACTIVE)) { >>> + AVD_AVND *node = su_ptr->su_on_node; >>> + if ((node->node_state == AVD_AVND_STATE_PRESENT) || >>> + (node->node_state == >>> + AVD_AVND_STATE_NO_CONFIG) || >> >> keep above expression on same line >> >>> + (node->node_state == >> AVD_AVND_STATE_NCS_INIT)) { >>> + if (avd_snd_su_msg(avd_cb, su_ptr) != >>> + NCSCC_RC_SUCCESS) { >>> + LOG_ER("%s: avd_snd_su_msg failed %s", >>> + __FUNCTION__, >>> + su_ptr->name.value); >> >> log to whom you failed to send and comp DN, one day you will need it... >> >>> + goto done; >>> + } >>> + } >>> } >>> >>> /* Set runtime cached attributes. */ >>> - avd_saImmOiRtObjectUpdate(&comp->su->name, >> const_cast<SaImmAttrNameT>("saAmfSUPreInstantiable"), >>> - SA_IMM_ATTR_SAUINT32T, &comp->su- >>> saAmfSUPreInstantiable); >>> + avd_saImmOiRtObjectUpdate(&su_ptr->name, >> const_cast<SaImmAttrNameT>("saAmfSUPreInstantiable"), >>> + SA_IMM_ATTR_SAUINT32T, &su_ptr- >>> saAmfSUPreInstantiable); >>> >>> avd_saImmOiRtObjectUpdate(&comp->comp_info.name, >> const_cast<SaImmAttrNameT>("saAmfCompReadinessState"), >>> SA_IMM_ATTR_SAUINT32T, &comp- >>> saAmfCompReadinessState); >>> diff --git a/osaf/services/saf/amf/amfd/ndproc.cc >> b/osaf/services/saf/amf/amfd/ndproc.cc >>> --- a/osaf/services/saf/amf/amfd/ndproc.cc >>> +++ b/osaf/services/saf/amf/amfd/ndproc.cc >>> @@ -608,9 +608,13 @@ void avd_data_update_req_evh(AVD_CL_CB * >>> case AVSV_SA_AMF_COMP:{ >>> /* Find the component record in the database, >> specified in the message. */ >>> if ((comp = avd_comp_get(&n2d_msg- >>> msg_info.n2d_data_req.param_info.name)) == NULL) { >>> - LOG_ER("%s: Invalid Comp '%s' (%u)", >> __FUNCTION__, >>> - n2d_msg- >>> msg_info.n2d_data_req.param_info.name.value, >>> - n2d_msg- >>> msg_info.n2d_data_req.param_info.name.length); >>> + /* In case of component delete, component >> gets >>> + deleted at Amfd first and then it gets >>> + uninstantiated and then deleted at Amfnd. >>> + So, when update for presence state comes >> from >>> + Amfnd, the component doesn't exists in >> Amfd*/ >>> + LOG_IN("%s: Invalid Comp '%s'", >> __FUNCTION__, >>> + n2d_msg- >>> msg_info.n2d_data_req.param_info.name.value); >>> goto done; >>> } >>> >>> diff --git a/osaf/services/saf/amf/amfnd/clc.cc >> b/osaf/services/saf/amf/amfnd/clc.cc >>> --- a/osaf/services/saf/amf/amfnd/clc.cc >>> +++ b/osaf/services/saf/amf/amfnd/clc.cc >>> @@ -1306,6 +1306,16 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_ >>> } >>> } >>> >>> + /* This is a case of 'component delete' when su is in intantiated >>> + state. Component can go into uninstantiated state or termination >>> + failed state, but the component information needs to be deleted from >>> + data base. */ >>> + if ((comp->pending_delete == true) && >>> + ((comp->pres == >> SA_AMF_PRESENCE_UNINSTANTIATED) || >>> + (comp->pres == >> SA_AMF_PRESENCE_TERMINATION_FAILED)) && >>> + (comp->su->pres == >> SA_AMF_PRESENCE_INSTANTIATED)) >>> + rc = avnd_compdb_rec_del(cb, &comp->name); >>> + >>> done: >>> TRACE_LEAVE2("%u", rc); >>> return rc; >>> diff --git a/osaf/services/saf/amf/amfnd/compdb.cc >> b/osaf/services/saf/amf/amfnd/compdb.cc >>> --- a/osaf/services/saf/amf/amfnd/compdb.cc >>> +++ b/osaf/services/saf/amf/amfnd/compdb.cc >>> @@ -837,15 +837,50 @@ uint32_t avnd_comp_oper_req(AVND_CB *cb, >>> >>> case AVSV_OBJ_OPR_DEL: >>> { >> >> the above opening brace is on the wrong line thus causing unnecessary >> indentation and hard top read code, please change >> or remove (if possible) it >> >>> - AVND_COMP *comp = 0; >>> + /* This request comes when any component is being >>> + deleted. When this request comes from Amfd, >>> + component can be in instantiated or uninstantiated >>> + state and in that case, Amfnd will uninstantiate the >>> + component and delete the record from its data base. >>> + This is kind of automatic termination of a component >>> + when comp is getting deleted. */ >>> + AVND_COMP *comp = >>> + m_AVND_COMPDB_REC_GET(cb->compdb, >> param->name); >>> + if (comp == NULL) { >>> + LOG_ER("%s: Comp '%s' not found", >> __FUNCTION__, >>> + param->name.value); >>> + goto done; >>> + } >>> >>> - /* get the record */ >>> - comp = m_AVND_COMPDB_REC_GET(cb->compdb, >> param->name); >>> - if (comp) { >>> - /* delete the record */ >>> - >> m_AVND_SEND_CKPT_UPDT_ASYNC_RMV(cb, comp, >> AVND_CKPT_COMP_CONFIG); >>> + if ((comp->pres == >> SA_AMF_PRESENCE_UNINSTANTIATED) && >>> + (comp->su->pres == >>> + >> SA_AMF_PRESENCE_UNINSTANTIATED)) { >> >> no line break above >> >>> + >> m_AVND_SEND_CKPT_UPDT_ASYNC_RMV(cb, comp, >>> + >> AVND_CKPT_COMP_CONFIG); >>> rc = avnd_compdb_rec_del(cb, ¶m- >>> name); >>> + goto done; >>> } >>> + >>> + /* Terminate the pi comp. It will terminate the >>> + component and delete the comp record. */ >>> + if >> (m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(comp)) { >>> + comp->pending_delete = true; >> >> isn't this setting common to both types of components? >> >>> + TRACE("Running the component clc FSM, >> terminate" >>> + " the component"); >> >> th above trace is not needed, remove >> >>> + rc = avnd_comp_clc_fsm_run(cb, comp, >>> + >> AVND_COMP_CLC_PRES_FSM_EV_TERM); >>> + goto done; >>> + } >>> + /* Terminate the Npi comp. After deleting the csi, comp >>> + will be already terminated, so we just want to delete >>> + the comp record.*/ >>> + if >> (!m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(comp)) { >>> + >> m_AVND_SEND_CKPT_UPDT_ASYNC_RMV(cb, comp, >>> + >> AVND_CKPT_COMP_CONFIG); >>> + rc = avnd_compdb_rec_del(cb, ¶m- >>> name); >>> + goto done; >>> + } >>> + >>> } >>> break; >>> >>> diff --git a/osaf/services/saf/amf/amfnd/include/avnd_comp.h >> b/osaf/services/saf/amf/amfnd/include/avnd_comp.h >>> --- a/osaf/services/saf/amf/amfnd/include/avnd_comp.h >>> +++ b/osaf/services/saf/amf/amfnd/include/avnd_comp.h >>> @@ -359,6 +359,8 @@ typedef struct avnd_comp_tag { >>> SaBoolT admin_oper; /*set to true if undergoing admin operation */ >>> int config_is_valid; /* Used to indicate that config has to be refreshed >> from IMM */ >>> bool assigned_flag; /* Used in finding multiple csi for a single comp >> while csi mod.*/ >>> + bool pending_delete; /* Used in deleting component when su is in >>> + instantiated state.*/ >>> >>> } AVND_COMP; >>> >>> diff --git a/osaf/services/saf/amf/amfnd/su.cc >> b/osaf/services/saf/amf/amfnd/su.cc >>> --- a/osaf/services/saf/amf/amfnd/su.cc >>> +++ b/osaf/services/saf/amf/amfnd/su.cc >>> @@ -79,9 +79,13 @@ uint32_t avnd_evt_avd_reg_su_evh(AVND_CB >>> >>> /* scan the su list & add each su to su-db */ >>> for (su_info = info->su_list; su_info; su = 0, su_info = su_info->next) >>> { >>> - su = avnd_sudb_rec_add(cb, su_info, &rc); >>> - if (!su) >>> - break; >>> + su = m_AVND_SUDB_REC_GET(cb->sudb, su_info->name); >>> + /* This function is common >>> + 1. for adding new SU in the data base >>> + 2. for adding a new componet in the existing su. >>> + So, we need to check wether the SU exists or not. */ >> >> two spelling issues above, remove the "we need to" >> >>> + if (su == NULL) >>> + su = avnd_sudb_rec_add(cb, su_info, &rc); >>> >>> m_AVND_SEND_CKPT_UPDT_ASYNC_ADD(cb, su, >> AVND_CKPT_SU_CONFIG); >>> >>> @@ -93,11 +97,28 @@ uint32_t avnd_evt_avd_reg_su_evh(AVND_CB >>> rc = NCSCC_RC_FAILURE; >>> break; >>> } >>> + >>> + /* When NPI comp is added into PI SU(that is >> UNINSTANTIATED), >>> + we don't have to run SU FSM as anyway, we are not >>> + instantiating NPI component. In this case, anyway, SU will >>> + remain in instantiated state. NPI comp will get instantiated >>> + when corresponding csi is added. */ >>> + bool su_is_instantiated; >>> + m_AVND_SU_IS_INSTANTIATED(su, su_is_instantiated); >>> + >>> + if ((su->pres == SA_AMF_PRESENCE_INSTANTIATED) && >>> + (su_is_instantiated == false)) { >>> + avnd_su_pres_state_set(su, >>> + >> SA_AMF_PRESENCE_UNINSTANTIATED); >>> + rc = avnd_su_pres_fsm_run(cb, su, 0, >>> + AVND_SU_PRES_FSM_EV_INST); >>> + } >>> } >>> >>> /*** send the response to AvD ***/ >>> rc = avnd_di_reg_su_rsp_snd(cb, &info->su_list->name, rc); >>> >>> + >>> done: >>> TRACE_LEAVE(); >>> return rc; >>> >>> > > ------------------------------------------------------------------------------ CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments & Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
