Hi Lennart,

The important change I think should be looked at, is a bug Praveen has 
found in Unsubscribed()/ReadFinalize(), which are not aligned with README.

Thanks,
Minh

On 03/03/16 23:58, Lennart Lund wrote:
> Hi Minh,
>
> Ack.
>
> I have not done a very deep analyze of this I assume it's the same code that 
> has already been tested for some time and that there are no significant 
> changes (and that I have reviewed once). Please tell me if there are any 
> changes that you think I should take a closer look at.
> I have applied all the patches, built and run the legacy tests in the 
> non-resilience configuration and all tests PASS.
>
> Regards
> Lennart
>
>> -----Original Message-----
>> From: Minh Hon Chau [mailto:[email protected]]
>> Sent: den 1 mars 2016 08:30
>> To: Lennart Lund; [email protected]; Vu Minh Nguyen; Minh
>> Chau H
>> Cc: [email protected]
>> Subject: [PATCH 0 of 5] Review Request for Add cloud resilience support
>> [#1180] V2
>>
>> Summary: ntf: Add cloud resilience support [#1180] V2
>> Review request for Trac Ticket(s): 1180
>> Peer Reviewer(s): Lennart, Praveen, Vu
>> Pull request to: NTF maintainers
>> 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            y
>>   OpenSAF services        n
>>   Core libraries          n
>>   Samples                 n
>>   Tests                   n
>>   Other                   n
>>
>>
>> Comments (indicate scope for each "y" above):
>> ---------------------------------------------
>> This V2 has revised comments:
>> - Update description of checkNtfServerState
>> - Not using conditional operator in ntfa_mds_svc_evt
>> - Update Unsubscribe() ReadFinalize() aligned with README
>> - Add lock/unlock ntfa_cb.cb_lock for client recovery
>> - Update ntftest options: -ve is for tag mode only, -vpe works
>>
>>
>> changeset 884d1bdbea715fbc81941a0941c2d3f799a4395e
>> Author:      Minh Hon Chau <[email protected]>
>> Date:        Tue, 01 Mar 2016 18:25:15 +1100
>>
>>      NTF: Add support cloud resilience for NTF libs common [#1180]
>>
>>      The patch contains support for cloud resilience feature in NTF
>> libs common
>>      which are mostly used in Agent code
>>
>> changeset 6d941afbcd475e1ecf58c6f9586e5ff60a7a3319
>> Author:      Minh Hon Chau <[email protected]>
>> Date:        Tue, 01 Mar 2016 18:26:53 +1100
>>
>>      NTF: Add support cloud resilience for NTF Agent [#1180] V2
>>
>>      The patch contains support for cloud resilience feature in NTF
>> Agent code.
>>      Please refer README.HYDRA for content of the changes
>>
>> changeset ddd2369c000c3648466c06b8babad4b5884a0058
>> Author:      Minh Hon Chau <[email protected]>
>> Date:        Tue, 01 Mar 2016 18:27:03 +1100
>>
>>      NTF: Add wrapper for usage of NTF API in ntftools to handle
>> TRY_AGAIN
>>      [#1180]
>>
>>      Since NTF support the SC outage which the NTF client has to
>> handle TRY_AGAIN
>>      return code, the patch adds wrapper for APIs being used in
>> ntftools that
>>      shall receives TRY_AGAIN when both SCs are down.
>>
>> changeset 67286bb9852bcfde837c009801826202f2905a5f
>> Author:      Minh Hon Chau <[email protected]>
>> Date:        Tue, 01 Mar 2016 18:27:09 +1100
>>
>>      NTF: Add new README file for description of cloud resilience
>> support [#1180]
>>      V2
>>
>>      Add description regarding general solution and API
>> implementation for cloud
>>      resilience support in NTF
>>
>> changeset ad9d91747c80faf1defd86a539f6238a997150b0
>> Author:      Minh Hon Chau <[email protected]>
>> Date:        Tue, 01 Mar 2016 18:27:18 +1100
>>
>>      NTF: Add tests for NTF cloud resilience feature [#1180] V2
>>
>>      The patch adds new test cases to ntftest for cloud resilience
>> feature.
>>
>>
>> Complete diffstat:
>> ------------------
>>   osaf/libs/agents/saf/ntfa/ntfa.h              |    31 +-
>>   osaf/libs/agents/saf/ntfa/ntfa_api.c          |   678
>> +++++++++++++++++++++++------
>>   osaf/libs/agents/saf/ntfa/ntfa_mds.c          |    14 +-
>>   osaf/libs/agents/saf/ntfa/ntfa_util.c         |   465 +++++++++++++++++++-
>>   osaf/libs/common/ntfsv/include/ntfsv_mem.h    |     7 +
>>   osaf/libs/common/ntfsv/include/ntfsv_msg.h    |     1 +
>>   osaf/libs/common/ntfsv/ntfsv_mem.c            |   159 ++++++
>>   osaf/services/saf/ntfsv/README.HYDRA          |   111 ++++
>>   osaf/tools/safntf/include/ntfclient.h         |    25 +
>>   osaf/tools/safntf/ntfread/ntfread.c           |    16 +-
>>   osaf/tools/safntf/ntfsend/ntfsend.c           |    24 +-
>>   osaf/tools/safntf/ntfsubscribe/ntfsubscribe.c |    22 +-
>>   osaf/tools/safntf/src/ntfclient.c             |   158 ++++++
>>   tests/ntfsv/Makefile.am                       |     4 +-
>>   tests/ntfsv/tet_ntf.h                         |     4 +-
>>   tests/ntfsv/tet_ntf_api_wrapper.c             |   438 +++++++++++++++++++
>>   tests/ntfsv/tet_ntf_common.c                  |    67 ++
>>   tests/ntfsv/tet_ntf_common.h                  |   187 ++++++++
>>   tests/ntfsv/tet_ntf_main.c                    |   154 ++++++-
>>   tests/ntfsv/tet_scOutage_reinitializeHandle.c |  1023
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   20 files changed, 3385 insertions(+), 203 deletions(-)
>>
>>
>> Testing Commands:
>> -----------------
>> Run all ntftest test cases
>>
>>
>> Testing, Expected Results:
>> --------------------------
>> All pass
>>
>>
>> Conditions of Submission:
>> -------------------------
>> ack from reviewers
>>
>>
>> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to