Hi I have now sent a patch that concludes this series of increments for creating the new CCB handler. I have created a test program (ccbhdl_test) that test many aspects of the handler including all possible modification types that can be added to a CCB, all attribute types that can be used with create and modify. It also test usage of a modification handler object in different scopes including using it several times without going out of scope. I have used Valgrind with this test program to test for memory leeks, important since a lot of memory allocation is done in the CCB handler.
I have not yet tested very much recovery handling. To do this the code has to be "instrumented" with extra test code that simulates IMM error return codes, finalize handles to test BAD HANDLE etc. I will do some more of that kind of testing. Next step is to concatenate all the increments into one commit and send it for an "official" review. When this is acked I plan to push without changing anything in SMF. After that SMF can be refactored using this handler instead of inline spot fixes all over the place. 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 > 2. Like immccb operations do we need to implement wrappers for > saImmOmAccessorGet_2, saImmOmAdminOperationInvoke_2 which are > used by SMF. > 3. Wrappers are added for IMM OM, do we need to have same kind of > wrapper for IMM OI implementation as well ? > 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_; } > 5. There are TODO's in some files, I hope you will address them before final > commit. > 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. > > 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
