Hi Hans, I will send the modified patch. Thanks -Nagu
> -----Original Message----- > From: Nagendra Kumar > Sent: 03 March 2016 10:50 > To: Hans Nordebäck; Praveen Malviya; Minh Chau H; Gary Lee > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] amfd: delete su and its child objects at > stdby amfd [#1683] > > Hi Hans N, > Thanks for your review. If I incorporate the comment, Is that > Ack ? > > Thanks > -Nagu > > -----Original Message----- > > From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] > > Sent: 29 February 2016 19:30 > > To: Nagendra Kumar; Praveen Malviya; Minh Chau H; Gary Lee > > Cc: opensaf-devel@lists.sourceforge.net; Hans Nordebäck > > Subject: RE: [PATCH 1 of 1] amfd: delete su and its child objects at > > stdby amfd [#1683] > > > > Hi Nagu, minor correction below, (not tested or compiled though): > > > > std::map<std::string, AVD_COMPCS_TYPE*>::iterator it = compcstype_db- > > >begin(); > > while (it != compcstype_db->end() { > > AVD_COMPCS_TYPE *compcstype = it->second; if (compcstype->comp == > > comp) { > > TRACE("Standby Amfd, compcstype '%s' not deleted", > > compcstype->name.value); > > it = compcstype_db->erase(it)); > > delete compcstype; > > else > > it++ > > } > > } > > > > /Thanks HansN > > -----Original Message----- > > From: Hans Nordebäck > > Sent: den 29 februari 2016 14:29 > > To: nagendr...@oracle.com; praveen.malv...@oracle.com; Minh Chau H; > > Gary Lee > > Cc: opensaf-devel@lists.sourceforge.net; Hans Nordebäck > > Subject: Re: [PATCH 1 of 1] amfd: delete su and its child objects at > > stdby amfd [#1683] > > > > Hi Nagu, > > > > the deepest for loop can be changed to: > > > > > > and db_template.h is changed to: > > > > // c++11, returns iterator > > iterator erase(const iterator &it) {return db.erase(it);} > > > > iterator begin() {return db.begin();} > > iterator end() {return db.end();} > > > > const_iterator cbegin() const {return db.cbegin();} > > const_iterator cend() const {return db.cend();} > > > > as c++11 returns an iterator in erase. It should be backwards > > compatible and all present calls to begin can be changed later as needed. > > > > The other two loops over su and comp, why is the iterator not updated? > > Elements in node list_of_su and > > su list_of_comp is deleted while iterating, an alternative could be to > > iterate over a copy of these lists and then the deletes should not > > affect respective iterators. > > > > /Thanks HansN > > > > > > > > > > On 02/26/2016 07:52 AM, nagendr...@oracle.com wrote: > > > osaf/services/saf/amf/amfd/comp.cc | 2 +- > > > osaf/services/saf/amf/amfd/include/comp.h | 1 + > > > osaf/services/saf/amf/amfd/include/su.h | 2 +- > > > osaf/services/saf/amf/amfd/node.cc | 54 > > +++++++++++++++++++++++++++++++ > > > osaf/services/saf/amf/amfd/su.cc | 2 +- > > > osaf/services/saf/amf/amfd/util.cc | 16 +++++++++ > > > 6 files changed, 74 insertions(+), 3 deletions(-) > > > > > > > > > If SUs are deleted at Act Amfd when Stdby Amfd has finished reading > > > objects from Imm database, Stdby Amfd still has those SUs. > > > Now, if node hosting those Sus are deleted, Act Amfd says Ok, but > > > Stdby Amfd says it still has some SUs left on that node to be deleted. > > > Hence in apply callback, Stdby Amfd crashes. > > > So, this patch makes sure that in this particular scenarios, if node > > > is being deleted and if there are some Sus and its child obkects are > > > still there, Stdby Amfd deletes them. > > > > > > 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 > > > @@ -1577,7 +1577,7 @@ static void comp_ccb_apply_modify_hdlr(s > > > TRACE_LEAVE(); > > > } > > > > > > -static void comp_ccb_apply_delete_hdlr(struct CcbUtilOperationData > > > *opdata) > > > +void comp_ccb_apply_delete_hdlr(struct CcbUtilOperationData > > > +*opdata) > > > { > > > TRACE_ENTER(); > > > > > > diff --git a/osaf/services/saf/amf/amfd/include/comp.h > > > b/osaf/services/saf/amf/amfd/include/comp.h > > > --- a/osaf/services/saf/amf/amfd/include/comp.h > > > +++ b/osaf/services/saf/amf/amfd/include/comp.h > > > @@ -275,4 +275,5 @@ bool comp_is_preinstantiable(const AVD_C > > > extern bool is_comp_assigned_any_csi(AVD_COMP *comp); > > > extern SaAisErrorT check_comp_stability(const AVD_COMP*); > > > extern AVD_CTCS_TYPE *get_ctcstype(const SaNameT > *comptype_name, > > > const SaNameT *cstype_name); > > > +extern void comp_ccb_apply_delete_hdlr(struct CcbUtilOperationData > > > +*opdata); > > > #endif > > > diff --git a/osaf/services/saf/amf/amfd/include/su.h > > > b/osaf/services/saf/amf/amfd/include/su.h > > > --- a/osaf/services/saf/amf/amfd/include/su.h > > > +++ b/osaf/services/saf/amf/amfd/include/su.h > > > @@ -171,5 +171,5 @@ extern SaAisErrorT avd_su_config_get(con > > > * Class constructor, must be called before any other function > > > */ > > > extern void avd_su_constructor(void); > > > - > > > +extern void su_ccb_apply_delete_hdlr(struct CcbUtilOperationData > > > +*opdata); > > > #endif > > > diff --git a/osaf/services/saf/amf/amfd/node.cc > > > b/osaf/services/saf/amf/amfd/node.cc > > > --- a/osaf/services/saf/amf/amfd/node.cc > > > +++ b/osaf/services/saf/amf/amfd/node.cc > > > @@ -165,9 +165,56 @@ AVD_AVND *avd_node_new(const SaNameT > *dn > > > > > > void avd_node_delete(AVD_AVND *node) > > > { > > > + TRACE_ENTER(); > > > osafassert(node->pg_csi_list.n_nodes == 0); > > > if (node->node_info.nodeId) > > > avd_node_delete_nodeid(node); > > > + /* Check if the SUs and related objects are still left. This can > > > + happen on Standby Amfd when it has just read the configuration > > > + and before it becomes applier, Act Amfd deletes SUs. Those SUs > > > + will be left out at Standby Amfd. Though this could be rare.*/ > > > + if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) { > > > + if (node->list_of_su.empty() != true) { > > > + std::vector<AVD_SU*> *su_list = nullptr; > > > + su_list = &node->list_of_su; > > > + std::vector<AVD_SU*>::const_iterator su_iter; > > > + for (su_iter = su_list->begin(); su_iter != su_list- > > >end(); ) { > > > + AVD_SU *su = *su_iter; > > > + TRACE("Standby Amfd, su '%s' not deleted", > > su->name.value); > > > + std::vector<AVD_COMP*> *comp_list = > > nullptr; > > > + comp_list = &su->list_of_comp; > > > + std::vector<AVD_COMP*>::const_iterator > > comp_iter; > > > + for (comp_iter = comp_list->begin(); > > comp_iter != comp_list->end(); ) { > > > + AVD_COMP *comp = *comp_iter; > > > + TRACE("Standby Amfd, comp '%s' not > > deleted", comp->comp_info.name.value); > > > + /* Delete the CompCsType. */ > > > + for (std::map<std::string, > > AVD_COMPCS_TYPE*>::const_iterator it = > > > + compcstype_db- > > >begin(); > > > + it != compcstype_db- > > >end(); it++) { > > > + AVD_COMPCS_TYPE > > *compcstype = it->second; > > > + if (compcstype->comp == > > comp) { > > > + TRACE("Standby > > Amfd, compcstype '%s' not deleted", > > > + > > compcstype->name.value); > > > + compcstype_db- > > >erase(Amf::to_string(&compcstype->name)); > > > + delete compcstype; > > > + } > > > + } > > > + > > > + /* Delete the Comp. */ > > > + struct CcbUtilOperationData opdata; > > > + memset(&opdata.objectName, 0, > > sizeof(SaNameT)); > > > + memcpy(opdata.objectName.value, > > comp->comp_info.name.value, > > > + comp- > > >comp_info.name.length); > > > + opdata.objectName.length = comp- > > >comp_info.name.length; > > > + > > comp_ccb_apply_delete_hdlr(&opdata); > > > + } > > > + /* Delete the SU. */ > > > + struct CcbUtilOperationData opdata; > > > + opdata.userData = su; > > > + su_ccb_apply_delete_hdlr(&opdata); > > > + } > > > + } > > > + } > > > m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(avd_cb, node, > > AVSV_CKPT_AVD_NODE_CONFIG); > > > node_name_db->erase(Amf::to_string(&node->name)); > > > delete [] node->node_name; > > > @@ -493,6 +540,10 @@ static SaAisErrorT node_ccb_completed_de > > > return SA_AIS_ERR_BAD_OPERATION; > > > } > > > > > > + if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) { > > > + goto done_ok; > > > + } > > > + > > > /* Check to see that no SUs exists on this node */ > > > if (node->list_of_su.empty() != true) { > > > /* check whether there exists a delete operation for @@ > > > - > > 569,6 > > > +620,9 @@ static SaAisErrorT node_ccb_completed_de > > > } > > > } > > > } > > > + > > > +done_ok: > > > + rc = SA_AIS_OK; > > > opdata->userData = node; > > > done: > > > TRACE_LEAVE(); > > > diff --git a/osaf/services/saf/amf/amfd/su.cc > > > b/osaf/services/saf/amf/amfd/su.cc > > > --- a/osaf/services/saf/amf/amfd/su.cc > > > +++ b/osaf/services/saf/amf/amfd/su.cc > > > @@ -1783,7 +1783,7 @@ static void su_ccb_apply_modify_hdlr(str > > > * > > > * @param su > > > */ > > > -static void su_ccb_apply_delete_hdlr(struct CcbUtilOperationData > > > *opdata) > > > +void su_ccb_apply_delete_hdlr(struct CcbUtilOperationData *opdata) > > > { > > > AVD_SU *su = static_cast<AVD_SU*>(opdata->userData); > > > AVD_AVND *su_node_ptr; > > > diff --git a/osaf/services/saf/amf/amfd/util.cc > > > b/osaf/services/saf/amf/amfd/util.cc > > > --- a/osaf/services/saf/amf/amfd/util.cc > > > +++ b/osaf/services/saf/amf/amfd/util.cc > > > @@ -1551,6 +1551,22 @@ int amfd_file_dump(const char *filename) > > > fprintf(f, " saAmfCompCurrProxyName: %s\n", > > comp->saAmfCompCurrProxyName.value); > > > } > > > > > > + fprintf(f, "COMPCS_TYPE:\n"); > > > + for (std::map<std::string, > > > + AVD_COMPCS_TYPE*>::const_iterator it = > > compcstype_db->begin(); > > > + it != compcstype_db->end(); it++) { > > > + const AVD_COMPCS_TYPE *compcs_type = it->second; > > > + fprintf(f, " dn: %s\n", compcs_type->name.value); > > > + fprintf(f, " saAmfCompNumMaxActiveCSIs: %u\n", > > > + compcs_type->saAmfCompNumMaxActiveCSIs); > > > + fprintf(f, " saAmfCompNumMaxStandbyCSIs: %u\n", > > > + compcs_type->saAmfCompNumMaxStandbyCSIs); > > > + fprintf(f, " saAmfCompNumCurrActiveCSIs: %u\n", > > > + compcs_type->saAmfCompNumCurrActiveCSIs); > > > + fprintf(f, " saAmfCompNumCurrStandbyCSIs: %u\n", > > > + > > > compcs_type->saAmfCompNumCurrStandbyCSIs); > > > + } > > > + > > > + > > > fprintf(f, "Node Groups:\n"); > > > for (std::map<std::string, AVD_AMF_NG*>::const_iterator it = > > nodegroup_db->begin(); > > > it != nodegroup_db->end(); it++) { > > > > ------------------------------------------------------------------------------ > Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + > Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor > end-to-end web transactions and take corrective actions now Troubleshoot > faster and improve end-user experience. Signup Now! > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > _______________________________________________ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
