Sorry, please read below 80 lines as 80 characters. > -----Original Message----- > From: Nagendra Kumar > Sent: 24 January 2014 09:37 > To: Hans Feldt; [email protected]; Praveen Malviya; Mathivanan > Naickan Palanivelu; Hrishikesh Chenna > Cc: [email protected] > Subject: Re: [devel] [PATCH 1 of 1] amf: support for automatic comp > instantiation and deletion [#597] > > > > + (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. > > Thanks > -Nagu > > > -----Original Message----- > > From: Hans Feldt [mailto:[email protected]] > > Sent: 23 January 2014 19:23 > > To: Nagendra Kumar; [email protected]; Praveen Malviya; > > Mathivanan Naickan Palanivelu; Hrishikesh Chenna > > Cc: [email protected] > > 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, [email protected] 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.clk > trk > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------ 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 [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
