[devel] [PATCH 1/1] imm: include CLM in poll before CLM handle is initialized [#2544]

[Zoran Milinkovic]

CLM selection object is initially set to -1. Included CLM selection
object in poll will be ignored until CLM selection object is created
and set to fds[FD_CLM].
---
 src/imm/immnd/immnd_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/imm/immnd/immnd_main.c b/src/imm/immnd/immnd_main.c
index bca8f80..67759ff 100644
--- a/src/imm/immnd/immnd_main.c
+++ b/src/imm/immnd/immnd_main.c
@@ -303,8 +303,7 @@ int main(int argc, char *argv[])
int maxEvt = 100;
struct timespec start_time;
struct pollfd fds[5];
-   int term_fd, nfds = 4;
-   ;
+   int term_fd, nfds = 5;
 
daemonize(argc, argv);
 
@@ -428,7 +427,6 @@ int main(int argc, char *argv[])
ncs_sel_obj_rmv_ind(_cb->clm_init_sel_obj,
true, true);
immnd_init_with_clm();
-   nfds = 5;
}
 
if (fds[FD_CLM].revents & POLLIN) {
-- 
1.9.1


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

Re: [devel] [PATCH 0/1] Review Request for log: fix cppcheck, cpplint and reorganize headers - part 1 [#2445]

[Lennart Lund]

Ack.
Minor comments in attached.diff file. See tag [Lennart]

Thanks
Lennart

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: den 20 juli 2017 08:31
> To: Lennart Lund ; Vu Minh Nguyen
> ; mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 0/1] Review Request for log: fix cppcheck, cpplint and
> reorganize headers - part 1 [#2445]
> 
> Summary: log: fix cppcheck, cpplint and reorganize headers - part 1 [#2445]
> Review request for Ticket(s): 2445
> Peer Reviewer(s): Lennart, Vu, Mahesh
> Pull request to: Vu
> Affected branch(es): develop
> Development branch: ticket-2445
> Base revision: 653edb5d9b217f1a3280b5aed8597fb53ffa5f61
> Personal repository: git://git.code.sf.net/u/canht32/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
> 
> 
> Comments (indicate scope for each "y" above):
> -
> *** EXPLAIN/COMMENT THE PATCH SERIES HERE ***
> 
> revision 8a7192da8b78065734da197211f19d4914706926
> Author:   Canh Van Truong 
> Date: Thu, 20 Jul 2017 13:21:52 +0700
> 
> log: fix cppcheck, cpplint and reorganize headers - part 1 [#2445]
> 
>Fix cppcheck, cpplint, replace nullptr for following files:
> - lgs_amf.*, lgs_config.*, lgs_dest.*, lgs_evt.*, lgs_file.*,
>   lgs_filehdl.*, lgs_nildest.*, lgs_unixsock_dest.*
>   lgs_util.*, lgs_dest_test.*
> 
> 
> 
> Complete diffstat:
> --
>  src/log/logd/lgs_amf.cc  |  25 ++--
>  src/log/logd/lgs_config.cc   | 238 
> +++
>  src/log/logd/lgs_config.h|   3 +-
>  src/log/logd/lgs_dest.cc |   2 +-
>  src/log/logd/lgs_dest.h  |   6 +-
>  src/log/logd/lgs_evt.cc  | 111 +-
>  src/log/logd/lgs_evt.h   |  17 ++-
>  src/log/logd/lgs_file.cc |  56 +
>  src/log/logd/lgs_filehdl.cc  |  84 +++---
>  src/log/logd/lgs_nildest.h   |   6 +-
>  src/log/logd/lgs_recov.cc|   4 +-
>  src/log/logd/lgs_stream.cc   |  16 +--
>  src/log/logd/lgs_unixsock_dest.h |   6 +-
>  src/log/logd/lgs_util.cc |  58 +-
>  src/log/logd/lgs_util.h  |   7 +-
>  src/log/tests/lgs_dest_test.cc   |   2 +-
>  16 files changed, 293 insertions(+), 348 deletions(-)
> 
> 
> Testing Commands:
> -
> *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES ***
> 
> 
> Testing, Expected Results:
> --
> *** PASTE COMMAND OUTPUTS / TEST RESULTS ***
> 
> 
> Conditions of Submission:
> -
> Ack from reviewer
> 
> 
> 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 

Re: [devel] [PATCH 0/1] Review Request for log: fix log supported maximum 2047 characters for long DN [#2525]

[Lennart Lund]

Ack
Minor comments can be found in the attached .diff file. See tag [Lennart]

Thanks
Lennart

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: den 12 juli 2017 10:11
> To: Lennart Lund ; Vu Minh Nguyen
> ; mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 0/1] Review Request for log: fix log supported maximum
> 2047 characters for long DN [#2525]
> 
> Summary: log: fix log supported maximum 2047 characters for long DN
> [#2525]
> Review request for Ticket(s): 2525
> Peer Reviewer(s): Lennart, Vu, Mahesh
> Pull request to: Vu
> Affected branch(es): develop, release
> Development branch: ticket-2525
> Base revision: 3be8e9adb4670607a93907a886b0cf301570d65a
> Personal repository: git://git.code.sf.net/u/canht32/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
> 
> 
> Comments (indicate scope for each "y" above):
> -
> *** EXPLAIN/COMMENT THE PATCH SERIES HERE ***
> 
> revision 81ba7f3323ab58677b706ca60075cf5b031cc4db
> Author:   Canh Van Truong 
> Date: Wed, 12 Jul 2017 14:56:34 +0700
> 
> log: fix log supported maximum 2047 characters for long DN [#2525]
> 
> Currently, log support maximum 2047 characters for long DN. it should
> support
> maximum 2048 characters.
> 
> The patch also fixes to check SA_AMF_COMPONENT_NAME maximum 2048
> characters
> and refactor encode and decode for SaNameT to process its name in one
> place
> 
> 
> 
> Complete diffstat:
> --
>  src/log/agent/lga_agent.cc   |   4 +-
>  src/log/agent/lga_mds.cc |  99 ++--
>  src/log/agent/lga_util.cc|   5 +-
>  src/log/apitest/tet_log_longDN.c | 131 +++--
> 
>  src/log/logd/lgs_mds.cc  | 137 
> +++
>  src/log/logd/lgs_recov.cc|   6 +-
>  src/log/logd/lgs_util.cc |   5 +-
>  7 files changed, 210 insertions(+), 177 deletions(-)
> 
> 
> Testing Commands:
> -
> Update test cases: logtest 13 6, logtest 13 7, logtest 13 8
> and create test case: logtest 13 11
> 
> 
> Testing, Expected Results:
> --
> All test cases pass
> 
> 
> Conditions of Submission:
> -
> Ack from 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 

Re: [devel] [PATCH 1/1] clm: Provide the node address as a parameter to the scale-out script [#2538]

[praveen malviya]

Ack.


Thanks
Praveen

On 01-Aug-17 4:42 PM, Anders Widell wrote:

Provide the node address as a command-line parameter when calling the scale-out
script. This can be useful if the scale-out script needs to contact the node
(e.g. copy some files to it or update some configuration on the node's local
disk) as part of the scale-out operation.
---
  src/clm/clmd/clms_evt.c | 57 ++---
  1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/src/clm/clmd/clms_evt.c b/src/clm/clmd/clms_evt.c
index ace140db4..84e7b3c6d 100644
--- a/src/clm/clmd/clms_evt.c
+++ b/src/clm/clmd/clms_evt.c
@@ -488,9 +488,17 @@ static void scale_out_node(CLMS_CB *cb,
queue_the_node = false;
}
if (queue_the_node) {
+   char node_address[SA_CLM_MAX_ADDRESS_LENGTH + 1];
+   size_t addr_len = nodeup_info->address.length;
+   if (addr_len > SA_CLM_MAX_ADDRESS_LENGTH)
+   addr_len = SA_CLM_MAX_ADDRESS_LENGTH;
+   if (nodeup_info->no_of_addresses == 0)
+   addr_len = 0;
+   memcpy(node_address, nodeup_info->address.value, addr_len);
+   node_address[addr_len] = '\0';
char *strp;
-   if (asprintf(, "%" PRIu32 ",%s,", nodeup_info->node_id,
-node_name) != -1) {
+   if (asprintf(, "%" PRIu32 ",%s,%s,", nodeup_info->node_id,
+node_name, node_address) != -1) {
LOG_NO("Queuing request to scale out node 0x%" PRIx32
   " (%s)",
   nodeup_info->node_id, node_name);
@@ -525,13 +533,10 @@ uint32_t proc_node_up_msg(CLMS_CB *cb, CLMSV_CLMS_EVT 
*evt)
  {
clmsv_clms_node_up_info_t *nodeup_info =
&(evt->info.msg.info.api_info.param).nodeup_info;
-   CLMS_CLUSTER_NODE *node = NULL;
-   SaUint32T nodeid;
uint32_t rc = NCSCC_RC_SUCCESS;
SaNameT node_name = {0};
CLMSV_MSG clm_msg;
SaBoolT check_member;
-   IPLIST *ip = NULL;
  
  	TRACE_ENTER2("Node up mesg for nodename length %d %s",

 nodeup_info->node_name.length,
@@ -542,10 +547,21 @@ uint32_t proc_node_up_msg(CLMS_CB *cb, CLMSV_CLMS_EVT 
*evt)
(char *)node_name.value, sizeof(node_name.value), "safNode=%s,%s",
nodeup_info->node_name.value, osaf_cluster->name.value);
  
-	nodeid = evt->info.msg.info.api_info.param.nodeup_info.node_id;

+   SaUint32T nodeid = nodeup_info->node_id;
+
+   /* Retrieve IP information */
+   IPLIST *ip = (IPLIST *)ncs_patricia_tree_get(_cb->iplist,
+(uint8_t *));
+
+   if (ip != NULL && ip->addr.length != 0 &&
+   nodeup_info->no_of_addresses == 0) {
+   nodeup_info->no_of_addresses = 1;
+   memcpy(&(nodeup_info->address), &(ip->addr), sizeof(ip->addr));
+   }
  
-	node = clms_node_get_by_name(_name);

+   CLMS_CLUSTER_NODE *node = clms_node_get_by_name(_name);
clm_msg.info.api_resp_info.rc = SA_AIS_OK;
+
if (node == NULL) {
/* The /etc/opensaf/node_name is an user exposed configuration
 * file. The node_name file contains the RDN value of the CLM
@@ -573,8 +589,7 @@ uint32_t proc_node_up_msg(CLMS_CB *cb, CLMSV_CLMS_EVT *evt)
  
  	if (node != NULL) {

/* Retrieve IP information */
-   if ((ip = (IPLIST *)ncs_patricia_tree_get(
-_cb->iplist, (uint8_t *))) == NULL) {
+   if (ip == NULL) {
clm_msg.info.api_resp_info.rc = SA_AIS_ERR_NOT_EXIST;
LOG_ER(
"IP information not found for: %s with node_id: %u",
@@ -653,8 +668,7 @@ uint32_t proc_node_up_msg(CLMS_CB *cb, CLMSV_CLMS_EVT *evt)
/* Self Node needs to be added tp patricia tree before hand during init
 */
if (NULL == clms_node_get_by_id(nodeid)) {
-   node->node_id =
-   evt->info.msg.info.api_info.param.nodeup_info.node_id;
+   node->node_id = nodeup_info->node_id;
  
  		TRACE("node->node_id %u node->nodeup %d", node->node_id,

  node->nodeup);
@@ -665,29 +679,18 @@ uint32_t proc_node_up_msg(CLMS_CB *cb, CLMSV_CLMS_EVT 
*evt)
"/node_name configuration");
}
}
-   node->boot_time =
-   evt->info.msg.info.api_info.param.nodeup_info.boot_time;
+
+   node->boot_time = nodeup_info->boot_time;
  
  	/* Update the node with ipaddress information */

-   if (ip->addr.length) {
-   memset(>node_addr, 0, sizeof(SaClmNodeAddressT));
-   node->node_addr.family = ip->addr.family;
-   node->node_addr.length = ip->addr.length;
-   memcpy(node->node_addr.value, ip->addr.value, 

Re: [devel] [PATCH 1/1] clm: Include boot time and node address in join request message [#2489]

[praveen malviya]

Ack.


Thanks,
Praveen

On 07-Aug-17 1:35 PM, Anders Widell wrote:
A node can have more than one single network address. If you run the 
ifconfig command, you get a list of network interfaces. Each one of 
these interfaces can have several address assigned to it: IPv4 
addresses, IPv6 addresses, and alias addresses. In addition, the node 
can have a TIPC address. So in the case of ticket [#2479], we might need 
to make both saClmNodeAddress and saClmNodeCurrAddress multi-value. 
However, I don't intend to implement [#2479] in the near future (or at 
all), since ticket [#2489] is probably enough for most real-world use 
cases. In most real-world use cases, it is enough for the application to 
get one single address for each node, but we need the flexibility to 
select which one of the addresses to present to the application.


saClmNodeAddressFamily and saClmNodeAddress are currently ignored by 
OpenSAF. I am not sure how saClmNodeAddressFamily and saClmNodeAddress 
are intended to be used, but my best guess is that saClmNodeAddress is 
intended for the case when you have statically assigned network 
addresses, and saClmNodeCurrAddress is intended for the case with 
dynamically assigned addresses, though there is no reason why we can't 
present a statically assigned address in saClmNodeCurrAddress as well. 
Since saClmNodeAddress is a configuration attribute, I am assuming here 
that you should actually be able to /set/ the node's address using the 
saClmNodeAddress configuration attribute! But in order for that to work, 
the node needs to read its IMM configuration immediately after booting, 
before it has configured its own network address. The only way this 
could work is if we are not actually talking about the network address 
used internally by OpenSAF, but the node's address on a separate network 
intended to be used by the application. Otherwise OpenSAF would not be 
able to communicate with IMM to read the node's own address. So 
according to this interpretation, each node has at least two addresses: 
one address used for internal OpenSAF communication, and another address 
used by the application. And it is the address used by the application 
which is configured using the saClmNodeAddress and presented in the 
saClmNodeCurrAddress attribute. Ticket doesn't favour any particular 
interpretation though, you are free to select the internal OpenSAF 
communication address or some other address to present in 
saClmNodeCurrAddress.


regards,

Anders Widell

On 08/07/2017 07:12 AM, praveen malviya wrote:

Hi Anders,

I have started reviewing this patch.

One initial query:
We have two sets of attributes for address in "SaClmNode":
set A) saClmNodeAddressFamily & saClmNodeAddress  and
set B )saClmNodeCurrAddressFamily & saClmNodeCurrAddress.

For ticket #2479, its description says making set B as Multi valued. I 
think it is set A that should be made multi-valued and set B should 
reflect the address currently in use. This will resolve some backward 
compatibility issue also as set B remains single valued.


Also the set B should reflect which address? address used by OpenSAF 
or by application? Till this time it has been OpenSAF internal 
communication address.



Thanks,
Praveen

On 31-Jul-17 6:40 PM, Anders Widell wrote:

The node join request message now has two new fields: boot time and node
address. This allows us to provide more accurate and correct 
information in the

CLM node runtime attributes in the information model:

* The boot time field transmits the node's actual boot time to the 
CLM server.
   Previously, the node join time was used as an approximation of the 
node boot
   time, but this might be inaccurate or incorrect. For example, if 
OpenSAF was
   started much later than the node was booted (e.g. if OpenSAF was 
restarted
   without a node reboot), then the node join time will differ 
significantly from

   the node boot time.

* The node address field transmits the node address to be presented 
to the
   application through the information model. Previously, the IP 
address which
   was used by OpenSAF internal communication was presented as the 
one and only
   node address, and there was no way to select some other address in 
case the
   node has multiple network addresses. The application now has the 
possibility

   to select which network address to present in the information model.
---
  00-README.conf |  8 
  src/clm/clmd/clms.h|  1 -
  src/clm/clmd/clms_evt.c| 15 ++-
  src/clm/clmd/clms_main.c   | 22 ++
  src/clm/clmd/clms_mbcsv.c  | 11 ++---
  src/clm/clmd/clms_mbcsv.h  |  2 -
  src/clm/clmd/clms_mds.c| 92 
+-

  src/clm/clmd/clms_util.c   | 15 ---
  src/clm/clmnd/cb.h | 14 ---
  src/clm/clmnd/clmna.conf   | 13 ++
  src/clm/clmnd/main.c   | 89 


  src/clm/common/clmsv_enc_dec.c 

Re: [devel] [PATCH 1/1] clm: Include boot time and node address in join request message [#2489]

[A V Mahesh]

Hi Anders & praveen,

On 8/7/2017 1:35 PM, Anders Widell wrote:
A node can have more than one single network address. If you run the 
ifconfig command, you get a list of network interfaces. Each one of 
these interfaces can have several address assigned to it: IPv4 
addresses, IPv6 addresses, and alias addresses. In addition, the node 
can have a TIPC address. So in the case of ticket [#2479], we might 
need to make both saClmNodeAddress and saClmNodeCurrAddress 
multi-value. However, I don't intend to implement [#2479] in the near 
future (or at all), since ticket [#2489] is probably enough for most 
real-world use cases. In most real-world use cases, it is enough for 
the application to get one single address for each node, but we need 
the flexibility to select which one of the addresses to present to the 
application. 
For your information , when I implemented the TCP transport , IP address 
/ CLM integration , I was pushing the  DTM configured IP address to  CLM.


-AVM

On 8/7/2017 1:35 PM, Anders Widell wrote:
A node can have more than one single network address. If you run the 
ifconfig command, you get a list of network interfaces. Each one of 
these interfaces can have several address assigned to it: IPv4 
addresses, IPv6 addresses, and alias addresses. In addition, the node 
can have a TIPC address. So in the case of ticket [#2479], we might 
need to make both saClmNodeAddress and saClmNodeCurrAddress 
multi-value. However, I don't intend to implement [#2479] in the near 
future (or at all), since ticket [#2489] is probably enough for most 
real-world use cases. In most real-world use cases, it is enough for 
the application to get one single address for each node, but we need 
the flexibility to select which one of the addresses to present to the 
application.


saClmNodeAddressFamily and saClmNodeAddress are currently ignored by 
OpenSAF. I am not sure how saClmNodeAddressFamily and saClmNodeAddress 
are intended to be used, but my best guess is that saClmNodeAddress is 
intended for the case when you have statically assigned network 
addresses, and saClmNodeCurrAddress is intended for the case with 
dynamically assigned addresses, though there is no reason why we can't 
present a statically assigned address in saClmNodeCurrAddress as well. 
Since saClmNodeAddress is a configuration attribute, I am assuming 
here that you should actually be able to /set/ the node's address 
using the saClmNodeAddress configuration attribute! But in order for 
that to work, the node needs to read its IMM configuration immediately 
after booting, before it has configured its own network address. The 
only way this could work is if we are not actually talking about the 
network address used internally by OpenSAF, but the node's address on 
a separate network intended to be used by the application. Otherwise 
OpenSAF would not be able to communicate with IMM to read the node's 
own address. So according to this interpretation, each node has at 
least two addresses: one address used for internal OpenSAF 
communication, and another address used by the application. And it is 
the address used by the application which is configured using the 
saClmNodeAddress and presented in the saClmNodeCurrAddress attribute. 
Ticket doesn't favour any particular interpretation though, you are 
free to select the internal OpenSAF communication address or some 
other address to present in saClmNodeCurrAddress.


regards,

Anders Widell

On 08/07/2017 07:12 AM, praveen malviya wrote:

Hi Anders,

I have started reviewing this patch.

One initial query:
We have two sets of attributes for address in "SaClmNode":
set A) saClmNodeAddressFamily & saClmNodeAddress  and
set B )saClmNodeCurrAddressFamily & saClmNodeCurrAddress.

For ticket #2479, its description says making set B as Multi valued. 
I think it is set A that should be made multi-valued and set B should 
reflect the address currently in use. This will resolve some backward 
compatibility issue also as set B remains single valued.


Also the set B should reflect which address? address used by OpenSAF 
or by application? Till this time it has been OpenSAF internal 
communication address.



Thanks,
Praveen

On 31-Jul-17 6:40 PM, Anders Widell wrote:
The node join request message now has two new fields: boot time and 
node
address. This allows us to provide more accurate and correct 
information in the

CLM node runtime attributes in the information model:

* The boot time field transmits the node's actual boot time to the 
CLM server.
   Previously, the node join time was used as an approximation of 
the node boot
   time, but this might be inaccurate or incorrect. For example, if 
OpenSAF was
   started much later than the node was booted (e.g. if OpenSAF was 
restarted
   without a node reboot), then the node join time will differ 
significantly from

   the node boot time.

* The node address field transmits the node address to be presented 
to the
   application through the 

Re: [devel] [PATCH 1/1] clm: Include boot time and node address in join request message [#2489]

[Anders Widell]

A node can have more than one single network address. If you run the 
ifconfig command, you get a list of network interfaces. Each one of 
these interfaces can have several address assigned to it: IPv4 
addresses, IPv6 addresses, and alias addresses. In addition, the node 
can have a TIPC address. So in the case of ticket [#2479], we might need 
to make both saClmNodeAddress and saClmNodeCurrAddress multi-value. 
However, I don't intend to implement [#2479] in the near future (or at 
all), since ticket [#2489] is probably enough for most real-world use 
cases. In most real-world use cases, it is enough for the application to 
get one single address for each node, but we need the flexibility to 
select which one of the addresses to present to the application.


saClmNodeAddressFamily and saClmNodeAddress are currently ignored by 
OpenSAF. I am not sure how saClmNodeAddressFamily and saClmNodeAddress 
are intended to be used, but my best guess is that saClmNodeAddress is 
intended for the case when you have statically assigned network 
addresses, and saClmNodeCurrAddress is intended for the case with 
dynamically assigned addresses, though there is no reason why we can't 
present a statically assigned address in saClmNodeCurrAddress as well. 
Since saClmNodeAddress is a configuration attribute, I am assuming here 
that you should actually be able to /set/ the node's address using the 
saClmNodeAddress configuration attribute! But in order for that to work, 
the node needs to read its IMM configuration immediately after booting, 
before it has configured its own network address. The only way this 
could work is if we are not actually talking about the network address 
used internally by OpenSAF, but the node's address on a separate network 
intended to be used by the application. Otherwise OpenSAF would not be 
able to communicate with IMM to read the node's own address. So 
according to this interpretation, each node has at least two addresses: 
one address used for internal OpenSAF communication, and another address 
used by the application. And it is the address used by the application 
which is configured using the saClmNodeAddress and presented in the 
saClmNodeCurrAddress attribute. Ticket doesn't favour any particular 
interpretation though, you are free to select the internal OpenSAF 
communication address or some other address to present in 
saClmNodeCurrAddress.


regards,

Anders Widell

On 08/07/2017 07:12 AM, praveen malviya wrote:

Hi Anders,

I have started reviewing this patch.

One initial query:
We have two sets of attributes for address in "SaClmNode":
set A) saClmNodeAddressFamily & saClmNodeAddress  and
set B )saClmNodeCurrAddressFamily & saClmNodeCurrAddress.

For ticket #2479, its description says making set B as Multi valued. I 
think it is set A that should be made multi-valued and set B should 
reflect the address currently in use. This will resolve some backward 
compatibility issue also as set B remains single valued.


Also the set B should reflect which address? address used by OpenSAF 
or by application? Till this time it has been OpenSAF internal 
communication address.



Thanks,
Praveen

On 31-Jul-17 6:40 PM, Anders Widell wrote:

The node join request message now has two new fields: boot time and node
address. This allows us to provide more accurate and correct 
information in the

CLM node runtime attributes in the information model:

* The boot time field transmits the node's actual boot time to the 
CLM server.
   Previously, the node join time was used as an approximation of the 
node boot
   time, but this might be inaccurate or incorrect. For example, if 
OpenSAF was
   started much later than the node was booted (e.g. if OpenSAF was 
restarted
   without a node reboot), then the node join time will differ 
significantly from

   the node boot time.

* The node address field transmits the node address to be presented 
to the
   application through the information model. Previously, the IP 
address which
   was used by OpenSAF internal communication was presented as the 
one and only
   node address, and there was no way to select some other address in 
case the
   node has multiple network addresses. The application now has the 
possibility

   to select which network address to present in the information model.
---
  00-README.conf |  8 
  src/clm/clmd/clms.h|  1 -
  src/clm/clmd/clms_evt.c| 15 ++-
  src/clm/clmd/clms_main.c   | 22 ++
  src/clm/clmd/clms_mbcsv.c  | 11 ++---
  src/clm/clmd/clms_mbcsv.h  |  2 -
  src/clm/clmd/clms_mds.c| 92 
+-

  src/clm/clmd/clms_util.c   | 15 ---
  src/clm/clmnd/cb.h | 14 ---
  src/clm/clmnd/clmna.conf   | 13 ++
  src/clm/clmnd/main.c   | 89 


  src/clm/common/clmsv_enc_dec.c | 34 +++-
  src/clm/common/clmsv_enc_dec.h |  2 +
  

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

[Nguyen Luu]

Thanks, Hans and Anders, for your review.

About the code refactoring, should we do it in a separate ticket for all 
AMF API's that use such constant and format specifier? That should be an 
enhancement rather than a defect ticket like this one.


Thanks,
Nguyen

On 8/2/2017 6:16 PM, Anders Widell wrote:
UINT32_MAX from  is a better constant here since we are 
dealing with a 32-bit integer.


regards,

Anders Widell


On 08/02/2017 09:19 AM, Hans Nordebäck wrote:

ack, code review only.

When refactoring the code, perhaps UINT_MAX from  can be 
used instead of AVSV_UNS32_HDL_MAX?


And PRIx64 from  instead of %llx?

/Regards HansN

On 08/02/2017 06:01 AM, Nguyen Luu wrote:

When called with an uninitialized or already finalized handle,
saAmfPmStart_3 and saAmfResponse_4 should return SA_AIS_ERR_BAD_HANDLE
instead of SA_AIS_ERR_VERSION as previously done.
---
  src/amf/agent/amf_agent.cc | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/src/amf/agent/amf_agent.cc b/src/amf/agent/amf_agent.cc
index 20528e9..b9191dd 100644
--- a/src/amf/agent/amf_agent.cc
+++ b/src/amf/agent/amf_agent.cc
@@ -2296,6 +2296,13 @@ SaAisErrorT AmfAgent::PmStart_3(SaAmfHandleT 
hdl, const SaNameT *comp_name,

SaAisErrorT rc = SA_AIS_OK;
TRACE_ENTER2("SaAmfHandleT passed is %llx", hdl);
  +  /* Verifying the input Handle & global handle */
+  if (!gl_ava_hdl || hdl > AVSV_UNS32_HDL_MAX) {
+TRACE_2("Invalid SaAmfHandle passed by component: %llx", hdl);
+rc = SA_AIS_ERR_BAD_HANDLE;
+goto done;
+  }
+
/* Version is previously set in in initialize function */
if (!ava_B4_ver_used(0)) {
  TRACE_2(
@@ -2844,6 +2851,13 @@ SaAisErrorT AmfAgent::Response_4(SaAmfHandleT 
hdl, SaInvocationT inv,

SaAisErrorT rc = SA_AIS_OK;
TRACE_ENTER2("SaAmfHandleT passed is %llx", hdl);
  +  /* Verifying the input Handle & global handle */
+  if (!gl_ava_hdl || hdl > AVSV_UNS32_HDL_MAX) {
+TRACE_2("Invalid SaAmfHandle passed by component: %llx", hdl);
+rc = SA_AIS_ERR_BAD_HANDLE;
+goto done;
+  }
+
/* Version is previously set in in initialize function */
if (!ava_B4_ver_used(0)) {
  TRACE_2(



-- 


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







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