Re: [devel] [PATCH 1/1] amfnd: unlock before releasing the monitoring thread to avoid deadlock [#2818]

2018-04-04 Thread Minh Hon Chau
Hi Ravi, Ack from me (legacy test run) with one comment. Since we move m_NCS_UNLOCK before releasing the mon thread, so it is ok to me if inside avnd_mon_pids(), after taking the LOCK(), we don't hit any cancellation point before UNLOCK(). Otherwise, the thread is canceled while the LOCK is

Re: [devel] [PATCH 1/1] smfd: Fix incorrect handling of SMFND NCSMDS_UP/DOWN events [#2821]

2018-04-04 Thread Nguyen Luu
Hi Lennart, Vijay, Please help me review this patch whenever possible for you. Thanks, Nguyen On 3/28/2018 3:07 PM, Nguyen Luu wrote: Current handling of SMFND DOWN event does not take into account failed SMFND UP event, which could eventually result in an inexact view of the actual number of

[devel] [PATCH 1/1] imm: fix memory leaked in immnd [#2825]

2018-04-04 Thread Vu Minh Nguyen
The allocated memory is not freed before returning from the function ImmModel::setCcbErrorString(). --- src/imm/immnd/ImmModel.cc | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/imm/immnd/ImmModel.cc b/src/imm/immnd/ImmModel.cc index f7c8fc0..87ded27 100644 ---

[devel] [PATCH 0/1] Review Request for imm: fix memory leaked in immnd [#2825] V2

2018-04-04 Thread Vu Minh Nguyen
Summary: imm: fix memory leaked in immnd [#2825] Review request for Ticket(s): 2825 Peer Reviewer(s): Hans, Ravi Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop, release Development branch: ticket-2825 Base revision:

Re: [devel] [PATCH 1/1] amfnd: unlock before releasing the monitoring thread to avoid deadlock [#2818]

2018-04-04 Thread Minh Hon Chau
Hi Ravi, I start reviewing today Thanks Minh On 04/04/18 17:19, Ravi Sekhar Reddy Konda wrote: Hi Minh, Did you get time to look at this patch, please consider reviewing it with priority Thanks, Ravi -Original Message- From: ravi-sekhar [mailto:ravisekhar.ko...@oracle.com] Sent:

Re: [devel] [PATCH 1/1] log: Handling of IMM OI BAD HANDLE in log server is incorrect [#2799]

2018-04-04 Thread Lennart Lund
Hi Vu, See my comments/Answers below [Lennart] Thanks Lennart > -Original Message- > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] > Sent: den 4 april 2018 12:10 > To: Lennart Lund ; Canh Van Truong > > Cc:

Re: [devel] [PATCH 1/1] log: Handling of IMM OI BAD HANDLE in log server is incorrect [#2799]

2018-04-04 Thread Lennart Lund
Hi Canh, See my comments below [Lennart] I have attached the fixes I have made for these comments Thanks Lennart From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au] Sent: den 4 april 2018 06:55 To: Lennart Lund ; Vu Minh Nguyen

Re: [devel] [PATCH 1/1] log: Handling of IMM OI BAD HANDLE in log server is incorrect [#2799]

2018-04-04 Thread Vu Minh Nguyen
Hi Lennart, Ack (code review only) with few comments inline, tagged [Vu] Regards, Vu > -Original Message- > From: Lennart Lund [mailto:lennart.l...@ericsson.com] > Sent: Tuesday, April 3, 2018 9:59 PM > To: vu.m.ngu...@dektech.com.au; canh.v.tru...@dektech.com.au > Cc:

Re: [devel] [PATCH 1/1] log: Handling of IMM OI BAD HANDLE in log server is incorrect [#2799]

2018-04-04 Thread Canh Van Truong
Hi Lennart, + // [Canh] below function is just called when ais_rc is 'SA_AIS_ERR_NO_OP' (stop_oi_create) + // or 'SA_AIS_OK'. In case 'SA_AIS_ERR_NO_OP', this function should not be called, + // because the old oi_handle_ will be used to finalize in lgsOiStop() I think that this my comment is

Re: [devel] [PATCH 1/1] imm: fix memory leaked in immnd [#2825]

2018-04-04 Thread Vu Minh Nguyen
Hi Hans, Now, I got your idea of using RAII for ` fmtError`. Thanks for your comment.  See my responses inline for your comment regarding variable arguments. Regards, Vu > -Original Message- > From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] > Sent: Wednesday, April 4, 2018

Re: [devel] [PATCH 1/1] imm: fix memory leaked in immnd [#2825]

2018-04-04 Thread Hans Nordebäck
Hi Zoran, yes you are right, hm, I didn't check the whole function... /BR Hans On 04/04/2018 09:57 AM, Zoran Milinkovic wrote: Hi Hans, Variable arguments with vsnprintf will work. The size of the new buffer is resized in if (len > errLen) { ... osafassert(vsnprintf(fmtError, len,

Re: [devel] [PATCH 1/1] imm: fix memory leaked in immnd [#2825]

2018-04-04 Thread Zoran Milinkovic
Hi Hans, Variable arguments with vsnprintf will work. The size of the new buffer is resized in if (len > errLen) { ... osafassert(vsnprintf(fmtError, len, errorString, vl) >= 0); } when there are variable arguments. BR, Zoran -Original Message- From: Hans Nordebäck Sent: den 4

Re: [devel] [PATCH 1/1] amfnd: unlock before releasing the monitoring thread to avoid deadlock [#2818]

2018-04-04 Thread Ravi Sekhar Reddy Konda
Hi Minh, Did you get time to look at this patch, please consider reviewing it with priority Thanks, Ravi -Original Message- From: ravi-sekhar [mailto:ravisekhar.ko...@oracle.com] Sent: Thursday, March 29, 2018 11:30 AM To: hans.nordeb...@ericsson.com; minh.c...@dektech.com.au;

[devel] [PATCH 3/5] amfd: adapt to new Consensus API [#2795]

2018-04-04 Thread Gary Lee
--- src/amf/amfd/role.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/amf/amfd/role.cc b/src/amf/amfd/role.cc index c8aa9cf1f..790983ee7 100644 --- a/src/amf/amfd/role.cc +++ b/src/amf/amfd/role.cc @@ -1217,7 +1217,7 @@ uint32_t amfd_switch_stdby_actv(AVD_CL_CB *cb) {

[devel] [PATCH 5/5] rded: adapt to new Consensus API [#2795]

2018-04-04 Thread Gary Lee
- add 3 new internal message: RDE_MSG_NODE_UP RDE_MSG_NODE_DOWN RDE_MSG_TAKEOVER_REQUEST_CALLBACK - subscribe to AMFND service up events to keep track of the number of cluster members - listen for takeover requests in KV store --- src/rde/rded/rde_cb.h| 12 ++---

[devel] [PATCH 4/5] fmd: adapt to new Consensus API [#2795]

2018-04-04 Thread Gary Lee
--- src/fm/fmd/fm_main.cc | 26 +- src/fm/fmd/fm_mds.cc | 2 ++ src/fm/fmd/fm_rda.cc | 15 +-- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/fm/fmd/fm_main.cc b/src/fm/fmd/fm_main.cc index 73c9b9ccd..3371ec5e8 100644 ---

[devel] [PATCH 0/5] Review Request for split-brain: select active SC from largest network partition [#2795]

2018-04-04 Thread Gary Lee
Summary: osaf: extend API to include a create key and an enhanced set key function [#2795] Review request for Ticket(s): 2795 Peer Reviewer(s): Anders, Hans, Ravi Pull request to: Affected branch(es): develop Development branch: ticket-2795 Base revision: b6539a3c61115c33f049a905fc05a899b30191b2

[devel] [PATCH 1/5] osaf: extend API to include a create key and an enhanced set key function [#2795]

2018-04-04 Thread Gary Lee
- add create_key function (fails if key already exists) - add setkey_match_prev function (set value if previous value matches) - add missing quotes --- src/osaf/consensus/plugins/etcd.plugin | 86 ++ 1 file changed, 77 insertions(+), 9 deletions(-) diff --git

[devel] [PATCH 2/5] osaf: add lock takeover request fuction [#2795]

2018-04-04 Thread Gary Lee
- add create and set (if previous value matches) functions to KeyValue class - add Consensus::MonitorTakeoverRequest() function for use by RDE to answer takeover requests - add Consensus::CreateTakeoverRequest() - before a SC is promoted to active, it will create a takeover request in the KV

Re: [devel] [PATCH 1/1] imm: fix memory leaked in immnd [#2825]

2018-04-04 Thread Hans Nordebäck
Hi Vu, not saying that you should change now, but an alternative can be to: instead of: char* fmtError = (char*) malloc(errLen);   osafassert(fmtError); a std::vector can be used, (or a std::array if fixed size): std::vector fmtError(errLen, 0);    : len = vsnprintf(fmtError.data(),