Re: [devel] [PATCH 1 of 1] amfnd: avoid null pointer access [#2213]

2017-03-06 Thread Hans Nordebäck
Ack, code review only/Thanks HansN -Original Message- From: nagendr...@oracle.com [mailto:nagendr...@oracle.com] Sent: den 7 mars 2017 07:31 To: Hans Nordebäck ; praveen.malv...@oracle.com; Minh Hon Chau ; Gary Lee Cc: opensaf-devel@lists.sourceforge.net Subject: [PATCH 1 of 1] amfnd

Re: [devel] [PATCH 1 of 1] nid: OpenSAF failed to start on controller randomly [#2346]

2017-03-07 Thread Hans Nordebäck
Hi Ramesh, I only increased the tolerance time, assuming that the time to start transportd in this case is longer than usual. /Regards HansN From: ramesh betham [mailto:ramesh.bet...@oracle.com] Sent: den 7 mars 2017 11:55 To: Hans Nordebäck Cc: Anders Widell ; opensaf-devel

Re: [devel] [PATCH 1 of 1] CLM : Cluster reset does not succed as reboot now command fails on SLES [#2339]

2017-03-07 Thread Hans Nordebäck
thanks Anders I'll update the patch/BR Hans On 03/07/2017 03:06 PM, Anders Widell wrote: > One more comment: the syslog message on the line before the shutdown > command also has to be updated. > > regards, > > Anders Widell > > > On 03/07/2017 03:03 PM, Anders Widell wrote: >> Ack with comment:

Re: [devel] [PATCH 1 of 1] mds: reduce log level to Err for TIPC_ERR_OVERLOAD in syslog [#2350]

2017-03-09 Thread Hans Nordebäck
that fails due to this change. /Regards Hans -Original Message- From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] Sent: den 7 mars 2017 07:57 To: mahesh.va...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] mds: reduce log level to Err for

Re: [devel] [PATCH 1 of 1] mds: reduce log level to Err for TIPC_ERR_OVERLOAD in syslog [#2350]

2017-03-09 Thread Hans Nordebäck
Hi Mahesh, Yes, I think 'TIPC_RETDATA' can have INFO, and TIPC_ERR_OVERLOAD as is, i.e. ERROR. /Regards Hans -Original Message- From: A V Mahesh [mailto:mahesh.va...@oracle.com] Sent: den 10 mars 2017 04:52 To: Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net Subject:

Re: [devel] [PATCH 1 of 1] CLM : Cluster reset does not succed as reboot now command fails on SLES [#2339]

2017-03-13 Thread Hans Nordebäck
Hi Ramesh, A gentle reminder for review/Thanks HansN -Original Message- From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] Sent: den 6 mars 2017 16:28 To: ramesh.bet...@oracle.com; Anders Widell Cc: opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 1 of 1] CLM : Cluster

Re: [devel] [PATCH 1 of 1] base: Improve trace by using tid instead of pid [#2370]

2017-03-15 Thread Hans Nordebäck
Hi Ramesh, reading the trace files with tid instead of pid will make it much easier to follow each threads flow of control. Tools such as trace2dot should also benefit. Example: : Mar 15 10:57:54.837368 osafamfd [475:src/imm/agent/imma_oi_api.cc:2960] TR attr:safCSIComp Mar 15 10:57:54.837

Re: [devel] [PATCH 1 of 1] CLM : Cluster reset does not succed as reboot now command fails on SLES [#2339]

2017-03-29 Thread Hans Nordebäck
Hi Ramesh, Is it ok to push this patch? It is acked by AndersW /Thanks HansN -Original Message- From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] Sent: den 13 mars 2017 14:37 To: ramesh.bet...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of

Re: [devel] [PATCH 1/1] samples: fix $piddir undefined in amf_demo_script [#2410]

2017-04-12 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 04/12/2017 02:36 PM, Nguyen Luu wrote: > The $piddir variable (containing path to amf_demo comp's pid file) > is missed to be defined in amf_demo_script. > > This could lead to the amf_demo process not getting truely killed > in some cases when cleanup is cal

Re: [devel] [PATCH 1 of 1] amfd: disallow delete of CtCs object if Ct maps to comp [#2428]

2017-04-18 Thread Hans Nordebäck
Hi Nagu, can you please use the new git submit for review. /Thanks HansN On 04/14/2017 03:22 PM, nagendr...@oracle.com wrote: > osaf/services/saf/amf/amfd/ctcstype.cc | 60 > - > 1 files changed, 58 insertions(+), 2 deletions(-) > > > diff --git a/osaf/servic

Re: [devel] [PATCH 1 of 1] amfd: disallow delete of CtCs object if Ct maps to comp [#2428]

2017-04-18 Thread Hans Nordebäck
Hi Nagu, Is the patch only valid for 5.1? /Regards Hans -Original Message- From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] Sent: den 18 april 2017 14:49 To: nagendr...@oracle.com; praveen.malv...@oracle.com; Minh Hon Chau ; Gary Lee Cc: opensaf-devel@lists.sourceforge.net

Re: [devel] [PATCH 1 of 1] amfd: disallow delete of CtCs object if Ct maps to comp [#2428]

2017-04-18 Thread Hans Nordebäck
Hi Nagu, there are some compiler errors related to TRACE, osaf_extended_name_borrow() is missing for SaNameT and c_str() for std::string. /Thanks Hans On 04/18/2017 03:04 PM, Hans Nordebäck wrote: > Hi Nagu, > > Is the patch only valid for 5.1? /Regards Hans > > -O

Re: [devel] [PATCH 1/1] base: Make pid file safe to read by rename it from temporary created file [#2432]

2017-05-02 Thread Hans Nordebäck
Hi Minh, some minor comments below, I discussed with Anders about using the existing flock instead of the rename. /BR HansN On 04/28/2017 11:36 AM, Minh Chau wrote: > At startup, osaftransportd waits for osafdtmd.pid file creation > and then reads dtm pid. If osafdtmd.pid has not been complete

Re: [devel] [PATCH 1/1] base: Make pid file safe to read by rename it from temporary created file [#2432]

2017-05-02 Thread Hans Nordebäck
Hi Minh, You can ignore my comment on pid in temp filename below. /Thanks HansN -Original Message- From: Hans Nordebäck Sent: den 2 maj 2017 15:11 To: Minh Hon Chau ; Anders Widell ; ramesh.bet...@oracle.com Cc: opensaf-devel@lists.sourceforge.net; Hans Nordebäck Subject: Re: [PATCH

Re: [devel] [PATCH 1/1] base: Make pid file safe to read by rename it from temporary created file [#2432]

2017-05-02 Thread Hans Nordebäck
another way because then we can have problems interacting with other >> programs that operate on the pid files (Ubuntu start-stop-daemon and >> LSB start_daemon). Maybe the solution is as simple as using flock() >> when reading the pid file? >> >> regards, >&g

Re: [devel] [PATCH 1/1] base: Improve state report for opensafd [#2459]

2017-05-17 Thread Hans Nordebäck
Hi Rafael, not sure if checking status of OpenSAF before ordering a reboot will have any effect as the result of the call will only be momentarily accurate. Is the problem this ticket address when SMF is ordering a reboot of a not fully started OpenSAF, and the "opensafd start/stop already in

Re: [devel] [PATCH 1/1] amfd: do not assert unnecessarily [#2458]

2017-05-17 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 05/11/2017 11:38 AM, Gary Lee wrote: > IMM APIs can fail if immnd finishes shutting down before amfd. > amfd should not unnecessarily assert and cause core dumps > to be created. > --- > src/amf/amfd/app.cc | 7 +-- > src/amf/amfd/comp.cc | 7 +--

Re: [devel] [PATCH 1/1] amfd: only increment su_cnt_admin_oper for non-opensaf SUs [#2466]

2017-05-23 Thread Hans Nordebäck
Ack, code review only/Thanks HansN -Original Message- From: Gary Lee [mailto:gary@dektech.com.au] Sent: den 24 maj 2017 02:52 To: praveen malviya ; Hans Nordebäck ; Minh Hon Chau ; nagendr...@oracle.com Cc: opensaf-devel@lists.sourceforge.net; Gary Lee Subject: [PATCH 1/1] amfd

Re: [devel] [PATCH 1/1] base: Improve state report for opensafd [#2459]

2017-05-31 Thread Hans Nordebäck
Hi Rafael, not tested, but perhaps we can do this check instead?: status() { amfpid=`pidofproc -p $amfnd_pid $amfnd_bin` if [[ -n "$amfpid" && ! -f "$lockfile_inprogress" ]; then amf-state siass ha RETVAL=$? else echo "The OpenSAF HA Framework is not run

Re: [devel] [PATCH 1/1] base: Improve state report for opensafd [#2459]

2017-05-31 Thread Hans Nordebäck
NBC. But as this script is pretty central to OpenSAF I am thinḱing NBC route would be better. On 05/31/2017 12:42 PM, Hans Nordebäck wrote: Hi Rafael, not tested, but perhaps we can do this check instead?: status() { amfpid=`pidofproc -p $amfnd_pid $amfnd_bin` if [[ -n &qu

Re: [devel] [PATCH 1/1] base: Improve state report for opensafd [#2459]

2017-06-01 Thread Hans Nordebäck
" opensafd but from services that can not handle a abrupt reboot." Which services are you referring to? The system may be abrubtly rebooted at any time. /Hans -Original Message- From: Rafael Odzakow Sent: den 1 juni 2017 10:41 To: Hans Nordebäck ; Anders Widell Cc: opens

Re: [devel] [PATCH 1/1] base: Try again for opensafd stop [#2459]

2017-06-01 Thread Hans Nordebäck
ack, code review only. Minor comments, in current opensafd script, the test and create of the lockfile is not atomic, so there is a window for race. Perhaps we can move the creation of the lockfile to function "wait_for_lockfile_clear", and make the test and create of the lockfile atomic?

Re: [devel] [PATCH 1/1] base: Try again for opensafd stop [#2459]

2017-06-01 Thread Hans Nordebäck
touch $lockfile_inprogress return 0 fi } /Hans On 06/02/2017 07:27 AM, Hans Nordebäck wrote: ack, code review only. Minor comments, in current opensafd script, the test and create of the lockfile is not atomic, so there is a window for race. Perhaps we can move the creation of the lockfile

Re: [devel] [PATCH 1/1] base: Atomically create pid file from temporary pid file V2 [#2432]

2017-06-02 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 05/25/2017 06:52 AM, Minh Chau wrote: At startup, osaftransportd waits for osafdtmd.pid file creation and then reads dtm pid. If osafdtmd.pid has not been completedly created but osaftransportd still receives IN_CREATE, osaftransported will fail to read pid

Re: [devel] [PATCH 3/3] amfnd: Refactor AVND_COMP for simpler cmd argument handling V2 [#1945]

2017-06-13 Thread Hans Nordebäck
Hi Praveen, The intention with V2 was to correct the case when a space is needed in the argument, but as you say it is not backward compatible. V1 of this patch is backward compatible, I'll put it back and send it out for a new review. /Thanks HansN On 06/13/2017 08:53 AM, praveen malviya

Re: [devel] [PATCH 1/3] amfnd: Refactor AVND_COMP for simpler environment variable handling [#1945]

2017-06-13 Thread Hans Nordebäck
Hi Praveen, good catch, I'll update. /Thanks HansN On 06/13/2017 08:53 AM, praveen malviya wrote: Hi Hans, One comment on this patch inline with [Praveen]. Thanks, Praveen On 18-May-17 3:32 PM, Hans Nordeback wrote: --- src/amf/amfnd/avnd_comp.h | 134 ++--

Re: [devel] [PATCH 1/3] amfnd: Refactor AVND_COMP for simpler environment variable handling [#1945]

2017-06-14 Thread Hans Nordebäck
Hi Praveen, thanks for reviewing, for info, the patch series also implies the following improvements: avnd_comp_clc_cmd_execute after refactoring: cyclomatic complexity reduced from 23 to 19 (-17%) # stmts reduced from 150 to 98 (-35%) # lines in function reduced from 259 to 184 (-29%) /Thank

Re: [devel] [PATCH 1/1] ntfd: Ensure mutex is not taken after cnsurvail_thread is canceled [#2508]

2017-06-27 Thread Hans Nordebäck
Hi Minh, Is trace enabled when this problem occurs? If so trace adds cancellation points, e.g. at fork/exec in create_imcnprocess. In stop_ntfimcn, pthread_join the join_ret variable is not used, use NULL instead, looking at the backtrace in the ticket, it's value is -1. /BR Hans -Origin

Re: [devel] [PATCH 1/1] ntfd: Ensure mutex is not taken after cnsurvail_thread is canceled [#2508]

2017-06-27 Thread Hans Nordebäck
Hi Minh, Perhaps you can add a new syslog, in stop_ntfimcn before the osaf_abort, add printout of lock, count and owner from the ntfimcn_mutex variable. /BR Hans -Original Message- From: Hans Nordebäck Sent: den 27 juni 2017 09:23 To: 'minh chau' ; Lennart Lund ; pr

Re: [devel] [PATCH 1/1] pyosaf: Ensure compatibility with Python 3 [#2492]

2017-06-28 Thread Hans Nordebäck
Hi Anders, minor comments below marked with [HansN] /BR Hans On 06/13/2017 04:07 PM, Anders Widell wrote: Ensure compatibility with Python 3 by running the Python source code files through the Automated Python 2 to 3 code translation tool "2to3". For more information about this tool, see: ht

Re: [devel] [PATCH 1/1] amfd: increase msg priority for node ups [#2510]

2017-07-05 Thread Hans Nordebäck
Hi, it may be that the MDS thread is a real time thread SCHED_RR and the main thread is SCHED_OTHER, and as long as the MDS thread do not yield, the main thread will not be scheduled. This may be more common in an container environment, I think we need to look into, if this starts to be a m

Re: [devel] [PATCH 1/1] amfd: increase msg priority for node ups [#2510]

2017-07-05 Thread Hans Nordebäck
Hi, agree with you Gary, the main thread looks busy, ack on this patch. /Thanks HansN On 07/06/2017 07:59 AM, Gary Lee wrote: Hi Praveen / Hans There are no errors in mds log. The messages are not lost, they are just buffered between the MDS thread and main thread, for up to around 15 seco

Re: [devel] [PATCH 1/1] amfa: Fix saAmfPmStart_3 and saAmfResponse_4 to correctly return BAD_HANDLE [#2539]

2017-08-02 Thread Hans Nordebäck
ack, code review only. When refactoring the code, perhaps UINT_MAX from can be used instead of AVSV_UNS32_HDL_MAX? And PRIx64 from instead of %llx? /Regards HansN On 08/02/2017 06:01 AM, Nguyen Luu wrote: When called with an uninitialized or already finalized handle, saAmfPmStart_3 and sa

Re: [devel] [PATCH 1/1] amfa: return BAD HANDLE in error report or error clear [#248]

2017-08-02 Thread Hans Nordebäck
ack, code review only. Same comment as for ticket [#2539], (UINT32_MAX and PRIx64)). /Regards HansN On 07/28/2017 11:25 AM, Nagendra Kumar wrote: --- src/amf/agent/amf_agent.cc | 15 +++ 1 file changed, 15 insertions(+) diff --git a/src/amf/agent/amf_agent.cc b/src/amf/agent/a

Re: [devel] [PATCH 1/1] nid: order of system log print out is not correct [#2541]

2017-08-02 Thread Hans Nordebäck
Ack, code review only/Regards HansN -Original Message- From: Rafael Odzakow Sent: den 2 augusti 2017 10:14 To: Anders Widell ; Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net; Rafael Odzakow Subject: [PATCH 1/1] nid: order of system log print out is not correct [#2541] Using

Re: [devel] [PATCH 1/1] amf: add option for controller status in amfclusterstatus [#2536]

2017-08-08 Thread Hans Nordebäck
ack, code review only. Minor comments below marked with [HansN] /Regards HansN On 08/04/2017 11:18 AM, Praveen wrote: --- src/amf/tools/amf_cluster_status.cc | 190 +++- 1 file changed, 167 insertions(+), 23 deletions(-) diff --git a/src/amf/tools/amf_cluste

Re: [devel] [PATCH 1/1] amfd: honor PrefAssignedSU in nway and nway active model during assignments [#2269]

2017-08-08 Thread Hans Nordebäck
Hi Praveen, ack, code review only, minor comments below. /Regards HansN On 07/27/2017 07:36 AM, Praveen wrote: SG attribute saAmfSGNumPrefAssignedSUs is applicable to N-Way and N-Way Active model. AMF is assigning more than saAmfSGNumPrefAssignedSUs in both N-Way and N-Way Active model. Pa

Re: [devel] [PATCH 1/1] amfnd: convert dnd_list to a vector [#1945]

2017-08-15 Thread Hans Nordebäck
ack, code review only. One minor comment below. /Regards HansN On 07/05/2017 10:45 AM, Gary Lee wrote: --- src/amf/amfnd/avnd_cb.h | 3 +- src/amf/amfnd/avnd_di.h | 36 -- src/amf/amfnd/avnd_mds.h | 11 +- src/amf/amfnd/di.cc | 321 +-

Re: [devel] [PATCH 1/1] amfd: update SI assignment state when SU is added or removed [#2269]

2017-08-31 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 09/01/2017 05:54 AM, Gary Lee wrote: --- src/amf/amfd/sg.cc | 5 + src/amf/amfd/su.cc | 6 ++ 2 files changed, 11 insertions(+) diff --git a/src/amf/amfd/sg.cc b/src/amf/amfd/sg.cc index 8f3590190..986bb 100644 --- a/src/amf/amfd/sg.cc +++

Re: [devel] [PATCH 1/1] amfd: harden completed and apply delete callbacks [#2566]

2017-09-06 Thread Hans Nordebäck
Hi Gary, ack, code review only. Minor comments below. /Thanks HansN On 09/06/2017 06:19 AM, Gary Lee wrote: It is possible for an object to be deleted in IMM, before a standby SC finishes initilization. Now, if the related callbacks are processed by the standby late, then unnecessary assertio

Re: [devel] [PATCH 1/1] osaf: Divide the safe reboot into two phases [#2542]

2017-09-14 Thread Hans Nordebäck
Hi Anders, I sent out a patch for review for ticket #2451, I think it covers this ticket also. /Thanks Hans On 08/02/2017 01:14 PM, Anders Widell wrote: Divide the safe reboot in the opensaf_reboot script into two phases: in the first phase we stop important OpenSAF services (especially IMM)

Re: [devel] [PATCH 1/1] mds: supervise receive thread throughput [#2570]

2017-09-14 Thread Hans Nordebäck
yes, I'll update the 00-README.conf before pushing. Praveen, is it ok to push this patch? We have run large clusters with the OSAF_MDS_WATCHDOG active for amfd and immd and it shows that both services performs well regarding receive throughput and latency. So e.g. the earlier improvements rel

Re: [devel] [PATCH 1/1] plm: fix systemd files for plm and opensaf [#2581]

2017-09-15 Thread Hans Nordebäck
Ack, review only. /Thanks HansN -Original Message- From: Alex Jones [mailto:alex.jo...@genband.com] Sent: den 14 september 2017 16:14 To: mathi.np@gmail.com; Anders Widell Cc: Alex Jones ; opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 1/1] plm: fix systemd files for plm

Re: [devel] [PATCH 1/1] amfnd: retry on TIMEOUT in amf_saImmOmAccessorGet_o2 [#2582]

2017-09-18 Thread Hans Nordebäck
Hi Gary, shouldn't we retry on SA_AIS_ERR_TRY_AGAIN? With the SA_AIS_ERR_TIMEOUT code the operation may have been executed, failed or is ongoing. /Hans On 09/18/2017 02:15 AM, Gary Lee wrote: Sometimes immutil_saImmOmAccessorGet_o2 returns TIMEOUT, AMF should retry a few times. --- src/am

Re: [devel] [PATCH 1/1] amf: ensure we do not reuse version when calling OpenSAF init functions [#2587]

2017-09-21 Thread Hans Nordebäck
ack, review only/Thanks HansN On 09/22/2017 03:23 AM, Gary Lee wrote: --- src/amf/amfd/imm.cc | 2 +- src/amf/amfnd/avnd_util.h | 2 +- src/amf/amfnd/util.cc | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/amf/amfd/imm.cc b/src/amf/amfd/imm.cc index

Re: [devel] [PATCH 1/1] mds: Improve log message at tipc overload [#2574]

2017-09-26 Thread Hans Nordebäck
Hi, Is it ok to push this patch? /Thanks Hans -Original Message- From: Hans Nordebäck Sent: den 7 september 2017 15:55 To: Anders Widell ; praveen.malv...@oracle.com Cc: opensaf-devel@lists.sourceforge.net; Hans Nordebäck Subject: [PATCH 1/1] mds: Improve log message at tipc overload

Re: [devel] [PATCH 1/1] osaf: Add gcov support [#2589]

2017-09-27 Thread Hans Nordebäck
Hi Anders, thanks I'll update, regarding the -lgcov it should not be included, just the --coverage should be necessary, it is according to gcc documentation a synonym for -fprofile-arcs, -ftest-coverage when compiling and -lgcov when linking. /Regards HansN On 09/27/2017 12:03 PM, Anders

Re: [devel] [PATCH 1/1] osaf: Add gcov support [#2589]

2017-09-27 Thread Hans Nordebäck
Hi Anders, Regarding disabling the gcov dump thread, the thread is only created if opensaf is configured with --enable-gcov and the thread creation is guarded with ifdef ENABLE_GCOV. /Regards Hans -Original Message- From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] Sent: den

Re: [devel] [PATCH 1/1] clm: Make the cluster reset admin op safe V3 [#2451]

2017-09-29 Thread Hans Nordebäck
Hi, I've checked Zoran's suggestion attached in the ticket, it handles safe cluster reboot in the most preferable way compared to the other three patches I think. /Regards Hans -Original Message----- From: Hans Nordebäck Sent: den 27 september 2017 13:26 To: Anders Widell ; pr

Re: [devel] [PATCH 1/1] amfd: remove node_up variable from AVD_AVND [#2595]

2017-09-29 Thread Hans Nordebäck
Ack, code review only/Thanks HansN -Original Message- From: Gary Lee [mailto:gary@dektech.com.au] Sent: den 27 september 2017 08:49 To: Hans Nordebäck ; Minh Hon Chau ; ravisekhar.ko...@oracle.com; praveen.malv...@oracle.com Cc: opensaf-devel@lists.sourceforge.net; Gary Lee

Re: [devel] [PATCH 1/1] clm: Add user data in CLMSV_CLUSTER_JOIN_REQ message [#2590]

2017-10-03 Thread Hans Nordebäck
Hi Anders, some comments below. /Regards Hans On 10/02/2017 01:25 PM, Anders Widell wrote: Ack with comments: 1) Add a final comma to the end of the command-line parameter for the scale-out script. The reason why there should be a final comma is to make sure the script (which may be modifi

Re: [devel] [PATCH 1/1] clm: Add user data in CLMSV_CLUSTER_JOIN_REQ message [#2590]

2017-10-03 Thread Hans Nordebäck
one more comment below. /HansN On 10/03/2017 02:31 PM, Hans Nordebäck wrote: Hi Anders, some comments below. /Regards Hans On 10/02/2017 01:25 PM, Anders Widell wrote: Ack with comments: 1) Add a final comma to the end of the command-line parameter for the scale-out script. The reason

Re: [devel] [PATCH 1/1] smf: try to wait for opensafd status before reboot [#2464]

2017-10-04 Thread Hans Nordebäck
Hi Rafael, I have one question on this patch below. /Thanks HansN -Original Message- From: Lennart Lund [mailto:lennart.l...@ericsson.com] Sent: den 25 september 2017 13:44 To: Rafael Odzakow Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1/1] smf: try to wait for o

Re: [devel] [PATCH 1/1] uml: Add support for IPv6 and update Linux version [#2593]

2017-10-06 Thread Hans Nordebäck
ack, review and ipv4/ipv6 tests run/Thanks HansN On 09/25/2017 04:43 PM, Anders Widell wrote: You can now select to use IPv6 addresses in UML, by using e.g. the following command before starting the UML cluster: export IPADDRBASE=fe80:: Linux kernel has also been updated to version 4.13.3.

Re: [devel] [PATCH 1/1] clm: Add user data in CLMSV_CLUSTER_JOIN_REQ message V2 [#2590]

2017-10-10 Thread Hans Nordebäck
Hi Anders, thanks, I'll fix the comments before pushing/Regards Hans On 10/10/2017 11:04 AM, Anders Widell wrote: Ack with minor comments inline, marked AndersW> regards, Anders Widell On 10/06/2017 11:10 AM, Hans Nordeback wrote: ---   scripts/opensaf_scale_out  | 14 +-   src/clm/R

Re: [devel] [PATCH 1/1] base: double start failed [#2622]

2017-10-12 Thread Hans Nordebäck
Hi Rafael, not tested, minor comment/question below. Regards /Hans On 10/10/2017 02:08 PM, Rafael Odzakow wrote: Previously named function "check_env" overwrites pid file. Move it to after running pidofproc for amfnd pid. --- src/nid/opensafd.in | 4 ++-- 1 file changed, 2 insertions(+),

Re: [devel] [PATCH 1/1] pyosaf: retry SAF initialize() function with original version [#2524]

2017-10-17 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 10/17/2017 07:47 AM, Hieu Nguyen wrote: --- python/pyosaf/utils/__init__.py | 44 +++ python/pyosaf/utils/clm/__init__.py | 11 + python/pyosaf/utils/immoi/__init__.py | 7 +++--- python/pyosaf/utils/imm

Re: [devel] [PATCH 3/3] base: Use the OpenSAF internal log service for trace [#2165]

2017-10-17 Thread Hans Nordebäck
ack, code review only. Minor comments below. /Thanks HansN On 10/10/2017 06:44 PM, Anders Widell wrote: Instead of writing trace messages directly to a file, use the OpenSAF internal log service which is implemented in osaftransportd. As a consequence, the trace files will now be rotated automa

Re: [devel] [PATCH 1/3] dtm: Support multiple OpenSAF internal log streams [#2165]

2017-10-17 Thread Hans Nordebäck
ack, code review only, minor comment below/Thanks HansN On 10/10/2017 06:44 PM, Anders Widell wrote: Extend the OpenSAF internal log service so that it supports multiple log streams in addition to the MDS log. All log streams are received on the same socket and the stream name is extracted from

Re: [devel] [PATCH 1/3] dtm: Support multiple OpenSAF internal log streams [#2165]

2017-10-17 Thread Hans Nordebäck
Hi Anders, the regex should be std::regex is_valid{R"([_[:alpha:]]+\.?\w+)"}; I think. The original function does not handle e.g. "." correctly I guess, but this regex does/Hans On 10/17/2017 12:45 PM, Hans Nordebäck wrote: ack, code review only, minor comment bel

Re: [devel] [PATCH 2/3] base: Convert logtrace to C++ [#2165]

2017-10-17 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 10/10/2017 06:44 PM, Anders Widell wrote: --- src/base/Makefile.am | 2 +- src/base/logtrace.c | 268 --- src/base/logtrace.cc | 253 src/base/logtrace.

Re: [devel] [PATCH 2/2] amf: improve error checking and display [#2628]

2017-10-17 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 10/17/2017 09:13 AM, Gary Lee wrote: * nodes are prefixed with '0x', eg '0x2010f' instead of '2010f' to match input * options 's', 'c', 'u' are incompatible and are rejected when used together --- src/amf/tools/amf_cluster_status.cc | 26 +

Re: [devel] [PATCH 1/2] amf: fix whitespace issues [#2628]

2017-10-17 Thread Hans Nordebäck
Ack, code review only/Thanks HansN -Original Message- From: Gary Lee [mailto:gary@dektech.com.au] Sent: den 17 oktober 2017 09:14 To: Hans Nordebäck ; ravisekhar.ko...@oracle.com; Minh Hon Chau Cc: opensaf-devel@lists.sourceforge.net; Gary Lee Subject: [PATCH 1/2] amf: fix

Re: [devel] [PATCH 1/1] amfnd: Add more details for synced SU, SISU from node director [#2575]

2017-10-17 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 09/11/2017 02:10 AM, Minh Chau wrote: --- src/amf/amfd/sg.cc | 1 + src/amf/amfnd/di.cc | 13 ++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/amf/amfd/sg.cc b/src/amf/amfd/sg.cc index 986bb..6019be955 100644 --- a/

Re: [devel] [PATCH 1/1] amfnd: change log level to notice for events during node failover/shutdown [#2605]

2017-10-17 Thread Hans Nordebäck
ack, code review only/Thanks HansN On 10/04/2017 06:21 AM, Gary Lee wrote: --- src/amf/amfnd/clc.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc index c317f093b..c8e60e61b 100644 --- a/src/amf/amfnd/clc.cc +++ b/src/amf/a

Re: [devel] [PATCH 1/3] dtm: Support multiple OpenSAF internal log streams [#2165]

2017-10-18 Thread Hans Nordebäck
r throwing an instance of 'std::regex_error'   what():  regex_error Aborted After googling this a bit, I think the reason could be that ECMAScript types of regular expressions are not fully implemented in GCC 4.8. / Anders Widell On 10/17/2017 01:03 PM, Hans Nordebäck wrote: Hi Ander

Re: [devel] [PATCH 1/3] dtm: Support multiple OpenSAF internal log streams [#2165]

2017-10-18 Thread Hans Nordebäck
Hi Anders, you can try this regex, it works in gcc 4.8.4, gcc 5.4.0 and gcc 7.2.0: std::regex is_valid{R"([_[:alpha:]]+\.?[_[:alpha:]]+)", std::regex::extended}; /Hans On 10/18/2017 09:23 AM, Hans Nordebäck wrote: Hi Anders, in gcc 4.8.4 it works with:  std::regex is_valid{R&

Re: [devel] [PATCH 1/3] dtm: Support multiple OpenSAF internal log streams [#2165]

2017-10-18 Thread Hans Nordebäck
perly with GCC 4.8 (when I tested the regex below only accepts single-character strings). regards, Anders Widell On 10/18/2017 09:40 AM, Hans Nordebäck wrote: Hi Anders, you can try this regex, it works in gcc 4.8.4, gcc 5.4.0 and gcc 7.2.0: std::regex is_valid{R"([_[:alpha:]]+\.

Re: [devel] [PATCH 1/1] ntf: Add support for code coverage to ntfimcn [#2623]

2017-10-20 Thread Hans Nordebäck
Ack, code review only/Thanks HansN -Original Message- From: Lennart Lund Sent: den 20 oktober 2017 12:45 To: Minh Hon Chau ; Hans Nordebäck ; Anders Widell Cc: opensaf-devel@lists.sourceforge.net; Lennart Lund Subject: [PATCH 1/1] ntf: Add support for code coverage to ntfimcn [#2623

Re: [devel] [PATCH 1/1] pyosaf: Create a pylint makefile target for pyosaf V2 [#2636]

2017-10-24 Thread Hans Nordebäck
Hi Nguyen, ok, I'll change to output format text, it seems to work for all pylint versions. /Thanks HansN On 10/24/2017 10:44 AM, Nguyen Luu wrote: Hi Hans, I have one comment inline marked with [Nguyen]. Thanks, Nguyen On 10/23/2017 7:40 PM, Hans Nordeback wrote: --- Makefile.am |

Re: [devel] [PATCH 1/2] base: Add utility function osaf_fdatasync for syncing to disk [#2451]

2017-10-24 Thread Hans Nordebäck
ack, code review only. Minor comments below/Thanks HansN On 10/23/2017 03:38 PM, Anders Widell wrote: --- src/base/Makefile.am| 3 +++ src/base/osaf_unistd.cc | 62 + src/base/osaf_unistd.h | 51

Re: [devel] [PATCH 1/2] nid: Derive node ID from TIPC address when not managing TIPC [#2653]

2017-10-25 Thread Hans Nordebäck
Hi Anders, ack review only minor comments below/Regards HansN Från: Anders Widell Skickat: den 25 oktober 2017 14:30 Till: Hans Nordebäck; ravisekhar.ko...@oracle.com Kopia: opensaf-devel@lists.sourceforge.net; Anders Widell Ämne: [PATCH 1/2] nid: Derive node ID

Re: [devel] [PATCH 2/2] dtm: Always call the configure_tipc script [#2653]

2017-10-25 Thread Hans Nordebäck
ack, review only/Thanks HansN Från: Anders Widell Skickat: den 25 oktober 2017 14:30 Till: Hans Nordebäck; ravisekhar.ko...@oracle.com Kopia: opensaf-devel@lists.sourceforge.net; Anders Widell Ämne: [PATCH 2/2] dtm: Always call the configure_tipc script [#2653

Re: [devel] [PATCH 2/2] clm: Make the cluster reset admin operation safe [#2451]

2017-10-26 Thread Hans Nordebäck
Hi Anders, I did some few tests: 1) a) Remove /sbin/reboot on SC-1 (active) to simulate slow/hanging reboots    b) immadm -o 4 safCluster=myClmCluster   (the  .clm_cluster_generation_id is created and contains e.g. 2)    c) sc-1 hangs and the rest of the cluster hangs til:   c1) opensaf_

Re: [devel] [PATCH 1/1] dtm: Use multiple backup files when rotating trace log files [#2638]

2017-10-26 Thread Hans Nordebäck
ack, code review only/Thanks HansN Från: Anders Widell Skickat: den 26 oktober 2017 17:16:55 Till: Hans Nordebäck; ravisekhar.ko...@oracle.com Kopia: opensaf-devel@lists.sourceforge.net; Anders Widell Ämne: [PATCH 1/1] dtm: Use multiple backup files when rotating

Re: [devel] [PATCH 1/1] clm: add new admin operations [#2649]

2017-10-27 Thread Hans Nordebäck
Hi Zoran, ack, code review and some tests run. A few comments/questions: 1) Why is osafclm_stop placed in two directories?   /usr/local/lib/opensaf/clm-scripts/osafclm_stop /usr/local/lib/opensaf/osafclm_stop 2) Perhaps extend the comment in clms_imm_admin_op_callback() to: /* Sectio

Re: [devel] [PATCH 1/1] amfnd: fix segv in ncs_tmr_stop [#2658]

2017-10-30 Thread Hans Nordebäck
Hi Gary, the problem is that the dnd_list is global and during iteration it is changed elsewhere via funciton avnd_last_step_clean(). I guess if comparing dnd_list and tmp after the call they may differ in some way. Perhaps tmp can be updated with these changes or we have to refactor thi

Re: [devel] [PATCH 1/1] amfnd: fix segv in ncs_tmr_stop [#2658]

2017-10-30 Thread Hans Nordebäck
, and thus not use iter again, it should be ok? Thanks Gary On 30 Oct. 2017 22:42, Hans Nordebäck wrote: Hi Gary, the problem is that the dnd_list is global and during iteration it is changed elsewhere via funciton avnd_last_step_clean(). I guess if comparing dnd_list and

Re: [devel] [PATCH 1/1] amfnd: fix segv in ncs_tmr_stop [#2658]

2017-10-30 Thread Hans Nordebäck
Hi Gary, fine, I'll change the patch to your suggestion. /Thanks -Original Message- From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] Sent: den 30 oktober 2017 13:00 To: Gary Lee Cc: Minh Hon Chau ; opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1/1]

Re: [devel] [PATCH 0/3] Review Request for osaf: Divide the safe reboot into two phases [#2542]

2017-10-30 Thread Hans Nordebäck
Hi Anders, ack for the series, code review and some basic tests. This solution relies on IMM and perhaps in the future this should be handled by CLMs instead. /Regards HansN On 10/27/2017 02:36 PM, Anders Widell wrote: Summary: osaf: Divide the safe reboot into two phases [#2542] Review r

Re: [devel] [PATCH 1/1] clm: wrapper CLM APIs with try_again handle inside [#2634]

2017-10-31 Thread Hans Nordebäck
Hi Vu, I think it would be good if we can align this with the style of the Pyhton decorators in pyosaf/utils, now there is a lot of redundant code and it should be possible to have e.g. one C++ object for each function where just the function pointer is unique and the code is shared/common.

Re: [devel] [PATCH 1/1] clm: wrapper CLM APIs with try_again handle inside [#2634]

2017-11-01 Thread Hans Nordebäck
corator(saImmOmSelectionObjectGet); rc = foo2(4712, "hello world!"); std::cout << "rc: " << rc << std::endl; return 0; } /Hans Från: Vu Minh Nguyen Skickat: den 1 november 2017 12:11:40 Till: Hans Nordebäck; Anders Widell Kop

Re: [devel] [PATCH 1/1] base: Add TimeoutStartSec to opensafd.service [#2678]

2017-11-10 Thread Hans Nordebäck
". Pass "infinity" to disable the timeout logic." /Regards HansN Från: Alex Jones Skickat: den 10 november 2017 15:42:38 Till: Hans Nordebäck; ravisekhar.ko...@oracle.com; ajo...@genband.com; Anders Widell Kopia: opensaf-devel@lists.source

Re: [devel] [PATCH 1/1] base: Add TimeoutStartSec to opensafd.service [#2678]

2017-11-13 Thread Hans Nordebäck
Hi Alex, it was tested on Ubuntu 16.04 and MontaVista Carrier Grade Express Linux 2.2.0. /Regards HansN On 11/10/2017 08:10 PM, Hans Nordebäck wrote: Hi Alex, I'll check which versions were used during test of the patch, but the current systemd documentation

Re: [devel] [PATCH 0/1] Review Request for clm: Fix unexpected join response error when executing immadm -o 4, 5 commands [#2661]

2017-11-14 Thread Hans Nordebäck
Hi, A gentle reminder for review. /Thanks HansN -Original Message- From: Hans Nordebäck Sent: den 1 november 2017 16:24 To: ravisekhar.ko...@oracle.com; Anders Widell ; Zoran Milinkovic Cc: opensaf-devel@lists.sourceforge.net; Hans Nordebäck Subject: [PATCH 0/1] Review Request for

Re: [devel] [PATCH 0/1] Review Request for clm: WA Two active controllers observed at cluster [#2677]

2017-11-14 Thread Hans Nordebäck
Hi, It is appreciated if you can prioritize reviewing this patch. /Thanks HansN -Original Message- From: Hans Nordebäck Sent: den 10 november 2017 09:29 To: Anders Widell ; ravisekhar.ko...@oracle.com Cc: opensaf-devel@lists.sourceforge.net; Hans Nordebäck Subject: [PATCH 0/1] Review

Re: [devel] [PATCH 1/1] clm: Fix unexpected join response error when executing immadm -o 4, 5 commands [#2661]

2017-11-14 Thread Hans Nordebäck
I'll add the free(msg) before pushing/Thanks HansN On 11/14/2017 10:06 PM, Anders Widell wrote: When I compare this with the fix that I did as part of ticket [#2451] (pasted in below), I think it looks like you forgot free(ms), and thus you get a memory leak eachh time you receive an CLMSV_CLM

Re: [devel] [PATCH 1/1] base: Add TimeoutStartSec to opensafd.service [#2678]

2017-11-15 Thread Hans Nordebäck
Hi Anders, yes, my first approach was, (mentioned in the ticket) was to: "the TimoutStartSec should at least be the sum of the services timeout values from file "nodeinit.conf*", " perhaps this is the prefered value? /Regards HansN On 11/15/2017 10:39 AM, Anders Widell wrote: Ack with the

Re: [devel] [PATCH 1/1] base: Add TimeoutStartSec to opensafd.service [#2678]

2017-11-15 Thread Hans Nordebäck
ome number. regards, Anders Widell On 11/15/2017 01:03 PM, Hans Nordebäck wrote: Hi Anders, yes, my first approach was, (mentioned in the ticket) was to: "the TimoutStartSec should at least be the sum of the services timeout values from file "nodeinit.conf*", " perh

Re: [devel] [PATCH 1/1] nid: Correctly handle the case when OPENSAF_MANAGE_TIPC=no [#2680]

2017-11-15 Thread Hans Nordebäck
ack, review only. A question, why was ETH_NAME, TIPC_NET id moved? /Regards Hans On 11/15/2017 01:55 PM, Anders Widell wrote: Fix a regression for the case when OPENSAF_MANAGE_TIPC=no and the file /var/lib/opensaf/node_id is present. In this case, the configure_tipc script should do nothing. -

Re: [devel] [PATCH 1/1] base: Add TimeoutStartSec to opensafd.service [#2678]

2017-11-15 Thread Hans Nordebäck
ards HansN On 11/15/2017 02:05 PM, Alex Jones wrote: This approach makes sense to me. Alex ---- *From:* Hans Nordebäck *Sent:* Wednesday, November 15, 2017 7:58:23 AM *To:* Anders Widell; ravisekhar.ko...@oracle.com; Alex

Re: [devel] [PATCH 1/1] tools: Fix pylint and pep8 issues for tools written in Python [#2664]

2017-11-17 Thread Hans Nordebäck
ack, code review only. /Thanks HansN On 10/31/2017 11:26 AM, Nguyen Luu wrote: Fix pylint and pep8 issues for the following Python files: ./tools/devel/dot/trace2dot ./tools/devel/review/patch-tokenize.py ./src/imm/tools/immxml-merge ./src/imm/tools/immxml-validate ./src/imm/tool

Re: [devel] [PATCH 1/1] pyosaf: add README for high level python interfaces [#2674]

2017-11-20 Thread Hans Nordebäck
Hi, ack with some comments below. /Thanks HansN On 11/10/2017 08:52 AM, Long H Buu Nguyen wrote: --- python/README_UTILS | 215 1 file changed, 215 insertions(+) create mode 100644 python/README_UTILS diff --git a/python/README_UTILS b

Re: [devel] [PATCH 1/1] amfd: Use only one variable for amfd status [#2454]

2017-11-28 Thread Hans Nordebäck
Hi Minh, ack code review only, perhaps we can do the following changes also? change typdef struct avnd_cb_tag to: struct AVND_CB {    enum class AvdStatus {   UP,   DOWN   };   AvdStatus avd_status() {return avd_status_;}   /* Temp: Indicates if AvD went down */   AvdStatus avd_statu

Re: [devel] [PATCH 1/1] tools: Fix pylint and pep8 issues for tools written in Python [#2664]

2017-11-28 Thread Hans Nordebäck
ack, review only. /Thanks HansN On 11/20/2017 12:59 PM, Nguyen Luu wrote: Fix pylint and pep8 issues for the following Python files: ./tools/devel/dot/trace2dot ./tools/devel/review/patch-tokenize.py ./src/imm/tools/immxml-merge ./src/imm/tools/immxml-validate ./src/imm/tools/bas

Re: [devel] [PATCH 1/1] base: Fix opensaf_scale_out script to handle binary arguments [#2703]

2017-11-29 Thread Hans Nordebäck
I'll add -n before pushing./Thanks HansN On 11/29/2017 02:32 PM, Anders Widell wrote: Ack with minor comment: maybe add -n option to the echo command, to avoid appending a newline at the end of the last field? regards, Anders Widell On 11/27/2017 04:30 PM, Hans Nordeback wrote: ---   script

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Hans Nordebäck
Hi Vu, please see some comments inlined below/Regards HansN On 11/28/2017 02:13 PM, Vu Minh Nguyen wrote: Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code of AIS APIs. --- src/base/Makefile.am | 5 +- src/base/tests/try_again_decorator_test.cc

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Hans Nordebäck
agree, constexpr should be fine. /HansN -Original Message- From: Anders Widell Sent: den 30 november 2017 13:05 To: Hans Nordebäck ; Vu Minh Nguyen Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Hans Nordebäck
Hi Vu,  I think it would be good if we can use the policy pattern for the retry, I reworked my previous sample a bit: #include #include #include #include // only to illustrate, the existing SAF C functions with some dummy arguments #include extern "C" int saImmOmInitialize(int i) {   p

Re: [devel] [PATCH 1/1] osaf: add /sbin/shutdown to sudoers file in 00-README.conf [#2729]

2017-12-05 Thread Hans Nordebäck
Ack, review only/Thanks HansN -Original Message- From: Zoran Milinkovic Sent: den 5 december 2017 15:04 To: Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net; Zoran Milinkovic Subject: [PATCH 1/1] osaf: add /sbin/shutdown to sudoers file in 00-README.conf [#2729] /sbin/shutdown

<    1   2   3   4   5   6   7   8   >