Hi zoran,
In future there may be chance of calling accessor_get_common(), with ccb
id, may be for other (usage).
Its good to have a redundant check there also as it is introduced in A.2.17.
/Neel.
On Friday 01 April 2016 05:13 PM, Zoran Milinkovic wrote:
> Hi Neelakanta,
>
> The version check is already done in saImmOmCcbObjectRead()
> if(!cl_node->isImmA2x11) {
> rc = SA_AIS_ERR_VERSION;
> TRACE_2("ERR_VERSION: saImmOmCcbObjectRead only supported for "
> "A.02.17 and above");
> goto done;
> }
>
> ccbId in accessor_get_common() can only be non-zero if it's called from
> saImmOmCcbObjectRead().
> So, no need for additional version check in accessor_get_common().
>
> Thanks,
> Zoran
>
> -----Original Message-----
> From: Zoran Milinkovic [mailto:[email protected]]
> Sent: Friday, April 01, 2016 1:35 PM
> To: Neelakanta Reddy
> Cc: [email protected]; [email protected]
> Subject: Re: [devel] [PATCH 0 of 3] Review Request for imm: support for
> transactional safe read [#48]
>
> 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
------------------------------------------------------------------------------
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
