[devel] [PATCH 0 of 1] Review Request for mds: Improve log message [#2193]

[Hans Nordeback]

Summary: mds: Improve log message
Review request for Trac Ticket(s):  [#2193]
Peer Reviewer(s): Mahesh
Pull request to: 
Affected branch(es): default
Development branch: default


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


Comments (indicate scope for each "y" above):
-
 <>

changeset 4cb9848dea359d2bef92a19e4ab0eaa94121dfb7
Author: Hans Nordeback 
Date:   Thu, 17 Nov 2016 08:26:08 +0100

mds: Improve log message [#2193]


Complete diffstat:
--
 osaf/libs/core/mds/mds_dt_tipc.c |  7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
 <>


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  n
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 ~/.hgrc file (i.e. username, 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.


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1 of 1] mds: Improve log message [#2193]

[Hans Nordeback]

 osaf/libs/core/mds/mds_dt_tipc.c |  7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)


diff --git a/osaf/libs/core/mds/mds_dt_tipc.c b/osaf/libs/core/mds/mds_dt_tipc.c
--- a/osaf/libs/core/mds/mds_dt_tipc.c
+++ b/osaf/libs/core/mds/mds_dt_tipc.c
@@ -2497,7 +2497,7 @@ static uint32_t mdtm_sendto(uint8_t *buf
 {
/* Can be made as macro even */
struct sockaddr_tipc server_addr;
-   int send_len = 0;
+   ssize_t send_len = 0;
 #ifdef MDS_CHECKSUM_ENABLE_FLAG
uint16_t checksum = 0;
 #endif
@@ -2524,8 +2524,11 @@ static uint32_t mdtm_sendto(uint8_t *buf
if (send_len == buff_len) {
m_MDS_LOG_INFO("MDTM: Successfully sent message");
return NCSCC_RC_SUCCESS;
+   } else if (send_len == -1) {
+   m_MDS_LOG_ERR("MDTM: Failed to send message err :%s", 
strerror(errno));
+   return NCSCC_RC_FAILURE;
} else {
-   m_MDS_LOG_ERR("MDTM: Failed to send message err :%s", 
strerror(errno));
+   m_MDS_LOG_ERR("MDTM: Failed to send message send_len :%zd", 
send_len);
return NCSCC_RC_FAILURE;
}
 }

--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 2] base: Add Mutex, Lock and ConditionVariable classes [#2173]

[ramesh betham]

Ack.

Thanks,
Ramesh.

On 11/8/2016 8:50 PM, Anders Widell wrote:
>   osaf/libs/core/cplusplus/base/Makefile.am   |   4 +
>   osaf/libs/core/cplusplus/base/condition_variable.cc |  39 
>   osaf/libs/core/cplusplus/base/condition_variable.h  |  95 
> +
>   osaf/libs/core/cplusplus/base/mutex.cc  |  46 ++
>   osaf/libs/core/cplusplus/base/mutex.h   |  82 ++
>   5 files changed, 266 insertions(+), 0 deletions(-)
>
>
> Add classes for Mutex, Lock and ConditionVariable - similar to std::mutex,
> std::unique_lock and std::condition_variable. One reason for adding these to
> OpenSAF is that the C++ standard library casses are unapproved by the Google 
> C++
> Style Guide. Another reason is that we can integrate them better into OpenSAF:
> we select the appropriate options when creating the mutex and condition
> variable, and we call osaf_abort() in case an unexpected error is
> encountered. Also, we will use an error checking mutex when the ENABLE_DEBUG
> preprocessor macro is defined - paving the way for special OpenSAF debug 
> builds.
>
> diff --git a/osaf/libs/core/cplusplus/base/Makefile.am 
> b/osaf/libs/core/cplusplus/base/Makefile.am
> --- a/osaf/libs/core/cplusplus/base/Makefile.am
> +++ b/osaf/libs/core/cplusplus/base/Makefile.am
> @@ -28,6 +28,8 @@ noinst_HEADERS = \
>   getenv.h \
>   log_message.h \
>   macros.h \
> + mutex.h \
> + condition_variable.h \
>   process.h \
>   time.h \
>   unix_client_socket.h \
> @@ -48,6 +50,8 @@ libbase_la_SOURCES = \
>   getenv.cc \
>   log_message.cc \
>   process.cc \
> + mutex.cc \
> + condition_variable.cc \
>   unix_client_socket.cc \
>   unix_server_socket.cc \
>   unix_socket.cc
> diff --git a/osaf/libs/core/cplusplus/base/condition_variable.cc 
> b/osaf/libs/core/cplusplus/base/condition_variable.cc
> new file mode 100644
> --- /dev/null
> +++ b/osaf/libs/core/cplusplus/base/condition_variable.cc
> @@ -0,0 +1,39 @@
> +/*  -*- OpenSAF  -*-
> + *
> + * (C) Copyright 2016 The OpenSAF Foundation
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
> + * under the GNU Lesser General Public License Version 2.1, February 1999.
> + * The complete license can be accessed from the following location:
> + * http://opensource.org/licenses/lgpl-license.php
> + * See the Copying file included with the OpenSAF distribution for full
> + * licensing terms.
> + *
> + * Author(s): Ericsson AB
> + *
> + */
> +
> +#include "osaf/libs/core/cplusplus/base/condition_variable.h"
> +
> +namespace base {
> +
> +ConditionVariable::ConditionVariable() : condition_variable_{} {
> +  pthread_condattr_t attr;
> +  int result = pthread_condattr_init();
> +  if (result != 0) osaf_abort(result);
> +  result = pthread_condattr_setclock(, CLOCK_MONOTONIC);
> +  if (result != 0) osaf_abort(result);
> +  result = pthread_cond_init(_variable_, );
> +  if (result != 0) osaf_abort(result);
> +  result = pthread_condattr_destroy();
> +  if (result != 0) osaf_abort(result);
> +}
> +
> +ConditionVariable::~ConditionVariable() {
> +  int result = pthread_cond_destroy(_variable_);
> +  if (result != 0) osaf_abort(result);
> +}
> +
> +}  // namespace base
> diff --git a/osaf/libs/core/cplusplus/base/condition_variable.h 
> b/osaf/libs/core/cplusplus/base/condition_variable.h
> new file mode 100644
> --- /dev/null
> +++ b/osaf/libs/core/cplusplus/base/condition_variable.h
> @@ -0,0 +1,95 @@
> +/*  -*- OpenSAF  -*-
> + *
> + * (C) Copyright 2016 The OpenSAF Foundation
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
> + * under the GNU Lesser General Public License Version 2.1, February 1999.
> + * The complete license can be accessed from the following location:
> + * http://opensource.org/licenses/lgpl-license.php
> + * See the Copying file included with the OpenSAF distribution for full
> + * licensing terms.
> + *
> + * Author(s): Ericsson AB
> + *
> + */
> +
> +#ifndef OSAF_LIBS_CORE_CPLUSPLUS_BASE_CONDITION_VARIABLE_H_
> +#define OSAF_LIBS_CORE_CPLUSPLUS_BASE_CONDITION_VARIABLE_H_
> +
> +#include 
> +#include 
> +#include "osaf/libs/core/common/include/osaf_utility.h"
> +#include "osaf/libs/core/cplusplus/base/macros.h"
> +#include "osaf/libs/core/cplusplus/base/mutex.h"
> +#include "osaf/libs/core/cplusplus/base/time.h"
> +
> +namespace base {
> +
> +enum class CvStatus { kNoTimeout, kTimeout };
> +
> +// A condition variable, with an API similar to std::condition_variable. The
> +// timed methods always use the monotonic clock (clock id CLOCK_MONOTONIC).
> +class 

[devel] [PATCH 1 of 1] log: handle TRY_AGAIN error code of saClmInitialize() [#2192]

[Vu Minh Nguyen]

 osaf/services/saf/logsv/lgs/lgs_clm.cc |  12 
 1 files changed, 12 insertions(+), 0 deletions(-)


LOG did not deal with TRY_AGAIN error code of `saClmInitialize()`,
LOG would exit, and cause node reboot if getting TRY_AGAIN.

The patch adds a while loop to do retry when getting TRY_AGAIN.

diff --git a/osaf/services/saf/logsv/lgs/lgs_clm.cc 
b/osaf/services/saf/logsv/lgs/lgs_clm.cc
--- a/osaf/services/saf/logsv/lgs/lgs_clm.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_clm.cc
@@ -348,13 +348,25 @@ void *lgs_clm_init_thread(void *cb) {
   static SaVersionT clmVersion = { 'B', 0x04, 0x01 };
   lgs_cb_t *_lgs_cb = reinterpret_cast (cb);
   SaAisErrorT rc;
+  uint32_t msecs_waited = 0;
+  const uint32_t max_waiting_time_10s = 10 * 1000; /* 10 secs */
+
   TRACE_ENTER();
+
   rc = saClmInitialize_4(&_lgs_cb->clm_hdl, _callbacks, );
+  while (((rc == SA_AIS_ERR_TRY_AGAIN) || (rc == SA_AIS_ERR_TIMEOUT) ||
+  (rc == SA_AIS_ERR_UNAVAILABLE)) &&
+ (msecs_waited < max_waiting_time_10s)) {
+usleep(100*1000);
+msecs_waited += 100;
+rc = saClmInitialize_4(&_lgs_cb->clm_hdl, _callbacks, );
+  }
   if (rc != SA_AIS_OK) {
 LOG_ER("saClmInitialize failed with error: %d", rc);
 TRACE_LEAVE();
 exit(EXIT_FAILURE);
   }
+
   rc = saClmSelectionObjectGet(_lgs_cb->clm_hdl, _cb->clmSelectionObject);
   if (rc != SA_AIS_OK) {
 LOG_ER("saClmSelectionObjectGet failed with error: %d", rc);

--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 0 of 1] Review Request for log: handle TRY_AGAIN error code of saClmInitialize() [#2192]

[Vu Minh Nguyen]

Summary: log: handle TRY_AGAIN error code of saClmInitialize() [#2192]
Review request for Trac Ticket(s): #2192
Peer Reviewer(s): LOG devs
Pull request to: <>
Affected branch(es): 5.1 & default
Development branch: default


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
 <>

changeset 34691c6a2aea13e67b0d50188d6e00f998580834
Author: Vu Minh Nguyen 
Date:   Thu, 17 Nov 2016 13:19:49 +0700

log: handle TRY_AGAIN error code of saClmInitialize() [#2192]

LOG did not deal with TRY_AGAIN error code of `saClmInitialize()`, LOG 
would
exit, and cause node reboot if getting TRY_AGAIN.

The patch adds a while loop to do retry when getting TRY_AGAIN.


Complete diffstat:
--
 osaf/services/saf/logsv/lgs/lgs_clm.cc |  12 
 1 files changed, 12 insertions(+), 0 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
 <>


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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.


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1 of 1] ntf: handle error code TRY_AGAIN of saClmInitialize() [#2191]

[Vu Minh Nguyen]

 osaf/services/saf/ntfsv/ntfs/ntfs_clm.c |  14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)


NTF did not deal with TRY_AGAIN error code of `saClmInitialize()`,
NTF would exit, and cause node reboot if getting TRY_AGAIN.

The patch adds a while loop to do retry when getting TRY_AGAIN.

diff --git a/osaf/services/saf/ntfsv/ntfs/ntfs_clm.c 
b/osaf/services/saf/ntfsv/ntfs/ntfs_clm.c
--- a/osaf/services/saf/ntfsv/ntfs/ntfs_clm.c
+++ b/osaf/services/saf/ntfsv/ntfs/ntfs_clm.c
@@ -101,13 +101,25 @@ void *ntfs_clm_init_thread(void *cb)
 {
ntfs_cb_t *_ntfs_cb = (ntfs_cb_t *) cb;
SaAisErrorT rc = SA_AIS_OK;
+   uint32_t msecs_waited = 0;
+   const uint32_t max_waiting_time_10s = 10 * 1000; /* 10 secs */
+
TRACE_ENTER();
+
rc = saClmInitialize_4(&_ntfs_cb->clm_hdl, _callbacks, );
+   while (((rc == SA_AIS_ERR_TRY_AGAIN) || (rc == SA_AIS_ERR_TIMEOUT) ||
+   (rc == SA_AIS_ERR_UNAVAILABLE)) &&
+  (msecs_waited < max_waiting_time_10s)) {
+   usleep(100*1000);
+   msecs_waited += 100;
+   rc = saClmInitialize_4(&_ntfs_cb->clm_hdl, _callbacks, 
);
+   }
if (rc != SA_AIS_OK) {
LOG_ER("saClmInitialize failed with error: %d", rc);
TRACE_LEAVE();
-exit(EXIT_FAILURE);
+   exit(EXIT_FAILURE);
}
+
rc = saClmSelectionObjectGet(_ntfs_cb->clm_hdl, 
_cb->clmSelectionObject);
if (rc != SA_AIS_OK) {
LOG_ER("saClmSelectionObjectGet failed with error: %d", rc);

--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 0 of 1] Review Request for ntf: handle error code TRY_AGAIN of saClmInitialize() [#2191]

[Vu Minh Nguyen]

Summary: ntf: handle error code TRY_AGAIN of saClmInitialize() [#2191]
Review request for Trac Ticket(s): #2191
Peer Reviewer(s): NTF devs
Pull request to: <>
Affected branch(es): all
Development branch: default


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
 <>

changeset 3eff9becd264da624cf0524422109615d7bd4383
Author: Vu Minh Nguyen 
Date:   Thu, 17 Nov 2016 13:10:24 +0700

ntf: handle error code TRY_AGAIN of saClmInitialize() [#2191]

NTF did not deal with TRY_AGAIN error code of `saClmInitialize()`, NTF 
would
exit, and cause node reboot if getting TRY_AGAIN.

The patch adds a while loop to do retry when getting TRY_AGAIN.


Complete diffstat:
--
 osaf/services/saf/ntfsv/ntfs/ntfs_clm.c |  14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
 <>


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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.


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] log: fix logtest 5 3 failed [#2168]

[A V Mahesh]

Hi,

ACK, not tested.

-AVM


On 11/17/2016 9:21 AM, Vu Minh Nguyen wrote:
>   tests/logsv/tet_LogOiOps.c |  2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> The test case was created to verify if all log streams' directories are 
> created
> successfully when changing `logRootDirectory`.
>
> There are mainly 03 steps:
> 1) Change `logRootDirectory`
> 2) Wait 100ms
> 3) Verify if the log stream's directory is created
>
> If step #1 takes more than 100ms, the verificaiton step will fail.
>
> This patch increase the time at step #2 to 1 second to make sure
> the step #1 done.
>
> diff --git a/tests/logsv/tet_LogOiOps.c b/tests/logsv/tet_LogOiOps.c
> --- a/tests/logsv/tet_LogOiOps.c
> +++ b/tests/logsv/tet_LogOiOps.c
> @@ -1076,7 +1076,7 @@ void change_root_path(void)
>   }
>   
>   // Verify if the directory and subdirectly are created successfully
> - usleep(100*1000); // to make sure logsv done processing of directories 
> creation
> + sleep(1); // to make sure logsv done processing of directories creation
>   sprintf(command, "ls %s/testRoot 1>/dev/null", tstdir);
>   rc = tet_system(command);
>   if (rc != 0) {


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] fix crash problem by checking null pointer before accessing its detail V2

[A V Mahesh]

Hi Hoang,

ACK with following :

We can replace  `CPD_NODE_USER_INFO *node_user = ckpt_node->node_users; 
`  with  `CPD_NODE_USER_INFO *node_user;`and
replace   `for (; node_user != NULL && count < 
ckpt_node->node_users_cnt; node_user = node_user->next) {` with
`for (node_user = ckpt_node->node_users; node_user != NULL && count < 
ckpt_node->node_users_cnt; node_user = node_user->next) {`

-AVM

On 11/17/2016 7:33 AM, Hoang Vo wrote:
>   osaf/services/saf/cpsv/cpd/cpd_red.c |  11 +++
>   1 files changed, 7 insertions(+), 4 deletions(-)
>
>
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_red.c 
> b/osaf/services/saf/cpsv/cpd/cpd_red.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_red.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_red.c
> @@ -301,7 +301,6 @@ void cpd_a2s_ckpt_dest_del(CPD_CB *cb, S
>   void cpd_a2s_ckpt_usr_info(CPD_CB *cb, CPD_CKPT_INFO_NODE *ckpt_node)
>   {
>   CPD_MBCSV_MSG cpd_msg;
> - int count = 0;
>   uint32_t rc = SA_AIS_OK;
>   memset(_msg, '\0', sizeof(CPD_MBCSV_MSG));
>   
> @@ -316,18 +315,22 @@ void cpd_a2s_ckpt_usr_info(CPD_CB *cb, C
>   cpd_msg.info.usr_info_2.ckpt_on_scxb2 = ckpt_node->ckpt_on_scxb2;
>   
>   if (ckpt_node->node_users_cnt) {
> + int count = 0;
>   CPD_NODE_USER_INFO *node_user = ckpt_node->node_users;
> - cpd_msg.info.usr_info_2.node_users_cnt = 
> ckpt_node->node_users_cnt;
>   cpd_msg.info.usr_info_2.node_list = 
> malloc(ckpt_node->node_users_cnt * sizeof(CPD_NODE_USER_INFO));
>   memset(cpd_msg.info.usr_info_2.node_list, '\0', 
> (sizeof(CPD_NODE_USER_INFO) * ckpt_node->node_users_cnt));
>   
> - for (count = 0; count < ckpt_node->node_users_cnt; count++) {
> + for (; node_user != NULL && count < ckpt_node->node_users_cnt; 
> node_user = node_user->next) {
>   cpd_msg.info.usr_info_2.node_list[count].dest = 
> node_user->dest;
>   cpd_msg.info.usr_info_2.node_list[count].num_users = 
> node_user->num_users;
>   cpd_msg.info.usr_info_2.node_list[count].num_readers = 
> node_user->num_readers;
>   cpd_msg.info.usr_info_2.node_list[count].num_writers = 
> node_user->num_writers;
> - node_user = node_user->next;
> + ++count;
>   }
> +
> + /* Update node_users_cnt in case of mismatch */
> + ckpt_node->node_users_cnt = count;
> + cpd_msg.info.usr_info_2.node_users_cnt = count;
>   }
>   
>   rc = cpd_mbcsv_async_update(cb, _msg);


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1 of 1] log: fix logtest 5 3 failed [#2168]

[Vu Minh Nguyen]

 tests/logsv/tet_LogOiOps.c |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


The test case was created to verify if all log streams' directories are created
successfully when changing `logRootDirectory`.

There are mainly 03 steps:
1) Change `logRootDirectory`
2) Wait 100ms
3) Verify if the log stream's directory is created

If step #1 takes more than 100ms, the verificaiton step will fail.

This patch increase the time at step #2 to 1 second to make sure
the step #1 done.

diff --git a/tests/logsv/tet_LogOiOps.c b/tests/logsv/tet_LogOiOps.c
--- a/tests/logsv/tet_LogOiOps.c
+++ b/tests/logsv/tet_LogOiOps.c
@@ -1076,7 +1076,7 @@ void change_root_path(void)
}
 
// Verify if the directory and subdirectly are created successfully
-   usleep(100*1000); // to make sure logsv done processing of directories 
creation
+   sleep(1); // to make sure logsv done processing of directories creation
sprintf(command, "ls %s/testRoot 1>/dev/null", tstdir);
rc = tet_system(command);
if (rc != 0) {

--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 0 of 1] Review Request for log: fix logtest 5 3 failed [#2168]

[Vu Minh Nguyen]

Summary: log: fix logtest 5 3 failed [#2168]
Review request for Trac Ticket(s): #2168
Peer Reviewer(s): all LOG devs
Pull request to: <>
Affected branch(es): 5.0, 5.1
Development branch: 5.0


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
 <>

changeset 8daa4a9ce2f24e4a5b19b702506188d908ebe944
Author: Vu Minh Nguyen 
Date:   Thu, 17 Nov 2016 10:45:56 +0700

log: fix logtest 5 3 failed [#2168]

The test case was created to verify if all log streams' directories are
created successfully when changing `logRootDirectory`.

There are mainly 03 steps: 
1) Change `logRootDirectory` 
2) Wait 100ms 
3) Verify if the log stream's directory is created

If step #1 takes more than 100ms, the verificaiton step will fail.

This patch increase the time at step #2 to 1 second to make sure the 
step #1
done.


Complete diffstat:
--
 tests/logsv/tet_LogOiOps.c |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


Testing Commands:
-
 Run `logtest 5 3`


Testing, Expected Results:
--
 The test is passed


Conditions of Submission:
-
 <>


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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.


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 0 of 1] Review Request for fix crash problem in cpd while rolling update got error [#2187]

[Hoang Vo]

Summary: cpd check null pointer before use [#2187]
Review request for Trac Ticket(s): #2187
Peer Reviewer(s): mahesh.va...@oracle.com; anders.wid...@ericsson.com
Pull request to: mahesh.va...@oracle.com
Affected branch(es): default
Development branch: default


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
V2: fix for loop following maintainer's comments, tested with all existing test 
cases

changeset 64fa8a63c6b21f0ea8ca09f1408ad3045f21c5c9
Author: Hoang Vo 
Date:   Thu, 17 Nov 2016 08:58:50 +0700

fix crash problem by checking null pointer before accessing its detail 
V2


Complete diffstat:
--
 osaf/services/saf/cpsv/cpd/cpd_red.c |  11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)


Testing Commands:
-


Testing, Expected Results:
--


Conditions of Submission:
-
ACK from maintainer

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 ~/.hgrc file (i.e. username, 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.


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1 of 1] fix crash problem by checking null pointer before accessing its detail V2

[Hoang Vo]

 osaf/services/saf/cpsv/cpd/cpd_red.c |  11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)


diff --git a/osaf/services/saf/cpsv/cpd/cpd_red.c 
b/osaf/services/saf/cpsv/cpd/cpd_red.c
--- a/osaf/services/saf/cpsv/cpd/cpd_red.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_red.c
@@ -301,7 +301,6 @@ void cpd_a2s_ckpt_dest_del(CPD_CB *cb, S
 void cpd_a2s_ckpt_usr_info(CPD_CB *cb, CPD_CKPT_INFO_NODE *ckpt_node)
 {
CPD_MBCSV_MSG cpd_msg;
-   int count = 0;
uint32_t rc = SA_AIS_OK;
memset(_msg, '\0', sizeof(CPD_MBCSV_MSG));
 
@@ -316,18 +315,22 @@ void cpd_a2s_ckpt_usr_info(CPD_CB *cb, C
cpd_msg.info.usr_info_2.ckpt_on_scxb2 = ckpt_node->ckpt_on_scxb2;
 
if (ckpt_node->node_users_cnt) {
+   int count = 0;
CPD_NODE_USER_INFO *node_user = ckpt_node->node_users;
-   cpd_msg.info.usr_info_2.node_users_cnt = 
ckpt_node->node_users_cnt;
cpd_msg.info.usr_info_2.node_list = 
malloc(ckpt_node->node_users_cnt * sizeof(CPD_NODE_USER_INFO));
memset(cpd_msg.info.usr_info_2.node_list, '\0', 
(sizeof(CPD_NODE_USER_INFO) * ckpt_node->node_users_cnt));
 
-   for (count = 0; count < ckpt_node->node_users_cnt; count++) {
+   for (; node_user != NULL && count < ckpt_node->node_users_cnt; 
node_user = node_user->next) {
cpd_msg.info.usr_info_2.node_list[count].dest = 
node_user->dest;
cpd_msg.info.usr_info_2.node_list[count].num_users = 
node_user->num_users;
cpd_msg.info.usr_info_2.node_list[count].num_readers = 
node_user->num_readers;
cpd_msg.info.usr_info_2.node_list[count].num_writers = 
node_user->num_writers;
-   node_user = node_user->next;
+   ++count;
}
+
+   /* Update node_users_cnt in case of mismatch */
+   ckpt_node->node_users_cnt = count;
+   cpd_msg.info.usr_info_2.node_users_cnt = count;
}
 
rc = cpd_mbcsv_async_update(cb, _msg);

--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] AMFD: Do not let non-active AMFD receives CLM track callback [#2141]

[minh chau]

Hi all,

Has anyone had chance to review this patch?

Thanks,
Minh

On 14/11/16 15:27, Minh Hon Chau wrote:
>   osaf/services/saf/amf/amfd/clm.cc   |  37 
> +++-
>   osaf/services/saf/amf/amfd/include/cb.h |   1 +
>   osaf/services/saf/amf/amfd/role.cc  |  16 +++---
>   3 files changed, 35 insertions(+), 19 deletions(-)
>
>
> In controller failover/switchover, sometimes active AMFD fails to stop
> CLM track callback. Therefore, when this AMFD become standby, AMFD can
> continue receiving CLM track callback and trigger the operations which
> should only be executed in active AMFD.
>
> diff --git a/osaf/services/saf/amf/amfd/clm.cc 
> b/osaf/services/saf/amf/amfd/clm.cc
> --- a/osaf/services/saf/amf/amfd/clm.cc
> +++ b/osaf/services/saf/amf/amfd/clm.cc
> @@ -219,7 +219,13 @@ static void clm_track_cb(const SaClmClus
>   LOG_ER("ClmTrackCallback received in error");
>   goto done;
>   }
> -
> + if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) {
> + if (avd_cb->is_clm_track_started == true) {
> + LOG_NO("Retry to stop clm track with AMFD state(%d)", 
> avd_cb->avail_state_avd);
> + avd_clm_track_stop();
> + }
> + goto done;
> + }
>   /*
>   ** The CLM cluster can be larger than the AMF cluster thus it is not an
>   ** error if the corresponding AMF node cannot be found.
> @@ -394,6 +400,7 @@ SaAisErrorT avd_clm_init(AVD_CL_CB* cb)
>   
>   cb->clmHandle = 0;
>   cb->clm_sel_obj = 0;
> + cb->is_clm_track_started = false;
>   TRACE_ENTER();
>   /*
>* TODO: This CLM initialization thread can be re-factored
> @@ -453,6 +460,8 @@ SaAisErrorT avd_clm_track_start(void)
>   } else {
>   LOG_ER("Failed to start cluster tracking %u", error);
>   }
> + } else {
> + avd_cb->is_clm_track_started = true;
>   }
>   TRACE_LEAVE();
>   return error;
> @@ -460,17 +469,23 @@ SaAisErrorT avd_clm_track_start(void)
>   
>   SaAisErrorT avd_clm_track_stop(void)
>   {
> -SaAisErrorT error = SA_AIS_OK;
> + SaAisErrorT error = SA_AIS_OK;
> + TRACE_ENTER();
> + error = saClmClusterTrackStop(avd_cb->clmHandle);
> + if (error != SA_AIS_OK) {
> + if (error == SA_AIS_ERR_TRY_AGAIN || error == 
> SA_AIS_ERR_TIMEOUT ||
> + error == SA_AIS_ERR_UNAVAILABLE) {
> + LOG_WA("Failed to stop cluster tracking %u", error);
> + } else {
> + LOG_ER("Failed to stop cluster tracking %u", error);
> + }
> + } else {
> + TRACE("Sucessfully stops cluster tracking");
> + avd_cb->is_clm_track_started = false;
> + }
>   
> -TRACE_ENTER();
> - error = saClmClusterTrackStop(avd_cb->clmHandle);
> -if (SA_AIS_OK != error)
> -LOG_ER("Failed to stop cluster tracking %u", error);
> - else
> - TRACE("Sucessfully stops cluster tracking");
> -
> -TRACE_LEAVE();
> -return error;
> + TRACE_LEAVE();
> + return error;
>   }
>   
>   void clm_node_terminate(AVD_AVND *node)
> diff --git a/osaf/services/saf/amf/amfd/include/cb.h 
> b/osaf/services/saf/amf/amfd/include/cb.h
> --- a/osaf/services/saf/amf/amfd/include/cb.h
> +++ b/osaf/services/saf/amf/amfd/include/cb.h
> @@ -210,6 +210,7 @@ typedef struct cl_cb_tag {
>   /* Clm stuff */
>   std::atomic clmHandle;
>   std::atomic clm_sel_obj;
> + bool is_clm_track_started;
>   
>   bool fully_initialized;
>   bool swap_switch; /* true - In middle of role switch. */
> diff --git a/osaf/services/saf/amf/amfd/role.cc 
> b/osaf/services/saf/amf/amfd/role.cc
> --- a/osaf/services/saf/amf/amfd/role.cc
> +++ b/osaf/services/saf/amf/amfd/role.cc
> @@ -1050,9 +1050,7 @@ uint32_t amfd_switch_actv_qsd(AVD_CL_CB
>   /*  Mark AVD as Quiesced. */
>   cb->avail_state_avd = SA_AMF_HA_QUIESCED;
>   
> - if (avd_clm_track_stop() != SA_AIS_OK) {
> - LOG_ER("ClmTrack stop failed");
> - }
> + avd_clm_track_stop();
>   
>   /* Go ahead and set mds role as already the NCS SU has been switched */
>   if (NCSCC_RC_SUCCESS != (rc = avd_mds_set_vdest_role(cb, 
> SA_AMF_HA_QUIESCED))) {
> @@ -1260,11 +1258,13 @@ uint32_t amfd_switch_stdby_actv(AVD_CL_C
>   if (NCSCC_RC_SUCCESS != avd_rde_set_role(SA_AMF_HA_ACTIVE)) {
>   LOG_ER("rde role change failed from stdy -> Active");
>   }
> -
> - if(avd_clm_track_start() != SA_AIS_OK) {
> - LOG_ER("Switch Standby --> Active, clm track start failed");
> - avd_d2d_chg_role_rsp(cb, NCSCC_RC_FAILURE, SA_AMF_HA_ACTIVE);
> - return NCSCC_RC_FAILURE;
> + // reuse clm track start
> + if (avd_cb->is_clm_track_started == false) {
> + if(avd_clm_track_start() != SA_AIS_OK) {
> + 

Re: [devel] [PATCH 1 of 1] log: implement SaLogFilterSetCallbackT [#2146]

[Lennart Lund]

Hi Canh,

I have some comments:

- API version handling (minor version) is a bit questionable. Maybe Mahesh CLM 
handling should get an "official" version 2 (is somewhat hidden as is) and 
version 3 to be used for the callback addition. I think there is a possibility 
that someone will set version 2 when initializing and assume that filter 
callback is supported but the server is running 5.1 that will accept version 2 
but does not support filter callback.

- It would be nice if you could find a way to contain filter handling in a 
better way. I can see that some of the new handling is hidden in the anonymous 
utility.cc file

- This is not a part of the new code but is there any test that verifies that 
filtered out log records are not written to the log files if sent by the client

Thanks
Lennart

> -Original Message-
> From: Canh Van Truong
> Sent: den 16 november 2016 08:11
> To: Lennart Lund ; Vu Minh Nguyen
> ; mahesh.va...@oracle.com; Anders Widell
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1 of 1] log: implement SaLogFilterSetCallbackT [#2146]
> 
> Hi Lennart,
> 
> I already did the upgrade test as your below and it was passed from my side.
> I will check and run all test again
> 
> Regards,
> Canh
> 
> -Original Message-
> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> Sent: Tuesday, November 15, 2016 11:16 PM
> To: Canh Van Truong; Vu Minh Nguyen; mahesh.va...@oracle.com; Anders
> Widell
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1 of 1] log: implement SaLogFilterSetCallbackT [#2146]
> 
> Hi Canh
> 
> I have done some testing and have found problems with compatibility
> between
> versions that will cause problems e.g. during an upgrade.
> I have used three nodes running different variants of OpenSAF 5.1 and
> default with your patch (5.2) and have found the following:
> 
> All nodes running 5.2: All tests Ok
> SC-1 and SC-2 (5.2) and PL-3 (5.1): Many tests fail. No test should fail
> SC-1 and SC-2 (5.1) and PL-3 (5.2): Many tests fail. No test except suite 17
> should fail
> SC-1 (5.2) SC-2 (5.1) and PL-3 (5.2): Ok
> 
> Yet to test:
> Does filter settings still work after switch over or fail over?
> 
> Thanks
> Lennart
> 
> 
> > -Original Message-
> > From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> > Sent: den 14 november 2016 11:43
> > To: Lennart Lund ; Vu Minh Nguyen
> > ; mahesh.va...@oracle.com; Anders
> Widell
> > 
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: [PATCH 1 of 1] log: implement SaLogFilterSetCallbackT [#2146]
> >
> >  osaf/libs/agents/saf/lga/lga.h|7 +-
> >  osaf/libs/agents/saf/lga/lga_api.c|4 +-
> >  osaf/libs/agents/saf/lga/lga_mds.c|   72 +-
> >  osaf/libs/agents/saf/lga/lga_util.c   |   58 -
> >  osaf/libs/common/logsv/include/lgsv_msg.h |8 +
> >  osaf/libs/saf/include/saLog.h |4 +
> >  osaf/services/saf/logsv/lgs/lgs_cb.h  |1 +
> >  osaf/services/saf/logsv/lgs/lgs_evt.cc|9 +-
> >  osaf/services/saf/logsv/lgs/lgs_evt.h |2 +-
> >  osaf/services/saf/logsv/lgs/lgs_imm.cc|7 +
> >  osaf/services/saf/logsv/lgs/lgs_mbcsv.cc  |3 +-
> >  osaf/services/saf/logsv/lgs/lgs_mbcsv.h   |1 +
> >  osaf/services/saf/logsv/lgs/lgs_mds.cc|   76 --
> >  osaf/services/saf/logsv/lgs/lgs_recov.cc  |5 +-
> >  osaf/services/saf/logsv/lgs/lgs_recov.h   |1 +
> >  osaf/services/saf/logsv/lgs/lgs_util.cc   |   81 ++
> >  osaf/services/saf/logsv/lgs/lgs_util.h|3 +-
> >  tests/logsv/tet_saLogFilterSetCallbackT.c |  364
> > +-
> >  tests/logsv/tet_saLogStreamOpen_2.c   |2 -
> >  19 files changed, 663 insertions(+), 45 deletions(-)
> >
> >
> > Implement SaLogFilterSetCallbackT which is mentioned at section 3.6.5
> > SaLogFilterSetCallbackT @ AIS LOG document.
> >
> > LGS: - Storing filter callback flag when client request open stream
> >  - Whenever severity filter is changed for app and systerm
> > streams, lgs will find which clients that need severity filter
> > callback and associate with the stream. Then lgs sends message callback to
> clients.
> >  - Encoding callback message for severity filter callback
> >
> > LGA: - Sending filter callback flag to lgs when open stream
> >  - Decoding callback message for severity callback from lgs
> >  - Dispatching severity filter callback
> >
> > diff --git a/osaf/libs/agents/saf/lga/lga.h
> > b/osaf/libs/agents/saf/lga/lga.h
> > --- a/osaf/libs/agents/saf/lga/lga.h
> > +++ b/osaf/libs/agents/saf/lga/lga.h
> > @@ -35,9 +35,9 @@
> >  #include "lgsv_msg.h"
> >  #include "lgsv_defs.h"
> >
> > -#define LGA_SVC_PVT_SUBPART_VERSION  1
> > +#define LGA_SVC_PVT_SUBPART_VERSION  2
> >  #define 

[devel] [PATCH 0 of 1] Review Request for amfd: si-swap timeout during multiple switchover test [#2190]

[Long HB Nguyen]

Summary: amfd: si-swap timeout during multiple switchover test [#2190]
Review request for Trac Ticket(s): #2190
Peer Reviewer(s): AMF devs
Pull request to: AMF maintainers
Affected branch(es): 5.0, 5.1, 5.2
Development branch: 5.2


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


Comments (indicate scope for each "y" above):
-
 IMM jobs are put in a queue to be performed later.
 When a si-swap on safSi=SC-2N,safApp=OpenSAF is performed,
 there may be a chance that the return code for si-swap operation
 has not been returned before OI is disconnected. In that case,
 admin operation returns timeout (SA_AIS_ERR_TIMEOUT).

changeset 7a51e509f6b2ce2e2f0f63663f2e3d75d1bf4553
Author: Long HB Nguyen
Date:   Wed, 16 Nov 2016 17:32:59 +0700

amfd: fix si-swap timeout [#2190]


Complete diffstat:
--
 osaf/services/saf/amf/amfd/imm.cc|  22 ++
 osaf/services/saf/amf/amfd/include/imm.h |   2 ++
 osaf/services/saf/amf/amfd/role.cc   |   5 +
 3 files changed, 29 insertions(+), 0 deletions(-)


Testing Commands:
-
 As the ticket.


Testing, Expected Results:
--
 si-swap successfully.


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 ~/.hgrc file (i.e. username, 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.


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1 of 1] amfd: fix si-swap timeout [#2190]

[Long HB Nguyen]

 osaf/services/saf/amf/amfd/imm.cc|  22 ++
 osaf/services/saf/amf/amfd/include/imm.h |   2 ++
 osaf/services/saf/amf/amfd/role.cc   |   5 +
 3 files changed, 29 insertions(+), 0 deletions(-)


diff --git a/osaf/services/saf/amf/amfd/imm.cc 
b/osaf/services/saf/amf/amfd/imm.cc
--- a/osaf/services/saf/amf/amfd/imm.cc
+++ b/osaf/services/saf/amf/amfd/imm.cc
@@ -423,6 +423,28 @@ AvdJobDequeueResultT Fifo::execute(const
return ret;
 }
 
+AvdJobDequeueResultT Fifo::executeAdminResp(const AVD_CL_CB *cb)
+{
+   Job *ajob;
+   AvdJobDequeueResultT ret = JOB_EXECUTED;
+
+   TRACE_ENTER();
+
+   while ((ajob = peek()) != nullptr) {
+   if (dynamic_cast(ajob) != nullptr) {
+   ret = ajob->exec(cb);
+   } else {
+   ajob = dequeue();
+   delete ajob;
+   ret = JOB_EXECUTED;
+   }
+   }
+
+   TRACE_LEAVE2("%d", ret);
+
+   return ret;
+}
+
 //
 void Fifo::empty()
 {
diff --git a/osaf/services/saf/amf/amfd/include/imm.h 
b/osaf/services/saf/amf/amfd/include/imm.h
--- a/osaf/services/saf/amf/amfd/include/imm.h
+++ b/osaf/services/saf/amf/amfd/include/imm.h
@@ -146,6 +146,8 @@ public:
 
 static AvdJobDequeueResultT execute(const AVD_CL_CB *cb);
 
+static AvdJobDequeueResultT executeAdminResp(const AVD_CL_CB *cb);
+
 static void empty();
 
static uint32_t size();
diff --git a/osaf/services/saf/amf/amfd/role.cc 
b/osaf/services/saf/amf/amfd/role.cc
--- a/osaf/services/saf/amf/amfd/role.cc
+++ b/osaf/services/saf/amf/amfd/role.cc
@@ -766,6 +766,11 @@ void avd_mds_qsd_role_evh(AVD_CL_CB *cb,
}
 
 try_again:
+   /* Execute admin op jobs before calling saImmOiImplementerClear to avoid
+* SA_AIS_ERR_TIMEOUT
+*/
+   Fifo::executeAdminResp(cb);
+
/* Take mutex here to sync with imm reinit thread.*/
osaf_mutex_lock_ordie(_reinit_mutex);
/* Give up IMM OI implementer role */

--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] cpsv: on cpnd down fist remove child safReplica object then parent safCkpt object [#2189]

[A V Mahesh]

Hi Hoang,

Additional TRACE will be removed after  testing.

-AVM

On 11/16/2016 2:28 PM, mahesh.va...@oracle.com wrote:
>   osaf/services/saf/cpsv/cpd/cpd_imm.c  |  20 
>   osaf/services/saf/cpsv/cpd/cpd_proc.c |   9 -
>   2 files changed, 20 insertions(+), 9 deletions(-)
>
>
> Bug :
> While cpd processing cpnd down for  COLLOCATED  cktp  and that checkpoint
> only exist on the went down cpnd ( no others Node opened this checkpoint in 
> cluster) ,
> then cpd  removes  that checkpoint and replica completely.
>
> In such case the current logic has as bug,  fist it removes ckpt node and 
> then replica,
> this is causing deletion of parent object safCkpt=...,*  first , then child 
> object safReplica=...,safCkpt=...,* next.
>
> as we know IMM removes child if parent is removed ,so this is causing the 
> issue out of
> sequence remove of safReplica object and ERR_NOT_EXIST  is returned.
>
> Fix :
> While cpd removing  that checkpoint and replica completely ,
> follow the sequence of  child object safReplica=...,safCkpt=...,*  fist then  
> parent object safCkpt=...,* next.
>
> This is focused fix , my be we need to review complete code for such 
> occurrences , if found
> will be addressed in new ticket.
>
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_imm.c 
> b/osaf/services/saf/cpsv/cpd/cpd_imm.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_imm.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_imm.c
> @@ -400,7 +400,9 @@ SaAisErrorT delete_runtime_replica_objec
>   osaf_extended_name_lend(replica_dn, _name);
>   rc = immutil_saImmOiRtObjectDelete(immOiHandle, _name);
>   if (rc != SA_AIS_OK) {
> - LOG_ER("Deleting run time object %s Failed - rc = 
> %d",replica_dn, rc);
> + LOG_ER("Deleting run time object %s Failed-1 - rc = 
> %d",replica_dn, rc);
> + } else {
> + TRACE("Deleting run time object %s Success-1 - rc = 
> %d",replica_dn, rc);
>   }
>   
>   free(replica_dn);
> @@ -522,9 +524,11 @@ SaAisErrorT delete_runtime_ckpt_object(C
>   osaf_extended_name_lend(ckpt_node->ckpt_name, _name);
>   
>   rc =  immutil_saImmOiRtObjectDelete(immOiHandle, _name);
> - if (rc != SA_AIS_OK)
> + if (rc != SA_AIS_OK) {
>   LOG_ER("Deleting run time object %s failed - rc = %d", 
> ckpt_node->ckpt_name, rc);
> -
> + } else {
> + TRACE("Deleting run time object %s success - rc = %d", 
> ckpt_node->ckpt_name, rc);
> + }
>   return rc;
>   }
>   
> @@ -917,11 +921,11 @@ SaAisErrorT cpd_clean_checkpoint_objects
>  /* Delete the runtime object and its children. */
>  rc = immutil_saImmOiRtObjectDelete(cb->immOiHandle, 
> _name);
>  if (rc == SA_AIS_OK) {
> -   TRACE("Object \"%s\" deleted", (char *) 
> osaf_extended_name_borrow(_name));
> -   } else {
> -   LOG_ER("%s saImmOiRtObjectDelete for \"%s\" FAILED 
> %d",
> -   __FUNCTION__, (char *) 
> osaf_extended_name_borrow(_name), rc);
> -   }
> +TRACE("saImmOiRtObjectDelete \"%s\" deleted 
> Successfully", (char *) osaf_extended_name_borrow(_name));
> +} else {
> +LOG_ER("%s saImmOiRtObjectDelete for \"%s\" FAILED %d",
> +__FUNCTION__, (char *) 
> osaf_extended_name_borrow(_name), rc);
> +}
>  }
>   
>  if (rc != SA_AIS_ERR_NOT_EXIST) {
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_proc.c 
> b/osaf/services/saf/cpsv/cpd/cpd_proc.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_proc.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_proc.c
> @@ -809,6 +809,11 @@ uint32_t cpd_process_cpnd_down(CPD_CB *c
>   send_evt.info.cpnd.info.ckpt_del.mds_dest = *cpnd_dest;
>   if (ckpt_node->dest_cnt == 0) {
>   TRACE_1("cpd ckpt del success for 
> ckpt_id:%llx",ckpt_node->ckpt_id);
> + /* Delete reploc fist*/
> + cpd_ckpt_reploc_get(>ckpt_reploc_tree, 
> _info, _info);
> + if (rep_info) {
> + cpd_ckpt_reploc_node_delete(cb, 
> rep_info, ckpt_node->is_unlink_set);
> + }
>   cpd_ckpt_map_node_get(>ckpt_map_tree, 
> ckpt_node->ckpt_name, _info);
>   
>   /* Remove the ckpt_node */
> @@ -875,7 +880,7 @@ uint32_t cpd_process_cpnd_down(CPD_CB *c
>   /* Send it to CPD(s), by sending ckpt_id = 0 */
>   /* This is to delete the node from reploc_tree */
>   cpd_ckpt_reploc_get(>ckpt_reploc_tree, _info, 
> _info);
> - if (rep_info) {
> + if ((rep_info) && (ckpt_node)) {
>   cpd_ckpt_reploc_node_delete(cb, rep_info, 
> ckpt_node->is_unlink_set);
>   }
>   
> @@ -1238,6 +1243,8 

[devel] [PATCH 0 of 1] Review Request for cpsv: on cpnd down fist remove child safReplica object then parent safCkpt object [#2189]

[mahesh . valla]

Summary:v: on cpnd down fist remove child safReplica object then parent safCkpt 
object [#2189]
Review request for Trac Ticket(s): #2189
Peer Reviewer(s): Hoang Vo
Pull request to: <>
Affected branch(es): default , 5.1
Development branch: default


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


Comments (indicate scope for each "y" above):
-

changeset b4465473c360355d61d1b7eecef3bb3a5e9502b4
Author: A V Mahesh 
Date:   Wed, 16 Nov 2016 14:24:41 +0530

cpsv: on cpnd down fist remove child safReplica object then parent 
safCkpt
object [#2189] Bug : While cpd processing cpnd down for COLLOCATED cktp
and that checkpoint only exist on the went down cpnd ( no others Node 
opened
this checkpoint in cluster) , then cpd removes that checkpoint and 
replica
completely.

In such case the current logic has as bug, fist it removes ckpt node and
then replica, this is causing deletion of parent object safCkpt=...,* 
first
, then child object safReplica=...,safCkpt=...,* next.

as we know IMM removes child if parent is removed ,so this is causing 
the
issue out of sequence remove of safReplica object and ERR_NOT_EXIST is
returned.

Fix : While cpd removing that checkpoint and replica completely , follow
the sequence of child object safReplica=...,safCkpt=...,* fist then
parent object safCkpt=...,* next.

This is focused fix , my be we need to review complete code for such
occurrences , if found will be addressed in new ticket.


Complete diffstat:
--
 osaf/services/saf/cpsv/cpd/cpd_imm.c  |  20 
 osaf/services/saf/cpsv/cpd/cpd_proc.c |   9 -
 2 files changed, 20 insertions(+), 9 deletions(-)


Testing Commands:
-
Reproduce steps:
- Using ckpt demo to create a checkpoint on PL-3.
- Reboot PL-3 and see the logs.

Testing, Expected Results:
--
Deleting run time safReplica object should not be return ERR_NOT_EXIST for 
above case.

Conditions of Submission:
-


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 ~/.hgrc file (i.e. username, 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 

[devel] [PATCH 1 of 1] cpsv: on cpnd down fist remove child safReplica object then parent safCkpt object [#2189]

[mahesh . valla]

 osaf/services/saf/cpsv/cpd/cpd_imm.c  |  20 
 osaf/services/saf/cpsv/cpd/cpd_proc.c |   9 -
 2 files changed, 20 insertions(+), 9 deletions(-)


Bug :
While cpd processing cpnd down for  COLLOCATED  cktp  and that checkpoint
only exist on the went down cpnd ( no others Node opened this checkpoint in 
cluster) ,
then cpd  removes  that checkpoint and replica completely.

In such case the current logic has as bug,  fist it removes ckpt node and then 
replica,
this is causing deletion of parent object safCkpt=...,*  first , then child 
object safReplica=...,safCkpt=...,* next.

as we know IMM removes child if parent is removed ,so this is causing the issue 
out of
sequence remove of safReplica object and ERR_NOT_EXIST  is returned.

Fix :
While cpd removing  that checkpoint and replica completely ,
follow the sequence of  child object safReplica=...,safCkpt=...,*  fist then  
parent object safCkpt=...,* next.

This is focused fix , my be we need to review complete code for such 
occurrences , if found
will be addressed in new ticket.

diff --git a/osaf/services/saf/cpsv/cpd/cpd_imm.c 
b/osaf/services/saf/cpsv/cpd/cpd_imm.c
--- a/osaf/services/saf/cpsv/cpd/cpd_imm.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_imm.c
@@ -400,7 +400,9 @@ SaAisErrorT delete_runtime_replica_objec
osaf_extended_name_lend(replica_dn, _name);
rc = immutil_saImmOiRtObjectDelete(immOiHandle, _name); 
if (rc != SA_AIS_OK) {
-   LOG_ER("Deleting run time object %s Failed - rc = 
%d",replica_dn, rc);
+   LOG_ER("Deleting run time object %s Failed-1 - rc = 
%d",replica_dn, rc);
+   } else {
+   TRACE("Deleting run time object %s Success-1 - rc = 
%d",replica_dn, rc);
}
 
free(replica_dn);
@@ -522,9 +524,11 @@ SaAisErrorT delete_runtime_ckpt_object(C
osaf_extended_name_lend(ckpt_node->ckpt_name, _name);
 
rc =  immutil_saImmOiRtObjectDelete(immOiHandle, _name);
-   if (rc != SA_AIS_OK)
+   if (rc != SA_AIS_OK) {
LOG_ER("Deleting run time object %s failed - rc = %d", 
ckpt_node->ckpt_name, rc);
-
+   } else {
+   TRACE("Deleting run time object %s success - rc = %d", 
ckpt_node->ckpt_name, rc);
+   }
return rc;
 }
 
@@ -917,11 +921,11 @@ SaAisErrorT cpd_clean_checkpoint_objects
/* Delete the runtime object and its children. */
rc = immutil_saImmOiRtObjectDelete(cb->immOiHandle, 
_name);
if (rc == SA_AIS_OK) {
-   TRACE("Object \"%s\" deleted", (char *) 
osaf_extended_name_borrow(_name));
-   } else {
-   LOG_ER("%s saImmOiRtObjectDelete for \"%s\" FAILED %d",
-   __FUNCTION__, (char *) 
osaf_extended_name_borrow(_name), rc);
-   }
+  TRACE("saImmOiRtObjectDelete \"%s\" deleted 
Successfully", (char *) osaf_extended_name_borrow(_name));
+  } else {
+  LOG_ER("%s saImmOiRtObjectDelete for \"%s\" FAILED %d",
+  __FUNCTION__, (char *) 
osaf_extended_name_borrow(_name), rc);
+  }
}
 
if (rc != SA_AIS_ERR_NOT_EXIST) {
diff --git a/osaf/services/saf/cpsv/cpd/cpd_proc.c 
b/osaf/services/saf/cpsv/cpd/cpd_proc.c
--- a/osaf/services/saf/cpsv/cpd/cpd_proc.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_proc.c
@@ -809,6 +809,11 @@ uint32_t cpd_process_cpnd_down(CPD_CB *c
send_evt.info.cpnd.info.ckpt_del.mds_dest = *cpnd_dest;
if (ckpt_node->dest_cnt == 0) {
TRACE_1("cpd ckpt del success for 
ckpt_id:%llx",ckpt_node->ckpt_id);
+   /* Delete reploc fist*/
+   cpd_ckpt_reploc_get(>ckpt_reploc_tree, 
_info, _info);
+   if (rep_info) {
+   cpd_ckpt_reploc_node_delete(cb, 
rep_info, ckpt_node->is_unlink_set);
+   }
cpd_ckpt_map_node_get(>ckpt_map_tree, 
ckpt_node->ckpt_name, _info);
 
/* Remove the ckpt_node */
@@ -875,7 +880,7 @@ uint32_t cpd_process_cpnd_down(CPD_CB *c
/* Send it to CPD(s), by sending ckpt_id = 0 */
/* This is to delete the node from reploc_tree */
cpd_ckpt_reploc_get(>ckpt_reploc_tree, _info, 
_info);
-   if (rep_info) {
+   if ((rep_info) && (ckpt_node)) {
cpd_ckpt_reploc_node_delete(cb, rep_info, 
ckpt_node->is_unlink_set);
}
 
@@ -1238,6 +1243,8 @@ uint32_t cpd_ckpt_reploc_imm_object_dele
LOG_ER("Deleting run time object %s FAILED", 
replica_dn);
free(replica_dn);
return NCSCC_RC_FAILURE;
+   } else {
+   TRACE("Deleting 

Re: [devel] [PATCH 1 of 1] fix crash problem by checking null pointer before accessing its detail

[A V Mahesh]

Ok, please re-test  and publish.

-AVM


On 11/16/2016 2:43 PM, Vo Minh Hoang wrote:
> Dear Mahesh,
>
> Thank you very much.
>
> Might it be like this acceptable, If yes I will send V2 patch.
>
>   if (ckpt_node->node_users_cnt) {
>   int count = 0;
>   CPD_NODE_USER_INFO *node_user = ckpt_node->node_users;
>   cpd_msg.info.usr_info_2.node_list =
> malloc(ckpt_node->node_users_cnt * sizeof(CPD_NODE_USER_INFO));
>   memset(cpd_msg.info.usr_info_2.node_list, '\0',
> (sizeof(CPD_NODE_USER_INFO) * ckpt_node->node_users_cnt));
>
>   for (; node_user != NULL && count <
> ckpt_node->node_users_cnt; node_user = node_user->next) {
>   cpd_msg.info.usr_info_2.node_list[count].dest =
> node_user->dest;
>   cpd_msg.info.usr_info_2.node_list[count].num_users =
> node_user->num_users;
>   cpd_msg.info.usr_info_2.node_list[count].num_readers
> = node_user->num_readers;
>   cpd_msg.info.usr_info_2.node_list[count].num_writers
> = node_user->num_writers;
>   ++count;
>   }
>
>   /* Update node_users_cnt in case of mismatch */
>   ckpt_node->node_users_cnt = count;
>   cpd_msg.info.usr_info_2.node_users_cnt = count;
>   }
>
>
> Sincerely,
> Hoang
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Wednesday, November 16, 2016 3:29 PM
> To: Vo Minh Hoang ; anders.wid...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] fix crash problem by checking null pointer
> before accessing its detail
>
> Hi,
>
> Still we can right the code, I just checked the code,  follow the way
> `node_user = node_user->next` is used in other occurrence part of the code.
>
> -AVM
>
> On 11/16/2016 12:16 PM, Vo Minh Hoang wrote:
>> Dear Mahesh,
>>
>> Thank you very much for your comments.
>>
>> Because the coredump occur when ckpt_node->node_users_cnt is not
>> updated correctly to the number of node_user so we count here to
>> handle that mismatch.
>> Might it possible to keep using like currently?
>>
>> Sincerely,
>> Hoang
>>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: Wednesday, November 16, 2016 11:00 AM
>> To: Vo Minh Hoang ;
>> anders.wid...@ericsson.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] fix crash problem by checking null pointer
>> before accessing its detail
>>
>>
>> Hi Hoang Vo,
>>
 Please let me know if you have any further inquiry.
>> Can you please also make the fix more readable replace  `for (count =
>> 0; count < ckpt_node->node_users_cnt; count++) {` some thing like `for
>> (node_user = ckpt_node->node_users; node_user != NULL; node_user =
>> node_user->next) {`
>> then, we can removevariable `int count = 0;`,   move
>> `cpd_msg.info.usr_info_2.node_users_cnt = ckpt_node->node_users_cnt;`
>> after for () loop.
>>
>> -AVM
>>
>>
>> On 11/16/2016 9:14 AM, Vo Minh Hoang wrote:
>>> Dear Mahesh,
>>>
>>> I am sorry that I cannot share the test steps because I cannot
>>> reproduce it in local environment.
>>> I've just received the coredump information point directly to this
>>> part, reviewed source code and found that pointer using is unsafe so
>>> I
>> correct it.
>>> Please let me know if you have any further inquiry.
>>>
>>> Thank you and best regard,
>>> Hoang
>>>
>>> -Original Message-
>>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>>> Sent: Wednesday, November 16, 2016 10:22 AM
>>> To: Hoang Vo ; anders.wid...@ericsson.com
>>> Cc: opensaf-devel@lists.sourceforge.net
>>> Subject: Re: [PATCH 1 of 1] fix crash problem by checking null
>>> pointer before accessing its detail
>>>
>>> Hi Hoang Vo,
>>>
>>> On 11/15/2016 12:57 PM, Hoang Vo wrote:
 Testing Commands:
 -


 Testing, Expected Results:
 --

>>> Can you please share test case .
>>>
>>> -AVM
>>>
>>> On 11/15/2016 12:57 PM, Hoang Vo wrote:
  osaf/services/saf/cpsv/cpd/cpd_red.c |  5 +
  1 files changed, 5 insertions(+), 0 deletions(-)


 diff --git a/osaf/services/saf/cpsv/cpd/cpd_red.c
>>> b/osaf/services/saf/cpsv/cpd/cpd_red.c
 --- a/osaf/services/saf/cpsv/cpd/cpd_red.c
 +++ b/osaf/services/saf/cpsv/cpd/cpd_red.c
 @@ -322,6 +322,11 @@ void cpd_a2s_ckpt_usr_info(CPD_CB *cb, C
memset(cpd_msg.info.usr_info_2.node_list, '\0',
>>> (sizeof(CPD_NODE_USER_INFO) * ckpt_node->node_users_cnt));
  
for (count = 0; count < ckpt_node->node_users_cnt;
> count++)
>>> {
 +  if (node_user == NULL) {
 +  ckpt_node->node_users_cnt = count;
 +  cpd_msg.info.usr_info_2.node_users_cnt =
>>> 

Re: [devel] [PATCH 1 of 1] fix crash problem by checking null pointer before accessing its detail

[Vo Minh Hoang]

Dear Mahesh,

Thank you very much.

Might it be like this acceptable, If yes I will send V2 patch.

if (ckpt_node->node_users_cnt) {
int count = 0;
CPD_NODE_USER_INFO *node_user = ckpt_node->node_users;
cpd_msg.info.usr_info_2.node_list =
malloc(ckpt_node->node_users_cnt * sizeof(CPD_NODE_USER_INFO));
memset(cpd_msg.info.usr_info_2.node_list, '\0',
(sizeof(CPD_NODE_USER_INFO) * ckpt_node->node_users_cnt));

for (; node_user != NULL && count <
ckpt_node->node_users_cnt; node_user = node_user->next) {
cpd_msg.info.usr_info_2.node_list[count].dest =
node_user->dest;
cpd_msg.info.usr_info_2.node_list[count].num_users =
node_user->num_users;
cpd_msg.info.usr_info_2.node_list[count].num_readers
= node_user->num_readers;
cpd_msg.info.usr_info_2.node_list[count].num_writers
= node_user->num_writers;
++count;
}

/* Update node_users_cnt in case of mismatch */
ckpt_node->node_users_cnt = count;
cpd_msg.info.usr_info_2.node_users_cnt = count;
}


Sincerely,
Hoang

-Original Message-
From: A V Mahesh [mailto:mahesh.va...@oracle.com] 
Sent: Wednesday, November 16, 2016 3:29 PM
To: Vo Minh Hoang ; anders.wid...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 1] fix crash problem by checking null pointer
before accessing its detail

Hi,

Still we can right the code, I just checked the code,  follow the way
`node_user = node_user->next` is used in other occurrence part of the code.

-AVM

On 11/16/2016 12:16 PM, Vo Minh Hoang wrote:
> Dear Mahesh,
>
> Thank you very much for your comments.
>
> Because the coredump occur when ckpt_node->node_users_cnt is not 
> updated correctly to the number of node_user so we count here to 
> handle that mismatch.
> Might it possible to keep using like currently?
>
> Sincerely,
> Hoang
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Wednesday, November 16, 2016 11:00 AM
> To: Vo Minh Hoang ; 
> anders.wid...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] fix crash problem by checking null pointer 
> before accessing its detail
>
>
> Hi Hoang Vo,
>
>>> Please let me know if you have any further inquiry.
>
> Can you please also make the fix more readable replace  `for (count = 
> 0; count < ckpt_node->node_users_cnt; count++) {` some thing like `for 
> (node_user = ckpt_node->node_users; node_user != NULL; node_user =
> node_user->next) {`
> then, we can removevariable `int count = 0;`,   move
> `cpd_msg.info.usr_info_2.node_users_cnt = ckpt_node->node_users_cnt;` 
> after for () loop.
>
> -AVM
>
>
> On 11/16/2016 9:14 AM, Vo Minh Hoang wrote:
>> Dear Mahesh,
>>
>> I am sorry that I cannot share the test steps because I cannot 
>> reproduce it in local environment.
>> I've just received the coredump information point directly to this 
>> part, reviewed source code and found that pointer using is unsafe so 
>> I
> correct it.
>> Please let me know if you have any further inquiry.
>>
>> Thank you and best regard,
>> Hoang
>>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: Wednesday, November 16, 2016 10:22 AM
>> To: Hoang Vo ; anders.wid...@ericsson.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] fix crash problem by checking null 
>> pointer before accessing its detail
>>
>> Hi Hoang Vo,
>>
>> On 11/15/2016 12:57 PM, Hoang Vo wrote:
>>> Testing Commands:
>>> -
>>>
>>>
>>> Testing, Expected Results:
>>> --
>>>
>> Can you please share test case .
>>
>> -AVM
>>
>> On 11/15/2016 12:57 PM, Hoang Vo wrote:
>>> osaf/services/saf/cpsv/cpd/cpd_red.c |  5 +
>>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>>
>>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_red.c
>> b/osaf/services/saf/cpsv/cpd/cpd_red.c
>>> --- a/osaf/services/saf/cpsv/cpd/cpd_red.c
>>> +++ b/osaf/services/saf/cpsv/cpd/cpd_red.c
>>> @@ -322,6 +322,11 @@ void cpd_a2s_ckpt_usr_info(CPD_CB *cb, C
>>> memset(cpd_msg.info.usr_info_2.node_list, '\0',
>> (sizeof(CPD_NODE_USER_INFO) * ckpt_node->node_users_cnt));
>>> 
>>> for (count = 0; count < ckpt_node->node_users_cnt;
count++)
>> {
>>> +   if (node_user == NULL) {
>>> +   ckpt_node->node_users_cnt = count;
>>> +   cpd_msg.info.usr_info_2.node_users_cnt =
>> count;
>>> +   break;
>>> +   }
>>>
cpd_msg.info.usr_info_2.node_list[count].dest =
>> node_user->dest;
>>>
cpd_msg.info.usr_info_2.node_list[count].num_users =
>> 

Re: [devel] [PATCH 1 of 1] fix crash problem by checking null pointer before accessing its detail

[A V Mahesh]

Hi,

Still we can right the code, I just checked the code,  follow the
way `node_user = node_user->next` is used in other occurrence part of 
the code.

-AVM

On 11/16/2016 12:16 PM, Vo Minh Hoang wrote:
> Dear Mahesh,
>
> Thank you very much for your comments.
>
> Because the coredump occur when ckpt_node->node_users_cnt is not updated
> correctly to the number of node_user so we count here to handle that
> mismatch.
> Might it possible to keep using like currently?
>
> Sincerely,
> Hoang
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Wednesday, November 16, 2016 11:00 AM
> To: Vo Minh Hoang ; anders.wid...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] fix crash problem by checking null pointer
> before accessing its detail
>
>
> Hi Hoang Vo,
>
>>> Please let me know if you have any further inquiry.
>
> Can you please also make the fix more readable replace  `for (count = 0;
> count < ckpt_node->node_users_cnt; count++) {` some thing like `for
> (node_user = ckpt_node->node_users; node_user != NULL; node_user =
> node_user->next) {`
> then, we can removevariable `int count = 0;`,   move
> `cpd_msg.info.usr_info_2.node_users_cnt = ckpt_node->node_users_cnt;` after
> for () loop.
>
> -AVM
>
>
> On 11/16/2016 9:14 AM, Vo Minh Hoang wrote:
>> Dear Mahesh,
>>
>> I am sorry that I cannot share the test steps because I cannot
>> reproduce it in local environment.
>> I've just received the coredump information point directly to this
>> part, reviewed source code and found that pointer using is unsafe so I
> correct it.
>> Please let me know if you have any further inquiry.
>>
>> Thank you and best regard,
>> Hoang
>>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: Wednesday, November 16, 2016 10:22 AM
>> To: Hoang Vo ; anders.wid...@ericsson.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] fix crash problem by checking null pointer
>> before accessing its detail
>>
>> Hi Hoang Vo,
>>
>> On 11/15/2016 12:57 PM, Hoang Vo wrote:
>>> Testing Commands:
>>> -
>>>
>>>
>>> Testing, Expected Results:
>>> --
>>>
>> Can you please share test case .
>>
>> -AVM
>>
>> On 11/15/2016 12:57 PM, Hoang Vo wrote:
>>> osaf/services/saf/cpsv/cpd/cpd_red.c |  5 +
>>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>>
>>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_red.c
>> b/osaf/services/saf/cpsv/cpd/cpd_red.c
>>> --- a/osaf/services/saf/cpsv/cpd/cpd_red.c
>>> +++ b/osaf/services/saf/cpsv/cpd/cpd_red.c
>>> @@ -322,6 +322,11 @@ void cpd_a2s_ckpt_usr_info(CPD_CB *cb, C
>>> memset(cpd_msg.info.usr_info_2.node_list, '\0',
>> (sizeof(CPD_NODE_USER_INFO) * ckpt_node->node_users_cnt));
>>> 
>>> for (count = 0; count < ckpt_node->node_users_cnt; 
>>> count++)
>> {
>>> +   if (node_user == NULL) {
>>> +   ckpt_node->node_users_cnt = count;
>>> +   cpd_msg.info.usr_info_2.node_users_cnt =
>> count;
>>> +   break;
>>> +   }
>>> cpd_msg.info.usr_info_2.node_list[count].dest =
>> node_user->dest;
>>> 
>>> cpd_msg.info.usr_info_2.node_list[count].num_users =
>> node_user->num_users;
>>> 
>>> cpd_msg.info.usr_info_2.node_list[count].num_readers
>> = node_user->num_readers;
>>
>>
>


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel