For each patch you should edit the opensaf immutils, then copy it over the 
samples immutils.
That way you will correct also the omission introduced earlier. 

Bottom line is that the two samples versions should be identical.
There could at some point be a reason for having some samples functionality that
only makes sense internally in OpenSAF (is useless for users) but I cant think 
of anything.
And unless it starts to become a lot of stuff, we will let it tag along to 
samples and the
users will just ignore it, or use it at their own peril. One rule is that 
immutil should only
Depend on the imm API. 

We want to keep things simple and simple means the two are the same.
The only reason we have two copies is that we dont want users to link to 
internal osaf
Libraries and immutils is not an officially supported API. Thus the samples copy
Is a take it or leave it piece of source that the user will build into *their* 
library.
And new versions of immutils are not aoutmatically copied to the users space.
The user takes full responsibility of versioning of their immutils.

/AndersBj

-----Original Message-----
From: Neelakanta Reddy [mailto:[email protected]] 
Sent: den 13 augusti 2014 10:54
To: Anders Björnerstedt
Cc: [email protected]
Subject: Re: [PATCH 0 of 1] Review Request for IMM: Dumpimg resource 
utilization data for IMMsv [#35] V2

Hi AndersBj,

The immutil is updated in #35 because a new utill API
immutil_saImmOiAdminOperationResult_o2 is added.
Which is used as a part of #35.

While pushing the patch, samples/immsv/immutils/ will be updated with new added 
API.

/Neel.

On Wednesday 13 August 2014 01:31 PM, Anders Bjornerstedt wrote:
> Hi Neel,
>
> I am reviewing #25 and #957 and I just have an initial comment about 
> immutils.
> Both patches update immutils (I actually dont remember the reason why
> #35 needed to update immutils but it
> must be due to reasons of convenience in the implementation of #35).
>
> The problem is that immutils.[h|c] should be identical between the 
> internal and the samples variant.
> The only reason they tend to diverge is that persons updating immutils 
> typically forget there is the samples version.
> Indeed,. in the current OpenSAF4.5 they differ even before you have 
> pushed either of these two fixes.
> But hat is an error and must be due to again someone omitting to 
> update the samples version.
> So please ensure that the two immutils variants are identical.
> Do this by copying the internal opensaf immutils OVER the samples 
> immutils.
>
> /AndersBj
>
>
> [email protected] wrote:
>> Summary:IMM: Dumpimg resource utilization data for IMMsv [#35] V2 
>> Review request for Trac Ticket(s): 35 Peer Reviewer(s): AndersBj, 
>> Zoran Pull request to: Affected branch(es): 4.5(default) Development 
>> branch: 4.5
>>
>> --------------------------------
>> 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 160f20828b44c16cc164356bec82c89463cc1bd9
>> Author:    Neelakanta Reddy<[email protected]>
>> Date:    Mon, 04 Aug 2014 19:12:54 +0530
>>
>>     IMM: Dumpimg resource utilization data for IMMsv [#35] V2 The 
>> enhancement
>>     dumps the IMMsv resource utilization data using IMM Admin 
>> Operation. The IMM
>>     AdminOperation must be directed to immsv object
>>     opensafImm=opensafImm,safApp=safImmService. PBE is implementer for
>>     opensafImm=opensafImm,safApp=safImmService object. The display admin
>>     operation request reaches PBE and PBE replys with extended 
>> adminoperation
>>     resut having adminownername and resource name as parameters.The 
>> enhancement
>>     Displays without 0PBE also. New immutil API
>>     immutil_saImmOiAdminOperationResult_o2 is added.
>>
>>     Presently two types of operationNames for dumping resource 
>> utilization is
>>     supported:
>>
>>     display -- returns the terse output for the requested resource. 
>> The number
>>     of requested resource will be returned. The supported resource are
>>     implementers, adminowners, ccbhandles and searchhandels.
>>
>>     Eg:
>>
>>     immadm -O display -p resource:SA_STRING_T:implementers
>>     opensafImm=opensafImm,safApp=safImmService
>>
>>     The output is the number of implementers present: Name Type Value(s)
>>     
>> =====================================================================
>> ===
>>
>>     count SA_INT64_T 9 (0x9)
>>
>>     displayverbose -- returns the verbose output for the requested 
>> resource. The
>>     verbose output is supported for the resources implementers and 
>> adminowners.
>>     If the number of resource is greater than 127 then the verbose 
>> output is
>>     displayed to syslog.
>>
>>     Eg: immadm -O displayverbose -p resource:SA_STRING_T:implementers
>>     opensafImm=opensafImm,safApp=safImmService [using admin-owner:
>>     'safImmService'] Object: 
>> opensafImm=opensafImm,safApp=safImmService Name
>>     Type Value(s)
>>     
>> =====================================================================
>> ===
>>
>>     safLogService SA_INT64_T 131343 (0x2010f) safClmService 
>> SA_INT64_T 131343
>>     (0x2010f) safAmfService SA_INT64_T 131343 (0x2010f)
>> MsgQueueService131343
>>     SA_INT64_T 131343 (0x2010f) safMsgGrpService SA_INT64_T 131343
>> (0x2010f)
>>     safCheckPointService SA_INT64_T 131343 (0x2010f) safEvtService 
>> SA_INT64_T
>>     131343 (0x2010f) safLckService SA_INT64_T 131343 (0x2010f) 
>> safSmfService
>>     SA_INT64_T 131343 (0x2010f)
>>
>>     If the operationName display-help is used, the return parameters 
>> will have
>>     presently supported resources for dumping resource utilization data.
>>
>>     Eg: immadm -O display-help
>> opensafImm=opensafImm,safApp=safImmService [using
>>     admin-owner: 'safImmService'] Object:
>>     opensafImm=opensafImm,safApp=safImmService Name Type Value(s)
>>     
>> =====================================================================
>> ===
>>
>>     supportedResources SA_STRING_T implementers supportedResources 
>> SA_STRING_T
>>     adminowners supportedResources SA_STRING_T ccbhandles 
>> supportedResources
>>     SA_STRING_T searchhandels
>>
>>     error-string is returned. if any of the dispaly operations fails.
>>
>>     immadm is modified for displaying the returned parameters when 
>> dumping of
>>     resource utilization is supported .
>>
>>
>> Complete diffstat:
>> ------------------
>>  osaf/libs/common/immsv/include/immutil.h         |    4 +
>>  osaf/services/saf/immsv/immnd/ImmModel.cc        |  281 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  osaf/services/saf/immsv/immnd/ImmModel.hh        |    8 +-
>>  osaf/services/saf/immsv/immnd/immnd_evt.c        |   57 +++++++++++++-
>>  osaf/services/saf/immsv/immnd/immnd_init.h       |    7 +-
>>  osaf/services/saf/immsv/immpbed/immpbe_daemon.cc |   57 +++++++++++++++
>>  osaf/tools/safimm/immadm/imm_admin.c             |   13 ++-
>>  osaf/tools/safimm/src/immutil.c                  |   17 ++++
>>  samples/immsv/immutils/immutil.c                 |   17 ++++
>>  samples/immsv/immutils/immutil.h                 |    4 +
>>  10 files changed, 447 insertions(+), 18 deletions(-)
>>
>>
>> Testing Commands:
>> -----------------
>> Test all the supported dumping operations on supported resources with 
>> PBE enable/disable.
>>
>> 1. display
>> 2. displayverbose
>> 3. dispaly-help
>>
>> Testing, Expected Results:
>> --------------------------
>> ALl supported dumping operations must dump the IMM resource 
>> utilization data.
>>
>> Conditions of Submission:
>> -------------------------
>> Ack from AndersBj
>>
>> 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.
>>
>


------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to