Hi Hans N,
                Please find the modified attached patch. Please find some time 
to review.

Thanks
-Nagu
> -----Original Message-----
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: 03 March 2016 12:48
> To: Nagendra Kumar; 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 Nagu,
> 
> good , my  questions below was related to the three iterators and deleting
> while iterating. /Thanks HansN
> 
> -----Original Message-----
> From: Nagendra Kumar [mailto:nagendr...@oracle.com]
> Sent: den 3 mars 2016 07:01
> 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,
> 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

Attachment: 1683_set.patch
Description: Binary data

------------------------------------------------------------------------------
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

Reply via email to