Hi Minh I have read the PR document and the README update and have no comments so you have my ACK
Thanks Lennart > -----Original Message----- > From: Minh Hon Chau [mailto:minh.c...@dektech.com.au] > Sent: den 12 februari 2018 00:03 > To: Lennart Lund <lennart.l...@ericsson.com>; > srinivas.mangip...@oracle.com; Canh Van Truong > <canh.v.tru...@dektech.com.au> > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 0/3] Review Request for ntf: Checkpoint and cold sync > reader information [#2757] > > Hi, > > A friendly reminder, I'm going to push the PR doc update as there is no > comment by today. > > Thanks, > > Minh > > > On 29/01/18 15:01, Minh Hon Chau wrote: > > Hi all, > > > > If the patch look ok to you, I would like to push it tomorrow. > > > > I have updated the README and PR doc at the below links, if you have > > time please have a look. > > > > > https://sourceforge.net/p/opensaf/tickets/_discuss/thread/77619155/0a9b/ > attachment/2757_README.diff > > > > > > > https://sourceforge.net/p/opensaf/tickets/_discuss/thread/5671255e/16a4/ > attachment/OpenSAF_NTFSv_PR_2735.odt > > > > > > Thanks, > > > > Minh > > > > > > On 24/01/18 12:49, minh.c...@dektech.com.au wrote: > >> Hi Lennart, > >> > >> I tested the APIs between versions with/without the changes. I will send > >> out for review the README and PR change after the code review is > >> done. One > >> limitation is that both active and standby require the patches to work. > >> > >> Thanks, > >> Minh > >> > >>> Hi Minh > >>> > >>> Ack. I have not tested much > >>> > >>> Have you tested using the reader API while running old version on > >>> standby > >>> and new version on active and vice versa (upgrade case)? Limitations? > >>> PR documentation update? > >>> > >>> Thanks > >>> Lennart > >>> > >>>> -----Original Message----- > >>>> From: Minh Hon Chau > >>>> Sent: den 22 januari 2018 05:19 > >>>> To: Lennart Lund <lennart.l...@ericsson.com>; > >>>> srinivas.mangip...@oracle.com; Canh Van Truong > >>>> <canh.v.tru...@dektech.com.au> > >>>> Cc: opensaf-devel@lists.sourceforge.net; Minh Hon Chau > >>>> <minh.c...@dektech.com.au> > >>>> Subject: [PATCH 0/3] Review Request for ntf: Checkpoint and cold sync > >>>> reader information [#2757] > >>>> > >>>> Summary: ntfd: Checkpoint reader to the standby when processes > reader > >>>> API requests [#2757] > >>>> Review request for Ticket(s): 2757 > >>>> Peer Reviewer(s): Lennart, Srinivas, Canh > >>>> Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** > >>>> Affected branch(es): develop > >>>> Development branch: ticket-2757 > >>>> Base revision: ee105cb3bf44eee4e8785e3de7d24f907641e2ab > >>>> Personal repository: git://git.code.sf.net/u/minh-chau/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): > >>>> --------------------------------------------- > >>>> *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** > >>>> > >>>> revision 74da3370accfa44a34a7abf9830ceaeae3ab5d4f > >>>> Author:Minh Chau <minh.c...@dektech.com.au> > >>>> Date:Mon, 22 Jan 2018 15:08:59 +1100 > >>>> > >>>> ntftest: Add new test cases of suite 41 for cold sync and > >>>> checkpoint of > >>>> reader > >>>> APIs [#2757] > >>>> > >>>> > >>>> > >>>> revision ad38745b1c411bc52905725281c84c69e4147fef > >>>> Author:Minh Chau <minh.c...@dektech.com.au> > >>>> Date:Mon, 22 Jan 2018 15:03:42 +1100 > >>>> > >>>> ntfd: Cold sync reader to the standby ntfd after rebooting the standby > >>>> controller [#2757] > >>>> > >>>> Assumpt that the reader information is updated to the standby ntfd via > >>>> checkpoint > >>>> upon reception of reader APIs requests. However, if the standby > >>>> controller > >>>> reboots > >>>> and comes up, the standby ntfd still has none of readers information > >>>> which is > >>>> available at the active ntfd. Now if a switchover happens, the new > >>>> active will > >>>> not > >>>> be able to process the reader APIs requests with existing reader > >>>> handles. > >>>> > >>>> This patch adds reader information as part of cold sync > >>>> > >>>> > >>>> > >>>> revision 47cf18850e6819c2db4642eb1e639aff5f0d8282 > >>>> Author:Minh Chau <minh.c...@dektech.com.au> > >>>> Date:Mon, 22 Jan 2018 14:12:00 +1100 > >>>> > >>>> ntfd: Checkpoint reader to the standby when processes reader API > >>>> requests > >>>> [#2757] > >>>> > >>>> When active ntfd receives reader API requests: ReaderIntialize, > >>>> ReadNext, > >>>> ReadFinalize, active ntfd does not update the readers information > >>>> to the > >>>> standby. Thus, either switchover or failover happens, the client > >>>> can not > >>>> continue to use the reader APIs, because there is no such reader > >>>> information > >>>> still available in the new active after switchover. > >>>> > >>>> The patch does checkpoint reader information to the standby when > >>>> completes > >>>> processing reader APIs request. > >>>> > >>>> > >>>> > >>>> Complete diffstat: > >>>> ------------------ > >>>> src/ntf/agent/ntfa_mds.c | 51 +-- > >>>> src/ntf/apitest/tet_coldsync.c | 690 > >>>> ++++++++++++++++++++++++++++++++++++++++- > >>>> src/ntf/common/ntfsv_enc_dec.c | 88 +++++- > >>>> src/ntf/common/ntfsv_enc_dec.h | 12 +- > >>>> src/ntf/ntfd/NtfAdmin.cc | 145 +++++++-- > >>>> src/ntf/ntfd/NtfAdmin.h | 17 +- > >>>> src/ntf/ntfd/NtfClient.cc | 68 +++- > >>>> src/ntf/ntfd/NtfClient.h | 11 +- > >>>> src/ntf/ntfd/NtfLogger.cc | 2 +- > >>>> src/ntf/ntfd/NtfReader.cc | 84 +++-- > >>>> src/ntf/ntfd/NtfReader.h | 13 +- > >>>> src/ntf/ntfd/ntfs_com.c | 105 +++++++ > >>>> src/ntf/ntfd/ntfs_com.h | 25 +- > >>>> src/ntf/ntfd/ntfs_evt.c | 14 +- > >>>> src/ntf/ntfd/ntfs_mbcsv.c | 287 ++++++++++++++--- > >>>> src/ntf/ntfd/ntfs_mbcsv.h | 16 + > >>>> src/ntf/ntfd/ntfs_mds.c | 42 +-- > >>>> 17 files changed, 1430 insertions(+), 240 deletions(-) > >>>> > >>>> > >>>> Testing Commands: > >>>> ----------------- > >>>> Run all test cases of suite 41, and legacy suites > >>>> > >>>> > >>>> Testing, Expected Results: > >>>> -------------------------- > >>>> All pass > >>>> > >>>> > >>>> Conditions of Submission: > >>>> ------------------------- > >>>> ack from reviewers > >>>> > >>>> > >>>> 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 ~/.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! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
