Hi zoran,
we are already checking in accessor_get_common:
if(bUseString && !cl_node->isImmA2f) {
rc = SA_AIS_ERR_VERSION;
TRACE_2("ERR_VERSION: saImmOmAccessorGet_o3 only
supported for "
"A.02.15 and above");
goto release_lock;
}
It is good to have a check.
/Neel.
On Friday 01 April 2016 05:20 PM, Zoran Milinkovic wrote:
> Hi,
>
> accessor_get_common() is a common function for handling accessor functions
> and safe read.
> The version check should be done in main functions, not in the common
> function.
>
> Thanks,
> Zoran
>
> -----Original Message-----
> From: Neelakanta Reddy [mailto:[email protected]]
> Sent: Friday, April 01, 2016 1:49 PM
> To: Zoran Milinkovic
> Cc: [email protected]; [email protected]
> Subject: Re: [devel] [PATCH 0 of 3] Review Request for imm: support for
> transactional safe read [#48]
>
> 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
