Hi Hung,

Reviewed the patches,

Apart from AndersBj comments, following are the comments for patch 3:

Why is the API version read from IMM server.

  API versions should be set  similarly how it is done for previous IMMA 
versions.

/Neel.


On Thursday 16 April 2015 12:50 PM, Hung Nguyen wrote:
> Summary: imm: remove all use of SaNameT from IMM server side and messages 
> [#969]
> Review request for Trac Ticket(s): 969
> Peer Reviewer(s): AndersBj, Neelakanta, Zoran
> Pull request to:
> Affected branch(es): default(4.7)
> Development branch: default(4.7)
>
> --------------------------------
> Impacted area       Impact y/n
> --------------------------------
>   Docs                    n
>   Build system            n
>   RPM/packaging           n
>   Configuration files     n
>   Startup scripts         n
>   SAF services            n
>   OpenSAF services        y
>   Core libraries          n
>   Samples                 n
>   Tests                   n
>   Other                   n
>
>
> Comments (indicate scope for each "y" above):
> ---------------------------------------------
>
> changeset f71cf86bfefaea3ab1d2cc15895a9fca4e906f31
> Author:       Hung Nguyen <[email protected]>
> Date: Mon, 13 Apr 2015 13:23:45 +0700
>
>       imm: Remove SaNameT from IMMND event handling routines [#969]
>
>       Replace SaNameT local variables with char*.
>
> changeset 26a8cbdd75fb4d8bcdc52cc2da25c6d3ae9250ed
> Author:       Hung Nguyen <[email protected]>
> Date: Mon, 13 Apr 2015 14:06:44 +0700
>
>       imm: Add OPENSAF_IMM_FLAG_PRT47_ALLOW value for noStdFlags [#969]
>
>       Add OPENSAF_IMM_FLAG_PRT47_ALLOW value for noStdFlags.
>
> changeset 70b2f04d94cb82b6ef38149043243d980400a94c
> Author:       Hung Nguyen <[email protected]>
> Date: Mon, 13 Apr 2015 18:48:16 +0700
>
>       imm: Check if server side supports protocol47 at handle initialization
>       [#969]
>
>       Add new member 'version' to IMMSV_ND2A_INIT_RSP. When protocol47 is
>       supported, it is set to 'A.2.16'. Otherwise, it is set to '_.0.0'.
>
>       Add new member 'isProto47' to IMMA_CLIENT_NODE. If protocol47 is 
> supported
>       at handle initialization, 'isProto47' is set to true.
>
>       The clients initialized before protocol47 will never use the new 
> protocol47
>       messages.
>
> changeset 9305a943af095551557b918545019022c85a9d21
> Author:       Hung Nguyen <[email protected]>
> Date: Tue, 14 Apr 2015 17:11:44 +0700
>
>       imm: Replace use of SaNameT from IMMSV_OM_ADMIN_OWNER_INITIALIZE [#969]
>
>       Define new message types: IMMD_EVT_ND2D_ADMINIT_REQ_2 and
>       IMMND_EVT_D2ND_ADMINIT_2. IMMND_EVT_A2ND_IMM_ADMINIT messages are only 
> sent
>       in scope of a node so no need to define a new message type.
>
>       Add new member 'stringAdminOwnerName' to 
> IMMSV_OM_ADMIN_OWNER_INITIALIZE.
>       'stringAdminOwnerName' is used in IMMND_EVT_A2ND_IMM_ADMINIT and new 
> message
>       types.
>
>       IMMND will use IMMD_EVT_ND2D_ADMINIT_REQ_2 messages if protocol47 is 
> enabled
>       and IMMD_EVT_ND2D_ADMINIT_REQ messages if protocol47 is disabled.
>
>       When receiving an IMMD_EVT_ND2D_ADMINIT_REQ message, IMMD will 
> broadcast an
>       IMMND_EVT_D2ND_ADMINIT message. When receiving an
>       IMMD_EVT_ND2D_ADMINIT_REQ_2 message, IMMD will broadcast an
>       IMMND_EVT_D2ND_ADMINIT_2 message.
>
>       ImmModel now has to handle both old and new messages. The message type 
> is
>       detected by checking if 'name' member of 
> IMMSV_OM_ADMIN_OWNER_INITIALIZE is
>       empty or not.
>
> changeset 82ed5258c4ca9a1810a1915cf2af7771a5578759
> Author:       Hung Nguyen <[email protected]>
> Date: Wed, 15 Apr 2015 11:17:32 +0700
>
>       imm: Replace use of SaNameT from IMMSV_OI_CCB_UPCALL_RSP [#969]
>
>       Define new message types: IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_3,
>       IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_4 and 
> IMMND_EVT_A2ND_OI_CCB_AUG_INIT_2.
>
>       Add new member 'stringName' to IMMSV_OI_CCB_UPCALL_RSP. 'stringName' is 
> used
>       in new message types.
>
>       Clients will use new messages when 'isProto47' is true. Otherwise, old
>       messages are used.
>
>       ImmModel now has to handle both old and new messages. The message type 
> is
>       detected by checking if 'name' member of IMMSV_OI_CCB_UPCALL_RSP is 
> empty or
>       not.
>
>
> Complete diffstat:
> ------------------
>   osaf/libs/agents/saf/imma/imma_cb.h              |    1 +
>   osaf/libs/agents/saf/imma/imma_oi_api.c          |   16 ++++-
>   osaf/libs/agents/saf/imma/imma_om_api.c          |   14 +++-
>   osaf/libs/agents/saf/imma/imma_proc.c            |   16 ++++-
>   osaf/libs/common/immsv/immsv_evt.c               |  336 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------
>   osaf/libs/common/immsv/include/immsv_api.h       |    1 +
>   osaf/libs/common/immsv/include/immsv_evt.h       |    8 ++
>   osaf/libs/common/immsv/include/immsv_evt_model.h |    2 +
>   osaf/services/saf/immsv/immd/immd_evt.c          |   19 +++++-
>   osaf/services/saf/immsv/immnd/ImmModel.cc        |   85 
> ++++++++++++++++++++++++----
>   osaf/services/saf/immsv/immnd/ImmModel.hh        |    3 +-
>   osaf/services/saf/immsv/immnd/immnd_evt.c        |   77 
> +++++++++++++++++++++-----
>   osaf/services/saf/immsv/immnd/immnd_init.h       |    7 +-
>   13 files changed, 418 insertions(+), 167 deletions(-)
>
>
> Testing Commands:
> -----------------
>
>
> Testing, Expected Results:
> --------------------------
>
>
> Conditions of Submission:
> -------------------------
> Ack from reviewers
>
>
> Arch      Built     Started    Linux distro
> -------------------------------------------
> mips        n          n
> mips64      n          n
> x86         n          n
> x86_64      n          n
> 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.
>
>
> ------------------------------------------------------------------------------
> BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
> Develop your own process in accordance with the BPMN 2 standard
> Learn Process modeling best practices with Bonita BPM through live exercises
> http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
> source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
> _______________________________________________
> Opensaf-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to