Hi Hans,
                I am ok to just reboot the node if both saAmfNodeAutoRepair and 
saAmfNodeFailfastOnTerminationFailure are set to true. I am not in favour of 
changing the existing behaviour until we don't have complete fix for it. So, in 
that sense, patch #1, #2 and #4(changes in amfd) are ok. If you are fine with 
that, we can apply #1,#2,#4 and proceed for tests.

Thanks
-Nagu

> -----Original Message-----
> From: Hans Feldt [mailto:hans.fe...@ericsson.com]
> Sent: 10 March 2014 15:57
> To: Nagendra Kumar; Praveen Malviya; Hans Nordebäck
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [devel] [PATCH 1 of 4] amfd: allow modification of node repair
> attributes [#538]
> 
> Could you please review one patch at time. What you are complaining about is
> changes from patch 3.
> 1, 2 & 4 is needed.
> 
> Thanks,
> Hans
> 
> 
> > -----Original Message-----
> > From: Nagendra Kumar [mailto:nagendr...@oracle.com]
> > Sent: den 10 mars 2014 07:56
> > To: Hans Feldt; Praveen Malviya; Hans Nordebäck
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: Re: [devel] [PATCH 1 of 4] amfd: allow modification of node
> > repair attributes [#538]
> >
> > I have tested with saAmfNodeAutoRepair and
> saAmfNodeFailfastOnTerminationFailure both set to false.
> > When I locked Act SU and csi timeout and cleanup script return error, then
> admin command just hangs.
> > Further admin command returns error as " Admin operation is already going
> on ".
> >
> > This looks problematic.
> >
> > My suggestion is to have reboot patch only and not change/disturb old
> > working logic until we have complete fix for saAmfNodeAutoRepair and
> saAmfNodeFailfastOnTerminationFailure both set to false.
> >
> > So, I have Nack for this patch series and would test further if only reboot 
> > patch
> is floated.
> >
> > Thanks
> > -Nagu
> >
> > > -----Original Message-----
> > > From: Hans Feldt [mailto:osafde...@gmail.com]
> > > Sent: 28 February 2014 13:25
> > > To: Praveen Malviya; Nagendra Kumar; hans.nordeb...@ericsson.com
> > > Cc: opensaf-devel@lists.sourceforge.net
> > > Subject: [PATCH 1 of 4] amfd: allow modification of node repair
> > > attributes [#538]
> > >
> > >  osaf/services/saf/amf/amfd/include/util.h |   1 +
> > >  osaf/services/saf/amf/amfd/node.cc        |  27
> > > +++++++++++++++++++++++++++
> > >  osaf/services/saf/amf/amfd/sg.cc          |   4 ++++
> > >  osaf/services/saf/amf/amfd/sgtype.cc      |   6 ++++++
> > >  osaf/services/saf/amf/amfd/util.cc        |  18 ++++++++++++++++++
> > >  5 files changed, 56 insertions(+), 0 deletions(-)
> > >
> > >
> > > To prepare for correct handling of TERMINATION-FAILED it is
> > > important that all the repair related attributes of the AMF system model 
> > > can
> be changed.
> > >
> > > This patch allows changing saAmfNodeAutoRepair and
> > > saAmfNodeFailfastOnTerminationFailure and also logs such change to SAF
> LOG.
> > >
> > > diff --git a/osaf/services/saf/amf/amfd/include/util.h
> > > b/osaf/services/saf/amf/amfd/include/util.h
> > > --- a/osaf/services/saf/amf/amfd/include/util.h
> > > +++ b/osaf/services/saf/amf/amfd/include/util.h
> > > @@ -94,6 +94,7 @@ struct avd_comp_tag;  struct avd_comp_csi_rel_tag;
> > > struct avd_csi_tag;
> > >
> > > +void amflog(int priority, const char *format, ...);
> > >  void d2n_msg_free(AVD_DND_MSG *msg);  uint32_t
> > > avd_d2n_msg_dequeue(struct cl_cb_tag *cb);  uint32_t
> > > avd_d2n_msg_snd(struct cl_cb_tag *cb, struct avd_avnd_tag *nd_node,
> > > AVD_DND_MSG *snd_msg); 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
> > > @@ -502,6 +502,24 @@ static SaAisErrorT node_ccb_completed_mo
> > >                   }
> > >           } else if (!strcmp(attribute->attrName,
> > > "saAmfNodeSuFailoverMax")) {
> > >                   /*  No validation needed, avoiding fall-through to
> Unknown
> > > Attribute error-case */
> > > +         } else if (!strcmp(attribute->attrName,
> > > "saAmfNodeAutoRepair")) {
> > > +                 uint32_t value = *((SaUint32T *)attribute-
> > > >attrValues[0]);
> > > +                 if (value > SA_TRUE) {
> > > +                         report_ccb_validation_error(opdata,
> > > +                                 "Invalid saAmfNodeAutoRepair '%s'",
> > > +                                 opdata->objectName.value);
> > > +                         rc = SA_AIS_ERR_BAD_OPERATION;
> > > +                         goto done;
> > > +                 }
> > > +         } else if (!strcmp(attribute->attrName,
> > > "saAmfNodeFailfastOnTerminationFailure")) {
> > > +                 uint32_t value = *((SaUint32T *)attribute-
> > > >attrValues[0]);
> > > +                 if (value > SA_TRUE) {
> > > +                         report_ccb_validation_error(opdata,
> > > +                                 "Invalid
> > > saAmfNodeFailfastOnTerminationFailure '%s'",
> > > +                                 opdata->objectName.value);
> > > +                         rc = SA_AIS_ERR_BAD_OPERATION;
> > > +                         goto done;
> > > +                 }
> > >           } else {
> > >                   report_ccb_validation_error(opdata, "Modification of
> '%s'
> > > failed-attribute '%s' cannot be modified",
> > >                                   opdata->objectName.value, attribute-
> > > >attrName); @@ -619,6 +637,15 @@ static void
> > > node_ccb_apply_modify_hdlr(C
> > >                   }
> > >                   TRACE("Modified saAmfNodeSuFailoverMax is '%u'",
> > > node->saAmfNodeSuFailoverMax);
> > >
> > > +         } else if (!strcmp(attribute->attrName,
> > > "saAmfNodeAutoRepair")) {
> > > +                 node->saAmfNodeAutoRepair = *((SaBoolT
> > > *)attribute->attrValues[0]);
> > > +                 amflog(LOG_NOTICE, "%s saAmfNodeAutoRepair
> > > changed to %u",
> > > +                         node->name.value, node-
> > > >saAmfNodeAutoRepair);
> > > +         } else if (!strcmp(attribute->attrName,
> > > "saAmfNodeFailfastOnTerminationFailure")) {
> > > +                 node->saAmfNodeFailfastOnTerminationFailure =
> > > +                                 *((SaBoolT *)attribute->attrValues[0]);
> > > +                 amflog(LOG_NOTICE, "%s
> > > saAmfNodeFailfastOnTerminationFailure changed to %u",
> > > +                         node->name.value, node-
> > > >saAmfNodeFailfastOnTerminationFailure);
> > >           } else {
> > >                   osafassert(0);
> > >           }
> > > 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
> > > @@ -850,6 +850,8 @@ static void ccb_apply_modify_hdlr(CcbUti
> > >                                   sg->saAmfSGAutoRepair_configured =
> true;
> > >                           }
> > >                           TRACE("Modified saAmfSGAutoRepair is '%u'",
> > > sg->saAmfSGAutoRepair);
> > > +                         amflog(LOG_NOTICE, "%s saAmfSGAutoRepair
> > > changed to %u",
> > > +                                 sg->name.value, sg-
> > > >saAmfSGAutoRepair);
> > >                   } else if (!strcmp(attribute->attrName,
> > > "saAmfSGAutoAdjust")) {
> > >                           TRACE("Old saAmfSGAutoAdjust is '%u'", sg-
> > > >saAmfSGAutoAdjust);
> > >                           if (value_is_deleted)
> > > @@ -1004,6 +1006,8 @@ static void ccb_apply_modify_hdlr(CcbUti
> > >                                   sg->saAmfSGAutoRepair_configured =
> true;
> > >                           }
> > >                           TRACE("Modified saAmfSGAutoRepair is '%u'",
> > > sg->saAmfSGAutoRepair);
> > > +                         amflog(LOG_NOTICE, "%s saAmfSGAutoRepair
> > > changed to %u",
> > > +                                 sg->name.value, sg-
> > > >saAmfSGAutoRepair);
> > >                   } else if (!strcmp(attribute->attrName,
> > > "saAmfSGSuHostNodeGroup")) {
> > >                           sg->saAmfSGSuHostNodeGroup = *((SaNameT
> *)value);
> > >                   } else {
> > > diff --git a/osaf/services/saf/amf/amfd/sgtype.cc
> > > b/osaf/services/saf/amf/amfd/sgtype.cc
> > > --- a/osaf/services/saf/amf/amfd/sgtype.cc
> > > +++ b/osaf/services/saf/amf/amfd/sgtype.cc
> > > @@ -18,6 +18,7 @@
> > >
> > >  #include <ncspatricia.h>
> > >  #include <logtrace.h>
> > > +#include <util.h>
> > >  #include <cluster.h>
> > >  #include <app.h>
> > >  #include <imm.h>
> > > @@ -445,12 +446,17 @@ static void sgtype_ccb_apply_modify_hdlr
> > >                           sgt->saAmfSgtDefAutoRepair_configured =
> true;
> > >                   }
> > >                   TRACE("Modified saAmfSgtDefAutoRepair is '%u'", sgt-
> > > >saAmfSgtDefAutoRepair);
> > > +                 amflog(LOG_NOTICE, "%s saAmfSgtDefAutoRepair
> > > changed to %u",
> > > +                         sgt->name.value, sgt-
> > > >saAmfSgtDefAutoRepair);
> > > +
> > >                   /* Modify saAmfSGAutoRepair for SGs which had
> inherited
> > > saAmfSgtDefAutoRepair.*/
> > >                   if (old_value != sgt->saAmfSgtDefAutoRepair) {
> > >                           for (AVD_SG *sg = sgt->list_of_sg; sg; sg = sg-
> > > >sg_list_sg_type_next) {
> > >                                   if (!sg-
> > > >saAmfSGAutoRepair_configured) {
> > >                                           sg->saAmfSGAutoRepair =
> > > static_cast<SaBoolT>(sgt->saAmfSgtDefAutoRepair);
> > >                                           TRACE("Modified
> > > saAmfSGAutoRepair is '%u'", sg->saAmfSGAutoRepair);
> > > +                                         amflog(LOG_NOTICE, "%s
> > > inherited saAmfSGAutoRepair value changed to %u",
> > > +                                                 sg->name.value, sg-
> > > >saAmfSGAutoRepair);
> > >                                   }
> > >                           }
> > >                   }
> > > 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
> > > @@ -1727,3 +1727,21 @@ void d2n_msg_free(AVSV_DND_MSG *msg)
> > >   /* free the message */
> > >   delete msg;
> > >  }
> > > +
> > > +/**
> > > + * Logs to saflog if active
> > > + * @param priority
> > > + * @param format
> > > + */
> > > +void amflog(int priority, const char *format, ...) {
> > > + if (avd_cb->avail_state_avd == SA_AMF_HA_ACTIVE) {
> > > +         va_list ap;
> > > +         char str[256];
> > > +
> > > +         va_start(ap, format);
> > > +         vsnprintf(str, sizeof(str), format, ap);
> > > +         va_end(ap);
> > > +         saflog(priority, amfSvcUsrName, "%s", str);
> > > + }
> > > +}
> >
> > ----------------------------------------------------------------------
> > -------- Learn Graph Databases - Download FREE O'Reilly Book "Graph
> > Databases" is the definitive new guide to graph databases and their
> > applications. Written by three acclaimed leaders in the field, this
> > first edition is now available. Download your free book today!
> > http://p.sf.net/sfu/13534_NeoTech
> > _______________________________________________
> > Opensaf-devel mailing list
> > Opensaf-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/opensaf-devel

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to