I had thought to implement #445 which would be making duplicate to 493. But since you have floated the patch, I ack it, but I have not tested extensible.
Thanks -Nagu > -----Original Message----- > From: Hans Feldt [mailto:osafde...@gmail.com] > Sent: 07 May 2014 16:33 > To: ajo...@genband.com; Nagendra Kumar > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 1 of 1] avd: fix SU in-service check [#493] > > osaf/services/saf/avsv/avd/avd_node.c | 11 +----- > osaf/services/saf/avsv/avd/avd_sgproc.c | 23 ++---------- > osaf/services/saf/avsv/avd/avd_su.c | 52 ++++++++++++++++++++++----- > - > osaf/services/saf/avsv/avd/include/avd_su.h | 10 +---- > 4 files changed, 48 insertions(+), 48 deletions(-) > > > SUs assigned before instantiated: > May 2 18:56:32 linux osafamfnd[12420]: NO Assigned 'safSi=Dataplane- > Np1-SI-1,safApp=DataplaneApp' STANDBY to 'safSu=Dataplane-SU1,safSg > =Dataplane-Np1,safApp=DataplaneApp' May 2 18:56:39 linux > osafamfnd[12420]: > NO 'safSu=Dataplane-SU1,safSg=Dataplane-Np1,safApp=DataplaneApp' > Presence > State INSTANTIATING => INSTANTIATED > > In some places where the macro m_AVD_APP_SU_IS_INSVC is used, the > presence state > for pre-instantiable SUs is not handled properly. > > By changing the macro into a function which correctly checks presence state > for > pre-instantiable SUs this problem can be solved. > > diff --git a/osaf/services/saf/avsv/avd/avd_node.c > b/osaf/services/saf/avsv/avd/avd_node.c > --- a/osaf/services/saf/avsv/avd/avd_node.c > +++ b/osaf/services/saf/avsv/avd/avd_node.c > @@ -860,16 +860,7 @@ void avd_node_admin_lock_unlock_shutdown > > su = node->list_of_su; > while (su != NULL) { > - m_AVD_GET_SU_NODE_PTR(cb, su, su_node_ptr); > - > - if (m_AVD_APP_SU_IS_INSVC(su, su_node_ptr) && > - ((su->saAmfSUPreInstantiable) ? > - (su->saAmfSUPresenceState == > SA_AMF_PRESENCE_INSTANTIATED):true)) { > - /* Pres state check is to prevent assignment to > SU in case node is instantiating > - * in Node locked state and somebody issues > UNLOCK on Node. Since SU are in instantiating > - * state, so AMFND will not assign the role to > components. Anyway when SU gets > - * instantiated, then assignment will be given > to > components/SU. > - */ > + if (su_is_insvc(su) == true) { > avd_su_readiness_state_set(su, > SA_AMF_READINESS_IN_SERVICE); > > su->sg_of_su->su_insvc(cb, su); > diff --git a/osaf/services/saf/avsv/avd/avd_sgproc.c > b/osaf/services/saf/avsv/avd/avd_sgproc.c > --- a/osaf/services/saf/avsv/avd/avd_sgproc.c > +++ b/osaf/services/saf/avsv/avd/avd_sgproc.c > @@ -259,7 +259,6 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb > AVD_AVND *node; > AVD_SU *su, *i_su; > SaAmfReadinessStateT old_state; > - AVD_AVND *su_node_ptr = NULL; > bool node_reboot_req = true; > > TRACE_ENTER2("id:%u, node:%x, '%s' state:%u", n2d_msg- > >msg_info.n2d_opr_state.msg_id, > @@ -297,8 +296,6 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb > goto done; > } > > - m_AVD_GET_SU_NODE_PTR(cb, su, su_node_ptr); > - > if (n2d_msg->msg_info.n2d_opr_state.rec_rcvr.saf_amf == > SA_AMF_NODE_SWITCHOVER) { > saflog(LOG_NOTICE, amfSvcUsrName, "Node Switch-Over > requested by '%s'", > node->name.value); > @@ -468,7 +465,6 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb > * the SG FSM. > */ > if (su->sg_of_su->sg_ncs_spec == SA_TRUE) { > - m_AVD_GET_SU_NODE_PTR(cb, su, su_node_ptr); > if (su->saAmfSUAdminState == > SA_AMF_ADMIN_UNLOCKED) { > avd_su_readiness_state_set(su, > SA_AMF_READINESS_IN_SERVICE); > /* Run the SG FSM */ > @@ -485,9 +481,8 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb > } else { /* if(su->sg_of_su->sg_ncs_spec == SA_TRUE) */ > > old_state = su->saAmfSuReadinessState; > - m_AVD_GET_SU_NODE_PTR(cb, su, su_node_ptr); > > - if (m_AVD_APP_SU_IS_INSVC(su, su_node_ptr)) { > + if (su_is_insvc(su) == true) { > avd_su_readiness_state_set(su, > SA_AMF_READINESS_IN_SERVICE); > if ((cb->init_state == AVD_APP_STATE) && > (old_state == SA_AMF_READINESS_OUT_OF_SERVICE)) { > /* An application SU has become in > service call SG FSM */ > @@ -1090,7 +1085,6 @@ void avd_su_si_assign_evh(AVD_CL_CB *cb, > void avd_sg_app_node_su_inst_func(AVD_CL_CB *cb, AVD_AVND *avnd) > { > AVD_SU *i_su; > - AVD_AVND *su_node_ptr = NULL; > > TRACE_ENTER2("'%s'", avnd->name.value); > > @@ -1121,9 +1115,7 @@ void avd_sg_app_node_su_inst_func(AVD_CL > /* mark the non preinstatiable as > enable. */ > avd_su_oper_state_set(i_su, > SA_AMF_OPERATIONAL_ENABLED); > > - m_AVD_GET_SU_NODE_PTR(cb, i_su, > su_node_ptr); > - > - if (m_AVD_APP_SU_IS_INSVC(i_su, > su_node_ptr)) { > + if (su_is_insvc(i_su) == true) { > > avd_su_readiness_state_set(i_su, SA_AMF_READINESS_IN_SERVICE); > } > } > @@ -1203,9 +1195,8 @@ uint32_t avd_sg_app_su_inst_func(AVD_CL_ > (su_node_ptr->saAmfNodeOperState == > SA_AMF_OPERATIONAL_ENABLED) && > (i_su->term_state == false)) { > avd_su_oper_state_set(i_su, > SA_AMF_OPERATIONAL_ENABLED); > - m_AVD_GET_SU_NODE_PTR(cb, i_su, > su_node_ptr); > > - if (m_AVD_APP_SU_IS_INSVC(i_su, > su_node_ptr)) { > + if (su_is_insvc(i_su) == true) { > avd_su_readiness_state_set(i_su, > SA_AMF_READINESS_IN_SERVICE); > i_su->sg_of_su->su_insvc(cb, i_su); > > @@ -1275,7 +1266,6 @@ uint32_t avd_sg_app_sg_admin_func(AVD_CL > { > uint32_t rc = NCSCC_RC_FAILURE; > AVD_SU *i_su; > - AVD_AVND *i_su_node_ptr = NULL; > > TRACE_ENTER2("'%s'", sg->name.value); > > @@ -1296,12 +1286,7 @@ uint32_t avd_sg_app_sg_admin_func(AVD_CL > * state. > */ > for (i_su = sg->list_of_su; i_su != NULL; i_su = i_su- > >sg_list_su_next) { > - m_AVD_GET_SU_NODE_PTR(cb, i_su, i_su_node_ptr); > - // TODO(nagu) remove saAmfSUPreInstantiable check > and move into m_AVD_APP_SU_IS_INSVC > - if (m_AVD_APP_SU_IS_INSVC(i_su, i_su_node_ptr) > && > - ((i_su->saAmfSUPreInstantiable) ? > - (i_su->saAmfSUPresenceState == > - > SA_AMF_PRESENCE_INSTANTIATED):true)) { > + if (su_is_insvc(i_su) == true) { > avd_su_readiness_state_set(i_su, > SA_AMF_READINESS_IN_SERVICE); > } > } > diff --git a/osaf/services/saf/avsv/avd/avd_su.c > b/osaf/services/saf/avsv/avd/avd_su.c > --- a/osaf/services/saf/avsv/avd/avd_su.c > +++ b/osaf/services/saf/avsv/avd/avd_su.c > @@ -28,6 +28,7 @@ > #include <avd_ntf.h> > #include <avd_proc.h> > #include <avd_csi.h> > +#include <avd_cluster.h> > > static NCS_PATRICIA_TREE su_db; > > @@ -960,17 +961,11 @@ static void su_admin_op_cb(SaImmOiHandle > switch (op_id) { > case SA_AMF_ADMIN_UNLOCK: > avd_su_admin_state_set(su, SA_AMF_ADMIN_UNLOCKED); > - if (((m_AVD_APP_SU_IS_INSVC(su, node)) || (su->sg_of_su- > >sg_ncs_spec == true)) && > - ((su->saAmfSUPreInstantiable) ? > - (su->saAmfSUPresenceState == > SA_AMF_PRESENCE_INSTANTIATED):true)) { > - /* Pres state check is to prevent assignment to SU in > case SU is instantiating in > - * locked state and somebody issues UNLOCK on SU. > Since comp are in instantiating state, > - * so AMFND will not assign the role to components. > Anyway when SU gets instantiated, then > - * assignment will be given to components/SU. > - */ > - /* Reason for adding "su->sg_of_su->sg_ncs_spec == > true" is for Middleware component > - * node oper state and SU oper state are marked > enabled after they gets assignments. > - * So, we cann't check compatibility with > m_AVD_APP_SU_IS_INSVC for them. > + if ((su_is_insvc(su) == true) || (su->sg_of_su->sg_ncs_spec == > true)) { > + /* Reason for adding "su->sg_of_su->sg_ncs_spec == > true" is for > + * Middleware component node oper state and SU > oper state are marked > + * enabled after they gets assignments. So, we can't > check > + * compatibility with m_AVD_APP_SU_IS_INSVC for > them. > */ > avd_su_readiness_state_set(su, > SA_AMF_READINESS_IN_SERVICE); > if (su->sg_of_su->su_insvc(cb, su) != > NCSCC_RC_SUCCESS) > @@ -1588,3 +1583,38 @@ void avd_su_constructor(void) > avd_class_impl_set("SaAmfSU", su_rt_attr_cb, su_admin_op_cb, > su_ccb_completed_cb, su_ccb_apply_cb); > } > > +/** > + * Checks if the SU can be made in-service > + * For reference see 3.2.1.4 and for pre-instantiable SUs Table 4 > + * > + * @param su > + * @return true if SU can be made in-service > + */ > +bool su_is_insvc(const AVD_SU *su) > +{ > + const AVD_AVND *node; > + const AVD_SG *sg = su->sg_of_su; > + const AVD_APP *app = sg->app; > + > + m_AVD_GET_SU_NODE_PTR(avd_cb, su, node); > + > + if (su->saAmfSUPreInstantiable == true) { > + return (avd_cluster->saAmfClusterAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (app->saAmfApplicationAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (su->saAmfSUAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (sg->saAmfSGAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (node->saAmfNodeAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (node->saAmfNodeOperState == > SA_AMF_OPERATIONAL_ENABLED) && > + (su->saAmfSUOperState == > SA_AMF_OPERATIONAL_ENABLED) && > + ((su->saAmfSUPresenceState == > SA_AMF_PRESENCE_INSTANTIATED || > + su->saAmfSUPresenceState == > SA_AMF_PRESENCE_RESTARTING)); > + } else { > + return (avd_cluster->saAmfClusterAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (app->saAmfApplicationAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (su->saAmfSUAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (sg->saAmfSGAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (node->saAmfNodeAdminState == > SA_AMF_ADMIN_UNLOCKED) && > + (node->saAmfNodeOperState == > SA_AMF_OPERATIONAL_ENABLED) && > + (su->saAmfSUOperState == > SA_AMF_OPERATIONAL_ENABLED); > + } > +} > 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 > @@ -130,18 +130,12 @@ su->su_switch = state;\ > m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, AVSV_CKPT_SU_SWITCH);\ > } > > -#define m_AVD_APP_SU_IS_INSVC(i_su,su_node_ptr) \ > -((su_node_ptr->saAmfNodeAdminState == SA_AMF_ADMIN_UNLOCKED) && > \ > -(su_node_ptr->saAmfNodeOperState == SA_AMF_OPERATIONAL_ENABLED) > && \ > -(i_su->sg_of_su->saAmfSGAdminState == SA_AMF_ADMIN_UNLOCKED) &&\ > -(i_su->saAmfSUAdminState == SA_AMF_ADMIN_UNLOCKED) &&\ > -(i_su->saAmfSUOperState == SA_AMF_OPERATIONAL_ENABLED)\ > -) > - > #define m_AVD_GET_SU_NODE_PTR(avd_cb,i_su,su_node_ptr) \ > if(true == i_su->su_is_external) su_node_ptr = avd_cb- > >ext_comp_info.local_avnd_node; \ > else su_node_ptr = i_su->su_on_node; > > +bool su_is_insvc(const AVD_SU *su); > + > /** > * Allocate SU memory and initialize attributes to defaults > * @param dn ------------------------------------------------------------------------------ Is your legacy SCM system holding you back? Join Perforce May 7 to find out: • 3 signs your SCM is hindering your productivity • Requirements for releasing software faster • Expert tips and advice for migrating your SCM now http://p.sf.net/sfu/perforce _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
