some comments inline./Regards HansN

-----Original Message-----
From: Hans Feldt [mailto:hans.fe...@ericsson.com] 
Sent: den 16 juni 2014 12:17
To: praveen malviya; Hans Feldt
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 2 of 4] amfd: add specialized red model specific SG 
classes [#713]

Inline
Thanks,
Hans

> -----Original Message-----
> From: praveen malviya [mailto:praveen.malv...@oracle.com]
> Sent: den 16 juni 2014 11:49
> To: Hans Feldt
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] [PATCH 2 of 4] amfd: add specialized red model 
> specific SG classes [#713]
> 
> Please find comments inline with [Praveen]
> 
> Thanks,
> Praveen
> On 16-Jun-14 10:37 AM, Hans Feldt wrote:
> >   osaf/services/saf/amf/amfd/ckpt_dec.cc       |   4 +-
> >   osaf/services/saf/amf/amfd/include/sg.h      |  48 
> > +++++++++++++++++++++++++--
> >   osaf/services/saf/amf/amfd/sg.cc             |  36 +++++++++++---------
> >   osaf/services/saf/amf/amfd/sg_2n_fsm.cc      |   4 ++
> >   osaf/services/saf/amf/amfd/sg_nored_fsm.cc   |   4 ++
> >   osaf/services/saf/amf/amfd/sg_npm_fsm.cc     |   4 ++
> >   osaf/services/saf/amf/amfd/sg_nway_fsm.cc    |   4 ++
> >   osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc |   4 ++
> >   8 files changed, 85 insertions(+), 23 deletions(-)
> >
> >
> > A first step in converting the C based SG FSM files into C++. The 
> > idea with that is to simplify the FSM code.
> >
> > diff --git a/osaf/services/saf/amf/amfd/ckpt_dec.cc 
> > b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> > --- a/osaf/services/saf/amf/amfd/ckpt_dec.cc
> > +++ b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> > @@ -403,7 +403,7 @@ static void decode_sg(NCS_UBAID *ub, AVD
> >   
> > \**************************************************************************/
> >   static uint32_t dec_sg_config(AVD_CL_CB *cb, NCS_MBCSV_CB_DEC *dec)
> >   {
> > -   AVD_SG sg;
> > +   SG_2N sg;
> >
> [Praveen] AMF will decode SGs of other red models also, then why on 2N?
[Hans] this is just to get an instance to store attributes in. AVD_SG is 
abstract and can't be used. It does not matter if it is SG_2N or SG_NORED here.
[HansN] AVD_SG is not abstract, i.e there are no pure virtual functions. Anyhow 
the SG_2N sg could be changed to AVD_SG *sg and method decode_sg do create the 
concrete type.

> >     TRACE_ENTER2("i_action '%u'", dec->i_action);
> >
> > @@ -2479,7 +2479,7 @@ static uint32_t dec_cs_sg_config(AVD_CL_
> >   {
> >     uint32_t status = NCSCC_RC_SUCCESS;
> >     uint32_t count;
> > -   AVD_SG dec_sg;
> > +   SG_2N dec_sg;
> >
> [Praveen] same as above.
> >     TRACE_ENTER();
> >
> > 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
> > @@ -62,13 +62,13 @@ typedef struct avd_sg_oper_tag {
> >     struct avd_sg_oper_tag *next;   /* The next SU undergoing operation. */
> >   } AVD_SG_OPER;
> >
> > -/* Availability directors Service Group structure(AVD_SG):
> > - * This data structure lives in the AvD and reflects data points
> > - * associated with the Service group (SG).
> > +/**
> > + * Service group abstract base class
> >    */
> >   class AVD_SG {
> >   public:
> >     AVD_SG();
> > +   virtual ~AVD_SG() {};
> >
> >     SaNameT name;           /* the service group DN used as the index.
> >                              * Checkpointing - Sent as a one time update.
> > @@ -251,6 +251,47 @@ private:
> >
> >   extern AmfDb<std::string, AVD_SG> *sg_db;
> >
> > +/**
> > + * 2N redundancy model SG specialization  */ class SG_2N : public 
> > +AVD_SG {
> > +public:
> > +   ~SG_2N();
> > +};
> > +
> > +/**
> > + * No redundancy specialization
> > + */
> > +class SG_NORED : public AVD_SG {
> > +public:
> > +   ~SG_NORED();
> > +};
> > +
> > +/**
> > + * N+M redundancy specialization
> > + */
> > +class SG_NPM : public AVD_SG {
> > +public:
> > +   ~SG_NPM();
> > +};
> > +
> > +/**
> > + * N-Way active specialization
> > + */
> > +class SG_NACV : public AVD_SG {
> > +public:
> > +   ~SG_NACV();
> > +};
> > +
> > +/**
> > + * N-Way specialization
> > + */
> > +class SG_NWAY : public AVD_SG {
> > +public:
> > +   ~SG_NWAY();
> > +};
> > +
> > +
> >   #define m_AVD_SET_SG_ADJUST(cb,sg,state) {\
> >     TRACE("adjust_state %u => %u", sg->adjust_state, state); \
> >     sg->adjust_state = state;\
> > @@ -321,7 +362,6 @@ extern AmfDb<std::string, AVD_SG> *sg_db
> >      }\
> >   }
> >
> > -extern AVD_SG *avd_sg_new(const SaNameT *dn);
> >   extern void avd_sg_delete(AVD_SG *sg);
> >   extern void avd_sg_db_add(AVD_SG *sg);
> >   extern void avd_sg_db_remove(AVD_SG *sg); 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
> > @@ -134,11 +134,23 @@ AVD_SG::AVD_SG():
> >     su_oper_list.next = NULL;
> >   }
> >
> > -AVD_SG *avd_sg_new(const SaNameT *dn)
> > +static AVD_SG *sg_new(const SaNameT *dn, SaAmfRedundancyModelT 
> > +redundancy_model)
> >   {
> >     AVD_SG *sg;
> >
> > -   sg = new AVD_SG();
> > +   if (redundancy_model == SA_AMF_2N_REDUNDANCY_MODEL)
> > +           sg = new SG_2N();
> > +   else if (redundancy_model == SA_AMF_NPM_REDUNDANCY_MODEL)
> > +           sg = new SG_NPM();
> > +   else if (redundancy_model == SA_AMF_N_WAY_REDUNDANCY_MODEL)
> > +           sg = new SG_NWAY();
> > +   else if (redundancy_model == SA_AMF_N_WAY_ACTIVE_REDUNDANCY_MODEL)
> > +           sg = new SG_NACV();
> > +   else if (redundancy_model == SA_AMF_NO_REDUNDANCY_MODEL)
> > +           sg = new SG_NORED();
> > +   else
> > +           assert(0);
> > +
> >     memcpy(sg->name.value, dn->value, dn->length);
> >     sg->name.length = dn->length;
> >
> > @@ -282,22 +294,13 @@ static AVD_SG *sg_create(const SaNameT *
> >
> >     TRACE_ENTER2("'%s'", sg_name->value);
> >
> > -   /*
> > -   ** If called at new active at failover, the object is found in the DB
> > -   ** but needs to get configuration attributes initialized.
> > -   */
> > -   if (NULL == (sg = sg_db->find(Amf::to_string(sg_name)))) {
> > -           if ((sg = avd_sg_new(sg_name)) == NULL)
> > -                   goto done;
> > -   } else
> > -           TRACE("already created, refreshing config...");
> > -
> > -
> > -   error = immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGType"), 
> > attributes, 0, &sg->saAmfSGType);
> > +   SaNameT sgtype_dn;
> > +   error = immutil_getAttr("saAmfSGType", attributes, 0, &sgtype_dn);
> >     osafassert(error == SA_AIS_OK);
> > -
> > -   sgt = sgtype_db->find(Amf::to_string(&sg->saAmfSGType));
> > +   sgt = sgtype_db->find(Amf::to_string(&sgtype_dn));
> >     osafassert(sgt);
> > +   sg = sg_new(sg_name, sgt->saAmfSgtRedundancyModel);
> [Praveen] According to spec saAmfSGType  in SG in config writable.
> Since all redundancy model's SG are derived classes from a abstract 
> base class, there will be problems if somebody modifies saAmfSGType in 
> SG to a value having different red model?
[Hans] we don't support such change today. Should we ever? I don't see the use 
case...
But yes then you would have to create a new instance of the correct class and 
migrate data.

> 
> 
> > +   sg->saAmfSGType = sgtype_dn;
> >     sg->sg_type = sgt;
> >
> >     
> >(void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGSuHostNodeGr
> >oup"), attributes, 0, &sg- saAmfSGSuHostNodeGroup);  @@ -399,7 +402,6 
> >@@ static AVD_SG *sg_create(const SaNameT *
> >
> >     rc = 0;
> >
> > -done:
> >     if (rc != 0) {
> >             avd_sg_delete(sg);
> >             sg = NULL;
> > diff --git a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc 
> > b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc
> > --- a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc
> > +++ b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc
> > @@ -4185,3 +4185,7 @@ void avd_sg_2n_init(AVD_SG *sg)
> >     sg->susi_success = avd_sg_2n_susi_sucss_func;
> >     sg->susi_failed = avd_sg_2n_susi_fail_func;
> >   }
> > +
> > +SG_2N::~SG_2N() {
> > +}
> > +
> > diff --git a/osaf/services/saf/amf/amfd/sg_nored_fsm.cc 
> > b/osaf/services/saf/amf/amfd/sg_nored_fsm.cc
> > --- a/osaf/services/saf/amf/amfd/sg_nored_fsm.cc
> > +++ b/osaf/services/saf/amf/amfd/sg_nored_fsm.cc
> > @@ -1452,3 +1452,7 @@ void avd_sg_nored_init(AVD_SG *sg)
> >     sg->susi_success = avd_sg_nored_susi_sucss_func;
> >     sg->susi_failed = avd_sg_nored_susi_fail_func;
> >   }
> > +
> > +SG_NORED::~SG_NORED() {
> > +}
> > +
> > diff --git a/osaf/services/saf/amf/amfd/sg_npm_fsm.cc 
> > b/osaf/services/saf/amf/amfd/sg_npm_fsm.cc
> > --- a/osaf/services/saf/amf/amfd/sg_npm_fsm.cc
> > +++ b/osaf/services/saf/amf/amfd/sg_npm_fsm.cc
> > @@ -4632,3 +4632,7 @@ void avd_sg_npm_init(AVD_SG *sg)
> >     sg->susi_success = avd_sg_npm_susi_sucss_func;
> >     sg->susi_failed = avd_sg_npm_susi_fail_func;
> >   }
> > +
> > +SG_NPM::~SG_NPM() {
> > +}
> > +
> > 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
> > @@ -3694,3 +3694,7 @@ void avd_sg_nway_init(AVD_SG *sg)
> >     sg->susi_success = avd_sg_nway_susi_sucss_func;
> >     sg->susi_failed = avd_sg_nway_susi_fail_func;
> >   }
> > +
> > +SG_NWAY::~SG_NWAY() {
> > +}
> > +
> > diff --git a/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc 
> > b/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc
> > --- a/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc
> > +++ b/osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc
> > @@ -2173,3 +2173,7 @@ void avd_sg_nacv_init(AVD_SG *sg)
> >     sg->susi_success = avd_sg_nacvred_susi_sucss_func;
> >     sg->susi_failed = avd_sg_nacvred_susi_fail_func;
> >   }
> > +
> > +SG_NACV::~SG_NACV() {
> > +}
> > +
> 
> 
> ----------------------------------------------------------------------
> -------- HPCC Systems Open Source Big Data Platform from LexisNexis 
> Risk Solutions Find What Matters Most in Your Big Data with HPCC 
> Systems Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
> Leverages Graph Analysis for Fast Processing & Easy Data Exploration 
> http://p.sf.net/sfu/hpccsystems 
> _______________________________________________
> Opensaf-devel mailing list
> Opensaf-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel

------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions Find 
What Matters Most in Your Big Data with HPCC Systems Open Source. Fast. 
Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration 
http://p.sf.net/sfu/hpccsystems _______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to