Hi Thanks for doing an early review.
I have started to create a test application for this IMM object handler. It will be located in the same directory as the demo applications and being built the same way. The test application will make it possible to test this new handler independently of SMF. I have found some problems that I am fixing. The most seriously is that in the lower layer API (imm_om_api) handles are given when the objects are created and cannot be restored without creating a new object. This means that recovery if for example BAD_HANDLE does not work. It is also means that a new ModelModification object has to be created for each CCB. Also, see below for some comments/answers regarding the points below. Thanks Lennart > -----Original Message----- > From: Syam Prasad Talluri [mailto:syam.tall...@oracle.com] > Sent: den 18 januari 2018 09:30 > To: Lennart Lund <lennart.l...@ericsson.com>; Vijay Roy > <vijay....@oracle.com> > Cc: opensaf-devel@lists.sourceforge.net > Subject: RE: [devel] [PATCH 0/5] Review Request for smf: Add capability to > redo CCBs that fail [#1398] > > Hi Lennart, > > I understand you are planning to send some more patches. Here are my > initial observations > > 1. In om_handle.cc we are reading the env variable > IMMOMCPP_TRACE_PATHNAME, we need to add about this in the ReadMe > file or remove this env [Lennart] Good finding, this has to be removed > 2. Like immccb operations do we need to implement wrappers for > saImmOmAccessorGet_2, saImmOmAdminOperationInvoke_2 which are > used by SMF. [Lennart] I agree, since we have the same problem with recovery handling etc. also here. However even if handlers for these OM APIs should be easier to implement and some of the code created for CCB handling is common it is projects that probably has to be handled one by one. > 3. Wrappers are added for IMM OM, do we need to have same kind of > wrapper for IMM OI implementation as well ? [Lennart] Yes, that would be really good to do but it will be more complicated and a bigger "project" than OM handling. In the log service there is a handler for the configuration object that is based on some ideas that can be used for an OI handler > 4. In om_handle.h is there any special reason to keep the below functions > void SetDummy(int i) { dummy_ = 1; } > int GetDummy(void) { return dummy_; } [Lennart] No, I have removed them > 5. There are TODO's in some files, I hope you will address them before final > commit. [Lennart] Yes. There is also a lot of LLDTEST tagged logs, traces and even code that also will be handled > 6. Looks like Wrappers implemented are generic Hence we can use these in > other services also and these can be moved to a common place going > forward. [Lennart] Yes, it is generic but let's use it with SMF first. If it works well it can be moved to a generic place and become library code. This also requires that the lower layer API wrappers are made generic library code. > > We will try to integrate the existing SMF OM Interface with these Wrappers > . > > > Thanks, > Syam. > -----Original Message----- > From: Lennart Lund [mailto:lennart.l...@ericsson.com] > Sent: Tuesday, January 16, 2018 8:11 PM > To: Vijay Roy <vijay....@oracle.com> > Cc: opensaf-devel@lists.sourceforge.net > Subject: [devel] [PATCH 0/5] Review Request for smf: Add capability to redo > CCBs that fail [#1398] > > Summary: smf: Add capability to redo CCBs that fail [#1398] Review request > for Ticket(s): 1398 Peer Reviewer(s): *** LIST THE TECH REVIEWER(S) / > MAINTAINER(S) HERE *** Pull request to: *** LIST THE PERSON WITH PUSH > ACCESS HERE *** Affected branch(es): develop Development branch: ticket- > 1398 Base revision: d082d2fb2b26437bbe6860e8efff8479748378c2 > Personal repository: git://git.code.sf.net/u/elunlen/review > > -------------------------------- > 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 > > NOTE: Patch(es) contain lines longer than 80 characers > > Comments (indicate scope for each "y" above): > --------------------------------------------- > Note: > Logging and tracing is still tagged LLDTEST. This will be removed For testing > use the demo programs as before > > revision 011fc764956ff8892f577b44a7174384c85cc878 > Author: Lennart Lund <lennart.l...@ericsson.com> > Date: Tue, 16 Jan 2018 15:28:10 +0100 > > smf: Add capability to redo CCBs that fail [#1398] > > Object modify is added > ccbdemo_modify updated with demo code for object modify > > Also some cleanup and bug fixes > > > > revision 5c2931adb8b8f5d030b869a48e0fcb91ceeda3ac > Author: Lennart Lund <lennart.l...@ericsson.com> > Date: Tue, 16 Jan 2018 15:14:42 +0100 > > smf: Add capability to redo CCBs that fail [#1398] > > Object delete is added > The ccbdemo1 command has changed name to ccbdemo_create A new > command ccbdemo_delete is added A new command ccbdemo_modify is > also added but is still dummy. This is in preparation for adding object modify > > Also some cleanup and some bug fixes > > > > revision c546f10cfe5344d071d4664099a62ab7f1f44209 > Author: Lennart Lund <lennart.l...@ericsson.com> > Date: Tue, 16 Jan 2018 15:14:42 +0100 > > smf: Add capability to redo CCBs that fail [#1398] > > All types can be used to set attributes at create time (SaAnyT and SaNameT > added) Help functions for converting SaAnyT and SaNameT to string is added > See SaAnytToString() and SaNametToString() in immccb.h > > > > revision 9d9063fc7e8699b20f88e0d26b8b121d83547b36 > Author: Lennart Lund <lennart.l...@ericsson.com> > Date: Tue, 16 Jan 2018 15:14:42 +0100 > > smf: Add capability to redo CCBs that fail [#1398] > > Added all numeric types including SaTimeT Still not implemented are > SaNameT and SaAnyT > > > > revision 8079298e0d1d7d9a3d69baa4d1f23187bec4ba69 > Author: Lennart Lund <lennart.l...@ericsson.com> > Date: Tue, 16 Jan 2018 15:14:42 +0100 > > smf: Add capability to redo CCBs that fail [#1398] > > Create a module for handling IMM CCB that implements all steps involved > and all rules regaring possible recovery and failing Replace current CCB > handling in SMF with useage of this module > > NOTE: This is an early version that is not well tested and everything is not > yet implemented. Also some refactoring can be done (redundency). In > some > places LLDTEST tagged traces, logs and printouts exists that will be > removed > or modified in a final version > - Only SaUint32T, SaInt32T and SaStringT is implemented. > Means that IMM type SA_IMM_ATTR_SAUINT32T, > SA_IMM_ATTR_SAINT32T and > SA_IMM_ATTR_SASTRINGT can be set as value type > - A program called ccbdemo1 is included that can be used as an example and > for some testing. This program is built and will be installed in an OpenSAF > uml cluster if --enable-tests is set in configure > - An IMM class definition, democlass.xml, for ccbdemo1 is provided. This > class > must be installed before ccbdemo1 can be executed > Example: # immcfg -f /hostfs/democlass.xml > > > > Added Files: > ------------ > src/smf/config/democlass.xml > src/smf/smfd/imm_modify_config/attribute.cc > src/smf/smfd/imm_modify_config/attribute.h > src/smf/smfd/imm_modify_config/creator.cc > src/smf/smfd/imm_modify_config/creator.h > src/smf/smfd/imm_modify_config/immccb.cc > src/smf/smfd/imm_modify_config/immccb.h > src/smf/smfd/imm_modify_config/README > src/smf/smfd/imm_modify_demo/ccbdemo1.cc > src/smf/smfd/imm_modify_demo/ccbdemo_delete.cc > src/smf/smfd/imm_modify_demo/ccbdemo_modify.cc > src/smf/smfd/imm_modify_demo/democlass.xml > src/smf/smfd/imm_modify_demo/Makefile > src/smf/smfd/imm_om_api/common/common.cc > src/smf/smfd/imm_om_api/common/common.h > src/smf/smfd/imm_om_api/common/imm_attribute.cc > src/smf/smfd/imm_om_api/common/imm_attribute.h > src/smf/smfd/imm_om_api/om_admin_owner_clear.cc > src/smf/smfd/imm_om_api/om_admin_owner_clear.h > src/smf/smfd/imm_om_api/om_admin_owner_handle.cc > src/smf/smfd/imm_om_api/om_admin_owner_handle.h > src/smf/smfd/imm_om_api/om_admin_owner_set.cc > src/smf/smfd/imm_om_api/om_admin_owner_set.h > src/smf/smfd/imm_om_api/om_ccb_handle.cc > src/smf/smfd/imm_om_api/om_ccb_handle.h > src/smf/smfd/imm_om_api/om_ccb_object_create.cc > src/smf/smfd/imm_om_api/om_ccb_object_create.h > src/smf/smfd/imm_om_api/om_ccb_object_delete.cc > src/smf/smfd/imm_om_api/om_ccb_object_delete.h > src/smf/smfd/imm_om_api/om_ccb_object_modify.cc > src/smf/smfd/imm_om_api/om_ccb_object_modify.h > src/smf/smfd/imm_om_api/om_handle.cc > src/smf/smfd/imm_om_api/om_handle.h > > > Complete diffstat: > ------------------ > src/smf/Makefile.am | 156 ++++- > src/smf/config/democlass.xml | 76 +++ > src/smf/smfd/SmfImmOperation.cc | 2 +- > src/smf/smfd/imm_modify_config/README | 129 ++++ > .../smfd/imm_modify_config/add_operation_to_ccb.cc | 262 +++++++++ > .../smfd/imm_modify_config/add_operation_to_ccb.h | 73 +++ > src/smf/smfd/imm_modify_config/attribute.cc | 631 > ++++++++++++++++++++ > src/smf/smfd/imm_modify_config/attribute.h | 277 +++++++++ > src/smf/smfd/imm_modify_config/immccb.cc | 649 > +++++++++++++++++++++ > src/smf/smfd/imm_modify_config/immccb.h | 438 ++++++++++++++ > src/smf/smfd/imm_modify_demo/Makefile | 19 + > src/smf/smfd/imm_modify_demo/ccbdemo_create.cc | 438 > ++++++++++++++ > src/smf/smfd/imm_modify_demo/ccbdemo_delete.cc | 173 ++++++ > src/smf/smfd/imm_modify_demo/ccbdemo_modify.cc | 274 +++++++++ > src/smf/smfd/imm_modify_demo/democlass.xml | 100 ++++ > src/smf/smfd/imm_om_api/common/common.cc | 56 ++ > src/smf/smfd/imm_om_api/common/common.h | 196 +++++++ > src/smf/smfd/imm_om_api/common/imm_attribute.cc | 115 ++++ > src/smf/smfd/imm_om_api/common/imm_attribute.h | 238 ++++++++ > src/smf/smfd/imm_om_api/om_admin_owner_clear.cc | 80 +++ > src/smf/smfd/imm_om_api/om_admin_owner_clear.h | 71 +++ > src/smf/smfd/imm_om_api/om_admin_owner_handle.cc | 88 +++ > src/smf/smfd/imm_om_api/om_admin_owner_handle.h | 94 +++ > src/smf/smfd/imm_om_api/om_admin_owner_set.cc | 122 ++++ > src/smf/smfd/imm_om_api/om_admin_owner_set.h | 91 +++ > src/smf/smfd/imm_om_api/om_ccb_handle.cc | 104 ++++ > src/smf/smfd/imm_om_api/om_ccb_handle.h | 82 +++ > src/smf/smfd/imm_om_api/om_ccb_object_create.cc | 85 +++ > src/smf/smfd/imm_om_api/om_ccb_object_create.h | 167 ++++++ > src/smf/smfd/imm_om_api/om_ccb_object_delete.cc | 60 ++ > src/smf/smfd/imm_om_api/om_ccb_object_delete.h | 74 +++ > src/smf/smfd/imm_om_api/om_ccb_object_modify.cc | 87 +++ > src/smf/smfd/imm_om_api/om_ccb_object_modify.h | 187 ++++++ > src/smf/smfd/imm_om_api/om_handle.cc | 151 +++++ > src/smf/smfd/imm_om_api/om_handle.h | 103 ++++ > 35 files changed, 5946 insertions(+), 2 deletions(-) > > > Testing Commands: > ----------------- > *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** > > > Testing, Expected Results: > -------------------------- > *** PASTE COMMAND OUTPUTS / TEST RESULTS *** > > > Conditions of Submission: > ------------------------- > *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** > > > 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 ~/.gitconfig file (i.e. user.name, user.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. > > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most engaging > tech sites, Slashdot.org! https://urldefense.proofpoint.com/v2/url?u=http- > 3A__sdm.link_slashdot&d=DwICAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIr > MUB65eapI_JnE&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO- > XRrmj38iQ&m=Ttbahyv5pgGAu1lhiV- > 0qp8ECEeN4OE3SG2vG8F3RT8&s=hwAVy244CDHi9XpvREj- > PropX6JtfS1odYCuGvqoNP0&e= > _______________________________________________ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__lists.sourceforge.net_lists_listinfo_opensaf- > 2Ddevel&d=DwICAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_Jn > E&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO- > XRrmj38iQ&m=Ttbahyv5pgGAu1lhiV- > 0qp8ECEeN4OE3SG2vG8F3RT8&s=Ih4I8JMs8Abr6sB3Ji0QNY2R0sRUfkEC8Yisk > ObVhno&e= ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
