> > +                   (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, &param-
> >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, &param-
> >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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to