I agree/HansN -----Original Message----- From: Hans Feldt Sent: den 22 september 2014 09:16 To: Hans Nordebäck; praveen.malv...@oracle.com; nagendr...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: RE: [devel] [PATCH 1 of 1] amf : disable oper state of faulty comp if SU is disabled [#1035]
> -----Original Message----- > From: Hans Nordebäck > Sent: den 22 september 2014 08:58 > To: praveen.malv...@oracle.com; Hans Feldt; nagendr...@oracle.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: RE: [devel] [PATCH 1 of 1] amf : disable oper state of faulty > comp if SU is disabled [#1035] > > Ack, code review only, (one minor comment below) /Regards HansN > > -----Original Message----- > From: praveen.malv...@oracle.com [mailto:praveen.malv...@oracle.com] > Sent: den 5 september 2014 07:00 > To: Hans Feldt; nagendr...@oracle.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: [devel] [PATCH 1 of 1] amf : disable oper state of faulty > comp if SU is disabled [#1035] > > osaf/services/saf/amf/amfd/ndproc.cc | 10 ++++++++++ > osaf/services/saf/amf/amfnd/clc.cc | 12 ++++++++---- > osaf/services/saf/amf/amfnd/susm.cc | 14 ++++++++++++++ > 3 files changed, 32 insertions(+), 4 deletions(-) > > > Operational state of faulty component is not disabled when SU gets disabled > after termination failure. > > AMFND is not disabling the oper state of component when its > instantiation and clean up fails. AMFND marks both compoent and its SU > TERM_FAILED but disables only SU. > > Patch updates the component oper state at AMFND and same update is > sent to AMFD. Also it replies for the pending operation > > diff --git a/osaf/services/saf/amf/amfd/ndproc.cc > b/osaf/services/saf/amf/amfd/ndproc.cc > --- a/osaf/services/saf/amf/amfd/ndproc.cc > +++ b/osaf/services/saf/amf/amfd/ndproc.cc > @@ -347,6 +347,16 @@ static void su_admin_op_report_to_imm(AV > su->name.value); > } > break; > + case SA_AMF_ADMIN_UNLOCK: > + if ((su->saAmfSUPreInstantiable == false) && (pres == > SA_AMF_PRESENCE_TERMINATION_FAILED)) { > + report_admin_op_error(cb->immOiHandle, > + su->pend_cbk.invocation, > + SA_AIS_ERR_REPAIR_PENDING, > + &su->pend_cbk, > + "SU '%s' moved to 'termination failed' > state", > + su->name.value); > + } > + break; > case SA_AMF_ADMIN_REPAIRED: > if (pres == SA_AMF_PRESENCE_UNINSTANTIATED) { > avd_saImmOiAdminOperationResult(cb->immOiHandle, > diff --git a/osaf/services/saf/amf/amfnd/clc.cc > b/osaf/services/saf/amf/amfnd/clc.cc > --- a/osaf/services/saf/amf/amfnd/clc.cc > +++ b/osaf/services/saf/amf/amfnd/clc.cc > @@ -963,8 +963,10 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_ > clear_error_report_alarm(comp); > } > > - /* instantiating -> inst-failed */ > - if ((SA_AMF_PRESENCE_INSTANTIATING == prv_st) && > (SA_AMF_PRESENCE_INSTANTIATION_FAILED == final_st)) { > + /* instantiating -> inst-failed/term-failed */ > + if ((SA_AMF_PRESENCE_INSTANTIATING == prv_st) && > + ((final_st == > SA_AMF_PRESENCE_INSTANTIATION_FAILED) || > + (final_st == > SA_AMF_PRESENCE_TERMINATION_FAILED))) { > /* instantiation failed.. log it */ > m_AVND_COMP_OPER_STATE_SET(comp, > SA_AMF_OPERATIONAL_DISABLED); > m_AVND_COMP_OPER_STATE_AVD_SYNC(cb, comp, rc); @@ > -1181,8 +1183,10 > @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_ > } > } > > - /* Instantiating -> Instantiationfailed */ > - if ((SA_AMF_PRESENCE_INSTANTIATING == prv_st) && > (SA_AMF_PRESENCE_INSTANTIATION_FAILED == final_st)) { > + /* Instantiating -> Instantiationfailed/Terminationfailed */ > + if ((SA_AMF_PRESENCE_INSTANTIATING == prv_st) && > + ((final_st == > SA_AMF_PRESENCE_INSTANTIATION_FAILED) || > + (final_st == > SA_AMF_PRESENCE_TERMINATION_FAILED))) { > m_AVND_COMP_FAILED_SET(comp); > m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, > AVND_CKPT_COMP_FLAG_CHANGE); > m_AVND_COMP_OPER_STATE_SET(comp, > SA_AMF_OPERATIONAL_DISABLED); > diff --git a/osaf/services/saf/amf/amfnd/susm.cc > b/osaf/services/saf/amf/amfnd/susm.cc > --- a/osaf/services/saf/amf/amfnd/susm.cc > +++ b/osaf/services/saf/amf/amfnd/susm.cc > @@ -1759,6 +1759,20 @@ uint32_t avnd_su_pres_st_chng_prc(AVND_C > /* si assignment/removal failed.. inform AvD */ > rc = avnd_di_susi_resp_send(cb, su, > m_AVND_SU_IS_ALL_SI(su) ? 0 : si); > } > + > + /* instantiating -> term-failed */ > [HansN] the if expression "reversed" compared to previous if stmts. > + if ((prv_st == SA_AMF_PRESENCE_INSTANTIATING) && > + (final_st == > SA_AMF_PRESENCE_TERMINATION_FAILED)) { [HansF] This is the preferred way (to me) to write. The other way around is not readable. I know it's some old recommendation to catch assignments instead of comparison but now we have good tools to catch this instead. So let's write readable and understandable code instead. > + > + m_AVND_SU_OPER_STATE_SET(su, > SA_AMF_OPERATIONAL_DISABLED); > + m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, > AVND_CKPT_SU_OPER_STATE); > + > + /* Don't send su-oper state msg, just update su oper > state > + * AMF has lost control over this component and the > operator needs > + * to repair this node. Failover is not possible in > this state. > + */ > + avnd_di_uns32_upd_send(AVSV_SA_AMF_SU, > saAmfSUOperState_ID, &su->name, su->oper); > + } > } > > done: > > ---------------------------------------------------------------------- > -------- > Slashdot TV. > Video for Nerds. Stuff that matters. > http://tv.slashdot.org/ > _______________________________________________ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
