Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-13 Thread Neelakanta Reddy
Hi zoran, Reviewed and tested the patch. Ack. /Neel. On Wednesday 07 October 2015 08:16 PM, Zoran Milinkovic wrote: > osaf/services/saf/immsv/immnd/ImmModel.cc | 12 +--- > osaf/services/saf/immsv/immnd/ImmModel.hh | 3 ++- > osaf/services/saf/immsv/immnd/immnd_evt.c | 15

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-09 Thread Anders Björnerstedt
Hi, I said somewhere at the beginning of this review process that the root cause of the problem is the dubious feature of implicit class/implementer-set, in particular for appliers. Unfortunately, even though this feature is just about useless from the functional perspective, it does have a

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-09 Thread Neelakanta Reddy
Hi , yes, I agree that the applier connection on newly sync node is documented and this is covered. If the ticket(description of the ticket needs to be modified) is to fix the applier re-connection in veteran nodes, based on information available in veteran nodes, then the check has to be

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-09 Thread Neelakanta Reddy
Hi zoran, Comments inline. /Neel. On Thursday 08 October 2015 07:56 PM, Zoran Milinkovic wrote: > Hi Neelakanta, > > From the sync, a new joined node gets only applier name and node id where > the applier is created. The new node does not know anything about on which > objects and classes

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-09 Thread Zoran Milinkovic
Hi Neelakanta, The case described in the ticket is the one with only implementer set where we get mismatch of appliers id. Since we use immcfg and immapplier to produce the problem, class implementer set is called just after implementer set call. In the beginning, I didn't understood why you

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-08 Thread Neelakanta Reddy
Hi zoran, The below patch does not solve the problem indicated in the ticket. . if (originatedAtThisNd) { /*Send reply to client from this ND. */ immnd_client_node_get(cb, clnt_hdl, _node); - if (cl_node == NULL) { + if ((cl_node ==

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-08 Thread Anders Björnerstedt
; Zoran Milinkovic Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Hi Neel, I think the solution for this should be > 2) Since the remote nodes doesn't have the applier mapping, they should not > do the ccb in

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-08 Thread Zoran Milinkovic
Hi Neelakanta, >From the sync, a new joined node gets only applier name and node id where the >applier is created. The new node does not know anything about on which objects >and classes appliers are set. That's a difference from implementers where each node knows on which objects or classes

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-08 Thread Hung Nguyen
eelaka...@oracle.com Sent: Thursday, October 08, 2015 8:17PM To: Zoran Milinkovic zoran.milinko...@ericsson.com Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Hi zoran, The below patch does n

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-08 Thread Anders Björnerstedt
eforge.net Subject: RE: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Yes I think I agree with Hung although this is getting messay and hard to follow in terminology. I don't like the patch slogan "synchronize applier set on all nodes". The erm "synch

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-07 Thread Hung Nguyen
Reddy reddy.neelaka...@oracle.com Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] osaf/services/saf/immsv/immnd/ImmModel.cc | 12 +--- osaf/services/saf/immsv/immnd/ImmModel.hh | 3

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-07 Thread Zoran Milinkovic
anders.bjornerst...@ericsson.com<mailto:anders.bjornerst...@ericsson.com> Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net<mailto:opensaf-devel@lists.sourceforge.net> Subject: RE: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Hi Hung, F

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-07 Thread Zoran Milinkovic
, October 07, 2015 10:10AM To: Neelakanta Reddy reddy.neelaka...@oracle.com<mailto:reddy.neelaka...@oracle.com> Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net<mailto:opensaf-devel@lists.sourceforge.net> Subject: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-07 Thread Hung Nguyen
Nguyen, Neelakanta Reddy, Anders Bjornerstedt hung.d.ngu...@dektech.com.au, reddy.neelaka...@oracle.com, anders.bjornerst...@ericsson.com Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net Subject: RE: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Hi Hung

[devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-07 Thread Zoran Milinkovic
osaf/services/saf/immsv/immnd/ImmModel.cc | 12 +--- osaf/services/saf/immsv/immnd/ImmModel.hh | 3 ++- osaf/services/saf/immsv/immnd/immnd_evt.c | 15 ++- osaf/services/saf/immsv/immnd/immnd_init.h | 2 +- 4 files changed, 22 insertions(+), 10 deletions(-) If

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-07 Thread Anders Björnerstedt
oran.milinko...@ericsson.com > Sent: Wednesday, October 07, 2015 10:10AM > To: Neelakanta Reddy > reddy.neelaka...@oracle.com > Cc: Opensaf-devel > opensaf-devel@lists.sourceforge.net > Subject: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-06 Thread Hung Nguyen
rceforge.net Subject: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] osaf/services/saf/immsv/immnd/ImmModel.cc | 44 +++ osaf/services/saf/immsv/immnd/immnd_evt.c | 7 +++- 2 files changed, 49 insertions(+), 2 deletions(-)

[devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-06 Thread Zoran Milinkovic
osaf/services/saf/immsv/immnd/ImmModel.cc | 44 +++ osaf/services/saf/immsv/immnd/immnd_evt.c | 7 +++- 2 files changed, 49 insertions(+), 2 deletions(-) In the first request of OiImplementerSet, from library to IMMND, a pre-check is added for checking if the

Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504]

2015-10-06 Thread Anders Björnerstedt
Almost ack from me, Code review only, not tested. Hung, can you please repeat the same test using this patch, that you did to expose the problem? The only issue I have is that there is some code duplication. The applier checks added to immModel_implIsFree() are almost the same as the