Ack,

Let's revisit this patch later like for eg:- making it configurable, etc. Iam 
waiting to understand the
big picture on how this feature has been coded in other services.

Cheers,
Mathi.

----- gary....@dektech.com.au wrote:

> HI Mathi
> 
> Any comments regarding this series? Can we push it?
> 
> Thanks
> Gary
> 
> > On 19 Jan 2016, at 11:29 PM, Anders Widell
> <anders.wid...@ericsson.com> wrote:
> > 
> > Ack for the series (code review only).
> > 
> > regards,
> > Anders Widell
> > 
> > On 01/07/2016 05:38 AM, Gary Lee wrote:
> >> Summary: clm: support simultaneous reboot of both controller nodes
> [#1646]
> >> Review request for Trac Ticket(s): 1646
> >> Peer Reviewer(s): Mathi, Anders W
> >> Pull request to:
> >> Affected branch(es): default
> >> Development branch: default
> >> 
> >> --------------------------------
> >> Impacted area       Impact y/n
> >> --------------------------------
> >>  Docs                    n
> >>  Build system            n
> >>  RPM/packaging           n
> >>  Configuration files     n
> >>  Startup scripts         n
> >>  SAF services            n
> >>  OpenSAF services        n
> >>  Core libraries          y
> >>  Samples                 n
> >>  Tests                   n
> >>  Other                   n
> >> 
> >> 
> >> Comments (indicate scope for each "y" above):
> >> ---------------------------------------------
> >> 
> >> changeset 44df7b651431e306911e9b327d182e86ce3022fb
> >> Author:    Gary Lee <gary....@dektech.com.au>
> >> Date:      Thu, 07 Jan 2016 15:27:12 +1100
> >> 
> >>    clma: send BAD_HANDLE to all clients if both controller nodes are
> >>    unavailable [#1646]
> >> 
> >>    If we detect clmd is unavailable on both controller nodes, then
> set
> >>    clma_cb.clms_reinit_required as true.
> >> 
> >>    Once an instance of clmd recovers, notify all clients that their
> CLM handle
> >>    is stale by returning BAD_HANDLE. Each client will need to call
> >>    saClmInitialize again.
> >> 
> >> changeset b61f9fb02800dc6878dfb41b7d24c8a798149ee0
> >> Author:    Gary Lee <gary....@dektech.com.au>
> >> Date:      Thu, 07 Jan 2016 15:27:20 +1100
> >> 
> >>    clm nodeagent: send nodeup again when clms is available after
> both
> >>    controllers are down [#1646]
> >> 
> >> 
> >> Complete diffstat:
> >> ------------------
> >>  osaf/libs/agents/saf/clma/clma.h         |   2 ++
> >>  osaf/libs/agents/saf/clma/clma_api.c     |  30
> +++++++++++++++++++++++-------
> >>  osaf/libs/agents/saf/clma/clma_mds.c     |  27
> ++++++++++++++++++++++++---
> >>  osaf/libs/agents/saf/clma/clma_util.c    |   2 ++
> >>  osaf/services/saf/clmsv/nodeagent/main.c |  10 ++++++++++
> >>  5 files changed, 61 insertions(+), 10 deletions(-)
> >> 
> >> 
> >> Testing Commands:
> >> -----------------
> >> Apply "headless feature" patches for other services
> >> Reboot both controllers
> >> 
> >> Testing, Expected Results:
> >> --------------------------
> >> Once a controller recovers, a CLM client should be signalled to
> call saClmDispatch() and
> >> receive SA_AIS_ERR_BAD_HANDLE
> >> 
> >> Conditions of Submission:
> >> -------------------------
> >>  <<HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC>>
> >> 
> >> 
> >> Arch      Built     Started    Linux distro
> >> -------------------------------------------
> >> mips        n          n
> >> mips64      n          n
> >> x86         n          n
> >> x86_64      y          y
> >> powerpc     n          n
> >> powerpc64   n          n
> >> 
> >> 
> >> Reviewer Checklist:
> >> -------------------
> >> [Submitters: make sure that your review doesn't trigger any
> checkmarks!]
> >> 
> >> 
> >> Your checkin has not passed review because (see checked entries):
> >> 
> >> ___ Your RR template is generally incomplete; it has too many blank
> entries
> >>     that need proper data filled in.
> >> 
> >> ___ You have failed to nominate the proper persons for review and
> push.
> >> 
> >> ___ Your patches do not have proper short+long header
> >> 
> >> ___ You have grammar/spelling in your header that is unacceptable.
> >> 
> >> ___ You have exceeded a sensible line length in your
> headers/comments/text.
> >> 
> >> ___ You have failed to put in a proper Trac Ticket # into your
> commits.
> >> 
> >> ___ You have incorrectly put/left internal data in your
> comments/files
> >>     (i.e. internal bug tracking tool IDs, product names etc)
> >> 
> >> ___ You have not given any evidence of testing beyond basic build
> tests.
> >>     Demonstrate some level of runtime or other sanity testing.
> >> 
> >> ___ You have ^M present in some of your files. These have to be
> removed.
> >> 
> >> ___ You have needlessly changed whitespace or added whitespace
> crimes
> >>     like trailing spaces, or spaces before tabs.
> >> 
> >> ___ You have mixed real technical changes with whitespace and
> other
> >>     cosmetic code cleanup changes. These have to be separate
> commits.
> >> 
> >> ___ You need to refactor your submission into logical chunks; there
> is
> >>     too much content into a single commit.
> >> 
> >> ___ You have extraneous garbage in your review (merge commits etc)
> >> 
> >> ___ You have giant attachments which should never have been sent;
> >>     Instead you should place your content in a public tree to be
> pulled.
> >> 
> >> ___ You have too many commits attached to an e-mail; resend as
> threaded
> >>     commits, or place in a public tree for a pull.
> >> 
> >> ___ You have resent this content multiple times without a clear
> indication
> >>     of what has changed between each re-send.
> >> 
> >> ___ You have failed to adequately and individually address all of
> the
> >>     comments and change requests that were proposed in the initial
> review.
> >> 
> >> ___ You have a misconfigured ~/.hgrc file (i.e. username, email
> etc)
> >> 
> >> ___ Your computer have a badly configured date and time; confusing
> the
> >>     the threaded patch review.
> >> 
> >> ___ Your changes affect IPC mechanism, and you don't present any
> results
> >>     for in-service upgradability test.
> >> 
> >> ___ Your changes affect user manual and documentation, your patch
> series
> >>     do not contain the patch that updates the Doxygen manual.
> >> 
> >

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