Hi Lennart, Thanks for you finding. In future I think we need to add more ntftest cases for using NTF API in threads concurrently. We had a few. I will publish next version patch
Thanks, Minh On 09/03/16 22:57, Lennart Lund wrote: > Hi Minh, > > I found a function checkNtfServerState() that read the global > ntfa_ntfsv_state variable. This function does not protect the variable with a > mutex. The corresponding ntfa_update_ntfsv_state() is called when mutex is > locked but the checkNtfServerState() is not, this is not thread safe. > > Regards > Lennart > >> -----Original Message----- >> From: minh chau [mailto:[email protected]] >> Sent: den 4 mars 2016 09:40 >> To: Lennart Lund; [email protected]; Vu Minh Nguyen >> Cc: [email protected] >> Subject: Re: [PATCH 0 of 5] Review Request for Add cloud resilience support >> [#1180] V2 >> >> 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. > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
