Hi Canh ACK with comments See comments inline [Lennart]
Thanks Lennart > -----Original Message----- > From: Canh Truong [mailto:canh.v.tru...@dektech.com.au] > Sent: den 4 augusti 2016 13:25 > To: Lennart Lund <lennart.l...@ericsson.com>; 'A V Mahesh' > <mahesh.va...@oracle.com>; Vu Minh Nguyen > <vu.m.ngu...@dektech.com.au> > Cc: opensaf-devel@lists.sourceforge.net > Subject: RE: [PATCH 1 of 1] log: fix issue with priority of messages adding to > mailbox in the server[1396] > > Hi Lennart, > > I'd like to send to you latest patch that shall be reviewed. > > Thanks, > Canh. > > -----Original Message----- > From: Lennart Lund [mailto:lennart.l...@ericsson.com] > Sent: Thursday, August 04, 2016 5:38 PM > To: A V Mahesh; Canh Van Truong; Vu Minh Nguyen > Cc: opensaf-devel@lists.sourceforge.net > Subject: RE: [PATCH 1 of 1] log: fix issue with priority of messages adding to > mailbox in the server[1396] > > Hi Canh > > My comments are very old, October 2015 and they are mostly about list > handling (see attached mail). Since then the log server has been refactored > to C++ so C++ tools for lists can be used. > There seems to be a lot of mail conversations with included patches etc. and > since I haven't been following this I have some problem understanding what > to review. Can you please send out a review request containing the latest > patches that shall actually be reviewed. > > Thanks > Lennart > > > -----Original Message----- > > From: A V Mahesh [mailto:mahesh.va...@oracle.com] > > Sent: den 21 juni 2016 08:49 > > 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 1 of 1] log: fix issue with priority of messages > > adding to mailbox in the server[1396] > > > > Hi Canh, > > > > It seems Lennart already provide comments please check. > > > > -AVM > > > > > > On 6/21/2016 12:07 PM, Canh Truong wrote: > > > Hi Mahesh, > > > > > > Thanks for your comments. I will send new version after Lennart > > comments. > > > I have checked your idea about multiple thread test case. The idea > > > may be > > same with patch that I sent it before. > > > > > > Regards, > > > Canh. > > > > > > -----Original Message----- > > > From: A V Mahesh [mailto:mahesh.va...@oracle.com] > > > Sent: Tuesday, June 21, 2016 11:47 AM > > > To: Canh Truong; vu.m.ngu...@dektech.com.au; > > lennart.l...@ericsson.com > > > Cc: opensaf-devel@lists.sourceforge.net > > > Subject: Re: [PATCH 1 of 1] log: fix issue with priority of messages > > > adding to > > mailbox in the server[1396] > > > > > > Hi Canh, > > > > > > It will be good to send V2 patch with Lennart comments and adding > > > new > > test cases. > > > > > > -AVM > > > On 6/21/2016 9:46 AM, A V Mahesh wrote: > > >> Hi Canh, > > >> > > >> Some how we were not able to open your attachment , so my self > > >> written > > >> > > >> a multi threaded test case ( attached > > >> `thread_LogInitializeFinalize.c` > > >> ) and reviewed/tested it is working fine. > > >> > > >> ACK from me with following : > > >> > > >> - Please add such as attached multi threaded test case to logtest, > > >> before pushing > > >> > > >> - add # before ticket number in commit message #1396 > > >> > > >> -AVM > > >> > > >> > > >> On 6/14/2016 2:47 PM, Canh Truong wrote: > > >>> Hi Mahesh, > > >>> > > >>> I have created new test case for multiple threads. Please help > > >>> check new patch in attachment. > > >>> > > >>> Thanks, > > >>> Canh. > > >>> > > >>> -----Original Message----- > > >>> From: A V Mahesh [mailto:mahesh.va...@oracle.com] > > >>> Sent: Tuesday, June 14, 2016 11:03 AM > > >>> To: Canh Van Truong; vu.m.ngu...@dektech.com.au; > > >>> lennart.l...@ericsson.com > > >>> Cc: opensaf-devel@lists.sourceforge.net > > >>> Subject: Re: [PATCH 1 of 1] log: fix issue with priority of > > >>> messages adding to mailbox in the server[1396] > > >>> > > >>> Hi Canh Van Truong, > > >>> > > >>> I hope this patch will work for multiple threads doing > > >>> saLogInitialize() , saLogStreamOpen_2() and saLogFinalize() > > >>> concurrently in a single process and reaches lga_use_count = 0 , > > >>> againg restarting > > >>> saLogInitialize() , saLogStreamOpen_2() and > > >>> saLogFinalize() > > >>> without process exist. > > >>> > > >>> If so let us have a test case like for multiple threads doing > > >>> saLogInitialize() , saLogStreamOpen_2() and saLogFinalize() > > >>> concurrently > > >>> > > >>> -AVM > > >>> > > >>> > > >>> On 6/13/2016 12:37 PM, Canh Van Truong wrote: > > >>>> osaf/libs/agents/saf/lga/lga_api.c | 3 ++- > > >>>> osaf/libs/agents/saf/lga/lga_util.c | 21 ++++++++++++--------- > > >>>> tests/logsv/logtest.h | 1 + > > >>>> tests/logsv/tet_saLogStreamOpen_2.c | 29 > > >>>> +++++++++++++++++++++++++++++ > > >>>> 4 files changed, 44 insertions(+), 10 deletions(-) > > >>>> > > >>>> > > >>>> Sometimes, Log service can handle LGSV_INITIALIZE_REQ before > > handle > > >>>> LGSV_LGS_EVT_LGA_DOWN, although LGSV_LGS_EVT_LGA_DOWN > > was sent first > > >>>> whenever > > >>>> finalize() is call for last client. Because lgs can not > > >>>> distinguish MDS address between new connection (initializing new > > >>>> client) and old connection (finalizing last client before). This > > >>>> patch fixes to keep MDS connection when last client finalized, > > >>>> and re-use it when next > > >>> initialize() called. > > >>>> diff --git a/osaf/libs/agents/saf/lga/lga_api.c > > >>>> b/osaf/libs/agents/saf/lga/lga_api.c > > >>>> --- a/osaf/libs/agents/saf/lga/lga_api.c > > >>>> +++ b/osaf/libs/agents/saf/lga/lga_api.c > > >>>> @@ -41,7 +41,8 @@ > > >>>> /* The main controle block */ > > >>>> lga_cb_t lga_cb = { > > >>>> .cb_lock = PTHREAD_MUTEX_INITIALIZER, > > >>>> - .lgs_state = LGS_START > > >>>> + .lgs_state = LGS_START, > > >>>> + .mds_hdl = 0 > > >>>> }; > > >>>> static bool is_well_know_stream(const char* dn) diff --git > > >>>> a/osaf/libs/agents/saf/lga/lga_util.c > > >>>> b/osaf/libs/agents/saf/lga/lga_util.c > > >>>> --- a/osaf/libs/agents/saf/lga/lga_util.c > > >>>> +++ b/osaf/libs/agents/saf/lga/lga_util.c > > >>>> @@ -308,21 +308,17 @@ unsigned int lga_startup(lga_cb_t *cb) > > >>>> osaf_mutex_lock_ordie(&lga_lock); > > >>>> TRACE_ENTER2("lga_use_count: %u", lga_use_count); > > >>>> - if (lga_use_count > 0) { > > >>>> - /* Already created, just increment the use_count */ > > >>>> - lga_use_count++; > > >>>> - goto done; > > >>>> - } else { > > >>>> + > > >>>> + if (cb->mds_hdl == 0) { > > >>>> if ((rc = ncs_agents_startup()) != NCSCC_RC_SUCCESS) { > > >>>> TRACE("ncs_agents_startup FAILED"); > > >>>> goto done; > > >>>> } > > >>>> if ((rc = lga_create()) != NCSCC_RC_SUCCESS) { > > >>>> + cb->mds_hdl = 0; > > >>>> ncs_agents_shutdown(); > > >>>> goto done; > > >>>> - } else { > > >>>> - lga_use_count = 1; > > >>>> } > > >>>> /* Agent has successfully been started including > > >>> communication @@ > > >>>> -331,6 +327,9 @@ unsigned int lga_startup(lga_cb_t *cb) > > >>>> set_lga_state(LGA_NORMAL); > > >>>> } > > >>>> + /* Increase the use_count */ > > >>>> + lga_use_count++; > > >>>> + > > >>>> done: > > >>>> osaf_mutex_unlock_ordie(&lga_lock); > > >>>> @@ -361,8 +360,12 @@ unsigned int lga_shutdown_after_last_cli > > >>>> /* Users still exist, just decrement the use count */ > > >>>> lga_use_count--; > > >>>> } else if (lga_use_count == 1) { > > >>>> - lga_destroy(); > > >>>> - rc = ncs_agents_shutdown(); > > >>>> + /* > > >>>> + * Disable MDS_UNINSTALL and ncs agent shutdown to keep > > >>>> + MDS > > >>> connection. > > >>>> + * Fix ticket 1396. Msg NCSMDS_DOWN and Initialize are > > >>>> + out > > >>> of order in Mbx. > > >>>> + */ [Lennart] Remove this comment. #1396 is fixed > > >>>> + /* lga_destroy(); */ > > >>>> + /* rc = ncs_agents_shutdown(); */ [Lennart] Remove out-commented code > > >>>> lga_use_count = 0; > > >>>> } > > >>>> diff --git a/tests/logsv/logtest.h b/tests/logsv/logtest.h > > >>>> --- a/tests/logsv/logtest.h > > >>>> +++ b/tests/logsv/logtest.h > > >>>> @@ -27,6 +27,7 @@ > > >>>> #include <utest.h> > > >>>> #include <util.h> > > >>>> #include <sys/wait.h> > > >>>> +#include <unistd.h> > > >>>> #include <osaf_time.h> > > >>>> #include <logtrace.h> > > >>>> diff --git a/tests/logsv/tet_saLogStreamOpen_2.c > > >>>> b/tests/logsv/tet_saLogStreamOpen_2.c > > >>>> --- a/tests/logsv/tet_saLogStreamOpen_2.c > > >>>> +++ b/tests/logsv/tet_saLogStreamOpen_2.c > > >>>> @@ -704,6 +704,34 @@ done: > > >>>> } > > >>>> } > > >>>> +/* > > >>>> + * Ticket 1396 > > >>>> + * Verify that saLogInitialize() then saLogFinalize() multiple > > >>>> +time OK */ void saLogMultipleInitialize(void) { > > >>>> + SaLogStreamHandleT logStreamHandle1; > > >>>> + int count = 100; > > >>>> + SaAisErrorT rc = SA_AIS_OK; > > >>>> + > > >>>> + while (count--) { > > >>>> + rc = saLogInitialize(&logHandle, &logCallbacks, > > >>> &logVersion); > > >>>> + if (rc != SA_AIS_OK) > > >>>> + break; > > >>>> + > > >>>> + rc = saLogStreamOpen_2(logHandle, > > >>>> + &app1StreamName, > > >>> &appStream1LogFileCreateAttributes, > > >>>> + SA_LOG_STREAM_CREATE, > > >>> SA_TIME_ONE_SECOND, &logStreamHandle1); > > >>>> + safassert(rc, SA_AIS_OK); > > >>>> + > > >>>> + rc = saLogFinalize(logHandle); > > >>>> + if (rc != SA_AIS_OK) > > >>>> + break; > > >>>> + > > >>>> + } > > >>>> + test_validate(rc, SA_AIS_OK); > > >>>> + > > >>>> +} > > >>>> + > > >>>> extern void saLogStreamOpenAsync_2_01(void); > > >>>> extern void saLogStreamOpenCallbackT_01(void); > > >>>> extern void saLogWriteLog_01(void); @@ -790,5 +818,6 @@ > > >>>> extern void saLogStreamClose_01(void); > > >>>> test_case_add(2, verFixLogRec_Max_Err, "saLogStreamOpen_2 > > >>>> with > > >>> maxLogRecordSize > MAX_RECSIZE, ERR"); > > >>>> test_case_add(2, verFixLogRec_Min_Err, "saLogStreamOpen_2 > > >>>> with > > >>> maxLogRecordSize < 150, ERR"); > > >>>> test_case_add(2, saLogStreamOpen_2_50, "saLogStreamOpen_2 > > >>>> with stream number out of the limitation, ERR"); > > >>>> + test_case_add(2, saLogMultipleInitialize, "saLogInitialize() > > >>>> + then > > >>>> + saLogFinalize() multiple time. keep MDS connection, OK"); > > >>>> } ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
