Fixed it. Thanks -Nagu
> -----Original Message----- > From: Nagendra Kumar > Sent: 07 March 2016 21:10 > To: [email protected] > Cc: [email protected]; [email protected] > Subject: Re: [devel] [PATCH 1 of 1] amfd: delete su and its child objects at > stdby amfd [#1683] > > Hi Hans N, > Thanks for the information. I am on leave today. I will correct it till this > weekend. > > Tx > Nagu > ----- Original Message ----- > From: [email protected] > To: [email protected], [email protected], > [email protected], [email protected] > Cc: [email protected], [email protected] > Sent: Monday, March 7, 2016 7:39:25 PM GMT +05:30 Chennai, Kolkata, > Mumbai, New Delhi > Subject: RE: [devel] [PATCH 1 of 1] amfd: delete su and its child objects at > stdby amfd [#1683] > > Hi Nagu, > > There are problems building for opensaf-4.6.x with this patch, as 4.6.x don't > have c++11 support enabled. > > /Thanks HansN > > -----Original Message----- > From: Nagendra Kumar [mailto:[email protected]] > Sent: den 4 mars 2016 11:14 > To: Hans Nordebäck; Praveen Malviya; Minh Chau H; Gary Lee > Cc: [email protected] > 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. > I thought that accessing based on node name is bit safer than using pointers > as in each loop the link is being deleted. But, anyway, I can clean this up > some time later. > > Thanks > -Nagu > > -----Original Message----- > > From: Hans Nordebäck [mailto:[email protected]] > > Sent: 04 March 2016 15:35 > > To: Nagendra Kumar; Praveen Malviya; Minh Chau H; Gary Lee > > Cc: [email protected] > > Subject: Re: [devel] [PATCH 1 of 1] amfd: delete su and its child > > objects at stdby amfd [#1683] > > > > Hi Nagu, > > > > ack code review only. One minor comment below, > > > > these loops, etc: > > > > std::set<std::string> su_list; > > std::set<std::string> comp_list; > > for (const auto& su : node->list_of_su) > > su_list.insert(Amf::to_string(&su->name)); > > : > > for (const auto& comp : su->list_of_comp) > > comp_list.insert(Amf::to_string(&comp->comp_info.name)); > > > > > > can be replaced by: > > > > > > if (node->list_of_su.empty() != true) { > > std::vector<AVD_SU*> su_list = node->list_of_su; > > for (auto iter = su_list.begin(); iter != su_list.end(); > > ++iter) { > > AVD_SU *su = *iter; > > TRACE("Standby Amfd, su '%s' not deleted", su->name.value); > > std::vector<AVD_COMP*> comp_list = su->list_of_comp; > > for (auto iter1 = comp_list.begin(); iter1 != > > comp_list.end(); ++iter1) { > > AVD_COMP *comp = *iter1; > > TRACE("Standby Amfd, comp '%s' not deleted", > > comp->comp_info.name.value); > > > > 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++; > > } > > > > I think it makes the code a bit cleaner. > > > > /Thanks HansN > > > > On 03/04/2016 08:19 AM, Nagendra Kumar wrote: > > > Hi Hans N, > > > Please find the modified attached patch. Please find some > > time to review. > > > > > > Thanks > > > -Nagu > > >> -----Original Message----- > > >> From: Hans Nordebäck [mailto:[email protected]] > > >> Sent: 03 March 2016 12:48 > > >> To: Nagendra Kumar; Praveen Malviya; Minh Chau H; Gary Lee > > >> Cc: [email protected] > > >> Subject: RE: [devel] [PATCH 1 of 1] amfd: delete su and its child > > >> objects at stdby amfd [#1683] > > >> > > >> Hi Nagu, > > >> > > >> good , my questions below was related to the three iterators and > > >> deleting while iterating. /Thanks HansN > > >> > > >> -----Original Message----- > > >> From: Nagendra Kumar [mailto:[email protected]] > > >> Sent: den 3 mars 2016 07:01 > > >> To: Hans Nordebäck; Praveen Malviya; Minh Chau H; Gary Lee > > >> Cc: [email protected] > > >> Subject: RE: [devel] [PATCH 1 of 1] amfd: delete su and its child > > >> objects at stdby amfd [#1683] > > >> > > >> 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: [email protected] > > >>> 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:[email protected]] > > >>>> Sent: 29 February 2016 19:30 > > >>>> To: Nagendra Kumar; Praveen Malviya; Minh Chau H; Gary Lee > > >>>> Cc: [email protected]; 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: [email protected]; [email protected]; Minh > > Chau > > >> H; > > >>>> Gary Lee > > >>>> Cc: [email protected]; 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, [email protected] 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 > > >>> [email protected] > > >>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > > > > > ------------------------------------------------------------------------------ > Transform Data into Opportunity. > Accelerate data analysis in your applications with Intel Data Analytics > Acceleration Library. > Click to learn more. > http://makebettercode.com/inteldaal-eval > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
