Hi Minh, Thanks for your comments. For #1: We are only using amf_saImmOmAccessorGet_o2() api, so shouldn't be any issue. But, I am working on it to read the information in imm thread, will publish it shortly, Minh. For #2: No, you didn't miss it, Minh. Since, this is internal to Amf and not exposed to the user, we can have few enums like this for making Amf work. For #3: I came to know and you may already know, Minh that AVND_COMP_TYPE_INTER_NODE is used for internode(also for external components) proxy-proxied components and this feature is badly broken in amfnd code. If I use it for error reporting, then it will collide with the feature of proxy-proxied and the code may go in some wrong flow in some use case and fixing that will take more energy and effort. Also proxy-proxied feature is diffferent than error report, so better to have a separate enum for this feature. So, I decided to use a separate enum for code-modularity/clarity/clean-code/easy-to-use. Please comment.
Thanks Anand Sundararaj Senior Solutions Architect | +1 480 686 4772 www.GetHighAvailability.com (https://am2.myprofessionalmail.com/appsuite/www.GetHighAvailability.com) Get High Availability Today! NJ, USA: +1 508-507-6507 > On 07/29/2020 4:49 PM minhchau <minh.c...@dektech.com.au> wrote: > > > Sorry, typo > > ... imm calls *while* immnd is restarted > > On 30/7/20 9:46 am, minhchau wrote: > > Hi Anand, > > > > 1. A general comment is that we should read from imm at amfnd's > > startup or in a thread, we had many problems in the past that amfnd's > > stuck in imm calls why immnd is restarted. > > > > 2. Can you please point to the saforum spec that how AMF should handle > > this new type of component (INTER_NODE_NP)? I tried but could not find > > it, or maybe I have missed it. > > > > 3. Is the existing type AVND_COMP_TYPE_INTER_NODE not good enough to > > reuse? There are avnd_evt_ava_csi_quiescing_compl_evh and > > avnd_evt_ava_resp_evh that have code patterns to forward to destined > > node to process the api calls. > > > > Thanks > > > > Minh > > > > On 24/7/20 12:28 pm, s.an...@gethighavailability.com wrote: > >> From: Anand Sundararaj <s.an...@gethighavailability.com> > >> > >> --- > >> src/amf/amfnd/amfnd.cc | 22 ++++-- > >> src/amf/amfnd/avnd_cb.h | 2 + > >> src/amf/amfnd/avnd_comp.h | 2 + > >> src/amf/amfnd/clm.cc | 33 ++++++++- > >> src/amf/amfnd/err.cc | 182 > >> +++++++++++++++++++++++++++++++++++++++++++++- > >> 5 files changed, 228 insertions(+), 13 deletions(-) > >> > >> diff --git a/src/amf/amfnd/amfnd.cc b/src/amf/amfnd/amfnd.cc > >> index 1bf057d..9c0aa03 100644 > >> --- a/src/amf/amfnd/amfnd.cc > >> +++ b/src/amf/amfnd/amfnd.cc > >> @@ -27,7 +27,6 @@ > >> #include <cinttypes> > >> #include "amf/amfnd/avnd.h" > >> - > >> // Remember MDS install version of Agents. It can be used to send > >> msg to Agent > >> // based on their versions. > >> std::map<MDS_DEST, MDS_SVC_PVT_SUB_PART_VER> agent_mds_ver_db; > >> @@ -203,12 +202,17 @@ uint32_t > >> avnd_evt_avnd_avnd_api_resp_msg_hdl(AVND_CB *cb, AVND_EVT *evt) { > >> AVND_COMP *o_comp = > >> m_AVND_INT_EXT_COMPDB_REC_GET(cb->internode_avail_comp_db, comp_name); > >> if (nullptr == o_comp) { > >> - LOG_ER("Couldn't find comp in Inter/Ext Comp DB"); > >> - res = NCSCC_RC_FAILURE; > >> - goto done; > >> + TRACE("Couldn't find comp in Inter/Ext Comp DB"); > >> + // Check if it is a internode responses like error report, etc. > >> + o_comp = avnd_cb->compdb_internode.find(comp_name); > >> + if (nullptr == o_comp) { > >> + LOG_ER("Couldn't find comp in internode comp DB"); > >> + res = NCSCC_RC_FAILURE; > >> + goto done; > >> + } else > >> + TRACE("Comp '%s' found in internode comp DB", > >> o_comp->name.c_str()); > >> } > >> reg_dest = o_comp->reg_dest; > >> - > >> /***************************************************************************** > >> * Some processing for registration and unregistration starts > >> *****************************************************************************/ > >> @@ -275,9 +279,13 @@ uint32_t > >> avnd_evt_avnd_avnd_api_resp_msg_hdl(AVND_CB *cb, AVND_EVT *evt) { > >> /* We have to fprwrd this message to AvA. */ > >> res = avnd_amf_resp_send(cb, resp_info->type, resp_info->rc, > >> - (uint8_t *)ha_state, ®_dest, > >> &avnd_msg->mds_ctxt, > >> + (uint8_t *)ha_state, &o_comp->reg_dest, > >> &avnd_msg->mds_ctxt, > >> nullptr, false); > >> - > >> + if (o_comp->comp_type == AVND_COMP_TYPE_INTER_NODE_NP) { > >> + TRACE("Comp '%s' deleted from internode comp DB", > >> o_comp->name.c_str()); > >> + cb->compdb_internode.erase(o_comp->name); > >> + delete o_comp; > >> + } > >> if (NCSCC_RC_SUCCESS != res) { > >> LOG_ER("%s: Msg Send to AvA Failed:Comp:%s ,Type: %u, rc:%u, > >> Dest:%" PRIu64, > >> __FUNCTION__, comp_name.c_str(), resp_info->type, > >> resp_info->rc, > >> diff --git a/src/amf/amfnd/avnd_cb.h b/src/amf/amfnd/avnd_cb.h > >> index 8b0cc23..a2e5213 100644 > >> --- a/src/amf/amfnd/avnd_cb.h > >> +++ b/src/amf/amfnd/avnd_cb.h > >> @@ -125,6 +125,8 @@ typedef struct avnd_cb_tag { > >> SaTimeT scs_absence_max_duration; > >> /* the timer for supervision of the absence of SC */ > >> AVND_TMR sc_absence_tmr; > >> + // Used for storing comp, residing on other node. > >> + AmfDb<std::string, AVND_COMP> compdb_internode; > >> } AVND_CB; > >> #define AVND_CB_NULL ((AVND_CB *)0) > >> diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h > >> index 49c610e..bbc2aa0 100644 > >> --- a/src/amf/amfnd/avnd_comp.h > >> +++ b/src/amf/amfnd/avnd_comp.h > >> @@ -296,6 +296,8 @@ typedef struct avnd_pxied_rec { > >> #define AVND_COMP_TYPE_LOCAL_NODE 0x00000001 > >> #define AVND_COMP_TYPE_INTER_NODE 0x00000002 > >> #define AVND_COMP_TYPE_EXT_CLUSTER 0x00000004 > >> +// Not for proxy or proxied, it is used for error report, etc. > >> +#define AVND_COMP_TYPE_INTER_NODE_NP 0x00000008 > >> /*########################################################################## > >> COMPONENT DEFINITION (TOP LEVEL) > >> diff --git a/src/amf/amfnd/clm.cc b/src/amf/amfnd/clm.cc > >> index 73c8ff8..111e70a 100644 > >> --- a/src/amf/amfnd/clm.cc > >> +++ b/src/amf/amfnd/clm.cc > >> @@ -226,9 +226,36 @@ static void clm_track_cb( > >> (avnd_cb->node_info.member == SA_TRUE)) { > >> // Act only once on CLM callback. > >> clm_node_left(notifItem->clusterNode.nodeId); > >> - } > >> - } > >> - > >> + } else { > >> + AVND_COMP *comp = nullptr; > >> + uint32_t rc; > >> + std::string name = ""; > >> + LOG_NO("Node Id:'%d'", notifItem->clusterNode.nodeId); > >> + // Delete the responses, which was expected to come from > >> this node. > >> + while (nullptr != (comp = > >> + avnd_compdb_rec_get_next(avnd_cb->compdb_internode, ""))) { > >> + name = comp->name; > >> + // Check the node id > >> + TRACE("Comp '%s'", comp->name.c_str()); > >> + if (comp->node_id == notifItem->clusterNode.nodeId) { > >> + TRACE("Comp '%s' matched", comp->name.c_str()); > >> + if (comp->comp_type == AVND_COMP_TYPE_INTER_NODE_NP) { > >> + // We have to forward this message to AvA. As of now > >> + // hardcode the value of ERROR_REPORT. > >> + rc = avnd_amf_resp_send(avnd_cb, AVSV_AMF_ERR_REP, > >> SA_AIS_ERR_TIMEOUT, > >> + nullptr, &comp->reg_dest, &comp->mds_ctxt, > >> + nullptr, false); > >> + if (NCSCC_RC_SUCCESS != rc) { > >> + LOG_ER("Sending error response failed for %s", > >> comp->name.c_str()); > >> + } > >> + TRACE("Comp '%s' deleted from internode comp DB", > >> comp->name.c_str()); > >> + avnd_cb->compdb_internode.erase(comp->name); > >> + delete comp; > >> + } // comp type > >> + } // Node id match > >> + } // while loop > >> + } // Different node id. > >> + } // if (false == avnd_cb->first_time_up) > >> } else if (notifItem->clusterChange == SA_CLM_NODE_RECONFIGURED) { > >> if (avnd_cb->node_info.nodeId == > >> notifItem->clusterNode.nodeId) { > >> TRACE("local CLM node reconfigured"); > >> diff --git a/src/amf/amfnd/err.cc b/src/amf/amfnd/err.cc > >> index aeec373..5ca142a 100644 > >> --- a/src/amf/amfnd/err.cc > >> +++ b/src/amf/amfnd/err.cc > >> @@ -59,9 +59,9 @@ > >> */ > >> #include "amf/amfnd/avnd.h" > >> +#include "osaf/immutil/immutil.h" > >> /* static function declarations */ > >> - > >> static uint32_t avnd_err_escalate(AVND_CB *, AVND_SU *, AVND_COMP *, > >> uint32_t *); > >> @@ -173,9 +173,185 @@ uint32_t avnd_evt_ava_err_rep_evh(AVND_CB > >> *cb, AVND_EVT *evt) { > >> /* check if component exists on local AvND node */ > >> comp = avnd_compdb_rec_get(cb->compdb, > >> Amf::to_string(&err_rep->err_comp)); > >> - if (!comp) amf_rc = SA_AIS_ERR_NOT_EXIST; > >> - /* determine other error codes, if any */ > >> + if (!comp) { > >> + // Check if this component is already in the internode db. > >> + // If it is, then it means that Error Report operation > >> + // is already undergoing on this comp, so return TRY_AGAIN. > >> + if (nullptr != > >> cb->compdb_internode.find(Amf::to_string(&err_rep->err_comp))) { > >> + LOG_NO("Already Error Report Operation undergoing on '%s'", > >> + Amf::to_string(&err_rep->err_comp).c_str()); > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + goto goto_error; > >> + } > >> + // Check if component is configured in the Cluster. > >> + SaImmAccessorHandleT accessorHandle = 0; > >> + const SaImmAttrValuesT_2 **attributes; > >> + SaImmHandleT immOmHandle = 0; > >> + SaVersionT immVersion = {'A', 2, 15}; > >> + std::string su_name; > >> + SaNameT node_name, clm_node_name; > >> + SaClmNodeIdT node_id; > >> + SaAmfOperationalStateT oper_status; > >> + bool is_member; > >> + SaAisErrorT error = saImmOmInitialize_cond(&immOmHandle, > >> nullptr, &immVersion); > >> + if (error != SA_AIS_OK) { > >> + LOG_ER("saImmOmInitialize failed: %u", error); > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + goto goto_error; > >> + } > >> + > >> + error = amf_saImmOmAccessorInitialize(immOmHandle, accessorHandle); > >> + if (error != SA_AIS_OK) { > >> + LOG_ER("amf_saImmOmAccessorInitialize failed: %u", error); > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + goto goto_finalize; > >> + } > >> + > >> + // Find the component in the Imm DB. > >> + if ((error = amf_saImmOmAccessorGet_o2( > >> + immOmHandle, accessorHandle, > >> Amf::to_string(&err_rep->err_comp), > >> + nullptr, (SaImmAttrValuesT_2 ***)&attributes)) != > >> SA_AIS_OK) { > >> + if (error == SA_AIS_ERR_NOT_EXIST) { > >> + LOG_NO("Component '%s' is not configured", > >> + Amf::to_string(&err_rep->err_comp).c_str()); > >> + amf_rc = SA_AIS_ERR_NOT_EXIST; > >> + } else { > >> + LOG_ER("amf_saImmOmAccessorGet_o2(for Comp '%s') failed: %u", > >> + Amf::to_string(&err_rep->err_comp).c_str(), error); > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + } > >> + > >> + goto goto_finalize; > >> + } > >> + > >> + // Component is configured. Now, extract SU name from comp name. > >> + avnd_cpy_SU_DN_from_DN(su_name, > >> Amf::to_string(&err_rep->err_comp)); > >> + TRACE("SU name '%s'", su_name.c_str()); > >> + > >> + // Get the SU attributes from Imm DB. > >> + if ((error = amf_saImmOmAccessorGet_o2( > >> + immOmHandle, accessorHandle, su_name, > >> + nullptr, (SaImmAttrValuesT_2 ***)&attributes)) != > >> SA_AIS_OK) { > >> + if (error == SA_AIS_ERR_NOT_EXIST) { > >> + LOG_NO("SU '%s' is not configured", su_name.c_str()); > >> + amf_rc = SA_AIS_ERR_NOT_EXIST; > >> + } else { > >> + LOG_ER("amf_saImmOmAccessorGet_o2(for SU '%s') failed: %u", > >> + su_name.c_str(), error); > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + } > >> + > >> + goto goto_finalize; > >> + } > >> + > >> + // Get Amf node name, where this SU is hosted. > >> + if (immutil_getAttr( > >> + const_cast<SaImmAttrNameT>("saAmfSUHostedByNode"), > >> + attributes, 0, &node_name) != SA_AIS_OK) { > >> + } > >> + TRACE("Hosting Amf node '%s'", Amf::to_string(&node_name).c_str()); > >> + > >> + // Get Amf node attributes from Imm DB. > >> + if ((error = amf_saImmOmAccessorGet_o2( > >> + immOmHandle, accessorHandle, Amf::to_string(&node_name), > >> + nullptr, (SaImmAttrValuesT_2 ***)&attributes)) != > >> SA_AIS_OK) { > >> + if (error == SA_AIS_ERR_NOT_EXIST) { > >> + LOG_NO("Amf Node '%s' is not configured", > >> Amf::to_string(&node_name).c_str()); > >> + amf_rc = SA_AIS_ERR_NOT_EXIST; > >> + } else { > >> + LOG_ER("amf_saImmOmAccessorGet_o2(for node '%s') failed: %u", > >> + Amf::to_string(&node_name).c_str(), error); > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + } > >> + > >> + goto goto_finalize; > >> + } > >> + TRACE("Hosting Amf node '%s' found.", > >> Amf::to_string(&node_name).c_str()); > >> + > >> + // Find the values of oper status from Amf node attributes. > >> + if (immutil_getAttr( > >> + const_cast<SaImmAttrNameT>("saAmfNodeOperState"), > >> + attributes, 0, &oper_status) != SA_AIS_OK) { > >> + } > >> + // Find the Clm Node name from Amf node attributes. > >> + if (immutil_getAttr( > >> + const_cast<SaImmAttrNameT>("saAmfNodeClmNode"), > >> + attributes, 0, &clm_node_name) != SA_AIS_OK) { > >> + } > >> + TRACE("Amf node saAmfNodeOperState '%u' and Clm node name '%s'", > >> + oper_status, Amf::to_string(&clm_node_name).c_str()); > >> + > >> + // Get Clm node attributes from Imm DB. > >> + if ((error = amf_saImmOmAccessorGet_o2( > >> + immOmHandle, accessorHandle, > >> Amf::to_string(&clm_node_name), > >> + nullptr, (SaImmAttrValuesT_2 ***)&attributes)) != > >> SA_AIS_OK) { > >> + if (error == SA_AIS_ERR_NOT_EXIST) { > >> + LOG_NO("Clm Node '%s' is not configured", > >> + Amf::to_string(&clm_node_name).c_str()); > >> + amf_rc = SA_AIS_ERR_NOT_EXIST; > >> + } else { > >> + LOG_ER("amf_saImmOmAccessorGet_o2(for clm node '%s') failed: > >> %u", > >> + Amf::to_string(&clm_node_name).c_str(), error); > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + } > >> + > >> + goto goto_finalize; > >> + } > >> + TRACE("Clm node '%s' found.", > >> Amf::to_string(&clm_node_name).c_str()); > >> + > >> + // Find the configured value of saClmNodeIsMember from Clm node > >> attributes. > >> + if (immutil_getAttr( > >> + const_cast<SaImmAttrNameT>("saClmNodeIsMember"), > >> + attributes, 0, &is_member) != SA_AIS_OK) { > >> + } > >> + // Find the configured value of saClmNodeID from Clm node > >> attributes. > >> + if (immutil_getAttr( > >> + const_cast<SaImmAttrNameT>("saClmNodeID"), > >> + attributes, 0, &node_id) != SA_AIS_OK) { > >> + } > >> + TRACE("Node id '%u' and Clm member '%u'", node_id, is_member); > >> + > >> + // If the node is disabled (DOWN) or is not a member of Clm > >> cluster, then > >> + // return Unavailable for any operation like this. > >> + if ((oper_status == SA_AMF_OPERATIONAL_DISABLED) || (is_member > >> == false)) { > >> + amf_rc = SA_AIS_ERR_UNAVAILABLE; > >> + goto goto_finalize; > >> + } > >> + > >> + // We have all the information, send the event to the destined > >> node. > >> + rc = avnd_avnd_msg_send(cb, (uint8_t *)api_info, api_info->type, > >> &evt->mds_ctxt, > >> + node_id); > >> + if (NCSCC_RC_SUCCESS != rc) { > >> + // We couldn't send this to the destined AvND, tell user to > >> try again. > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + LOG_ER("avnd_int_ext_comp_hdlr:Msg Send Failed: '%u'", rc); > >> + goto goto_finalize; > >> + } else { > >> + TRACE("avnd_avnd_msg_send:Msg Send Success"); > >> + // Add the internode component in the DB > >> + comp = new AVND_COMP(); > >> + comp->name = Amf::to_string(&err_rep->err_comp); > >> + comp->node_id = node_id; > >> + comp->reg_dest = api_info->dest; > >> + comp->mds_ctxt = evt->mds_ctxt; > >> + comp->comp_type = AVND_COMP_TYPE_INTER_NODE_NP; > >> + // Add to the internode available compdb. > >> + rc = avnd_cb->compdb_internode.insert(comp->name, comp); > >> + if (NCSCC_RC_SUCCESS != rc) { > >> + delete comp; > >> + comp = nullptr; > >> + amf_rc = SA_AIS_ERR_TRY_AGAIN; > >> + LOG_ER("compdb_internode.insert failed: '%u'", rc); > >> + goto goto_finalize; > >> + } > >> + immutil_saImmOmFinalize(immOmHandle); > >> + goto done; > >> + } > >> +goto_finalize: > >> + immutil_saImmOmFinalize(immOmHandle); > >> + } > >> +goto_error: > >> /* We need not entertain errors when comp is not in shape */ > >> if (comp && (m_AVND_COMP_PRES_STATE_IS_UNINSTANTIATED(comp) || _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
