Re: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in Headless state [#2601]

2017-10-24 Thread Minh Hon Chau 

Hi,

Agree with Ravi that the msg N2D_REG_SU should be queued too, otherwise 
there will be no D2N_PRESENCE_SU msg.


So the condition to queue N2D_INFO_SU_SI_ASSIGN, N2D_OPERATION_STATE and 
D2N_PRESENCE_SU should be:


cb->is_avd_down == true || cb->amfd_sync_required == true

The reason is the above 3 messages won't be processed until amfd 
finishes sync


Also agree with Gary that N2D_DATA_REQUEST should be queued with: 
cb->is_avd_down == false && cb->amfd_sync_required == true
The reason is the DATA is carried in sync_info msg. After amfd is up and 
before node_up is accepted, if the DATA_REQUEST msg is sent and rejected 
by amfd, it's still in the queue and will be resent after sync by 
avnd_diq_rec_send_buffered_msg(). However, the msgs in queue could have 
the msg_id not in ascending order.


For the PG track msg, I think we probably need to return TRY_AGAIN on 
waiting for acceptance of node_up too, because the pg track is resent 
when node_up is accepted (on stop resp_tmr timer), so the PG track may 
be duplicated if PG track is called just right after amfd is up. Also, 
it could cause the msg id out of order in the same way of DATA_REQUEST 
in this ticket too.


I can check the D2N_PRESENCE_SU and PG track and raise separated tickets 
if there're bugs (most likely)

The solution of the patch looks ok to me within Gary's comment.

Thanks,
Minh
On 25/10/17 14:58, Gary Lee wrote:

Hi Ravi

>From what I can see, the main problem is AMFND is sending messages after AMFD 
has come up, but before NODE_UP has been accepted by AMFD.
If that is correct, then PG track messages, like DATA_REQUEST, should be in the 
same situation as it’s only checking is_avd_down.

uint32_t avnd_evt_ava_pg_start_evh(AVND_CB *cb, AVND_EVT *evt) {

   // if headless, return TRY_AGAIN to application
   if (cb->is_avd_down == true) {
 LOG_NO("Director is down. Return try again for PG start.");
  ..
   }
   ..
}

I think we should only buffer these messages if “is_avd_down == false && 
amfd_sync_required == true”.
We should be able to drop DATA_REQUEST when it’s headless, since that type of 
information is sync’ed with AVSV_N2D_ND_CSICOMP_STATE_INFO_MSG and 
AVSV_N2D_ND_SISU_STATE_INFO_MSG.

Thanks
Gary

On 24/10/17, 11:14 pm, "Ravi Sekhar Reddy Konda"  
wrote:

 Hi Gary,
 
 There are some messages which does not required to be queued like PG_TRACK & NODE_DOWN messages

 During SC absence period, we are returning SA_AMF_ERROR_TRY_AGAIN for PG 
Track operations
 
 Currently along with this Fix we are queuing the following messages

 AVSV_N2D_INFO_SU_SI_ASSIGN_MSG
 AVSV_N2D_OPERATION_STATE_MSG
 AVSV_N2D_DATA_REQUEST_MSG
 
 only missing event is Reg SU response event (AVSV_N2D_REG_SU_MSG), I think this event has to be queued.
 
 if you agree with this. I will update the patch by adding queuing for Reg SU response event also
 
 
 Thanks,

 Ravi
 
 -Original Message-

 From: Gary Lee [mailto:gary@dektech.com.au]
 Sent: Wednesday, October 18, 2017 10:57 AM
 To: ravi-sekhar ; hans.nordeb...@ericsson.com
 Cc: opensaf-devel@lists.sourceforge.net
 Subject: Re: [devel] [PATCH 1/1] amf: Buffer and resend data req messages 
in Headless state [#2601]
 
 Hi Ravi
 
 I’ve started looking at this. My initial thought is that perhaps we need to queue up all messages when is_avd_down == false && amfd_sync_required == true (ie. AMFD has come up but haven’t accepted node_up). What do you think?
 
 Will get back to you.
 
 /Gary
 
 -Original Message-

 From: ravi-sekhar 
 Date: Tuesday, 17 October 2017 at 10:39 pm
 To: 
 Cc: 
 Subject: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in 
Headless state [#2601]
 
 ---

  src/amf/amfnd/di.cc | 42 +++---
  1 file changed, 31 insertions(+), 11 deletions(-)
 
 diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc

 index 2dc023c..1e0d682 100644
 --- a/src/amf/amfnd/di.cc
 +++ b/src/amf/amfnd/di.cc
 @@ -998,21 +998,30 @@ uint32_t avnd_di_object_upd_send(AVND_CB *cb, 
AVSV_PARAM_INFO *param) {
uint32_t rc = NCSCC_RC_SUCCESS;
TRACE_ENTER2("Comp '%s'", osaf_extended_name_borrow(>name));
  
 -  if (cb->is_avd_down == true) {

 -TRACE_LEAVE2("AVD is down. %u", rc);
 -return rc;
 -  }
 -
 -  memset(, 0, sizeof(AVND_MSG));
 -
/* populate the msg */
 +  memset(, 0, sizeof(AVND_MSG));
msg.info.avd = static_cast(calloc(1, 
sizeof(AVSV_DND_MSG)));
msg.type = AVND_MSG_AVD;
msg.info.avd->msg_type =

[devel] [PATCH 0/1] Review Request for clm: fix errors in clmprint tool [#2651] V3

2017-10-24 Thread Vu Minh Nguyen 
Summary: clm: fix errors in clmprint tool [#2651]
Review request for Ticket(s): 2651
Peer Reviewer(s): Hans, Zoran
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop, release
Development branch: ticket-2651
Base revision: ee7cb901b8eb023502e3c21180afb78dd15c328b
Personal repository: git://git.code.sf.net/u/winhvu/review


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   y

NOTE: Patch(es) contain lines longer than 80 characers

Comments (indicate scope for each "y" above):
-
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision e070300a38f0f564c8c8493f112c68c442c6528c
Author: Vu Minh Nguyen 
Date:   Wed, 25 Oct 2017 10:57:17 +0700

clm: fix errors in clmprint tool [#2651]

Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.



Added Files:

 src/clm/tools/Makefile


Complete diffstat:
--
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 191 +++---
 2 files changed, 182 insertions(+), 27 deletions(-)


Testing Commands:
-
Refer to the ticket description for testing commands.


Testing, Expected Results:
--
Result OK.


Conditions of Submission:
-
Get ack from peer reviewers.


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 ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

2017-10-24 Thread Vu Minh Nguyen 
Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.
---
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 191 +++---
 2 files changed, 182 insertions(+), 27 deletions(-)
 create mode 100644 src/clm/tools/Makefile

diff --git a/src/clm/tools/Makefile b/src/clm/tools/Makefile
new file mode 100644
index 000..1e39957
--- /dev/null
+++ b/src/clm/tools/Makefile
@@ -0,0 +1,18 @@
+#  -*- OpenSAF  -*-
+#
+# Copyright Ericsson AB 2017 - All Rights Reserved.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+# 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
+#
+
+all:
+   $(MAKE) -C ../../.. bin/clmprint
diff --git a/src/clm/tools/clm_print.c b/src/clm/tools/clm_print.c
index eb01d3e..f44aee2 100644
--- a/src/clm/tools/clm_print.c
+++ b/src/clm/tools/clm_print.c
@@ -1,6 +1,7 @@
 /*  -*- OpenSAF  -*-
  *
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
  *
  * This program is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -59,9 +60,9 @@ static char *clm_step[] = {
 
 static void print_node(const SaClmClusterNodeT_4 *clusterNode)
 {
-   printf("Node Info for nodeId: %u>\n",
+   printf("\nNode Info for nodeId: %u>\n",
clusterNode->nodeId);
-   printf("  Node Id: %u(%x)\n",
+   printf("  Node Id: %u(0x%x)\n",
clusterNode->nodeId, clusterNode->nodeId);
printf("  Address Family : %s\n",
clusterNode->nodeAddress.family == 1
@@ -84,6 +85,7 @@ static void print_node(const SaClmClusterNodeT_4 *clusterNode)
clusterNode->bootTimestamp);
printf("  Initial View Number: %llu\n",
clusterNode->initialViewNumber);
+   printf("<===\n\n");
 }
 
 static void  clm_node_get_callback(SaInvocationT invocation,
@@ -133,7 +135,6 @@ static void clm_track_callback(const 
SaClmClusterNotificationBufferT_4
print_node(>notification[i].clusterNode);
printf("  Change : %s\n",
clm_change[notificationBuffer->notification[i].clusterChange]);
-   printf("<\n\n");
}
 
if ((step == SA_CLM_CHANGE_VALIDATE) || (step == SA_CLM_CHANGE_START)) {
@@ -217,6 +218,93 @@ static void clm_dispatch(void)
exit(EXIT_SUCCESS);
}
 }
+
+/*
+ * Exit the program with EXIT_FAILURE if passing invalid options,
+ * update the output `bit_mask` otherwise.
+ */
+static void validate_option(const char opt, uint32_t *bit_mask)
+{
+   enum mask_position {
+   OPT_HELP,
+   OPT_NODEGET,
+   OPT_ASYNCGET,
+   OPT_TRACK,
+   OPT_BTRACK,
+   OPT_TIMEOUT
+   };
+
+   switch (opt) {
+   case 'n':
+   if ((*bit_mask >> OPT_NODEGET) & 1) {
+   fprintf(stderr, "error - duplicated options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_NODEGET);
+   break;
+
+   case 'a':
+   if ((*bit_mask >> OPT_ASYNCGET) & 1) {
+   fprintf(stderr, "error - duplicated options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_ASYNCGET);
+   break;
+
+   case 'b':
+   if ((*bit_mask >> OPT_BTRACK) & 1) {
+   fprintf(stderr, "error - duplicated options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+

Re: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in Headless state [#2601]

2017-10-24 Thread Gary Lee 
Hi Ravi

From what I can see, the main problem is AMFND is sending messages after AMFD 
has come up, but before NODE_UP has been accepted by AMFD.
If that is correct, then PG track messages, like DATA_REQUEST, should be in the 
same situation as it’s only checking is_avd_down.

uint32_t avnd_evt_ava_pg_start_evh(AVND_CB *cb, AVND_EVT *evt) {

  // if headless, return TRY_AGAIN to application
  if (cb->is_avd_down == true) {
LOG_NO("Director is down. Return try again for PG start.");
 ..
  }
  ..
}

I think we should only buffer these messages if “is_avd_down == false && 
amfd_sync_required == true”.
We should be able to drop DATA_REQUEST when it’s headless, since that type of 
information is sync’ed with AVSV_N2D_ND_CSICOMP_STATE_INFO_MSG and 
AVSV_N2D_ND_SISU_STATE_INFO_MSG.

Thanks
Gary

On 24/10/17, 11:14 pm, "Ravi Sekhar Reddy Konda"  
wrote:

Hi Gary,

There are some messages which does not required to be queued like PG_TRACK 
& NODE_DOWN messages
During SC absence period, we are returning SA_AMF_ERROR_TRY_AGAIN for PG 
Track operations 

Currently along with this Fix we are queuing the following messages 
AVSV_N2D_INFO_SU_SI_ASSIGN_MSG
AVSV_N2D_OPERATION_STATE_MSG
AVSV_N2D_DATA_REQUEST_MSG

only missing event is Reg SU response event (AVSV_N2D_REG_SU_MSG), I think 
this event has to be queued.

if you agree with this. I will update the patch by adding queuing for Reg 
SU response event also


Thanks,
Ravi

-Original Message-
From: Gary Lee [mailto:gary@dektech.com.au] 
Sent: Wednesday, October 18, 2017 10:57 AM
To: ravi-sekhar ; hans.nordeb...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1/1] amf: Buffer and resend data req messages 
in Headless state [#2601]

Hi Ravi

I’ve started looking at this. My initial thought is that perhaps we need to 
queue up all messages when is_avd_down == false && amfd_sync_required == true 
(ie. AMFD has come up but haven’t accepted node_up). What do you think?

Will get back to you.

/Gary

-Original Message-
From: ravi-sekhar 
Date: Tuesday, 17 October 2017 at 10:39 pm
To: 
Cc: 
Subject: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in 
Headless state [#2601]

---
 src/amf/amfnd/di.cc | 42 +++---
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc
index 2dc023c..1e0d682 100644
--- a/src/amf/amfnd/di.cc
+++ b/src/amf/amfnd/di.cc
@@ -998,21 +998,30 @@ uint32_t avnd_di_object_upd_send(AVND_CB *cb, 
AVSV_PARAM_INFO *param) {
   uint32_t rc = NCSCC_RC_SUCCESS;
   TRACE_ENTER2("Comp '%s'", osaf_extended_name_borrow(>name));
 
-  if (cb->is_avd_down == true) {
-TRACE_LEAVE2("AVD is down. %u", rc);
-return rc;
-  }
-
-  memset(, 0, sizeof(AVND_MSG));
-
   /* populate the msg */
+  memset(, 0, sizeof(AVND_MSG));
   msg.info.avd = static_cast(calloc(1, 
sizeof(AVSV_DND_MSG)));
   msg.type = AVND_MSG_AVD;
   msg.info.avd->msg_type = AVSV_N2D_DATA_REQUEST_MSG;
-  msg.info.avd->msg_info.n2d_data_req.msg_id = ++(cb->snd_msg_id);
   msg.info.avd->msg_info.n2d_data_req.node_id = cb->node_info.nodeId;
   msg.info.avd->msg_info.n2d_data_req.param_info = *param;
 
+  if ((cb->is_avd_down == true) || (cb->amfd_sync_required == true)) {
+msg.info.avd->msg_info.n2d_data_req.msg_id = 0;
+if (avnd_diq_rec_add(cb, ) == nullptr) {
+  rc = NCSCC_RC_FAILURE;
+}
+LOG_NO(
+"avnd_di_object_upd_send() deferred as AMF director is 
offline(%d),"
+" or sync is required(%d)",
+cb->is_avd_down, cb->amfd_sync_required);
+
+TRACE_LEAVE2("AVD is down. %u", rc);
+return rc;
+  } else {
+msg.info.avd->msg_info.n2d_data_req.msg_id = ++(cb->snd_msg_id);
+  }
+
   /* send the msg to AvD */
   rc = avnd_di_msg_send(cb, );
   if (NCSCC_RC_SUCCESS == rc) msg.info.avd = 0;
@@ -1515,9 +1524,20 @@ void avnd_diq_rec_send_buffered_msg(AVND_CB *cb) 
{
 pending_rec->msg.info.avd->msg_info.n2d_opr_state.rec_rcvr
 .raw);
 ++iter;
-} else {
-  ++iter;
-}
+} else if (pending_rec->msg.info.avd->msg_type == 
AVSV_N2D_DATA_REQUEST_MSG &&
+

Re: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in Headless state [#2601]

2017-10-24 Thread Ravi Sekhar Reddy Konda 
Hi Gary,

There are some messages which does not required to be queued like PG_TRACK & 
NODE_DOWN messages
During SC absence period, we are returning SA_AMF_ERROR_TRY_AGAIN for PG Track 
operations 

Currently along with this Fix we are queuing the following messages 
AVSV_N2D_INFO_SU_SI_ASSIGN_MSG
AVSV_N2D_OPERATION_STATE_MSG
AVSV_N2D_DATA_REQUEST_MSG

only missing event is Reg SU response event (AVSV_N2D_REG_SU_MSG), I think this 
event has to be queued.

if you agree with this. I will update the patch by adding queuing for Reg SU 
response event also


Thanks,
Ravi

-Original Message-
From: Gary Lee [mailto:gary@dektech.com.au] 
Sent: Wednesday, October 18, 2017 10:57 AM
To: ravi-sekhar ; hans.nordeb...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in 
Headless state [#2601]

Hi Ravi

I’ve started looking at this. My initial thought is that perhaps we need to 
queue up all messages when is_avd_down == false && amfd_sync_required == true 
(ie. AMFD has come up but haven’t accepted node_up). What do you think?

Will get back to you.

/Gary

-Original Message-
From: ravi-sekhar 
Date: Tuesday, 17 October 2017 at 10:39 pm
To: 
Cc: 
Subject: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in 
Headless state [#2601]

---
 src/amf/amfnd/di.cc | 42 +++---
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc
index 2dc023c..1e0d682 100644
--- a/src/amf/amfnd/di.cc
+++ b/src/amf/amfnd/di.cc
@@ -998,21 +998,30 @@ uint32_t avnd_di_object_upd_send(AVND_CB *cb, 
AVSV_PARAM_INFO *param) {
   uint32_t rc = NCSCC_RC_SUCCESS;
   TRACE_ENTER2("Comp '%s'", osaf_extended_name_borrow(>name));
 
-  if (cb->is_avd_down == true) {
-TRACE_LEAVE2("AVD is down. %u", rc);
-return rc;
-  }
-
-  memset(, 0, sizeof(AVND_MSG));
-
   /* populate the msg */
+  memset(, 0, sizeof(AVND_MSG));
   msg.info.avd = static_cast(calloc(1, 
sizeof(AVSV_DND_MSG)));
   msg.type = AVND_MSG_AVD;
   msg.info.avd->msg_type = AVSV_N2D_DATA_REQUEST_MSG;
-  msg.info.avd->msg_info.n2d_data_req.msg_id = ++(cb->snd_msg_id);
   msg.info.avd->msg_info.n2d_data_req.node_id = cb->node_info.nodeId;
   msg.info.avd->msg_info.n2d_data_req.param_info = *param;
 
+  if ((cb->is_avd_down == true) || (cb->amfd_sync_required == true)) {
+msg.info.avd->msg_info.n2d_data_req.msg_id = 0;
+if (avnd_diq_rec_add(cb, ) == nullptr) {
+  rc = NCSCC_RC_FAILURE;
+}
+LOG_NO(
+"avnd_di_object_upd_send() deferred as AMF director is 
offline(%d),"
+" or sync is required(%d)",
+cb->is_avd_down, cb->amfd_sync_required);
+
+TRACE_LEAVE2("AVD is down. %u", rc);
+return rc;
+  } else {
+msg.info.avd->msg_info.n2d_data_req.msg_id = ++(cb->snd_msg_id);
+  }
+
   /* send the msg to AvD */
   rc = avnd_di_msg_send(cb, );
   if (NCSCC_RC_SUCCESS == rc) msg.info.avd = 0;
@@ -1515,9 +1524,20 @@ void avnd_diq_rec_send_buffered_msg(AVND_CB *cb) {
 pending_rec->msg.info.avd->msg_info.n2d_opr_state.rec_rcvr
 .raw);
 ++iter;
-} else {
-  ++iter;
-}
+} else if (pending_rec->msg.info.avd->msg_type == 
AVSV_N2D_DATA_REQUEST_MSG &&
+   pending_rec->msg.info.avd->msg_info.n2d_data_req.msg_id == 
0) {
+pending_rec->msg.info.avd->msg_info.n2d_data_req.msg_id =
+  ++(cb->snd_msg_id);
+
+LOG_NO(
+"Found and resend buffered Data Req msg for SU:'%s', 
msg_id:'%u'",
+osaf_extended_name_borrow(_rec->msg.info.avd->msg_info
+   .n2d_data_req.param_info.name),
+pending_rec->msg.info.avd->msg_info.n2d_data_req.msg_id);
+   ++iter;
+ } else {
+   ++iter;
+ }
   }
 
   TRACE("retransmit message to amfd");
-- 
1.9.1



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! 
https://urldefense.proofpoint.com/v2/url?u=http-3A__sdm.link_slashdot=DwIFaQ=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10=rFCQ76TW5HZUgA7b20ApVcXgXru6mvz4fvCm1_H6w1k=_B8vfWmVodcwGLVWeK6LJ5kV9_6Z39lakp7QX8LKQzk=tpxAiV8CXUFAPD5RvnnpQZoFhOdQrBc7h-EA0c9AfZM=
 
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net

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 
  3 files changed, 116 insertions(+)
  create mode 100644 src/base/osaf_unistd.cc
  create mode 100644 src/base/osaf_unistd.h

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index d0d67bbf0..be6f757c7 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -54,11 +54,13 @@ lib_libopensaf_core_la_SOURCES += \
src/base/ncsdlib.c \
src/base/os_defs.c \
src/base/osaf_extended_name.c \
+   src/base/osaf_gcov.c \
src/base/osaf_poll.c \
src/base/osaf_secutil.c \
src/base/osaf_time.c \
src/base/osaf_timerfd.c \
src/base/osaf_unicode.c \
+   src/base/osaf_unistd.cc \
src/base/osaf_utility.c \
src/base/patricia.c \
src/base/process.cc \
@@ -132,6 +134,7 @@ noinst_HEADERS += \
src/base/osaf_time.h \
src/base/osaf_timerfd.h \
src/base/osaf_unicode.h \
+   src/base/osaf_unistd.h \
src/base/osaf_utility.h \
src/base/process.h \
src/base/saf_def.h \
diff --git a/src/base/osaf_unistd.cc b/src/base/osaf_unistd.cc
new file mode 100644
index 0..a08b62ed2
--- /dev/null
+++ b/src/base/osaf_unistd.cc
@@ -0,0 +1,62 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * 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.
+ *
+ */
+
+#include "base/osaf_unistd.h"
+#include 
+#include 
+#include 
+#include 
+#include "base/osaf_utility.h"
+

[HansN] perhaps use const std::string & ?

+int osaf_fdatasync(const char *pathname) {
+  int fd;
+
+  for (;;) {
+fd = open(pathname, O_RDONLY);
+if (fd >= 0) break;
+int e = errno;
+
+if (e == EFAULT) osaf_abort(e);
+if (e != EINTR) return -1;
+  }
+  for (;;) {
+if (fdatasync(fd) >= 0) break;
+int e = errno;
+
+if (e == EBADF) osaf_abort(e);
+if (e != EINTR) {
+  close(fd);
+  errno = e;
+  return -1;
+}
+  }
+  int result = close(fd);
+
+  if (result < 0 && errno == EINTR) result = 0;
+  return result;
+}
+
+int osaf_fdatasync(int fd) {
+  for (;;) {
+if (fdatasync(fd) >= 0) break;
+int e = errno;
+
+if (e == EBADF) osaf_abort(e);
+if (e != EINTR) {
+  return -1;
+}
+  }
+  return 0;
+}
diff --git a/src/base/osaf_unistd.h b/src/base/osaf_unistd.h
new file mode 100644
index 0..734548df4
--- /dev/null
+++ b/src/base/osaf_unistd.h
@@ -0,0 +1,51 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * 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.
+ *
+ */
+
+/** @file
+ *
+ * This file contains convenience functions similar to the ones declared in the
+ * system header file unistd.h
+ */
+
+#ifndef BASE_OSAF_UNISTD_H_
+#define BASE_OSAF_UNISTD_H_
+
+/**
+ * @brief Sync a file or a directory to disk.
+ *
+ * This is a convenience function that calls open(2) and fdatasync(2) to sync
+ * the file or the directory specified by @a pathname. It handles EINTR
+ * internally with a loop. For other errors, this function will return -1 and
+ * set errno apropriately.

[HansN] or osaf_abort at other faults ?

+ *
+ * Please be aware that in order to sync a file that has been written, you
+ * probably also need to sync the directory containing the file.
+ */
+int osaf_fdatasync(const char* pathname);
+
+/**
+ * @brief Sync a file or a directory to disk.
+ *
+ * This is a convenience function that calls fdatasync(2) to sync the file or
+ * the directory specified by the open file descriptor @a fd. It handles EINTR
+ * internally with a loop. For other errors, this function will return -1 and
+ * set errno apropriately.

[HansN] or osaf_abort at other faults ?

+ *
+ * Please be aware that in order to sync a file that has been written, you
+ * probably also need to sync

[devel] [PATCH 1/1] pyosaf: High level python interfaces for CLM [#2602]

2017-10-24 Thread Long H Buu Nguyen 
- Add more error handling
- Refactor clm code
- Keep raising exceptions for existing python methods
---
 python/pyosaf/utils/__init__.py | 267 ++---
 python/pyosaf/utils/clm/__init__.py | 581 ++--
 2 files changed, 717 insertions(+), 131 deletions(-)

diff --git a/python/pyosaf/utils/__init__.py b/python/pyosaf/utils/__init__.py
index 7a1c21b79..fac3c5b80 100644
--- a/python/pyosaf/utils/__init__.py
+++ b/python/pyosaf/utils/__init__.py
@@ -16,17 +16,29 @@
 #
 
 """ pyosaf common utils """
+import os
+import syslog
 import time
+import warnings
 from copy import deepcopy
+from ctypes import POINTER
 
-from pyosaf.saAis import eSaAisErrorT
+from pyosaf import saImmOm
+from pyosaf.saAis import eSaAisErrorT, SaStringT
 
 
-TRY_AGAIN_COUNT = 60
+# The 'MAX_RETRY_TIME' and 'RETRY_INTERVAL' environment variables shall be set
+# with user-defined values BEFORE importing the pyosaf 'utils' module;
+# Otherwise, the default values for MAX_RETRY_TIME(60s) and RETRY_INTERVAL(1s)
+# will be used throughout.
+MAX_RETRY_TIME = int(os.environ.get("MAX_RETRY_TIME")) \
+if "MAX_RETRY_TIME" in os.environ else 60
+RETRY_INTERVAL = int(os.environ.get("RETRY_INTERVAL")) \
+if "RETRY_INTERVAL" in os.environ else 1
 
 
 class SafException(Exception):
-""" SAF Exception that can be printed """
+""" SAF Exception for error during executing SAF functions """
 def __init__(self, value, msg=None):
 Exception.__init__(self)
 self.value = value
@@ -44,73 +56,244 @@ def raise_saf_exception(func, error):
 raise SafException(error, error_string)
 
 
+def check_resource_abort(ccb_handle):
+""" Get error strings from IMM and check if it is a resource abort case
+
+Args:
+ccb_handle (SaImmCcbHandleT): CCB handle
+
+Returns:
+bool: 'True' if it is a resource abort case. 'False', otherwise.
+"""
+c_error_strings = POINTER(SaStringT)()
+saImmOmCcbGetErrorStrings = decorate(saImmOm.saImmOmCcbGetErrorStrings)
+
+# Get error strings
+# As soon as the ccb_handle is finalized, the strings are freed
+rc = saImmOmCcbGetErrorStrings(ccb_handle, c_error_strings)
+
+if rc == eSaAisErrorT.SA_AIS_OK:
+if c_error_strings:
+for c_error_string in c_error_strings:
+if c_error_string.startswith("IMM: Resource abort: "):
+return True
+
+return False
+
+
 def decorate(func):
 """ Decorate the given SAF function so that it retries a fixed number of
-times if needed and raises an exception if it encounters any fault other
-than SA_AIS_ERR_TRY_AGAIN.
+times for certain returned error codes during execution
+
+Args:
+func (function): The decorated SAF function
+
+Returns:
+SaAisErrorT: Return code of the decorated SAF function
 """
 
 def inner(*args):
-""" Call "function" in the lexical scope in a retry loop and raise an
-exception if it encounters any fault other than TRY_AGAIN
+""" Call the decorated function in the lexical scope in a retry loop
+
+Args:
+args (tuple): Arguments of the decorated SAF function
 """
-one_sec_sleeps = 0
-error = func(*args)
+count_sec_sleeps = 0
 
-while error == eSaAisErrorT.SA_AIS_ERR_TRY_AGAIN:
-if one_sec_sleeps == TRY_AGAIN_COUNT:
+rc = func(*args)
+while rc != eSaAisErrorT.SA_AIS_OK:
+
+if count_sec_sleeps >= MAX_RETRY_TIME:
 break
-time.sleep(1)
-one_sec_sleeps += 1
 
-error = func(*args)
+if rc == eSaAisErrorT.SA_AIS_ERR_TRY_AGAIN:
+sleep_time_interval = RETRY_INTERVAL
+elif rc == eSaAisErrorT.SA_AIS_ERR_NO_RESOURCES:
+sleep_time_interval = RETRY_INTERVAL
+elif rc == eSaAisErrorT.SA_AIS_ERR_BUSY:
+sleep_time_interval = 3 * RETRY_INTERVAL
+elif rc == eSaAisErrorT.SA_AIS_ERR_FAILED_OPERATION:
+# Retry on getting FAILED_OPERATION only applies to IMM
+# CCB-related operations in case of a resource abort;
+ccb_handle = args[0]
+resource_abort = check_resource_abort(ccb_handle)
+if resource_abort:
+sleep_time_interval = RETRY_INTERVAL
+else:
+break  # Break out of the retry loop
 
-if error != eSaAisErrorT.SA_AIS_OK:
-raise_saf_exception(func, error)
+# Check sleep_time_interval to sleep and retry the function
+if sleep_time_interval > 0:
+time.sleep(sleep_time_interval)
+count_sec_sleeps += sleep_time_interval
+rc = func(*args)
 
-return error
+return rc
 
 return inner
 
 
-def initialize_decorate(func):

[devel] [PATCH 0/1] Review Request for pyosaf: High level python interfaces for CLM V3 [#2602]

2017-10-24 Thread Long H Buu Nguyen 
Summary: pyosaf: High level python interfaces for CLM V3[#2602]
Review request for Ticket(s): 2602
Peer Reviewer(s): Hans Nordeback, Anders Widell, Srinivas Mangipudy
Pull request to: Hans Nordeback, Anders Widell
Affected branch(es): develop
Development branch: ticket-2602
Base revision: 48340ca7a3c45c1bd12fe37d7c05e026abd55531
Personal repository: git://git.code.sf.net/u/xlobung/review


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   y


Comments (indicate scope for each "y" above):
-
revision c21fbe69db47f33f88464a969d9434a1476105eb
Author: Long H Buu Nguyen 
Date:   Tue, 24 Oct 2017 18:29:20 +0700

pyosaf: High level python interfaces for CLM [#2602]

- Add more error handling
- Refactor clm code
- Keep raising exceptions for existing python methods



Complete diffstat:
--
 python/pyosaf/utils/__init__.py | 267 ++---
 python/pyosaf/utils/clm/__init__.py | 581 ++--
 2 files changed, 717 insertions(+), 131 deletions(-)


Testing Commands:
-
Write a new clm demo or modify clm-tool to test.


Testing, Expected Results:
--
Can track CLM cluster node changes suscessfully.


Conditions of Submission:
-
ACK from one of the reviewers.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/1] pyosaf: High level python interfaces for CLM [#2602]

2017-10-24 Thread Long H Buu Nguyen 
- Add more error handling
- Refactor clm code
- Keep raising exceptions for existing python methods
---
 python/pyosaf/utils/__init__.py | 267 ++---
 python/pyosaf/utils/clm/__init__.py | 579 ++--
 2 files changed, 716 insertions(+), 130 deletions(-)

diff --git a/python/pyosaf/utils/__init__.py b/python/pyosaf/utils/__init__.py
index 7a1c21b79..fac3c5b80 100644
--- a/python/pyosaf/utils/__init__.py
+++ b/python/pyosaf/utils/__init__.py
@@ -16,17 +16,29 @@
 #
 
 """ pyosaf common utils """
+import os
+import syslog
 import time
+import warnings
 from copy import deepcopy
+from ctypes import POINTER
 
-from pyosaf.saAis import eSaAisErrorT
+from pyosaf import saImmOm
+from pyosaf.saAis import eSaAisErrorT, SaStringT
 
 
-TRY_AGAIN_COUNT = 60
+# The 'MAX_RETRY_TIME' and 'RETRY_INTERVAL' environment variables shall be set
+# with user-defined values BEFORE importing the pyosaf 'utils' module;
+# Otherwise, the default values for MAX_RETRY_TIME(60s) and RETRY_INTERVAL(1s)
+# will be used throughout.
+MAX_RETRY_TIME = int(os.environ.get("MAX_RETRY_TIME")) \
+if "MAX_RETRY_TIME" in os.environ else 60
+RETRY_INTERVAL = int(os.environ.get("RETRY_INTERVAL")) \
+if "RETRY_INTERVAL" in os.environ else 1
 
 
 class SafException(Exception):
-""" SAF Exception that can be printed """
+""" SAF Exception for error during executing SAF functions """
 def __init__(self, value, msg=None):
 Exception.__init__(self)
 self.value = value
@@ -44,73 +56,244 @@ def raise_saf_exception(func, error):
 raise SafException(error, error_string)
 
 
+def check_resource_abort(ccb_handle):
+""" Get error strings from IMM and check if it is a resource abort case
+
+Args:
+ccb_handle (SaImmCcbHandleT): CCB handle
+
+Returns:
+bool: 'True' if it is a resource abort case. 'False', otherwise.
+"""
+c_error_strings = POINTER(SaStringT)()
+saImmOmCcbGetErrorStrings = decorate(saImmOm.saImmOmCcbGetErrorStrings)
+
+# Get error strings
+# As soon as the ccb_handle is finalized, the strings are freed
+rc = saImmOmCcbGetErrorStrings(ccb_handle, c_error_strings)
+
+if rc == eSaAisErrorT.SA_AIS_OK:
+if c_error_strings:
+for c_error_string in c_error_strings:
+if c_error_string.startswith("IMM: Resource abort: "):
+return True
+
+return False
+
+
 def decorate(func):
 """ Decorate the given SAF function so that it retries a fixed number of
-times if needed and raises an exception if it encounters any fault other
-than SA_AIS_ERR_TRY_AGAIN.
+times for certain returned error codes during execution
+
+Args:
+func (function): The decorated SAF function
+
+Returns:
+SaAisErrorT: Return code of the decorated SAF function
 """
 
 def inner(*args):
-""" Call "function" in the lexical scope in a retry loop and raise an
-exception if it encounters any fault other than TRY_AGAIN
+""" Call the decorated function in the lexical scope in a retry loop
+
+Args:
+args (tuple): Arguments of the decorated SAF function
 """
-one_sec_sleeps = 0
-error = func(*args)
+count_sec_sleeps = 0
 
-while error == eSaAisErrorT.SA_AIS_ERR_TRY_AGAIN:
-if one_sec_sleeps == TRY_AGAIN_COUNT:
+rc = func(*args)
+while rc != eSaAisErrorT.SA_AIS_OK:
+
+if count_sec_sleeps >= MAX_RETRY_TIME:
 break
-time.sleep(1)
-one_sec_sleeps += 1
 
-error = func(*args)
+if rc == eSaAisErrorT.SA_AIS_ERR_TRY_AGAIN:
+sleep_time_interval = RETRY_INTERVAL
+elif rc == eSaAisErrorT.SA_AIS_ERR_NO_RESOURCES:
+sleep_time_interval = RETRY_INTERVAL
+elif rc == eSaAisErrorT.SA_AIS_ERR_BUSY:
+sleep_time_interval = 3 * RETRY_INTERVAL
+elif rc == eSaAisErrorT.SA_AIS_ERR_FAILED_OPERATION:
+# Retry on getting FAILED_OPERATION only applies to IMM
+# CCB-related operations in case of a resource abort;
+ccb_handle = args[0]
+resource_abort = check_resource_abort(ccb_handle)
+if resource_abort:
+sleep_time_interval = RETRY_INTERVAL
+else:
+break  # Break out of the retry loop
 
-if error != eSaAisErrorT.SA_AIS_OK:
-raise_saf_exception(func, error)
+# Check sleep_time_interval to sleep and retry the function
+if sleep_time_interval > 0:
+time.sleep(sleep_time_interval)
+count_sec_sleeps += sleep_time_interval
+rc = func(*args)
 
-return error
+return rc
 
 return inner
 
 
-def initialize_decorate(func):

[devel] [PATCH 0/1] Review Request for pyosaf: High level python interfaces for CLM V2 [#2602]

2017-10-24 Thread Long H Buu Nguyen 
Summary: pyosaf: High level python interfaces for CLM V2 [#2602]
Review request for Ticket(s): 2602
Peer Reviewer(s): Hans Nordeback, Anders Widell, Srinivas Mangipudy
Pull request to: Hans Nordeback, Anders Widell
Affected branch(es): develop
Development branch: ticket-2602
Base revision: 48340ca7a3c45c1bd12fe37d7c05e026abd55531
Personal repository: git://git.code.sf.net/u/xlobung/review


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   y


Comments (indicate scope for each "y" above):
-
revision 84a33fc95422a80265e1c01c837d5ce87bec460e
Author: Long H Buu Nguyen 
Date:   Tue, 24 Oct 2017 17:25:33 +0700

pyosaf: High level python interfaces for CLM [#2602]

- Add more error handling
- Refactor clm code
- Keep raising exceptions for existing python methods



Complete diffstat:
--
 python/pyosaf/utils/__init__.py | 267 ++---
 python/pyosaf/utils/clm/__init__.py | 579 ++--
 2 files changed, 716 insertions(+), 130 deletions(-)


Testing Commands:
-
Write a new clm demo or modify clm-tool to test.


Testing, Expected Results:
--
Can track CLM cluster node changes suscessfully.


Conditions of Submission:
-
ACK from one of the reviewers.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1/1] 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 |   9 +-
  python/pylintrc | 381 
  2 files changed, 389 insertions(+), 1 deletion(-)
  create mode 100644 python/pylintrc

diff --git a/Makefile.am b/Makefile.am
index a7548d274..f9ba87b35 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -429,7 +429,14 @@ checkpatch:
perl "$$builddir/bin/checkpatch.pl" --no-tree --file --ignore SPLIT_STRING 
--summary-file $$(cat "$$builddir/checkpatch_files.txt") 1>&2; \
true
  
-.PHONY: cppcheck.xml cppcheck cpplint shellcheck checkpatch

+pylint:
+   @srcdir=$$(cd $(top_srcdir); pwd; cd - > /dev/null); \
+   cd "$$srcdir/python"; find . -name '*.py' | xargs pylint --rcfile=pylintrc 
--output-format=html > pylint.html; \
[Nguyen] The HTML reporter (--output-format=html) has been removed 
since pylint 1.7 in favor of the JSON reporter. So the above 
--output-format option would produce error when run with pylint 
version >= 1.7. Could you check and reconsider the output format?

+   true
+   @echo "The pylint result can be found in the file python/pylint.html"
+   @echo "Use the follwing command to view it: firefox python/pylint.html"
+
+.PHONY: cppcheck.xml cppcheck cpplint shellcheck checkpatch pylint
  
  clean-local:

-rm -f $(PACKAGE_NAME)-$(PACKAGE_VERSION).tar.gz
diff --git a/python/pylintrc b/python/pylintrc
new file mode 100644
index 0..c4d2e222d
--- /dev/null
+++ b/python/pylintrc
@@ -0,0 +1,381 @@
+[MASTER]
+
+# Specify a configuration file.
+#rcfile=
+
+# Python code to execute, usually for sys.path manipulation such as
+# pygtk.require().
+#init-hook=
+
+# Profiled execution.
+# option profile is obsolete and it is slated for removal in Pylint 1.6.
+# profile=no
+
+# Add files or directories to the blacklist. They should be base names, not
+# paths.
+ignore=CVS
+
+# Pickle collected data for later comparisons.
+persistent=yes
+
+# List of plugins (as comma separated values of python modules names) to load,
+# usually to register additional checkers.
+load-plugins=
+
+# Use multiple processes to speed up Pylint.
+jobs=1
+
+# Allow loading of arbitrary C extensions. Extensions are imported into the
+# active Python interpreter and may run arbitrary code.
+unsafe-load-any-extension=no
+
+# A comma-separated list of package or module names from where C extensions may
+# be loaded. Extensions are loading into the active Python interpreter and may
+# run arbitrary code
+extension-pkg-whitelist=
+
+# Allow optimization of some AST trees. This will activate a peephole AST
+# optimizer, which will apply various small optimizations. For instance, it can
+# be used to obtain the result of joining multiple strings with the addition
+# operator. Joining a lot of strings can lead to a maximum recursion error in
+# Pylint and this flag can prevent that. It has one side effect, the resulting
+# AST will be different than the one from reality.
+optimize-ast=no
+
+
+[MESSAGES CONTROL]
+
+# Only show warnings with the listed confidence levels. Leave empty to show
+# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED
+confidence=
+
+# Enable the message, report, category or checker with the given id(s). You can
+# either give multiple identifier separated by comma (,) or put this option
+# multiple time. See also the "--disable" option for examples.
+#enable=
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifiers separated by comma (,) or put this
+# option multiple times (only on the command line, not in the configuration
+# file where it should appear only once).You can also use "--disable=all" to
+# disable everything first and then reenable specific checks. For example, if
+# you want to run only the similarities checker, you can use "--disable=all
+# --enable=similarities". If you want to run only the classes checker, but have
+# no Warning level messages displayed, use"--disable=all --enable=classes
+# --disable=W"
+#
+# Added "W0105": pointless-string-statement
+# Added "W0511": Used when a warning note as FIXME or XXX is detected.
+# Added "W0511": Used when a warning note as FIXME or XXX is detected.
+#disable=W0511,W0105,E1608,W1627,E1601,E1603,E1602,E1605,E1604,E1607,E1606,W1621,W1620,W1623,W1622,W1625,W1624,W1609,#W1608,W1607,W1606,W1605,W1604,W1603,W1602,W1601,W1639,W1640,I0021,W1638,I0020,W1618,W1619,W1630,W1626,W1637,W1634,W1#635,W1610,W1611,W1612,W1613,W1614,W1615,W1616,W1617,W1632,W1633,W0704,W1628,W1629,W1636
+
+
+[REPORTS]
+
+# Set the output format. Available formats are text, parseable, colorized, msvs
+# (visual studio) and html. You can also give a reporter class, eg
+#

Re: [devel] [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

2017-10-24 Thread Zoran Milinkovic 
Hi Vu,

Find my comments inline.

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] 
Sent: den 24 oktober 2017 10:56
To: Hans Nordebäck ; Zoran Milinkovic 

Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen 

Subject: [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.
---
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 186 +++---
 2 files changed, 176 insertions(+), 28 deletions(-)
 create mode 100644 src/clm/tools/Makefile

diff --git a/src/clm/tools/Makefile b/src/clm/tools/Makefile
new file mode 100644
index 000..1e39957
--- /dev/null
+++ b/src/clm/tools/Makefile
@@ -0,0 +1,18 @@
+#  -*- OpenSAF  -*-
+#
+# Copyright Ericsson AB 2017 - All Rights Reserved.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+# 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
+#
+
+all:
+   $(MAKE) -C ../../.. bin/clmprint
diff --git a/src/clm/tools/clm_print.c b/src/clm/tools/clm_print.c
index eb01d3e..5d578b2 100644
--- a/src/clm/tools/clm_print.c
+++ b/src/clm/tools/clm_print.c
@@ -1,6 +1,7 @@
 /*  -*- OpenSAF  -*-
  *
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
  *
  * This program is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -23,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -59,9 +61,9 @@ static char *clm_step[] = {
 
 static void print_node(const SaClmClusterNodeT_4 *clusterNode)
 {
-   printf("Node Info for nodeId: %u>\n",
+   printf("\nNode Info for nodeId: %u>\n",
clusterNode->nodeId);
-   printf("  Node Id: %u(%x)\n",
+   printf("  Node Id: %u(0x%x)\n",
clusterNode->nodeId, clusterNode->nodeId);
printf("  Address Family : %s\n",
clusterNode->nodeAddress.family == 1
@@ -84,6 +86,7 @@ static void print_node(const SaClmClusterNodeT_4 *clusterNode)
clusterNode->bootTimestamp);
printf("  Initial View Number: %llu\n",
clusterNode->initialViewNumber);
+   printf("<===\n\n");
 }
 
 static void  clm_node_get_callback(SaInvocationT invocation,
@@ -133,7 +136,6 @@ static void clm_track_callback(const 
SaClmClusterNotificationBufferT_4
print_node(>notification[i].clusterNode);
printf("  Change : %s\n",
clm_change[notificationBuffer->notification[i].clusterChange]);
-   printf("<\n\n");
}
 
if ((step == SA_CLM_CHANGE_VALIDATE) || (step == SA_CLM_CHANGE_START)) {
@@ -217,6 +219,93 @@ static void clm_dispatch(void)
exit(EXIT_SUCCESS);
}
 }
+
+/*
+ * Exit the program with EXIT_FAILURE if passing invalid options,
+ * update the output `bit_mask` otherwise.
+ */
+void validate_option(const char opt, uint32_t *bit_mask)
+{
+   enum mask_position {
+   OPT_HELP,
+   OPT_NODEGET,
+   OPT_ASYNCGET,
+   OPT_TRACK,
+   OPT_BTRACK,
+   OPT_TIMEOUT
+   };
+
+   switch (opt) {
+   case 'n':
+   if ((*bit_mask >> OPT_NODEGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_NODEGET);
+   break;
+
+   case 'a':
+   if ((*bit_mask >> OPT_ASYNCGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr,

Re: [devel] [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

2017-10-24 Thread Vu Minh Nguyen 
Hi Zoran,

Thanks for your quick look and good comments. I just sent the V2.  Thanks!

Regards, Vu

> -Original Message-
> From: Zoran Milinkovic [mailto:zoran.milinko...@ericsson.com]
> Sent: Tuesday, October 24, 2017 3:23 PM
> To: Vu Minh Nguyen ; Hans Nordebäck
> 
> Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> 
> Subject: RE: [PATCH 1/1] clm: fix errors in clmprint tool [#2651]
> 
> Hi Vu,
> 
> Find my comments inline
> 
> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 24 oktober 2017 09:43
> To: Hans Nordebäck ; Zoran Milinkovic
> 
> Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> 
> Subject: [PATCH 1/1] clm: fix errors in clmprint tool [#2651]
> 
> Fix the problems:
> 1) clmprint returns 0 for the error case.
> 2) clmprint does not handle invalid inputs.
> 3) clmprint does not deal with non-member node.
> ---
>  src/clm/tools/Makefile|  18 +
>  src/clm/tools/clm_print.c | 186
> +++---
>  2 files changed, 178 insertions(+), 26 deletions(-)
>  create mode 100644 src/clm/tools/Makefile
> 
> diff --git a/src/clm/tools/Makefile b/src/clm/tools/Makefile
> new file mode 100644
> index 000..de2407b
> --- /dev/null
> +++ b/src/clm/tools/Makefile
> @@ -0,0 +1,18 @@
> +#  -*- 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
> +#
> +
> +all:
> + $(MAKE) -C ../../.. bin/clmprint
> diff --git a/src/clm/tools/clm_print.c b/src/clm/tools/clm_print.c
> index eb01d3e..09f3f34 100644
> --- a/src/clm/tools/clm_print.c
> +++ b/src/clm/tools/clm_print.c
> @@ -1,6 +1,7 @@
>  /*  -*- OpenSAF  -*-
>   *
>   * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
> + * Copyright Ericsson AB 2017 - All Rights Reserved.
>   *
>   * This program is distributed in the hope that it will be useful, but
>   * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> @@ -23,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
>  #include 
> @@ -59,9 +61,9 @@ static char *clm_step[] = {
> 
>  static void print_node(const SaClmClusterNodeT_4 *clusterNode)
>  {
> - printf("Node Info for nodeId:
> %u>\n",
> + printf("\nNode Info for nodeId:
> %u>\n",
>   clusterNode->nodeId);
> - printf("  Node Id: %u(%x)\n",
> + printf("  Node Id: %u(0x%x)\n",
>   clusterNode->nodeId, clusterNode->nodeId);
>   printf("  Address Family : %s\n",
>   clusterNode->nodeAddress.family == 1
> @@ -84,6 +86,7 @@ static void print_node(const SaClmClusterNodeT_4
> *clusterNode)
>   clusterNode->bootTimestamp);
>   printf("  Initial View Number: %llu\n",
>   clusterNode->initialViewNumber);
> +
>   printf("<=
> ==\n\n");
>  }
> 
>  static void  clm_node_get_callback(SaInvocationT invocation,
> @@ -133,7 +136,6 @@ static void clm_track_callback(const
> SaClmClusterNotificationBufferT_4
>   print_node(
> >notification[i].clusterNode);
>   printf("  Change : %s\n",
>   clm_change[notificationBuffer-
> >notification[i].clusterChange]);
> -
>   printf("<\
> n\n");
>   }
> 
>   if ((step == SA_CLM_CHANGE_VALIDATE) || (step ==
> SA_CLM_CHANGE_START)) {
> @@ -217,6 +219,98 @@ static void clm_dispatch(void)
>   exit(EXIT_SUCCESS);
>   }
>  }
> +
> +/*
> + * Exit the program with EXIT_FAILURE if passing invalid options,
> + * update the output `bit_mask` otherwise.
> + */
> +void validate_option(const char opt, uint32_t *bit_mask)
> +{
> + enum mask_position {
> + OPT_HELP,
> + OPT_NODEGET,
> + OPT_ASYNCGET,
> + OPT_TRACK,
> + OPT_BTRACK,
> + OPT_TIMEOUT
> + };
> +
> + switch (opt) {
> + case 'n':
> + if ((*bit_mask >> OPT_NODEGET) & 1) {
> + fprintf(stderr, "error - duplicated

[devel] [PATCH 0/1] Review Request for clm: fix errors in clmprint tool [#2651] V2

2017-10-24 Thread Vu Minh Nguyen 
Summary: clm: fix errors in clmprint tool [#2651]
Review request for Ticket(s): 2651
Peer Reviewer(s): *** Hans, Zoran
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop, release
Development branch: ticket-2651
Base revision: 48340ca7a3c45c1bd12fe37d7c05e026abd55531
Personal repository: git://git.code.sf.net/u/winhvu/review


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   y

NOTE: Patch(es) contain lines longer than 80 characers

Comments (indicate scope for each "y" above):
-
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision 65f50f8228c875c06de151556cb327e9d5e25901
Author: Vu Minh Nguyen 
Date:   Tue, 24 Oct 2017 15:49:43 +0700

clm: fix errors in clmprint tool [#2651]

Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.



Added Files:

 src/clm/tools/Makefile


Complete diffstat:
--
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 186 +++---
 2 files changed, 176 insertions(+), 28 deletions(-)


Testing Commands:
-
Refer to the ticket description for testing commands


Testing, Expected Results:
--
*** PASTE COMMAND OUTPUTS / TEST RESULTS ***


Conditions of Submission:
-
Get ack from peer reviewers


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 ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

2017-10-24 Thread Vu Minh Nguyen 
Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.
---
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 186 +++---
 2 files changed, 176 insertions(+), 28 deletions(-)
 create mode 100644 src/clm/tools/Makefile

diff --git a/src/clm/tools/Makefile b/src/clm/tools/Makefile
new file mode 100644
index 000..1e39957
--- /dev/null
+++ b/src/clm/tools/Makefile
@@ -0,0 +1,18 @@
+#  -*- OpenSAF  -*-
+#
+# Copyright Ericsson AB 2017 - All Rights Reserved.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+# 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
+#
+
+all:
+   $(MAKE) -C ../../.. bin/clmprint
diff --git a/src/clm/tools/clm_print.c b/src/clm/tools/clm_print.c
index eb01d3e..5d578b2 100644
--- a/src/clm/tools/clm_print.c
+++ b/src/clm/tools/clm_print.c
@@ -1,6 +1,7 @@
 /*  -*- OpenSAF  -*-
  *
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
  *
  * This program is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -23,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -59,9 +61,9 @@ static char *clm_step[] = {
 
 static void print_node(const SaClmClusterNodeT_4 *clusterNode)
 {
-   printf("Node Info for nodeId: %u>\n",
+   printf("\nNode Info for nodeId: %u>\n",
clusterNode->nodeId);
-   printf("  Node Id: %u(%x)\n",
+   printf("  Node Id: %u(0x%x)\n",
clusterNode->nodeId, clusterNode->nodeId);
printf("  Address Family : %s\n",
clusterNode->nodeAddress.family == 1
@@ -84,6 +86,7 @@ static void print_node(const SaClmClusterNodeT_4 *clusterNode)
clusterNode->bootTimestamp);
printf("  Initial View Number: %llu\n",
clusterNode->initialViewNumber);
+   printf("<===\n\n");
 }
 
 static void  clm_node_get_callback(SaInvocationT invocation,
@@ -133,7 +136,6 @@ static void clm_track_callback(const 
SaClmClusterNotificationBufferT_4
print_node(>notification[i].clusterNode);
printf("  Change : %s\n",
clm_change[notificationBuffer->notification[i].clusterChange]);
-   printf("<\n\n");
}
 
if ((step == SA_CLM_CHANGE_VALIDATE) || (step == SA_CLM_CHANGE_START)) {
@@ -217,6 +219,93 @@ static void clm_dispatch(void)
exit(EXIT_SUCCESS);
}
 }
+
+/*
+ * Exit the program with EXIT_FAILURE if passing invalid options,
+ * update the output `bit_mask` otherwise.
+ */
+void validate_option(const char opt, uint32_t *bit_mask)
+{
+   enum mask_position {
+   OPT_HELP,
+   OPT_NODEGET,
+   OPT_ASYNCGET,
+   OPT_TRACK,
+   OPT_BTRACK,
+   OPT_TIMEOUT
+   };
+
+   switch (opt) {
+   case 'n':
+   if ((*bit_mask >> OPT_NODEGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_NODEGET);
+   break;
+
+   case 'a':
+   if ((*bit_mask >> OPT_ASYNCGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_ASYNCGET);
+   break;
+
+   case 'b':
+   if ((*bit_mask >> OPT_BTRACK) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if

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

2017-10-24 Thread Nguyen Luu 

Hi Hans,

I have one comment inline marked with [Nguyen].

Thanks,
Nguyen

On 10/23/2017 7:40 PM, Hans Nordeback wrote:

---
  Makefile.am |   9 +-
  python/pylintrc | 381 
  2 files changed, 389 insertions(+), 1 deletion(-)
  create mode 100644 python/pylintrc

diff --git a/Makefile.am b/Makefile.am
index a7548d274..f9ba87b35 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -429,7 +429,14 @@ checkpatch:
perl "$$builddir/bin/checkpatch.pl" --no-tree --file --ignore SPLIT_STRING 
--summary-file $$(cat "$$builddir/checkpatch_files.txt") 1>&2; \
true
  
-.PHONY: cppcheck.xml cppcheck cpplint shellcheck checkpatch

+pylint:
+   @srcdir=$$(cd $(top_srcdir); pwd; cd - > /dev/null); \
+   cd "$$srcdir/python"; find . -name '*.py' | xargs pylint --rcfile=pylintrc 
--output-format=html > pylint.html; \
[Nguyen] The HTML reporter (--output-format=html) has been removed since 
pylint 1.7 in favor of the JSON reporter. So the above --output-format 
option would produce error when run with pylint version >= 1.7. Could 
you check and reconsider the output format?

+   true
+   @echo "The pylint result can be found in the file python/pylint.html"
+   @echo "Use the follwing command to view it: firefox python/pylint.html"
+
+.PHONY: cppcheck.xml cppcheck cpplint shellcheck checkpatch pylint
  
  clean-local:

-rm -f $(PACKAGE_NAME)-$(PACKAGE_VERSION).tar.gz
diff --git a/python/pylintrc b/python/pylintrc
new file mode 100644
index 0..c4d2e222d
--- /dev/null
+++ b/python/pylintrc
@@ -0,0 +1,381 @@
+[MASTER]
+
+# Specify a configuration file.
+#rcfile=
+
+# Python code to execute, usually for sys.path manipulation such as
+# pygtk.require().
+#init-hook=
+
+# Profiled execution.
+# option profile is obsolete and it is slated for removal in Pylint 1.6.
+# profile=no
+
+# Add files or directories to the blacklist. They should be base names, not
+# paths.
+ignore=CVS
+
+# Pickle collected data for later comparisons.
+persistent=yes
+
+# List of plugins (as comma separated values of python modules names) to load,
+# usually to register additional checkers.
+load-plugins=
+
+# Use multiple processes to speed up Pylint.
+jobs=1
+
+# Allow loading of arbitrary C extensions. Extensions are imported into the
+# active Python interpreter and may run arbitrary code.
+unsafe-load-any-extension=no
+
+# A comma-separated list of package or module names from where C extensions may
+# be loaded. Extensions are loading into the active Python interpreter and may
+# run arbitrary code
+extension-pkg-whitelist=
+
+# Allow optimization of some AST trees. This will activate a peephole AST
+# optimizer, which will apply various small optimizations. For instance, it can
+# be used to obtain the result of joining multiple strings with the addition
+# operator. Joining a lot of strings can lead to a maximum recursion error in
+# Pylint and this flag can prevent that. It has one side effect, the resulting
+# AST will be different than the one from reality.
+optimize-ast=no
+
+
+[MESSAGES CONTROL]
+
+# Only show warnings with the listed confidence levels. Leave empty to show
+# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED
+confidence=
+
+# Enable the message, report, category or checker with the given id(s). You can
+# either give multiple identifier separated by comma (,) or put this option
+# multiple time. See also the "--disable" option for examples.
+#enable=
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifiers separated by comma (,) or put this
+# option multiple times (only on the command line, not in the configuration
+# file where it should appear only once).You can also use "--disable=all" to
+# disable everything first and then reenable specific checks. For example, if
+# you want to run only the similarities checker, you can use "--disable=all
+# --enable=similarities". If you want to run only the classes checker, but have
+# no Warning level messages displayed, use"--disable=all --enable=classes
+# --disable=W"
+#
+# Added "W0105": pointless-string-statement
+# Added "W0511": Used when a warning note as FIXME or XXX is detected.
+# Added "W0511": Used when a warning note as FIXME or XXX is detected.
+#disable=W0511,W0105,E1608,W1627,E1601,E1603,E1602,E1605,E1604,E1607,E1606,W1621,W1620,W1623,W1622,W1625,W1624,W1609,#W1608,W1607,W1606,W1605,W1604,W1603,W1602,W1601,W1639,W1640,I0021,W1638,I0020,W1618,W1619,W1630,W1626,W1637,W1634,W1#635,W1610,W1611,W1612,W1613,W1614,W1615,W1616,W1617,W1632,W1633,W0704,W1628,W1629,W1636
+
+
+[REPORTS]
+
+# Set the output format. Available formats are text, parseable, colorized, msvs
+# (visual studio) and html. You can also give a reporter class, eg
+# mypackage.mymodule.MyReporterClass.
+# output-format=text
+
+# Put messages in a separate file for each module / package specified on the
+# command line instead of

Re: [devel] [PATCH 1/1] clm: fix return wrong error code [#2652]

2017-10-24 Thread Zoran Milinkovic 
Hi Vu,

Reviewed the code.
Ack from me.

Thanks,
Zoran

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] 
Sent: den 24 oktober 2017 09:58
To: Hans Nordebäck ; Zoran Milinkovic 
; Anders Widell 
Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen 

Subject: [PATCH 1/1] clm: fix return wrong error code [#2652]

saClmClusterNodeGet_4() and saClmClusterNodeGetAsync() returns
SA_AIS_ERR_UNAVAILABLE(31) when querying non-member node information from a 
member node.

According to AIS, they should return SA_AIS_ERR_NOT_EXIST.
SA_AIS_ERR_UNAVAILABLE should be returned when invoking process is not 
executing on a member node.
---
 src/clm/clmd/clms_evt.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/clm/clmd/clms_evt.c b/src/clm/clmd/clms_evt.c index 
8352af8..4d7010d 100644
--- a/src/clm/clmd/clms_evt.c
+++ b/src/clm/clmd/clms_evt.c
@@ -1492,13 +1492,20 @@ static uint32_t proc_node_get_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
.bootTimestamp = node->boot_time;
clm_msg.info.api_resp_info.param.node_get
.initialViewNumber = node->init_view;
-   } else {
+   } else if (local_node->member == SA_FALSE) {
clm_msg.evt_type =
CLMSV_CLMS_TO_CLMA_API_RESP_MSG;
clm_msg.info.api_resp_info.type =
CLMSV_NODE_GET_RESP;
clm_msg.info.api_resp_info.rc =
SA_AIS_ERR_UNAVAILABLE;
+   } else {
+   clm_msg.evt_type =
+   CLMSV_CLMS_TO_CLMA_API_RESP_MSG;
+   clm_msg.info.api_resp_info.type =
+   CLMSV_NODE_GET_RESP;
+   clm_msg.info.api_resp_info.rc =
+   SA_AIS_ERR_NOT_EXIST;
}
} else {
clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_API_RESP_MSG; @@ 
-1508,7 +1515,7 @@ static uint32_t proc_node_get_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
} else {
clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_API_RESP_MSG;
clm_msg.info.api_resp_info.type = CLMSV_NODE_GET_RESP;
-   clm_msg.info.api_resp_info.rc = SA_AIS_ERR_NOT_EXIST;
+   clm_msg.info.api_resp_info.rc = SA_AIS_ERR_UNAVAILABLE;
}
 
TRACE_LEAVE();
@@ -1573,6 +1580,16 @@ static uint32_t proc_node_get_async_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
ais_rc;
clm_msg.info.cbk_info.client_id =
param->client_id;
+   } else if (local_node->member == SA_FALSE) {
+   clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_CBK_MSG;
+   clm_msg.info.cbk_info.type =
+   CLMSV_NODE_ASYNC_GET_CBK;
+   clm_msg.info.cbk_info.param.node_get.inv =
+   param->inv;
+   clm_msg.info.cbk_info.param.node_get.err =
+   SA_AIS_ERR_UNAVAILABLE;
+   clm_msg.info.cbk_info.client_id =
+   param->client_id;
} else {
TRACE(
"Node exists in the database but is 
non-member"); @@ -1582,7 +1599,7 @@ static uint32_t 
proc_node_get_async_msg(CLMS_CB *cb, CLMSV_CLMS_EVT *evt)
clm_msg.info.cbk_info.param.node_get.inv =
param->inv;
clm_msg.info.cbk_info.param.node_get.err =
-   SA_AIS_ERR_UNAVAILABLE;
+   SA_AIS_ERR_NOT_EXIST;
clm_msg.info.cbk_info.client_id =
param->client_id;
}
@@ -1596,11 +1613,11 @@ static uint32_t proc_node_get_async_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
clm_msg.info.cbk_info.client_id = param->client_id;
}
} else {
-   TRACE("Node doesn't exist in the data base");
+   TRACE("Invoking process is not in cluster membership");
clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_CBK_MSG;
clm_msg.info.cbk_info.type = CLMSV_NODE_ASYNC_GET_CBK;
clm_msg.info.cbk_info.param.node_get.inv = param->inv;
-

Re: [devel] [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

2017-10-24 Thread Zoran Milinkovic 
Hi Vu,

Find my comments inline

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] 
Sent: den 24 oktober 2017 09:43
To: Hans Nordebäck ; Zoran Milinkovic 

Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen 

Subject: [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.
---
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 186 +++---
 2 files changed, 178 insertions(+), 26 deletions(-)
 create mode 100644 src/clm/tools/Makefile

diff --git a/src/clm/tools/Makefile b/src/clm/tools/Makefile
new file mode 100644
index 000..de2407b
--- /dev/null
+++ b/src/clm/tools/Makefile
@@ -0,0 +1,18 @@
+#  -*- 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
+#
+
+all:
+   $(MAKE) -C ../../.. bin/clmprint
diff --git a/src/clm/tools/clm_print.c b/src/clm/tools/clm_print.c
index eb01d3e..09f3f34 100644
--- a/src/clm/tools/clm_print.c
+++ b/src/clm/tools/clm_print.c
@@ -1,6 +1,7 @@
 /*  -*- OpenSAF  -*-
  *
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
  *
  * This program is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -23,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -59,9 +61,9 @@ static char *clm_step[] = {
 
 static void print_node(const SaClmClusterNodeT_4 *clusterNode)
 {
-   printf("Node Info for nodeId: %u>\n",
+   printf("\nNode Info for nodeId: %u>\n",
clusterNode->nodeId);
-   printf("  Node Id: %u(%x)\n",
+   printf("  Node Id: %u(0x%x)\n",
clusterNode->nodeId, clusterNode->nodeId);
printf("  Address Family : %s\n",
clusterNode->nodeAddress.family == 1
@@ -84,6 +86,7 @@ static void print_node(const SaClmClusterNodeT_4 *clusterNode)
clusterNode->bootTimestamp);
printf("  Initial View Number: %llu\n",
clusterNode->initialViewNumber);
+   printf("<===\n\n");
 }
 
 static void  clm_node_get_callback(SaInvocationT invocation,
@@ -133,7 +136,6 @@ static void clm_track_callback(const 
SaClmClusterNotificationBufferT_4
print_node(>notification[i].clusterNode);
printf("  Change : %s\n",
clm_change[notificationBuffer->notification[i].clusterChange]);
-   printf("<\n\n");
}
 
if ((step == SA_CLM_CHANGE_VALIDATE) || (step == SA_CLM_CHANGE_START)) {
@@ -217,6 +219,98 @@ static void clm_dispatch(void)
exit(EXIT_SUCCESS);
}
 }
+
+/*
+ * Exit the program with EXIT_FAILURE if passing invalid options,
+ * update the output `bit_mask` otherwise.
+ */
+void validate_option(const char opt, uint32_t *bit_mask)
+{
+   enum mask_position {
+   OPT_HELP,
+   OPT_NODEGET,
+   OPT_ASYNCGET,
+   OPT_TRACK,
+   OPT_BTRACK,
+   OPT_TIMEOUT
+   };
+
+   switch (opt) {
+   case 'n':
+   if ((*bit_mask >> OPT_NODEGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_NODEGET);
+   break;
+
+   case 'a':
+   if ((*bit_mask >> OPT_ASYNCGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error -

[devel] [PATCH 0/1] Review Request for clm: fix return wrong error code [#2652]

2017-10-24 Thread Vu Minh Nguyen 
Summary: clm: fix return wrong error code [#2652]
Review request for Ticket(s): 2652
Peer Reviewer(s): Hans, Anders, Zoran
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop, release
Development branch: ticket-2652
Base revision: 48340ca7a3c45c1bd12fe37d7c05e026abd55531
Personal repository: git://git.code.sf.net/u/winhvu/review


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

NOTE: Patch(es) contain lines longer than 80 characers

Comments (indicate scope for each "y" above):
-
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision ce03000bfe56b68b66f5da4f5621c8e58e4b18d9
Author: Vu Minh Nguyen 
Date:   Tue, 24 Oct 2017 14:44:14 +0700

clm: fix return wrong error code [#2652]

saClmClusterNodeGet_4() and saClmClusterNodeGetAsync() returns
SA_AIS_ERR_UNAVAILABLE(31) when querying non-member node information
from a member node.

According to AIS, they should return SA_AIS_ERR_NOT_EXIST.
SA_AIS_ERR_UNAVAILABLE should be returned when invoking process is not
executing on a member node.



Complete diffstat:
--
 src/clm/clmd/clms_evt.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)


Testing Commands:
-
Query non-member node information from a member node, using clmprint.
`clmprint -n 0x3060f` and `clmprint -a 0x3060f`


Testing, Expected Results:
--
saClmClusterNodeGet_4()/saClmClusterNodeGetAsync() should return 
SA_AIS_ERR_NOT_EXIST.


Conditions of Submission:
-
Get ack from peer reviewers


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 ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/1] clm: fix return wrong error code [#2652]

2017-10-24 Thread Vu Minh Nguyen 
saClmClusterNodeGet_4() and saClmClusterNodeGetAsync() returns
SA_AIS_ERR_UNAVAILABLE(31) when querying non-member node information
from a member node.

According to AIS, they should return SA_AIS_ERR_NOT_EXIST.
SA_AIS_ERR_UNAVAILABLE should be returned when invoking process is not
executing on a member node.
---
 src/clm/clmd/clms_evt.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/clm/clmd/clms_evt.c b/src/clm/clmd/clms_evt.c
index 8352af8..4d7010d 100644
--- a/src/clm/clmd/clms_evt.c
+++ b/src/clm/clmd/clms_evt.c
@@ -1492,13 +1492,20 @@ static uint32_t proc_node_get_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
.bootTimestamp = node->boot_time;
clm_msg.info.api_resp_info.param.node_get
.initialViewNumber = node->init_view;
-   } else {
+   } else if (local_node->member == SA_FALSE) {
clm_msg.evt_type =
CLMSV_CLMS_TO_CLMA_API_RESP_MSG;
clm_msg.info.api_resp_info.type =
CLMSV_NODE_GET_RESP;
clm_msg.info.api_resp_info.rc =
SA_AIS_ERR_UNAVAILABLE;
+   } else {
+   clm_msg.evt_type =
+   CLMSV_CLMS_TO_CLMA_API_RESP_MSG;
+   clm_msg.info.api_resp_info.type =
+   CLMSV_NODE_GET_RESP;
+   clm_msg.info.api_resp_info.rc =
+   SA_AIS_ERR_NOT_EXIST;
}
} else {
clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_API_RESP_MSG;
@@ -1508,7 +1515,7 @@ static uint32_t proc_node_get_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
} else {
clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_API_RESP_MSG;
clm_msg.info.api_resp_info.type = CLMSV_NODE_GET_RESP;
-   clm_msg.info.api_resp_info.rc = SA_AIS_ERR_NOT_EXIST;
+   clm_msg.info.api_resp_info.rc = SA_AIS_ERR_UNAVAILABLE;
}
 
TRACE_LEAVE();
@@ -1573,6 +1580,16 @@ static uint32_t proc_node_get_async_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
ais_rc;
clm_msg.info.cbk_info.client_id =
param->client_id;
+   } else if (local_node->member == SA_FALSE) {
+   clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_CBK_MSG;
+   clm_msg.info.cbk_info.type =
+   CLMSV_NODE_ASYNC_GET_CBK;
+   clm_msg.info.cbk_info.param.node_get.inv =
+   param->inv;
+   clm_msg.info.cbk_info.param.node_get.err =
+   SA_AIS_ERR_UNAVAILABLE;
+   clm_msg.info.cbk_info.client_id =
+   param->client_id;
} else {
TRACE(
"Node exists in the database but is 
non-member");
@@ -1582,7 +1599,7 @@ static uint32_t proc_node_get_async_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
clm_msg.info.cbk_info.param.node_get.inv =
param->inv;
clm_msg.info.cbk_info.param.node_get.err =
-   SA_AIS_ERR_UNAVAILABLE;
+   SA_AIS_ERR_NOT_EXIST;
clm_msg.info.cbk_info.client_id =
param->client_id;
}
@@ -1596,11 +1613,11 @@ static uint32_t proc_node_get_async_msg(CLMS_CB *cb, 
CLMSV_CLMS_EVT *evt)
clm_msg.info.cbk_info.client_id = param->client_id;
}
} else {
-   TRACE("Node doesn't exist in the data base");
+   TRACE("Invoking process is not in cluster membership");
clm_msg.evt_type = CLMSV_CLMS_TO_CLMA_CBK_MSG;
clm_msg.info.cbk_info.type = CLMSV_NODE_ASYNC_GET_CBK;
clm_msg.info.cbk_info.param.node_get.inv = param->inv;
-   clm_msg.info.cbk_info.param.node_get.err = SA_AIS_ERR_NOT_EXIST;
+   clm_msg.info.cbk_info.param.node_get.err = 
SA_AIS_ERR_UNAVAILABLE;
clm_msg.info.cbk_info.client_id = param->client_id;
}
TRACE_LEAVE();
-- 
1.9.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[devel] [PATCH 1/1] clm: fix errors in clmprint tool [#2651]

2017-10-24 Thread Vu Minh Nguyen 
Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.
---
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 186 +++---
 2 files changed, 178 insertions(+), 26 deletions(-)
 create mode 100644 src/clm/tools/Makefile

diff --git a/src/clm/tools/Makefile b/src/clm/tools/Makefile
new file mode 100644
index 000..de2407b
--- /dev/null
+++ b/src/clm/tools/Makefile
@@ -0,0 +1,18 @@
+#  -*- 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
+#
+
+all:
+   $(MAKE) -C ../../.. bin/clmprint
diff --git a/src/clm/tools/clm_print.c b/src/clm/tools/clm_print.c
index eb01d3e..09f3f34 100644
--- a/src/clm/tools/clm_print.c
+++ b/src/clm/tools/clm_print.c
@@ -1,6 +1,7 @@
 /*  -*- OpenSAF  -*-
  *
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
  *
  * This program is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -23,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -59,9 +61,9 @@ static char *clm_step[] = {
 
 static void print_node(const SaClmClusterNodeT_4 *clusterNode)
 {
-   printf("Node Info for nodeId: %u>\n",
+   printf("\nNode Info for nodeId: %u>\n",
clusterNode->nodeId);
-   printf("  Node Id: %u(%x)\n",
+   printf("  Node Id: %u(0x%x)\n",
clusterNode->nodeId, clusterNode->nodeId);
printf("  Address Family : %s\n",
clusterNode->nodeAddress.family == 1
@@ -84,6 +86,7 @@ static void print_node(const SaClmClusterNodeT_4 *clusterNode)
clusterNode->bootTimestamp);
printf("  Initial View Number: %llu\n",
clusterNode->initialViewNumber);
+   printf("<===\n\n");
 }
 
 static void  clm_node_get_callback(SaInvocationT invocation,
@@ -133,7 +136,6 @@ static void clm_track_callback(const 
SaClmClusterNotificationBufferT_4
print_node(>notification[i].clusterNode);
printf("  Change : %s\n",
clm_change[notificationBuffer->notification[i].clusterChange]);
-   printf("<\n\n");
}
 
if ((step == SA_CLM_CHANGE_VALIDATE) || (step == SA_CLM_CHANGE_START)) {
@@ -217,6 +219,98 @@ static void clm_dispatch(void)
exit(EXIT_SUCCESS);
}
 }
+
+/*
+ * Exit the program with EXIT_FAILURE if passing invalid options,
+ * update the output `bit_mask` otherwise.
+ */
+void validate_option(const char opt, uint32_t *bit_mask)
+{
+   enum mask_position {
+   OPT_HELP,
+   OPT_NODEGET,
+   OPT_ASYNCGET,
+   OPT_TRACK,
+   OPT_BTRACK,
+   OPT_TIMEOUT
+   };
+
+   switch (opt) {
+   case 'n':
+   if ((*bit_mask >> OPT_NODEGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_NODEGET);
+   break;
+
+   case 'a':
+   if ((*bit_mask >> OPT_ASYNCGET) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   if ((*bit_mask) && (*bit_mask ^ (1 << OPT_TIMEOUT))) {
+   fprintf(stderr, "error - multiple options!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |= (1 << OPT_ASYNCGET);
+   break;
+
+   case 'b':
+   if ((*bit_mask >> OPT_BTRACK) & 1) {
+   fprintf(stderr, "error - duplicated option!\n");
+   exit(EXIT_FAILURE);
+   }
+
+   *bit_mask |=

[devel] [PATCH 0/1] Review Request for clm: fix errors in clmprint tool [#2651]

2017-10-24 Thread Vu Minh Nguyen 
Summary: clm: fix errors in clmprint tool [#2651]
Review request for Ticket(s): 2651
Peer Reviewer(s): Hans, Zoran
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop, release
Development branch: ticket-2651
Base revision: 48340ca7a3c45c1bd12fe37d7c05e026abd55531
Personal repository: git://git.code.sf.net/u/winhvu/review


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   y

NOTE: Patch(es) contain lines longer than 80 characers

Comments (indicate scope for each "y" above):
-
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision cda2e3dd84e3c23b23dabee515b04a920384e580
Author: Vu Minh Nguyen 
Date:   Tue, 24 Oct 2017 14:34:34 +0700

clm: fix errors in clmprint tool [#2651]

Fix the problems:
1) clmprint returns 0 for the error case.
2) clmprint does not handle invalid inputs.
3) clmprint does not deal with non-member node.



Added Files:

 src/clm/tools/Makefile


Complete diffstat:
--
 src/clm/tools/Makefile|  18 +
 src/clm/tools/clm_print.c | 186 +++---
 2 files changed, 178 insertions(+), 26 deletions(-)


Testing Commands:
-
Refer to the ticket for testing commands.


Testing, Expected Results:
--
*** PASTE COMMAND OUTPUTS / TEST RESULTS ***


Conditions of Submission:
-
Get ack from peer reviewers


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 ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel