Re: [devel] [PATCH 1/1] amfd: Trigger dependent SI assignment if currActiveAssignment is less than preferred active assignment [#2803]

2018-03-15 Thread Gary Lee

ack from me (review)


On 16/03/18 16:14, Minh Hon Chau wrote:

Hi Hans, I will update the PR.

Hi Gary, Ravi,

Do we have any comments?

Thanks,

Minh


On 16/03/18 01:51, Hans Nordebäck wrote:
Hi Minh, ack review only. Good if the AMF Programmer's Reference also 
be updated regarding this, (si dependencies/sponsors).


/Regards HansN


On 03/14/2018 12:53 AM, Minh Chau wrote:
In SI dependency configuration that set NwayActive SI as dependent 
SI, which
is assigned to all SUs hosted on all nodes. After stop and restart 
SCs, the

NwayActive SI becomes PARTIALLY_ASSIGNED.

The reason of PARTIALLY_ASSIGNED SI is that the SI currently is not 
assigned
in SC nodes. This patch triggers assignment for dependent SI if the 
SI has

not had enough preferred active assignment.

Please note that the additional case in this patch only hits if the 
SC absence
feature is enabled. In normal cluster, the dependency state should 
firstly
go from READ_TO_ASSIGN and the SG procedure will create active 
assignments

up to the preferred number.
---
  src/amf/amfd/si_dep.cc | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/amf/amfd/si_dep.cc b/src/amf/amfd/si_dep.cc
index a4ccbe7..f63b1b0 100644
--- a/src/amf/amfd/si_dep.cc
+++ b/src/amf/amfd/si_dep.cc
@@ -799,7 +799,10 @@ void avd_sidep_assign_evh(AVD_CL_CB *cb, 
AVD_EVT *evt) {

    } else {
  /*Check sponsors state once agian then take action*/
  sidep_update_si_self_dep_state(dep_si);
-    if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN) {
+    if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN ||
+    (dep_si->si_dep_state == AVD_SI_ASSIGNED &&
+    dep_si->saAmfSINumCurrActiveAssignments <
+    dep_si->pref_active_assignments())) {
    if ((sidep_sg_red_si_process_assignment(avd_cb, dep_si) ==
 NCSCC_RC_FAILURE) &&
    (dep_si->num_dependents != 0)) {
@@ -980,6 +983,10 @@ void sidep_take_action_on_dependents(AVD_SI *si) {
    sidep_process_ready_to_unassign_depstate(dep_si);
  } else if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN) {
    sidep_si_dep_state_evt_send(avd_cb, dep_si, 
AVD_EVT_ASSIGN_SI_DEP_STATE);

+    } else if (dep_si->si_dep_state == AVD_SI_ASSIGNED &&
+    si->sg_of_si->sg_fsm_state == AVD_SG_FSM_STABLE &&
+    si->saAmfSINumCurrActiveAssignments < 
si->pref_active_assignments()) {
+  sidep_si_dep_state_evt_send(avd_cb, dep_si, 
AVD_EVT_ASSIGN_SI_DEP_STATE);

  }
    }








--
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


Re: [devel] [PATCH 1/1] base: Only close inherited fd(s) after fork() in child process [#2805]

2018-03-15 Thread Minh Hon Chau

Thanks Anders for your comments.

Hi Hans, Ravi,

Is there any comment you would like to add, otherwise I update the patch 
with Anders' comments.


Thanks,

Minh


On 16/03/18 01:41, Anders Widell wrote:

Ack with minor comments, marked AndersW> below.

regards,

Anders Widell


On 03/15/2018 07:50 AM, Minh Chau wrote:

---
  src/base/os_defs.c | 25 -
  1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/base/os_defs.c b/src/base/os_defs.c
index 6f9ec52..e914011 100644
--- a/src/base/os_defs.c
+++ b/src/base/os_defs.c
@@ -1052,14 +1052,29 @@ uint32_t 
ncs_os_process_execute_timed(NCS_OS_PROC_EXECUTE_TIMED_INFO *req)

   * child */
  if (getenv("OPENSAF_KEEP_FD_OPEN_AFTER_FORK") == NULL) {
  /* Close all inherited file descriptors */
-    int i = sysconf(_SC_OPEN_MAX);
-    if (i == -1) {
+    int fd_max = sysconf(_SC_OPEN_MAX);


AndersW> sysconf() returns a long. Maybe fd_max should have the type 
long to match the return type of sysconf()?



+
+    if (fd_max == -1) {
  syslog(LOG_ERR, "%s: sysconf failed - %s",
-   __FUNCTION__, strerror(errno));
+    __FUNCTION__, strerror(errno));
  exit(EXIT_FAILURE);
  }
-    for (i--; i >= 0; --i)
-    (void)close(i); /* close all descriptors */
+    struct dirent *pentry = NULL;


AndersW> pentry is not a good name (avoid abbreviations, and separate 
words with underscores). Maybe rename it to dir_entry or just entry?



+    DIR *dir = opendir("/proc/self/fd");
+
+    if (dir != NULL) {
+    while ((pentry = readdir(dir)) != NULL) {
+    int fd = strtoimax(pentry->d_name, NULL, 10);


AndersW> strtoimax() is declared in the inttypes.h header file. Add an 
#include at the top of the file.
AndersW> strtoimax() returns an intmax_t. Change the type of fd to 
intmax_t.



+    if (fd > INT_MIN && fd < fd_max) (void)close(fd);


AndersW> File descriptors cannot be negative. Use fd >= 0 instead of 
fd > INT_MIN.

AndersW> Remove (void).


+    }
+    (void)closedir(dir);


AndersW> Remove (void).


+    } else {
+    /* fall back, close all possible descriptors */
+    syslog(LOG_ERR, "%s: opendir failed - %s",
+    __FUNCTION__, strerror(errno));
+    for (fd_max--; fd_max >= 0; --fd_max)
+    (void)close(fd_max);


AndersW> Remove (void).


+    }
    /* Redirect standard files to /dev/null */
  if (freopen("/dev/null", "r", stdin) == NULL)






--
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


Re: [devel] [PATCH 1/1] amfd: Trigger dependent SI assignment if currActiveAssignment is less than preferred active assignment [#2803]

2018-03-15 Thread Minh Hon Chau

Hi Hans, I will update the PR.

Hi Gary, Ravi,

Do we have any comments?

Thanks,

Minh


On 16/03/18 01:51, Hans Nordebäck wrote:
Hi Minh, ack review only. Good if the AMF Programmer's Reference also 
be updated regarding this, (si dependencies/sponsors).


/Regards HansN


On 03/14/2018 12:53 AM, Minh Chau wrote:
In SI dependency configuration that set NwayActive SI as dependent 
SI, which
is assigned to all SUs hosted on all nodes. After stop and restart 
SCs, the

NwayActive SI becomes PARTIALLY_ASSIGNED.

The reason of PARTIALLY_ASSIGNED SI is that the SI currently is not 
assigned
in SC nodes. This patch triggers assignment for dependent SI if the 
SI has

not had enough preferred active assignment.

Please note that the additional case in this patch only hits if the 
SC absence
feature is enabled. In normal cluster, the dependency state should 
firstly
go from READ_TO_ASSIGN and the SG procedure will create active 
assignments

up to the preferred number.
---
  src/amf/amfd/si_dep.cc | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/amf/amfd/si_dep.cc b/src/amf/amfd/si_dep.cc
index a4ccbe7..f63b1b0 100644
--- a/src/amf/amfd/si_dep.cc
+++ b/src/amf/amfd/si_dep.cc
@@ -799,7 +799,10 @@ void avd_sidep_assign_evh(AVD_CL_CB *cb, AVD_EVT 
*evt) {

    } else {
  /*Check sponsors state once agian then take action*/
  sidep_update_si_self_dep_state(dep_si);
-    if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN) {
+    if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN ||
+    (dep_si->si_dep_state == AVD_SI_ASSIGNED &&
+    dep_si->saAmfSINumCurrActiveAssignments <
+    dep_si->pref_active_assignments())) {
    if ((sidep_sg_red_si_process_assignment(avd_cb, dep_si) ==
 NCSCC_RC_FAILURE) &&
    (dep_si->num_dependents != 0)) {
@@ -980,6 +983,10 @@ void sidep_take_action_on_dependents(AVD_SI *si) {
    sidep_process_ready_to_unassign_depstate(dep_si);
  } else if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN) {
    sidep_si_dep_state_evt_send(avd_cb, dep_si, 
AVD_EVT_ASSIGN_SI_DEP_STATE);

+    } else if (dep_si->si_dep_state == AVD_SI_ASSIGNED &&
+    si->sg_of_si->sg_fsm_state == AVD_SG_FSM_STABLE &&
+    si->saAmfSINumCurrActiveAssignments < 
si->pref_active_assignments()) {
+  sidep_si_dep_state_evt_send(avd_cb, dep_si, 
AVD_EVT_ASSIGN_SI_DEP_STATE);

  }
    }






--
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


[devel] Errors in running OpenSAF message sample

2018-03-15 Thread Feng Xie
Hi,

Thanks a lot to Anders' suggestion. After downgrading from Python 3 to Python 
2.7, the errors in running immxml-configure were resolved!

I was able to start OpenSAF related demos and compile sample programs. The 
first OpenSAF sample program I ran is msg_demo under 
~/local/share/opensaf/samples/mqsv. The sample was run in a single-node Ubuntu 
VM. However, I encountered error code 2 from saMsgInitialize. I checked the 
source code in ~/src/msg/agent/mqa_api.cc, it seems that error code 2 maps to 
SA_AIS_ERR_LIBRARY. There are may occurrences of it. I am stuck at this step. 
Any help will be highly appreciated!

In addition, I have two additional questions:

  1.  Is there a timer service provided by OpenSAF?
  2.  How to turn on internal tracing when running OpenSAF sample programs?
The output from ./amf-state, error messages and opensaf demos are attached 
at the end of the email for reference. Thanks.

Feng



  1.  Error messages when running msg_demo
On one terminal (as a receiver):

~/local/share/opensaf/samples/mqsv$ sudo msg_demo 0

STARTING THE MQSv DEMO
==
MessageQ Receiver Application
#

DEMO SCENARIO#1: Receiving messages via Sync API - saMsgMessageGet START

Error Initialising saMsgInitialize with error - 2Press Enter Key to Continue...


On the other terminal (as a sender):

~/local/share/opensaf/samples/mqsv$ sudo msg_demo 1

STARTING THE MQSv DEMO
==
MessageQ Sender Application
##

SCENARIO#1:Sending Messages via Sync API - saMsgMessageSend START

Error Initialising saMsgInitialize with error - 2Press Enter Key to Continue...

##



  1.  OpenSAF related demos
~/local/share/opensaf/samples/mqsv$ ps -A


2697 ?00:00:00 osafdtmd
  2705 ?00:00:00 osaftransportd
  2713 ?00:00:00 osafclmna
  2724 ?00:00:00 osafrded
  2735 ?00:00:00 osaffmd
  2747 ?00:00:00 osafimmd
  2760 ?00:00:03 osafimmnd
  2777 ?00:00:00 osaflogd
  2791 ?00:00:00 osafntfd
  2803 ?00:00:00 osafclmd
  2815 ?00:00:00 osafamfd
  2833 ?00:00:00 osafamfnd
  2853 ?00:00:00 osafsmfd
  2855 ?00:00:00 osafsmfnd
  2883 ?00:00:03 osafmsgnd
  2896 ?00:00:00 osafmsgd
  2957 ?00:00:00 osaflckd
  2967 ?00:00:00 osaflcknd
  2994 ?00:00:00 osafckptnd
  3002 ?00:00:01 osafevtd
  3017 ?00:00:01 osafckptd
  3022 ?00:00:00 osafamfwd
..


  1.  Output of amf-state

~/local/bin$ ./amf-state

Service Instances UNLOCKED and UNASSIGNED:
   safSi=NoRed2,safApp=OpenSAF

Service Units UNLOCKED and DISABLED:
   safSu=SC-2,safSg=2N,safApp=OpenSAF
   safSu=SC-2,safSg=NoRed,safApp=OpenSAF

Nodes UNLOCKED and DISABLED:
   safAmfNode=SC-2,safAmfCluster=myAmfCluster





From: Anders Widell 
Sent: Thursday, March 8, 2018 7:20 AM
To: Feng Xie ; Dheeroj Ram ; 
opensaf-us...@lists.sourceforge.net; gary@dektech.com.au
Subject: Re: Errors in running immxml-configure with OpenSaf 5.3 in a Ubuntu VM


Just a wild guess: maybe your Linux distribution installs Python version 3 as 
default? We haven't fully converted our Python code to be compatible with 
version 3 yet (it ought to be done, though). Could you run 'python --version' 
to check your default version. If it is version 3, you probably need to install 
version 2 and use it to run the OpenSAF Python code.

OpenSAF supports running in virtual machines.

regards,

Anders Widell
On 03/07/2018 09:59 PM, Feng Xie wrote:
Hi Dheeraj, Gary and Anders,

   Thanks a lot for the quick response! With your help, I was able to solve 
the original issue of locating the share libraries. The current issue I am 
facing now is with immxml-configure, which failed in File "./immxml-merge", 
line 370, in save_result
self.imm_content_element.toxml(encoding).replace("/>", ">") + "\n"). As a 
result, IMMD can't be started. My debug procedure and detailed error messages 
are attached at the end for your reference. I am stuck at this step. Any help 
will be highly appreciated!

   Let me provide some background information about my current effort so 
that you can better understand my situation:


  1.  I am trying to use opensaf for core middleware services like messaging 
(IPC), log, timer, etc.. The target environment is a single-node multicore 
(ARM) Linux embedded system. If opensaf works in a single-node environment, we 
may extend to a cluster environment in the future.



  1.  I created a virtual machine using VMWare on a X86 Dell PC with ubuntu 
Linux and installed the latest opensaf tarball (OpenSAF package 
opensaf-5.18.02.tar.gz in a Ubuntu Linux VM (Linux version 

Re: [devel] [PATCH 1/1] amfd: Trigger dependent SI assignment if currActiveAssignment is less than preferred active assignment [#2803]

2018-03-15 Thread Hans Nordebäck
Hi Minh, ack review only. Good if the AMF Programmer's Reference also be 
updated regarding this, (si dependencies/sponsors).


/Regards HansN


On 03/14/2018 12:53 AM, Minh Chau wrote:

In SI dependency configuration that set NwayActive SI as dependent SI, which
is assigned to all SUs hosted on all nodes. After stop and restart SCs, the
NwayActive SI becomes PARTIALLY_ASSIGNED.

The reason of PARTIALLY_ASSIGNED SI is that the SI currently is not assigned
in SC nodes. This patch triggers assignment for dependent SI if the SI has
not had enough preferred active assignment.

Please note that the additional case in this patch only hits if the SC absence
feature is enabled. In normal cluster, the dependency state should firstly
go from READ_TO_ASSIGN and the SG procedure will create active assignments
up to the preferred number.
---
  src/amf/amfd/si_dep.cc | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/amf/amfd/si_dep.cc b/src/amf/amfd/si_dep.cc
index a4ccbe7..f63b1b0 100644
--- a/src/amf/amfd/si_dep.cc
+++ b/src/amf/amfd/si_dep.cc
@@ -799,7 +799,10 @@ void avd_sidep_assign_evh(AVD_CL_CB *cb, AVD_EVT *evt) {
} else {
  /*Check sponsors state once agian then take action*/
  sidep_update_si_self_dep_state(dep_si);
-if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN) {
+if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN ||
+(dep_si->si_dep_state == AVD_SI_ASSIGNED &&
+dep_si->saAmfSINumCurrActiveAssignments <
+dep_si->pref_active_assignments())) {
if ((sidep_sg_red_si_process_assignment(avd_cb, dep_si) ==
 NCSCC_RC_FAILURE) &&
(dep_si->num_dependents != 0)) {
@@ -980,6 +983,10 @@ void sidep_take_action_on_dependents(AVD_SI *si) {
sidep_process_ready_to_unassign_depstate(dep_si);
  } else if (dep_si->si_dep_state == AVD_SI_READY_TO_ASSIGN) {
sidep_si_dep_state_evt_send(avd_cb, dep_si, 
AVD_EVT_ASSIGN_SI_DEP_STATE);
+} else if (dep_si->si_dep_state == AVD_SI_ASSIGNED &&
+si->sg_of_si->sg_fsm_state == AVD_SG_FSM_STABLE &&
+si->saAmfSINumCurrActiveAssignments < si->pref_active_assignments()) {
+  sidep_si_dep_state_evt_send(avd_cb, dep_si, AVD_EVT_ASSIGN_SI_DEP_STATE);
  }
}
  



--
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


Re: [devel] [PATCH 1/1] base: Only close inherited fd(s) after fork() in child process [#2805]

2018-03-15 Thread Anders Widell

Ack with minor comments, marked AndersW> below.

regards,

Anders Widell


On 03/15/2018 07:50 AM, Minh Chau wrote:

---
  src/base/os_defs.c | 25 -
  1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/base/os_defs.c b/src/base/os_defs.c
index 6f9ec52..e914011 100644
--- a/src/base/os_defs.c
+++ b/src/base/os_defs.c
@@ -1052,14 +1052,29 @@ uint32_t 
ncs_os_process_execute_timed(NCS_OS_PROC_EXECUTE_TIMED_INFO *req)
 * child */
if (getenv("OPENSAF_KEEP_FD_OPEN_AFTER_FORK") == NULL) {
/* Close all inherited file descriptors */
-   int i = sysconf(_SC_OPEN_MAX);
-   if (i == -1) {
+   int fd_max = sysconf(_SC_OPEN_MAX);


AndersW> sysconf() returns a long. Maybe fd_max should have the type 
long to match the return type of sysconf()?



+
+   if (fd_max == -1) {
syslog(LOG_ERR, "%s: sysconf failed - %s",
-  __FUNCTION__, strerror(errno));
+   __FUNCTION__, strerror(errno));
exit(EXIT_FAILURE);
}
-   for (i--; i >= 0; --i)
-   (void)close(i); /* close all descriptors */
+   struct dirent *pentry = NULL;


AndersW> pentry is not a good name (avoid abbreviations, and separate 
words with underscores). Maybe rename it to dir_entry or just entry?



+   DIR *dir = opendir("/proc/self/fd");
+
+   if (dir != NULL) {
+   while ((pentry = readdir(dir)) != NULL) {
+   int fd = strtoimax(pentry->d_name, 
NULL, 10);


AndersW> strtoimax() is declared in the inttypes.h header file. Add an 
#include at the top of the file.

AndersW> strtoimax() returns an intmax_t. Change the type of fd to intmax_t.


+   if (fd > INT_MIN && fd < fd_max) 
(void)close(fd);


AndersW> File descriptors cannot be negative. Use fd >= 0 instead of fd 
> INT_MIN.

AndersW> Remove (void).


+   }
+   (void)closedir(dir);


AndersW> Remove (void).


+   } else {
+   /* fall back, close all possible descriptors */
+   syslog(LOG_ERR, "%s: opendir failed - %s",
+   __FUNCTION__, strerror(errno));
+   for (fd_max--; fd_max >= 0; --fd_max)
+   (void)close(fd_max);


AndersW> Remove (void).


+   }
  
  			/* Redirect standard files to /dev/null */

if (freopen("/dev/null", "r", stdin) == NULL)



--
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


Re: [devel] [PATCH 1/1] tools: Fix trace2dot, it stopped working after ticket #2165 [#2668]

2018-03-15 Thread Anders Widell

Ack.

regards,

Anders Widell


On 03/09/2018 02:30 PM, Hans Nordeback wrote:

---
  tools/devel/dot/trace2dot | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/devel/dot/trace2dot b/tools/devel/dot/trace2dot
index 160617c7a..1977276ea 100755
--- a/tools/devel/dot/trace2dot
+++ b/tools/devel/dot/trace2dot
@@ -2,7 +2,7 @@
  #
  #
  # (C) Copyright 2015 The OpenSAF Foundation
-# (C) Copyright Ericsson AB 2015, 2016, 2017. All rights reserved.
+# (C) Copyright Ericsson AB 2015, 2016, 2017, 2018. All rights reserved.
  #
  # This program is distributed in the hope that it will be useful, but
  # WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -59,13 +59,13 @@ def check_infile(trace_file):
  trace_leave = set()
  for line in infile:
  items = line.split()
-if items.__len__() > 6:
-if items[5] == '>>':
-name = items[6].rstrip(':')
+if len(items) > 10:
+if items[9] == '>>':
+name = items[10].rstrip(':')
  trace_enter.add(name)
  
-if items[5] == '<<':

-name = items[6].rstrip(':')
+if items[9] == '<<':
+name = items[10].rstrip(':')
  trace_leave.add(name)
  
  for name in trace_enter:

@@ -86,9 +86,9 @@ def process_infile(infile, from_function, outfile):
  
  for line in infile:

  items = line.split()
-if items.__len__() > 6:
-if items[5] == '>>':
-func_enter = items[6].rstrip(':')
+if len(items) > 10:
+if items[9] == '>>':
+func_enter = items[10].rstrip(':')
  if from_function and not from_func_found:
  if from_function != func_enter:
  continue
@@ -108,8 +108,8 @@ def process_infile(infile, from_function, outfile):
  ' [ordering=out, color=red, shape=box, label="' +
  func_enter + '"];\n')
  
-if items[5] == '<<':

-func_leave = items[6].rstrip(':')
+if items[9] == '<<':
+func_leave = items[10].rstrip(':')
  if from_function:
  if from_function == func_leave:
  break



--
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


Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Hans Nordebäck

Hi Vu,

yes, it seems ok to remove the assert, as it is now a nop. (The other 
conditions reply_dest and isObjSync


may/seems not need to be asserted anymore).

/Regards HansN


On 03/15/2018 11:16 AM, Vu Minh Nguyen wrote:

Hi Hans,

I am going to remove the assertion since the original one is now always true 
with #2794 fix.

Do you have any concern with that?

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Thursday, March 15, 2018 4:56 PM
To: Vu Minh Nguyen 
Cc: opensaf-devel@lists.sourceforge.net; ravisekhar.ko...@oracle.com;
zoran.milinko...@ericsson.com; anders.wid...@ericsson.com;
lennart.l...@ericsson.com
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

Hi Vu,

I guess the assert should be kept but remove the

reply_dest == cb->immnd_mdest_id

condtition, or?

/Regards HansN


On 03/15/2018 10:46 AM, Vu Minh Nguyen wrote:

Hi Hans,

You are right.  I will remove that assert. Thanks! 

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Thursday, March 15, 2018 4:34 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

Hi Vu,

this osafassert will never assert, no matter what values reply_dest or
isObjSync have,

if reply_dest is 0 then osafassert(!reply_dest will be true.


/Regards HansN


On 03/15/2018 10:20 AM, Vu Minh Nguyen wrote:

Hi Hans,

I don't think so. In some cases, reply_dest could be 0 and not equal to cb-
immd_mdest_id.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Thursday, March 15, 2018 4:10 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

Hi Vu,

not a big deal, but you are wrong, as I mentioned, the osafassert is now
a nop as

"reply_dest == cb->immnd_mdest_id" are always true at the osafassert

stmt.

/Regards HansN




On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote:

Hi Hans,

See my response inline.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Tuesday, March 13, 2018 9:10 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

ack, review only. One question below.

/Thanks HansN


On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:

When IMMND restarts (e.g: OUT OF ORDER detection), it may get

message

from active IMMD which is originated from just-dead IMMND

process.

In such case, we are in confused situation - messages come from
local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!

This patch discards such messages, notify the case to syslog
instead of aborting the IMMND progress.
---
  src/imm/immnd/immnd_evt.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/src/imm/immnd/immnd_evt.c

b/src/imm/immnd/immnd_evt.c

index 228b7dd..43611a3 100644
--- a/src/imm/immnd/immnd_evt.c
+++ b/src/imm/immnd/immnd_evt.c
@@ -10766,6 +10766,18 @@ static uint32_t

immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,

(m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb-

node_id);

if (originatedAtThisNd) {
+   /* Get the message comes from local IMMND but not

me

+  (cb->immnd_mdest_id). Probably IMMND just restarts
+  (e.g: OUT OF ORDER detection), and this message

belongs

+  to previous (dead) IMMND. So, discard this message.
+*/
+   if (reply_dest && reply_dest != cb->immnd_mdest_id) {
+   LOG_WA("DISCARD FEVS message sent by

previous dead

IMMND");

+   dequeue_outgoing(cb);
+   TRACE_LEAVE();
+   return NCSCC_RC_SUCCESS;
+   }
+
osafassert(!reply_dest || (reply_dest == cb-

immnd_mdest_id)

||
[HansN] in osafassert, isn't

... ||(reply_dest == cb->immnd_mdest_id) ||

redundant as

if (reply_dest && reply_dest != cb->immnd_mdest_id)

is checked before?

[Vu] The check should be there, otherwise assertion could get failed

when

reply_dest != 0 and isObjSync is FALSE.

   isObjSync);
if (cb->fevs_replies_pending) {





--
Check out 

[devel] [PATCH 1/1] imm: sPbeRtMutations is updated even when validation for duplicate values fails [#2422]

2018-03-15 Thread Danh Vo
When immnd receives event to update persistent runtime attribute
from immd, there is 2 loops: the first loop validates the change,
the second one updates the afim object. The root cause comes from
the check (attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) which is
inside the second loop instead of the first loop. So the
validation is still passed even when the attribute does not allow
duplicate value. As a result, sPbeRtMutations is updated in the
second loop.
---
 src/imm/immnd/ImmModel.cc | 65 ++-
 1 file changed, 36 insertions(+), 29 deletions(-)

diff --git a/src/imm/immnd/ImmModel.cc b/src/imm/immnd/ImmModel.cc
index c539fda..9f81c6b 100644
--- a/src/imm/immnd/ImmModel.cc
+++ b/src/imm/immnd/ImmModel.cc
@@ -18102,19 +18102,23 @@ SaAisErrorT ImmModel::rtObjectUpdate(
   err = SA_AIS_ERR_INVALID_PARAM;
   break;  // out of switch
 }
-if (doIt) {
-  osafassert(attrValue->isMultiValued());
-  ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue;
-  if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) &&
-  (multiattr->hasMatchingValue(tmpos))) {
-LOG_NO(
-"ERR_INVALID_PARAM: multivalued attr '%s' with 
NO_DUPLICATES "
-"yet duplicate values provided in rta-update call. 
Object:'%s'.",
-attrName.c_str(), objectName.c_str());
-err = SA_AIS_ERR_INVALID_PARAM;
-break;  // out of for switch
-  }
 
+osafassert(attrValue->isMultiValued());
+ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue;
+eduAtValToOs(, &(p->attrValue.attrValue),
+ 
(SaImmValueTypeT)p->attrValue.attrValueType);
+
+if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) &&
+(multiattr->hasMatchingValue(tmpos))) {
+  LOG_NO(
+  "ERR_INVALID_PARAM: multivalued attr '%s' with NO_DUPLICATES 
"
+  "yet duplicate values provided in rta-update call. 
Object:'%s'.",
+  attrName.c_str(), objectName.c_str());
+  err = SA_AIS_ERR_INVALID_PARAM;
+  break;  // out of for switch
+}
+
+if (doIt) {
   multiattr->setExtraValue(tmpos);
 }
   }
@@ -18128,27 +18132,30 @@ SaAisErrorT ImmModel::rtObjectUpdate(
   attrName.c_str());
   err = SA_AIS_ERR_INVALID_PARAM;
   break;  // out of switch
-} else if (doIt) {
-  osafassert(attrValue->isMultiValued());
-  ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue;
+}
 
-  IMMSV_EDU_ATTR_VAL_LIST* al = p->attrValue.attrMoreValues;
-  while (al) {
-eduAtValToOs(, &(al->n),
- (SaImmValueTypeT)p->attrValue.attrValueType);
-if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) &&
-(multiattr->hasMatchingValue(tmpos))) {
-  LOG_NO(
-  "ERR_INVALID_PARAM: multivalued attr '%s' with 
NO_DUPLICATES "
-  "yet duplicate values provided in rta-update call. 
Object:'%s'.",
-  attrName.c_str(), objectName.c_str());
-  err = SA_AIS_ERR_INVALID_PARAM;
-  break;  // out of loop
-}
+osafassert(attrValue->isMultiValued());
+ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue;
+IMMSV_EDU_ATTR_VAL_LIST* al = p->attrValue.attrMoreValues;
 
+while (al) {
+  eduAtValToOs(, &(al->n),
+  (SaImmValueTypeT)p->attrValue.attrValueType);
+  if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) &&
+  (multiattr->hasMatchingValue(tmpos))) {
+LOG_NO(
+"ERR_INVALID_PARAM: multivalued attr '%s' with 
NO_DUPLICATES "
+"yet duplicate values provided in rta-update call. 
Object:'%s'.",
+attrName.c_str(), objectName.c_str());
+err = SA_AIS_ERR_INVALID_PARAM;
+break;  // out of loop
+  }
+
+  if (doIt) {
 multiattr->setExtraValue(tmpos);
-al = al->next;
   }
+
+  al = al->next;
 }
   }
   break;
-- 
2.7.4


--
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


[devel] [PATCH 0/1] Review Request for imm: sPbeRtMutations is updated even when validation for duplicate values fails [#2422]

2018-03-15 Thread Danh Vo
Summary: imm: sPbeRtMutations is updated even when validation for duplicate 
values fails [#2422]
Review request for Ticket(s): 2422
Peer Reviewer(s): Ravi, Anders, Hans, Lennart, Vu
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2422
Base revision: 3d8e8a959e298ff438fe5a6e7cc02e2404305542
Personal repository: git://git.code.sf.net/u/zvoxdan/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesy
 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 fff1ace1d400ec5b56c391d0389f228032f00e7e
Author: Danh Vo 
Date:   Thu, 15 Mar 2018 17:16:56 +0700

imm: sPbeRtMutations is updated even when validation for duplicate values fails 
[#2422]

When immnd receives event to update persistent runtime attribute
from immd, there is 2 loops: the first loop validates the change,
the second one updates the afim object. The root cause comes from
the check (attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) which is
inside the second loop instead of the first loop. So the
validation is still passed even when the attribute does not allow
duplicate value. As a result, sPbeRtMutations is updated in the
second loop.



Complete diffstat:
--
 src/imm/immnd/ImmModel.cc | 65 ++-
 1 file changed, 36 insertions(+), 29 deletions(-)


Testing Commands:
-
Create Test class as below:
  
SA_CONFIG

test
SA_STRING_T
SA_CONFIG
SA_INITIALIZED


list
SA_UINT32_T
SA_RUNTIME
SA_CACHED
SA_PERSISTENT
SA_MULTI_VALUE
SA_NO_DUPLICATES

  
Create object test=1 by command:
  immcfg -c Test test=1
Create an implementer which has:
  - saImmOiRtAttrUpdateCallback returns SA_AIS_OK.
  - saImmOiAdminOperationCallback does saImmOiRtObjectUpdate_2() command to 
update "list" attribute as bellow sequence:
   + Add value=9 to 'list' attribute
   + Add value=9 to 'list' attribute
   + Add value=10 to 'list' attribute
Perform admin operation on test=1 by command:
  immadm -o 1 test=1
See the result if it is expected.


Testing, Expected Results:
--
2018-03-15 15:43:09.889 SC-1 2422-implemter: >> AdminOperationCallbackT_2
2018-03-15 15:43:09.890 SC-1 2422-implemter: Add 9 into "list" attribute
2018-03-15 15:43:10.001 SC-1 2422-implemter: saImmOiRtObjectUpdate_2 return 
code: 1
2018-03-15 15:43:10.001 SC-1 2422-implemter: Add 9 into "list" attribute
2018-03-15 15:43:10.004 SC-1 osafimmnd[207]: NO ERR_INVALID_PARAM: multivalued 
attr 'list' with NO_DUPLICATES yet duplicate values provided in rta-update 
call. Object:'test=1'.
2018-03-15 15:43:10.004 SC-1 osafimmnd[207]: WA Got error on non local rt 
object update err: 7
2018-03-15 15:43:10.004 SC-1 2422-implemter: saImmOiRtObjectUpdate_2 return 
code: 7
2018-03-15 15:43:10.004 SC-1 2422-implemter: Add 10 into "list" attribute
2018-03-15 15:43:10.050 SC-1 2422-implemter: saImmOiRtObjectUpdate_2 return 
code: 1
2018-03-15 15:43:10.051 SC-1 2422-implemter: << AdminOperationCallbackT_2


Conditions of Submission:
-
Ack from one of reviewers


Arch  Built StartedLinux distro
---
mipsn  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 

Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Vu Minh Nguyen
Hi Hans,

I am going to remove the assertion since the original one is now always true 
with #2794 fix. 

Do you have any concern with that?

Regards, Vu

> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Thursday, March 15, 2018 4:56 PM
> To: Vu Minh Nguyen 
> Cc: opensaf-devel@lists.sourceforge.net; ravisekhar.ko...@oracle.com;
> zoran.milinko...@ericsson.com; anders.wid...@ericsson.com;
> lennart.l...@ericsson.com
> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> configuration [#2794]
> 
> Hi Vu,
> 
> I guess the assert should be kept but remove the
> 
> reply_dest == cb->immnd_mdest_id
> 
> condtition, or?
> 
> /Regards HansN
> 
> 
> On 03/15/2018 10:46 AM, Vu Minh Nguyen wrote:
> > Hi Hans,
> >
> > You are right.  I will remove that assert. Thanks! 
> >
> > Regards, Vu
> >
> >> -Original Message-
> >> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> >> Sent: Thursday, March 15, 2018 4:34 PM
> >> To: Vu Minh Nguyen ;
> >> ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
> >> anders.wid...@ericsson.com; lennart.l...@ericsson.com
> >> Cc: opensaf-devel@lists.sourceforge.net
> >> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> >> configuration [#2794]
> >>
> >> Hi Vu,
> >>
> >> this osafassert will never assert, no matter what values reply_dest or
> >> isObjSync have,
> >>
> >> if reply_dest is 0 then osafassert(!reply_dest will be true.
> >>
> >>
> >> /Regards HansN
> >>
> >>
> >> On 03/15/2018 10:20 AM, Vu Minh Nguyen wrote:
> >>> Hi Hans,
> >>>
> >>> I don't think so. In some cases, reply_dest could be 0 and not equal to 
> >>> cb-
> >>> immd_mdest_id.
> >>>
> >>> Regards, Vu
> >>>
>  -Original Message-
>  From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
>  Sent: Thursday, March 15, 2018 4:10 PM
>  To: Vu Minh Nguyen ;
>  ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
>  anders.wid...@ericsson.com; lennart.l...@ericsson.com
>  Cc: opensaf-devel@lists.sourceforge.net
>  Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
>  configuration [#2794]
> 
>  Hi Vu,
> 
>  not a big deal, but you are wrong, as I mentioned, the osafassert is now
>  a nop as
> 
>  "reply_dest == cb->immnd_mdest_id" are always true at the osafassert
> >> stmt.
>  /Regards HansN
> 
> 
> 
> 
>  On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote:
> > Hi Hans,
> >
> > See my response inline.
> >
> > Regards, Vu
> >
> >> -Original Message-
> >> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> >> Sent: Tuesday, March 13, 2018 9:10 PM
> >> To: Vu Minh Nguyen ;
> >> ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
> >> anders.wid...@ericsson.com; lennart.l...@ericsson.com
> >> Cc: opensaf-devel@lists.sourceforge.net
> >> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> >> configuration [#2794]
> >>
> >> ack, review only. One question below.
> >>
> >> /Thanks HansN
> >>
> >>
> >> On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:
> >>> When IMMND restarts (e.g: OUT OF ORDER detection), it may get
>  message
> >>> from active IMMD which is originated from just-dead IMMND
> process.
> >>> In such case, we are in confused situation - messages come from
> >>> local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!
> >>>
> >>> This patch discards such messages, notify the case to syslog
> >>> instead of aborting the IMMND progress.
> >>> ---
> >>>  src/imm/immnd/immnd_evt.c | 12 
> >>>  1 file changed, 12 insertions(+)
> >>>
> >>> diff --git a/src/imm/immnd/immnd_evt.c
> >> b/src/imm/immnd/immnd_evt.c
> >>> index 228b7dd..43611a3 100644
> >>> --- a/src/imm/immnd/immnd_evt.c
> >>> +++ b/src/imm/immnd/immnd_evt.c
> >>> @@ -10766,6 +10766,18 @@ static uint32_t
> >> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> >>>   (m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb-
> >>> node_id);
> >>>   if (originatedAtThisNd) {
> >>> + /* Get the message comes from local IMMND but not
> >> me
> >>> +(cb->immnd_mdest_id). Probably IMMND just restarts
> >>> +(e.g: OUT OF ORDER detection), and this message
> >> belongs
> >>> +to previous (dead) IMMND. So, discard this message.
> >>> +  */
> >>> + if (reply_dest && reply_dest != cb->immnd_mdest_id) {
> >>> + LOG_WA("DISCARD FEVS message sent by
> >> previous dead
> >> IMMND");
> >>> + dequeue_outgoing(cb);
> >>> + TRACE_LEAVE();
> >>> +   

Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Hans Nordebäck

Hi Vu,

I guess the assert should be kept but remove the

reply_dest == cb->immnd_mdest_id

condtition, or?

/Regards HansN


On 03/15/2018 10:46 AM, Vu Minh Nguyen wrote:

Hi Hans,

You are right.  I will remove that assert. Thanks! 

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Thursday, March 15, 2018 4:34 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

Hi Vu,

this osafassert will never assert, no matter what values reply_dest or
isObjSync have,

if reply_dest is 0 then osafassert(!reply_dest will be true.


/Regards HansN


On 03/15/2018 10:20 AM, Vu Minh Nguyen wrote:

Hi Hans,

I don't think so. In some cases, reply_dest could be 0 and not equal to cb-
immd_mdest_id.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Thursday, March 15, 2018 4:10 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

Hi Vu,

not a big deal, but you are wrong, as I mentioned, the osafassert is now
a nop as

"reply_dest == cb->immnd_mdest_id" are always true at the osafassert

stmt.

/Regards HansN




On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote:

Hi Hans,

See my response inline.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Tuesday, March 13, 2018 9:10 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

ack, review only. One question below.

/Thanks HansN


On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:

When IMMND restarts (e.g: OUT OF ORDER detection), it may get

message

from active IMMD which is originated from just-dead IMMND process.
In such case, we are in confused situation - messages come from
local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!

This patch discards such messages, notify the case to syslog
instead of aborting the IMMND progress.
---
 src/imm/immnd/immnd_evt.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/imm/immnd/immnd_evt.c

b/src/imm/immnd/immnd_evt.c

index 228b7dd..43611a3 100644
--- a/src/imm/immnd/immnd_evt.c
+++ b/src/imm/immnd/immnd_evt.c
@@ -10766,6 +10766,18 @@ static uint32_t

immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,

(m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb-

node_id);

if (originatedAtThisNd) {
+   /* Get the message comes from local IMMND but not

me

+  (cb->immnd_mdest_id). Probably IMMND just restarts
+  (e.g: OUT OF ORDER detection), and this message

belongs

+  to previous (dead) IMMND. So, discard this message.
+*/
+   if (reply_dest && reply_dest != cb->immnd_mdest_id) {
+   LOG_WA("DISCARD FEVS message sent by

previous dead

IMMND");

+   dequeue_outgoing(cb);
+   TRACE_LEAVE();
+   return NCSCC_RC_SUCCESS;
+   }
+
osafassert(!reply_dest || (reply_dest == cb-

immnd_mdest_id)

||
[HansN] in osafassert, isn't

... ||(reply_dest == cb->immnd_mdest_id) ||

redundant as

if (reply_dest && reply_dest != cb->immnd_mdest_id)

is checked before?

[Vu] The check should be there, otherwise assertion could get failed when

reply_dest != 0 and isObjSync is FALSE.

   isObjSync);
if (cb->fevs_replies_pending) {





--
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


Re: [devel] [PATCH 1/1] fmd: avoid conflict with split-brain prevention if two nodes are elected [#2801]

2018-03-15 Thread Anders Widell

Ack.

regards,

Anders Widell


On 03/09/2018 06:57 AM, Gary Lee wrote:

If we have a 'tied election' and split-brain prevention is enabled,
then the 'old active' is fenced, or the 'old active' will self-reboot
when it is notified a new node is active.

We need to disable this redundant check in fmd. Otherwise, the 'new active'
will also reboot, along with the 'old active'.
---
  src/fm/fmd/fm_main.cc | 19 ---
  1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/fm/fmd/fm_main.cc b/src/fm/fmd/fm_main.cc
index 1244c2347..73c9b9ccd 100644
--- a/src/fm/fmd/fm_main.cc
+++ b/src/fm/fmd/fm_main.cc
@@ -600,13 +600,18 @@ static void fm_mbx_msg_handler(FM_CB *fm_cb, FM_EVT 
*fm_mbx_evt) {
 * progress of shutdown (i.e., amfd/immd is still alive).
 */
if ((fm_cb->role == PCS_RDA_ACTIVE) && (fm_cb->csi_assigned == false)) {
-LOG_WA(
-"Two active controllers observed in a cluster, newActive: %x and "
-"old-Active: %x",
-unsigned(fm_cb->node_id), unsigned(fm_cb->peer_node_id));
-opensaf_reboot(0, NULL,
-   "Received svc up from peer node (old-active is not "
-   "fully DOWN), hence rebooting the new Active");
+Consensus consensus_service;
+if (consensus_service.IsEnabled() == false) {
+  // If split-brain prevention is enabled, then the 'old active' has
+  // already initiated a self-reboot, or it is fenced.
+  LOG_WA(
+  "Two active controllers observed in a cluster, newActive: %x and 
"
+  "old-Active: %x",
+  unsigned(fm_cb->node_id), unsigned(fm_cb->peer_node_id));
+  opensaf_reboot(0, NULL,
+ "Received svc up from peer node (old-active is not "
+ "fully DOWN), hence rebooting the new Active");
+}
}
  
/* Peer fm came up so sending ee_id of this node */



--
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


Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Vu Minh Nguyen
Hi Hans,

You are right.  I will remove that assert. Thanks! 

Regards, Vu

> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Thursday, March 15, 2018 4:34 PM
> To: Vu Minh Nguyen ;
> ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
> anders.wid...@ericsson.com; lennart.l...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> configuration [#2794]
> 
> Hi Vu,
> 
> this osafassert will never assert, no matter what values reply_dest or
> isObjSync have,
> 
> if reply_dest is 0 then osafassert(!reply_dest will be true.
> 
> 
> /Regards HansN
> 
> 
> On 03/15/2018 10:20 AM, Vu Minh Nguyen wrote:
> > Hi Hans,
> >
> > I don't think so. In some cases, reply_dest could be 0 and not equal to cb-
> >immd_mdest_id.
> >
> > Regards, Vu
> >
> >> -Original Message-
> >> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> >> Sent: Thursday, March 15, 2018 4:10 PM
> >> To: Vu Minh Nguyen ;
> >> ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
> >> anders.wid...@ericsson.com; lennart.l...@ericsson.com
> >> Cc: opensaf-devel@lists.sourceforge.net
> >> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> >> configuration [#2794]
> >>
> >> Hi Vu,
> >>
> >> not a big deal, but you are wrong, as I mentioned, the osafassert is now
> >> a nop as
> >>
> >> "reply_dest == cb->immnd_mdest_id" are always true at the osafassert
> stmt.
> >>
> >> /Regards HansN
> >>
> >>
> >>
> >>
> >> On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote:
> >>> Hi Hans,
> >>>
> >>> See my response inline.
> >>>
> >>> Regards, Vu
> >>>
>  -Original Message-
>  From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
>  Sent: Tuesday, March 13, 2018 9:10 PM
>  To: Vu Minh Nguyen ;
>  ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
>  anders.wid...@ericsson.com; lennart.l...@ericsson.com
>  Cc: opensaf-devel@lists.sourceforge.net
>  Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
>  configuration [#2794]
> 
>  ack, review only. One question below.
> 
>  /Thanks HansN
> 
> 
>  On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:
> > When IMMND restarts (e.g: OUT OF ORDER detection), it may get
> >> message
> > from active IMMD which is originated from just-dead IMMND process.
> > In such case, we are in confused situation - messages come from
> > local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!
> >
> > This patch discards such messages, notify the case to syslog
> > instead of aborting the IMMND progress.
> > ---
> > src/imm/immnd/immnd_evt.c | 12 
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/src/imm/immnd/immnd_evt.c
> b/src/imm/immnd/immnd_evt.c
> > index 228b7dd..43611a3 100644
> > --- a/src/imm/immnd/immnd_evt.c
> > +++ b/src/imm/immnd/immnd_evt.c
> > @@ -10766,6 +10766,18 @@ static uint32_t
>  immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> > (m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb-
> >node_id);
> >
> > if (originatedAtThisNd) {
> > +   /* Get the message comes from local IMMND but not
> me
> > +  (cb->immnd_mdest_id). Probably IMMND just restarts
> > +  (e.g: OUT OF ORDER detection), and this message
> belongs
> > +  to previous (dead) IMMND. So, discard this message.
> > +*/
> > +   if (reply_dest && reply_dest != cb->immnd_mdest_id) {
> > +   LOG_WA("DISCARD FEVS message sent by
> previous dead
>  IMMND");
> > +   dequeue_outgoing(cb);
> > +   TRACE_LEAVE();
> > +   return NCSCC_RC_SUCCESS;
> > +   }
> > +
> > osafassert(!reply_dest || (reply_dest == cb-
> >immnd_mdest_id)
>  ||
>  [HansN] in osafassert, isn't
> 
>  ... ||(reply_dest == cb->immnd_mdest_id) ||
> 
>  redundant as
> 
>  if (reply_dest && reply_dest != cb->immnd_mdest_id)
> 
>  is checked before?
> >>> [Vu] The check should be there, otherwise assertion could get failed when
> >> reply_dest != 0 and isObjSync is FALSE.
> >isObjSync);
> > if (cb->fevs_replies_pending) {
> >



--
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


Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Hans Nordebäck

Hi Vu,

this osafassert will never assert, no matter what values reply_dest or 
isObjSync have,


if reply_dest is 0 then osafassert(!reply_dest will be true.


/Regards HansN


On 03/15/2018 10:20 AM, Vu Minh Nguyen wrote:

Hi Hans,

I don't think so. In some cases, reply_dest could be 0 and not equal to 
cb->immd_mdest_id.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Thursday, March 15, 2018 4:10 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

Hi Vu,

not a big deal, but you are wrong, as I mentioned, the osafassert is now
a nop as

"reply_dest == cb->immnd_mdest_id" are always true at the osafassert stmt.

/Regards HansN




On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote:

Hi Hans,

See my response inline.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Tuesday, March 13, 2018 9:10 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

ack, review only. One question below.

/Thanks HansN


On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:

When IMMND restarts (e.g: OUT OF ORDER detection), it may get

message

from active IMMD which is originated from just-dead IMMND process.
In such case, we are in confused situation - messages come from
local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!

This patch discards such messages, notify the case to syslog
instead of aborting the IMMND progress.
---
src/imm/immnd/immnd_evt.c | 12 
1 file changed, 12 insertions(+)

diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c
index 228b7dd..43611a3 100644
--- a/src/imm/immnd/immnd_evt.c
+++ b/src/imm/immnd/immnd_evt.c
@@ -10766,6 +10766,18 @@ static uint32_t

immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,

(m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb->node_id);

if (originatedAtThisNd) {
+   /* Get the message comes from local IMMND but not me
+  (cb->immnd_mdest_id). Probably IMMND just restarts
+  (e.g: OUT OF ORDER detection), and this message belongs
+  to previous (dead) IMMND. So, discard this message.
+*/
+   if (reply_dest && reply_dest != cb->immnd_mdest_id) {
+   LOG_WA("DISCARD FEVS message sent by previous dead

IMMND");

+   dequeue_outgoing(cb);
+   TRACE_LEAVE();
+   return NCSCC_RC_SUCCESS;
+   }
+
osafassert(!reply_dest || (reply_dest == cb->immnd_mdest_id)

||
[HansN] in osafassert, isn't

... ||(reply_dest == cb->immnd_mdest_id) ||

redundant as

if (reply_dest && reply_dest != cb->immnd_mdest_id)

is checked before?

[Vu] The check should be there, otherwise assertion could get failed when

reply_dest != 0 and isObjSync is FALSE.

   isObjSync);
if (cb->fevs_replies_pending) {





--
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


Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Vu Minh Nguyen
Hi Hans,

I don't think so. In some cases, reply_dest could be 0 and not equal to 
cb->immd_mdest_id.

Regards, Vu

> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Thursday, March 15, 2018 4:10 PM
> To: Vu Minh Nguyen ;
> ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
> anders.wid...@ericsson.com; lennart.l...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> configuration [#2794]
> 
> Hi Vu,
> 
> not a big deal, but you are wrong, as I mentioned, the osafassert is now
> a nop as
> 
> "reply_dest == cb->immnd_mdest_id" are always true at the osafassert stmt.
> 
> /Regards HansN
> 
> 
> 
> 
> On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote:
> > Hi Hans,
> >
> > See my response inline.
> >
> > Regards, Vu
> >
> >> -Original Message-
> >> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> >> Sent: Tuesday, March 13, 2018 9:10 PM
> >> To: Vu Minh Nguyen ;
> >> ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
> >> anders.wid...@ericsson.com; lennart.l...@ericsson.com
> >> Cc: opensaf-devel@lists.sourceforge.net
> >> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> >> configuration [#2794]
> >>
> >> ack, review only. One question below.
> >>
> >> /Thanks HansN
> >>
> >>
> >> On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:
> >>> When IMMND restarts (e.g: OUT OF ORDER detection), it may get
> message
> >>> from active IMMD which is originated from just-dead IMMND process.
> >>> In such case, we are in confused situation - messages come from
> >>> local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!
> >>>
> >>> This patch discards such messages, notify the case to syslog
> >>> instead of aborting the IMMND progress.
> >>> ---
> >>>src/imm/immnd/immnd_evt.c | 12 
> >>>1 file changed, 12 insertions(+)
> >>>
> >>> diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c
> >>> index 228b7dd..43611a3 100644
> >>> --- a/src/imm/immnd/immnd_evt.c
> >>> +++ b/src/imm/immnd/immnd_evt.c
> >>> @@ -10766,6 +10766,18 @@ static uint32_t
> >> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> >>>   (m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb->node_id);
> >>>
> >>>   if (originatedAtThisNd) {
> >>> + /* Get the message comes from local IMMND but not me
> >>> +(cb->immnd_mdest_id). Probably IMMND just restarts
> >>> +(e.g: OUT OF ORDER detection), and this message belongs
> >>> +to previous (dead) IMMND. So, discard this message.
> >>> +  */
> >>> + if (reply_dest && reply_dest != cb->immnd_mdest_id) {
> >>> + LOG_WA("DISCARD FEVS message sent by previous dead
> >> IMMND");
> >>> + dequeue_outgoing(cb);
> >>> + TRACE_LEAVE();
> >>> + return NCSCC_RC_SUCCESS;
> >>> + }
> >>> +
> >>>   osafassert(!reply_dest || (reply_dest == 
> >>> cb->immnd_mdest_id)
> >> ||
> >> [HansN] in osafassert, isn't
> >>
> >> ... ||(reply_dest == cb->immnd_mdest_id) ||
> >>
> >> redundant as
> >>
> >> if (reply_dest && reply_dest != cb->immnd_mdest_id)
> >>
> >> is checked before?
> > [Vu] The check should be there, otherwise assertion could get failed when
> reply_dest != 0 and isObjSync is FALSE.
> >
> >>>  isObjSync);
> >>>   if (cb->fevs_replies_pending) {
> >



--
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


Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Hans Nordebäck

Hi Vu,

not a big deal, but you are wrong, as I mentioned, the osafassert is now 
a nop as


"reply_dest == cb->immnd_mdest_id" are always true at the osafassert stmt.

/Regards HansN




On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote:

Hi Hans,

See my response inline.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Tuesday, March 13, 2018 9:10 PM
To: Vu Minh Nguyen ;
ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
anders.wid...@ericsson.com; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
configuration [#2794]

ack, review only. One question below.

/Thanks HansN


On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:

When IMMND restarts (e.g: OUT OF ORDER detection), it may get message
from active IMMD which is originated from just-dead IMMND process.
In such case, we are in confused situation - messages come from
local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!

This patch discards such messages, notify the case to syslog
instead of aborting the IMMND progress.
---
   src/imm/immnd/immnd_evt.c | 12 
   1 file changed, 12 insertions(+)

diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c
index 228b7dd..43611a3 100644
--- a/src/imm/immnd/immnd_evt.c
+++ b/src/imm/immnd/immnd_evt.c
@@ -10766,6 +10766,18 @@ static uint32_t

immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,

(m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb->node_id);

if (originatedAtThisNd) {
+   /* Get the message comes from local IMMND but not me
+  (cb->immnd_mdest_id). Probably IMMND just restarts
+  (e.g: OUT OF ORDER detection), and this message belongs
+  to previous (dead) IMMND. So, discard this message.
+*/
+   if (reply_dest && reply_dest != cb->immnd_mdest_id) {
+   LOG_WA("DISCARD FEVS message sent by previous dead

IMMND");

+   dequeue_outgoing(cb);
+   TRACE_LEAVE();
+   return NCSCC_RC_SUCCESS;
+   }
+
osafassert(!reply_dest || (reply_dest == cb->immnd_mdest_id)

||
[HansN] in osafassert, isn't

... ||(reply_dest == cb->immnd_mdest_id) ||

redundant as

if (reply_dest && reply_dest != cb->immnd_mdest_id)

is checked before?

[Vu] The check should be there, otherwise assertion could get failed when 
reply_dest != 0 and isObjSync is FALSE.


   isObjSync);
if (cb->fevs_replies_pending) {





--
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


Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Vu Minh Nguyen
Hi Hans,

See my response inline.

Regards, Vu

> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Tuesday, March 13, 2018 9:10 PM
> To: Vu Minh Nguyen ;
> ravisekhar.ko...@oracle.com; zoran.milinko...@ericsson.com;
> anders.wid...@ericsson.com; lennart.l...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large
> configuration [#2794]
> 
> ack, review only. One question below.
> 
> /Thanks HansN
> 
> 
> On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote:
> > When IMMND restarts (e.g: OUT OF ORDER detection), it may get message
> > from active IMMD which is originated from just-dead IMMND process.
> > In such case, we are in confused situation - messages come from
> > local IMMND, but not me (reply_dest != cb->immnd_mdest_id)!
> >
> > This patch discards such messages, notify the case to syslog
> > instead of aborting the IMMND progress.
> > ---
> >   src/imm/immnd/immnd_evt.c | 12 
> >   1 file changed, 12 insertions(+)
> >
> > diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c
> > index 228b7dd..43611a3 100644
> > --- a/src/imm/immnd/immnd_evt.c
> > +++ b/src/imm/immnd/immnd_evt.c
> > @@ -10766,6 +10766,18 @@ static uint32_t
> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> > (m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb->node_id);
> >
> > if (originatedAtThisNd) {
> > +   /* Get the message comes from local IMMND but not me
> > +  (cb->immnd_mdest_id). Probably IMMND just restarts
> > +  (e.g: OUT OF ORDER detection), and this message belongs
> > +  to previous (dead) IMMND. So, discard this message.
> > +*/
> > +   if (reply_dest && reply_dest != cb->immnd_mdest_id) {
> > +   LOG_WA("DISCARD FEVS message sent by previous dead
> IMMND");
> > +   dequeue_outgoing(cb);
> > +   TRACE_LEAVE();
> > +   return NCSCC_RC_SUCCESS;
> > +   }
> > +
> > osafassert(!reply_dest || (reply_dest == cb->immnd_mdest_id)
> ||
> [HansN] in osafassert, isn't
> 
> ... ||(reply_dest == cb->immnd_mdest_id) ||
> 
> redundant as
> 
> if (reply_dest && reply_dest != cb->immnd_mdest_id)
> 
> is checked before?
[Vu] The check should be there, otherwise assertion could get failed when 
reply_dest != 0 and isObjSync is FALSE.

> >isObjSync);
> > if (cb->fevs_replies_pending) {



--
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


[devel] [PATCH 0/1] Review Request for base: Only close inherited fd(s) after fork() in child process [#2805]

2018-03-15 Thread Minh Chau
Summary: base: Only close inherited fd(s) after fork() in child process [#2805]
Review request for Ticket(s): 2805
Peer Reviewer(s): Anders, Hans, and Ravi
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2805
Base revision: c26df46a7e3c42fab714b8579614d5ace15de441
Personal repository: git://git.code.sf.net/u/minh-chau/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesn
 Core libraries  y
 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 e4854e5708327b6596a37ac9962fea18e7499c24
Author: Minh Chau 
Date:   Thu, 15 Mar 2018 17:41:27 +1100

base: Only close inherited fd(s) after fork() in child process [#2805]



Complete diffstat:
--
 src/base/os_defs.c | 25 -
 1 file changed, 20 insertions(+), 5 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 StartedLinux distro
---
mipsn  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


Re: [devel] [PATCH 1/1] imm: coredump during scale-in on large configuration [#2794]

2018-03-15 Thread Ravi Sekhar Reddy Konda
Hi Vu,

Ack for the patch, code review only

Thanks,
Ravi
-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] 
Sent: Monday, March 12, 2018 12:43 PM
To: ravisekhar.ko...@oracle.com; hans.nordeb...@ericsson.com; 
zoran.milinko...@ericsson.com; anders.wid...@ericsson.com; 
lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen 

Subject: [PATCH 1/1] imm: coredump during scale-in on large configuration 
[#2794]

When IMMND restarts (e.g: OUT OF ORDER detection), it may get message from 
active IMMD which is originated from just-dead IMMND process.
In such case, we are in confused situation - messages come from local IMMND, 
but not me (reply_dest != cb->immnd_mdest_id)!

This patch discards such messages, notify the case to syslog instead of 
aborting the IMMND progress.
---
 src/imm/immnd/immnd_evt.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c index 
228b7dd..43611a3 100644
--- a/src/imm/immnd/immnd_evt.c
+++ b/src/imm/immnd/immnd_evt.c
@@ -10766,6 +10766,18 @@ static uint32_t immnd_evt_proc_fevs_rcv(IMMND_CB *cb, 
IMMND_EVT *evt,
(m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb->node_id);
 
if (originatedAtThisNd) {
+   /* Get the message comes from local IMMND but not me
+  (cb->immnd_mdest_id). Probably IMMND just restarts
+  (e.g: OUT OF ORDER detection), and this message belongs
+  to previous (dead) IMMND. So, discard this message.
+*/
+   if (reply_dest && reply_dest != cb->immnd_mdest_id) {
+   LOG_WA("DISCARD FEVS message sent by previous dead 
IMMND");
+   dequeue_outgoing(cb);
+   TRACE_LEAVE();
+   return NCSCC_RC_SUCCESS;
+   }
+
osafassert(!reply_dest || (reply_dest == cb->immnd_mdest_id) ||
   isObjSync);
if (cb->fevs_replies_pending) {
--
1.9.1

--
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