Hi Neelakanta, Find my comments inline
-----Original Message----- From: Neelakanta Reddy [mailto:[email protected]] Sent: Friday, April 01, 2016 12:35 PM To: Zoran Milinkovic Cc: [email protected]; [email protected] Subject: Re: [PATCH 0 of 3] Review Request for imm: support for transactional safe read [#48] Hi Zoran/AndersBj, Reviewed and tested the patch. 1. @@ -5765,6 +5768,9 @@ static SaAisErrorT accessor_get_common(S SaUint32T timeout; TRACE_ENTER(); + if(ccbId) { + TRACE_2("This is a SAFE read, ccbId:%u", ccbId); + } API version has to be checked if ccbId is given [Zoran] I will add the check when I push the code. 2. why can not IMMND_EVT_A2ND_OBJ_SAFE_READ can be sent as fevs from IMMND in case of INTERRUPT rather than sending them back from agent. [Zoran] This is another way to implement safe read, and can be considered as a small improvement. Also the improvement will include more complexity (continuations, ...) I guess that this implementation comes when Anders had another message for locking the object, which he later removed and the same message is sent over fevs for locking the object. Thanks, Zoran /Neel. On Thursday 31 March 2016 01:26 PM, Zoran Milinkovic wrote: > Summary: imm: support for transactional safe read [#48] Review request > for Trac Ticket(s): 48 Peer Reviewer(s): Hung, Neelakanta Pull request > to: Zoran Affected branch(es): default(5.0) Development branch: > default(5.0) > > -------------------------------- > 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): > --------------------------------------------- > > changeset 339c2755f9cf83bdaee6689c6c129982c628e8e6 > Author: Zoran Milinkovic <[email protected]> > Date: Thu, 31 Mar 2016 09:49:45 +0200 > > imm: add support to the library for transactional safe read [#48] > > The patch contains code for the library part of transactional safe > read > > changeset f88495b085e13d84baf02dff770c819397686334 > Author: Zoran Milinkovic <[email protected]> > Date: Thu, 31 Mar 2016 09:50:34 +0200 > > imm: add support to IMM service for transactional safe read [#48] > > The patch contains code for IMM service part of transactional safe > read > > changeset 4c5162eb5f51e2efb80e664763376225250a27ff > Author: Zoran Milinkovic <[email protected]> > Date: Thu, 31 Mar 2016 09:51:35 +0200 > > imm: add tests for transactional safe read [#48] > > New tests for transactional safe read > > > Complete diffstat: > ------------------ > osaf/libs/agents/saf/imma/imma_cb.h | 1 + > osaf/libs/agents/saf/imma/imma_db.c | 15 ++- > osaf/libs/agents/saf/imma/imma_oi_api.c | 2 +- > osaf/libs/agents/saf/imma/imma_om_api.c | 422 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > osaf/libs/agents/saf/imma/imma_proc.c | 38 +++++++++- > osaf/libs/common/immsv/immsv_evt.c | 24 +++++- > osaf/libs/common/immsv/include/immsv_evt.h | 2 + > osaf/libs/common/immsv/include/immsv_evt_model.h | 1 + > osaf/libs/saf/include/saImmOm_A_2_17.h | 6 + > osaf/services/saf/immsv/README | 44 > +++++++++++ > osaf/services/saf/immsv/immnd/ImmModel.cc | 631 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------- > osaf/services/saf/immsv/immnd/ImmModel.hh | 10 ++- > osaf/services/saf/immsv/immnd/immnd_evt.c | 162 > +++++++++++++++++++++++++++++++++++++++++- > osaf/services/saf/immsv/immnd/immnd_init.h | 3 + > tests/immsv/common/immtest.c | 2 + > tests/immsv/common/immtest.h | 1 + > tests/immsv/implementer/test_saImmOiAugmentCcbInitialize.c | 63 > +++++++++++++++- > tests/immsv/management/test_saImmOmCcbInitialize.c | 7 + > tests/immsv/management/test_saImmOmCcbObjectModify_2.c | 126 > ++++++++++++++++++++++++++++----- > 19 files changed, 1459 insertions(+), 101 deletions(-) > > > Testing Commands: > ----------------- > immomtest > immoitest > > > Testing, Expected Results: > -------------------------- > All tests must pass > > > Conditions of Submission: > ------------------------- > Ack from Neelakanta and Hung > > > 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. > ------------------------------------------------------------------------------ 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=278785471&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
