Hi Canh

+      // [Canh1] Following the ticket #1396, As I remember that you, Ander, Vu
+      // and me had discussed a lot of about ticket 1396 and we had final 
agreement
+      // for the solution of ticket 1396: the mds connection never shutdown 
after first client
+      // is initialized although this client(last client) is finalized after 
that.
+      // This mean that  when mds connection is initialized successfully,
+      // it will never be shutdown. So ncs_agents_shutdown() should not be 
called here
+      // if we finalize the last client, to keep the mds connection is alive

Yes, you are right.  So Ack.
But please look at the minor comments I had as well

Thanks
Lennart

> -----Original Message-----
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: den 6 mars 2018 05:19
> To: Lennart Lund <lennart.l...@ericsson.com>; Vu Minh Nguyen
> <vu.m.ngu...@dektech.com.au>
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> after recovery fails [#2670]
> 
> Hi Lennart,
> 
> Please see my reply comment [Canh1] in the attachment.
> 
> Thanks
> Canh
> 
> -----Original Message-----
> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> Sent: Monday, March 5, 2018 7:40 PM
> To: Canh Van Truong <canh.v.tru...@dektech.com.au>; Vu Minh Nguyen
> <vu.m.ngu...@dektech.com.au>; Lennart Lund
> <lennart.l...@ericsson.com>
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> after recovery fails [#2670]
> 
> Hi Canh,
> 
> I have added some new comments. See [Lennart1] in the attached .diff
> Note that this patch contains all comments so I shall be applied alone, not
> on top of the other comment patches
> 
> Thanks
> Lennart
> 
> > -----Original Message-----
> > From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> > Sent: den 2 mars 2018 07:29
> > To: Lennart Lund <lennart.l...@ericsson.com>; Vu Minh Nguyen
> > <vu.m.ngu...@dektech.com.au>
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> > after recovery fails [#2670]
> >
> > Hi Lennart,
> >
> > Please see my replied comment in attachment.
> >
> > Thanks
> > Canh
> >
> > -----Original Message-----
> > From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> > Sent: Thursday, February 22, 2018 5:00 PM
> > To: Canh Van Truong <canh.v.tru...@dektech.com.au>; Vu Minh Nguyen
> > <vu.m.ngu...@dektech.com.au>
> > Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> > <canh.v.tru...@dektech.com.au>; Lennart Lund
> > <lennart.l...@ericsson.com>
> > Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> > after recovery fails [#2670]
> >
> > Hi Canh
> >
> > I have done a review and have some comments. See attached diff
> >
> > Thanks
> > Lennart
> >
> > > -----Original Message-----
> > > From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> > > Sent: den 9 november 2017 04:25
> > > To: Lennart Lund <lennart.l...@ericsson.com>; Vu Minh Nguyen
> > > <vu.m.ngu...@dektech.com.au>
> > > Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> > > <canh.v.tru...@dektech.com.au>
> > > Subject: [PATCH 0/1] Review Request for log: fix log agent may crash
> after
> > > recovery fails [#2670]
> > >
> > > Summary: log: fix log agent may crash after recovery fails [#2670]
> > > Review request for Ticket(s): 2670
> > > Peer Reviewer(s): Lennart, Vu
> > > Pull request to: Vu
> > > Affected branch(es): develop, release
> > > Development branch: ticket-2670
> > > Base revision: ce78275348c06f5d69577744f0dab525e79443e7
> > > Personal repository: git://git.code.sf.net/u/canht32/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
> > >
> > >
> > > Comments (indicate scope for each "y" above):
> > > ---------------------------------------------
> > > *** EXPLAIN/COMMENT THE PATCH SERIES HERE ***
> > >
> > > revision 980226974b1ab266b068eded487e6ce26b9645c0
> > > Author:   Canh Van Truong <canh.v.tru...@dektech.com.au>
> > > Date:     Thu, 9 Nov 2017 10:14:53 +0700
> > >
> > > log: fix log agent may crash after recovery fails [#2670]
> > >
> > > In log api, the client is deleted from the list in the agent after
> > recovery fails.
> > > there is no check if this client is used by other user.
> > >
> > > The patch fix to make sure that the deletion is just processed when it
> is
> > not
> > > being used.
> > >
> > >
> > >
> > > Complete diffstat:
> > > ------------------
> > >  src/log/agent/lga_agent.cc | 62 ++++++++++++++++++++++++++++-----
> --
> > --
> > > ---------
> > >  src/log/agent/lga_agent.h  |  2 +-
> > >  src/log/agent/lga_client.h | 19 +++++---------
> > >  src/log/agent/lga_util.cc  | 25 +++----------------
> > >  src/log/agent/lga_util.h   |  3 +--
> > >  5 files changed, 50 insertions(+), 61 deletions(-)
> > >
> > >
> > > Testing Commands:
> > > -----------------
> > > *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES ***
> > >
> > >
> > > Testing, Expected Results:
> > > --------------------------
> > > *** PASTE COMMAND OUTPUTS / TEST RESULTS ***
> > >
> > >
> > > Conditions of Submission:
> > > -------------------------
> > > Ack from reviewers
> > >
> > >
> > > 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! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to