Ack with minor comment: Extra line of " sg->adminOp = static_cast<SaAmfAdminOperationIdT>(0);" here:
+ sg->adminOp = static_cast<SaAmfAdminOperationIdT>(0); + sg->adminOp_invocationId = 0; + sg->adminOp = static_cast<SaAmfAdminOperationIdT>(0); + } > -----Original Message----- > From: Praveen Malviya > Sent: 01 August 2014 00:18 > To: [email protected]; Nagendra Kumar; [email protected] > Cc: [email protected] > Subject: [PATCH 1 of 1] amfd : reply for admin operation on SG after its > completion [#293] > > osaf/services/saf/amf/amfd/include/amfd.h | 1 + > osaf/services/saf/amf/amfd/include/sg.h | 8 ++ > osaf/services/saf/amf/amfd/ndproc.cc | 20 +++++ > osaf/services/saf/amf/amfd/sg.cc | 114 > +++++++++++++++++++++++++++++- > osaf/services/saf/amf/amfd/sg_nway_fsm.cc | 2 +- > 5 files changed, 143 insertions(+), 2 deletions(-) > > > SG admin operations on SG are returned without actual completion. > > Currently AMF replies to IMM for admin operations without checking > the actual states of SIs, SUs, SUSIs and sg_fsm. > > Patch fixes the problem by replying to IMM for the completion > of admin operation after Sg becomes stable or if the affected > entities are in stable state. > > diff --git a/osaf/services/saf/amf/amfd/include/amfd.h > b/osaf/services/saf/amf/amfd/include/amfd.h > --- a/osaf/services/saf/amf/amfd/include/amfd.h > +++ b/osaf/services/saf/amf/amfd/include/amfd.h > @@ -36,6 +36,7 @@ > #include "logtrace.h" > > #include "amf.h" > +#include "imm.h" > > #include "ncsencdec_pub.h" > #include "amf_d2nmsg.h" > diff --git a/osaf/services/saf/amf/amfd/include/sg.h > b/osaf/services/saf/amf/amfd/include/sg.h > --- a/osaf/services/saf/amf/amfd/include/sg.h > +++ b/osaf/services/saf/amf/amfd/include/sg.h > @@ -189,6 +189,8 @@ public: > * this group in the descending order > * of the rank. > */ > + SaInvocationT adminOp_invocationId; > + SaAmfAdminOperationIdT adminOp; > > AVD_SG *sg_list_sg_type_next; > struct avd_amf_sg_type_tag *sg_type; > @@ -489,6 +491,11 @@ public: > }\ > if (state == AVD_SG_FSM_STABLE) {\ > osafassert(sg->su_oper_list.su == NULL); \ > + if (sg->adminOp_invocationId != 0) { \ > + avd_saImmOiAdminOperationResult(avd_cb- > >immOiHandle, sg->adminOp_invocationId, SA_AIS_OK);\ > + sg->adminOp_invocationId = 0; \ > + sg->adminOp = > static_cast<SaAmfAdminOperationIdT>(0); \ > + }\ > }\ > } > > @@ -562,6 +569,7 @@ extern void avd_su_role_failover(AVD_SU > extern bool sg_is_tolerance_timer_running_for_any_si(AVD_SG *sg); > extern void avd_sg_adjust_config(AVD_SG *sg); > extern uint32_t sg_instantiated_su_count(const AVD_SG *sg); > +extern bool sg_stable_after_lock_in_or_unlock_in(AVD_SG *sg); > > > #endif > 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 > @@ -544,6 +544,22 @@ done: > return false; > } > > +/** > + * handler to report error response to imm for any pending admin operation > on sg > + * > + * @param sg > + */ > +static void sg_admin_op_report_to_imm(AVD_SG *sg) > +{ > + if (sg_stable_after_lock_in_or_unlock_in(sg) == true) { > + avd_saImmOiAdminOperationResult(avd_cb->immOiHandle, > + sg->adminOp_invocationId, SA_AIS_OK); > + sg->adminOp = static_cast<SaAmfAdminOperationIdT>(0); > + sg->adminOp_invocationId = 0; > + sg->adminOp = static_cast<SaAmfAdminOperationIdT>(0); > + } > + > +} > > /************************************************************ > ***************** > * Function: avd_data_update_req_func > * > @@ -758,6 +774,10 @@ void avd_data_update_req_evh(AVD_CL_CB * > } else if (su->pend_cbk.invocation != 0) > { > > su_admin_op_report_to_imm(su, > static_cast<SaAmfPresenceStateT>(l_val)); > } > + > + if (su->sg_of_su- > >adminOp_invocationId != 0) > + > sg_admin_op_report_to_imm(su->sg_of_su); > + > /* send response to pending clm > callback */ > if (su->su_on_node->clm_pend_inv != > 0) > clm_pend_response(su, > static_cast<SaAmfPresenceStateT>(l_val)); > diff --git a/osaf/services/saf/amf/amfd/sg.cc > b/osaf/services/saf/amf/amfd/sg.cc > --- a/osaf/services/saf/amf/amfd/sg.cc > +++ b/osaf/services/saf/amf/amfd/sg.cc > @@ -132,6 +132,7 @@ AVD_SG::AVD_SG(): > memset(&saAmfSGSuHostNodeGroup, 0, sizeof(SaNameT)); > su_oper_list.su = NULL; > su_oper_list.next = NULL; > + adminOp_invocationId = 0; > } > > static AVD_SG *sg_new(const SaNameT *dn, SaAmfRedundancyModelT > redundancy_model) > @@ -1204,6 +1205,11 @@ static void sg_admin_op_cb(SaImmOiHandle > } > } > > + if ((sg->adminOp_invocationId != 0) || (sg->adminOp != 0)) { > + report_admin_op_error(immOiHandle, invocation, > SA_AIS_ERR_TRY_AGAIN, NULL, > + "Admin operation is going on (%s)", sg- > >name.value); > + goto done; > + } > /* Avoid if any single Csi assignment is undergoing on SG. */ > if (csi_assignment_validate(sg) == true) { > report_admin_op_error(immOiHandle, invocation, > SA_AIS_ERR_TRY_AGAIN, NULL, > @@ -1240,6 +1246,7 @@ static void sg_admin_op_cb(SaImmOiHandle > NULL); > goto done; > } > + sg->adminOp = SA_AMF_ADMIN_UNLOCK; > break; > > case SA_AMF_ADMIN_LOCK: > @@ -1264,6 +1271,7 @@ static void sg_admin_op_cb(SaImmOiHandle > goto done; > } > > + sg->adminOp = SA_AMF_ADMIN_LOCK; > break; > case SA_AMF_ADMIN_SHUTDOWN: > if (sg->saAmfSGAdminState == > SA_AMF_ADMIN_SHUTTING_DOWN) { > @@ -1287,6 +1295,7 @@ static void sg_admin_op_cb(SaImmOiHandle > NULL); > goto done; > } > + sg->adminOp = SA_AMF_ADMIN_SHUTDOWN; > break; > case SA_AMF_ADMIN_LOCK_INSTANTIATION: > if (sg->saAmfSGAdminState == > SA_AMF_ADMIN_LOCKED_INSTANTIATION) { > @@ -1310,6 +1319,8 @@ static void sg_admin_op_cb(SaImmOiHandle > goto done; > } > > + > + sg->adminOp = SA_AMF_ADMIN_LOCK_INSTANTIATION; > break; > case SA_AMF_ADMIN_UNLOCK_INSTANTIATION: > if (sg->saAmfSGAdminState == SA_AMF_ADMIN_LOCKED) { > @@ -1338,7 +1349,13 @@ static void sg_admin_op_cb(SaImmOiHandle > } > > avd_sg_admin_state_set(sg, SA_AMF_ADMIN_LOCKED); > + > + if ((sg->list_of_su != NULL) && (sg->list_of_su- > >saAmfSUPreInstantiable == false)) { > + avd_saImmOiAdminOperationResult(immOiHandle, > invocation, SA_AIS_OK); > + goto done; > + } > sg_app_sg_admin_unlock_inst(avd_cb, sg); > + sg->adminOp = SA_AMF_ADMIN_UNLOCK_INSTANTIATION; > > break; > case SA_AMF_ADMIN_SG_ADJUST: > @@ -1347,7 +1364,23 @@ static void sg_admin_op_cb(SaImmOiHandle > "Admin Operation '%llu' not supported", > op_id); > goto done; > } > - avd_saImmOiAdminOperationResult(immOiHandle, invocation, > SA_AIS_OK); > + > + if ((op_id != SA_AMF_ADMIN_UNLOCK_INSTANTIATION) && (op_id != > SA_AMF_ADMIN_LOCK_INSTANTIATION) > + && (sg->sg_fsm_state == AVD_SG_FSM_STABLE)) { > + avd_saImmOiAdminOperationResult(immOiHandle, invocation, > SA_AIS_OK); > + sg->adminOp = static_cast<SaAmfAdminOperationIdT>(0); > + goto done; > + } > + > + if ((op_id == SA_AMF_ADMIN_UNLOCK_INSTANTIATION) || (op_id == > SA_AMF_ADMIN_LOCK_INSTANTIATION)) { > + if (sg_stable_after_lock_in_or_unlock_in(sg) == true) { > + avd_saImmOiAdminOperationResult(immOiHandle, > invocation, SA_AIS_OK); > + sg->adminOp = > static_cast<SaAmfAdminOperationIdT>(0); > + goto done; > + } > + } > + > + sg->adminOp_invocationId = invocation; > done: > TRACE_LEAVE(); > } > @@ -1582,6 +1615,11 @@ void AVD_SG::set_fsm_state(AVD_SG_FSM_ST > > if (state == AVD_SG_FSM_STABLE) { > osafassert(su_oper_list.su == NULL); > + if (adminOp_invocationId != 0) { > + avd_saImmOiAdminOperationResult(avd_cb- > >immOiHandle, adminOp_invocationId, SA_AIS_OK); > + adminOp_invocationId = 0; > + adminOp = static_cast<SaAmfAdminOperationIdT>(0); > + } > } > } > > @@ -1732,3 +1770,77 @@ SaAisErrorT AVD_SG::si_swap(AVD_SI *si, > return SA_AIS_ERR_NOT_SUPPORTED; > } > > +/** > + * @brief Checks if SG is stable with respect to lock-in or unlock-on > operation. > + * > + * @param[in] sg > + * > + * @return true/false > + **/ > +bool sg_stable_after_lock_in_or_unlock_in(AVD_SG *sg) > +{ > + uint32_t instantiated_sus = 0, to_be_instantiated_sus = 0; > + SaAmfAdminStateT node_admin_state; > + > + switch (sg->adminOp) { > + case SA_AMF_ADMIN_LOCK_INSTANTIATION : > + for (AVD_SU *su = sg->list_of_su; su; su = su->sg_list_su_next) > { > + if ((su->saAmfSUPresenceState != > SA_AMF_PRESENCE_UNINSTANTIATED) && > + (su->saAmfSUPresenceState != > SA_AMF_PRESENCE_INSTANTIATION_FAILED) && > + (su->saAmfSUPresenceState != > SA_AMF_PRESENCE_TERMINATION_FAILED)) > + return false; > + } > + break; > + case SA_AMF_ADMIN_UNLOCK_INSTANTIATION : > + /* Unlock-in of SG will not instantiate any component in NPI > SU.*/ > + if ((sg->list_of_su != NULL) && (sg->list_of_su- > >saAmfSUPreInstantiable == false)) > + return true; > + > + for (AVD_SU *su = sg->list_of_su; su; su = su->sg_list_su_next) > { > + node_admin_state = su->su_on_node- > >saAmfNodeAdminState; > + > + if ((su->saAmfSUPresenceState == > SA_AMF_PRESENCE_INSTANTIATION_FAILED) || > + (su->saAmfSUPresenceState == > SA_AMF_PRESENCE_TERMINATION_FAILED)) > + continue; > + > + if (su->saAmfSUPresenceState == > SA_AMF_PRESENCE_INSTANTIATED) { > + instantiated_sus++; > + continue; > + } > + > + if (node_admin_state == > SA_AMF_ADMIN_LOCKED_INSTANTIATION) > + continue; > + > + if ((node_admin_state != > SA_AMF_ADMIN_LOCKED_INSTANTIATION) && > + (su->saAmfSUAdminState != > SA_AMF_ADMIN_LOCKED_INSTANTIATION) && > + (su->saAmfSUOperState == > SA_AMF_OPERATIONAL_ENABLED) && > + (su->su_on_node->node_state == > AVD_AVND_STATE_PRESENT)) { > + > + if (su->saAmfSUPresenceState == > SA_AMF_PRESENCE_INSTANTIATING) > + return false; > + if (su->saAmfSUPresenceState == > SA_AMF_PRESENCE_UNINSTANTIATED) { > + to_be_instantiated_sus++; > + continue; > + } > + > + } > + > + } > + > + if (instantiated_sus >= sg->saAmfSGNumPrefInserviceSUs) > + return true; > + else { > + if (to_be_instantiated_sus == 0) > + return true; > + else > + return false; > + } > + > + break; > + default: > + TRACE("Called for wrong admin operation"); > + break; > + } > + > + return true; > +} > diff --git a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > --- a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > +++ b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > @@ -1257,7 +1257,7 @@ uint32_t avd_sg_nway_si_assign(AVD_CL_CB > > TRACE_ENTER2("%s", sg->name.value); > > - sg->sg_fsm_state = AVD_SG_FSM_STABLE; > + m_AVD_SET_SG_FSM(cb, sg, AVD_SG_FSM_STABLE); > m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, sg, > AVSV_CKPT_SG_FSM_STATE); > > avd_sidep_update_si_dep_state_for_all_sis(sg); ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
