Hi Anders,

some comments below.

/Regards Hans


On 10/02/2017 01:25 PM, Anders Widell wrote:
Ack with comments:

1) Add a final comma to the end of the command-line parameter for the scale-out script. The reason why there should be a final comma is to make sure the script (which may be modified by the user) is backwards-compatible with future enhancements where we add more parameters. By adding a comma after the final parameter, new parameters added in the future are less likely to cause compatibility problems for the user-modified script.

[HansN] ok, I'll put the comma back
2) Add a vetting function that checks that the user data doesn't contain illegal characters. If the user data contains illegal characters we shall treat it in the same way as if the file was missing. At least the comma character shall be considered illegal. I think we can be even more restrictive - maybe only allow printable ASCII characters (including space). It is easy to remove restrictions later on without impacting backwards compatibility. Adding restrictions later on will on the other hand not be backwards compatible.

[HansN] I think it would be good to not put restrictions on the user data, for the IP address we can use inet_pton to validate both IPv4 and IPv6.
3) Use the vetting function also for the IP address that was added in ticket [#2489] (I forgot this :-).

4) The name of the file: "user_data" does not describe the purpose so well. Maybe change it to "clm_node_info"? At least I think the name should mention "CLM". Since it is only used for scaling it could perhaps be called "clm_scale_out_info", although in the future we may use it even when there is no scaling.
[HansN] ok, clm_user_data. I think user_data is rather common/general for data that can be specified by an "user/application".

/ Anders W


On 09/25/2017 04:32 PM, Hans Nordeback wrote:
---
  src/clm/README             |  8 +++++---
  src/clm/clmd/clms_evt.c    | 11 +++++++++--
  src/clm/clmd/clms_mds.c    | 11 +++++++++++
  src/clm/clmnd/cb.h         |  1 +
  src/clm/clmnd/main.c       | 28 +++++++++++++++++++++++++++-
  src/clm/common/clmsv_msg.h |  1 +
  6 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/src/clm/README b/src/clm/README
index dee3e64f2..10e60849b 100644
--- a/src/clm/README
+++ b/src/clm/README
@@ -65,10 +65,12 @@ and if so add the necessary IMM objects so that the node will be able to join   the next time it tries. The script will be called with one or more command-line   arguments, where each argument is a comma-separated list of properties of a node   that wishes to join the cluster. Currently, the comma-separated list in each -command-line argument contains only two entries, but the script should be -forwards compatible with future extensions where more entries may be added to +command-line argument contains four entries, node-id, node-name, node-ipaddress, user-data. +But the script should be forwards compatible with future extensions where more entries may be added to   the comma-separated list. The first entry in the list is the node id represented -as a decimal number. The second entry in the list is the name of the node. +as a decimal number. The second entry in the list is the name of the node. The third entry is the +IP address of the node. The fourth entry is optional user data that can be entered in a file named
+/etc/opensaf/user_data, up to 256 characters can be given.
    NOTE: the script must be idempotent, i.e. it must be harmless to call it more   than one time with the same parameters. The second call should do nothing since
diff --git a/src/clm/clmd/clms_evt.c b/src/clm/clmd/clms_evt.c
index 84e7b3c6d..eb771c105 100644
--- a/src/clm/clmd/clms_evt.c
+++ b/src/clm/clmd/clms_evt.c
@@ -453,6 +453,7 @@ static void scale_out_node(CLMS_CB *cb,
                 const clmsv_clms_node_up_info_t *nodeup_info)
  {
      char node_name[SA_MAX_NAME_LENGTH];
+    char user_data[SA_MAX_NAME_LENGTH];
        TRACE_ENTER();
      size_t name_len = nodeup_info->node_name.length < SA_MAX_NAME_LENGTH
@@ -461,6 +462,12 @@ static void scale_out_node(CLMS_CB *cb,
      memcpy(node_name, nodeup_info->node_name.value, name_len);
      node_name[name_len] = '\0';
  +    size_t user_data_len = nodeup_info->user_data.length < SA_MAX_NAME_LENGTH
+                  ? nodeup_info->user_data.length
+                  : (SA_MAX_NAME_LENGTH - 1);
+    memcpy(user_data, nodeup_info->user_data.value, user_data_len);
+    user_data[user_data_len] = '\0';
+
      osaf_mutex_lock_ordie(&cb->scale_out_data_mutex);
      size_t no_of_pending_nodes = cb->no_of_pending_nodes;
      size_t no_of_inprogress_nodes = cb->no_of_inprogress_nodes;
@@ -497,8 +504,8 @@ static void scale_out_node(CLMS_CB *cb,
          memcpy(node_address, nodeup_info->address.value, addr_len);
          node_address[addr_len] = '\0';
          char *strp;
-        if (asprintf(&strp, "%" PRIu32 ",%s,%s,", nodeup_info->node_id,
-                 node_name, node_address) != -1) {
+        if (asprintf(&strp, "%" PRIu32 ",%s,%s,%s", nodeup_info->node_id,
+                 node_name, node_address, user_data) != -1) {
              LOG_NO("Queuing request to scale out node 0x%" PRIx32
                     " (%s)",
                     nodeup_info->node_id, node_name);
diff --git a/src/clm/clmd/clms_mds.c b/src/clm/clmd/clms_mds.c
index 1eb0e1e62..ecfe0da3b 100644
--- a/src/clm/clmd/clms_mds.c
+++ b/src/clm/clmd/clms_mds.c
@@ -787,6 +787,17 @@ static uint32_t clms_dec_nodeup_msg(NCS_UBAID *uba, CLMSV_MSG *msg)
      TRACE("nodename %s length %d",
msg->info.api_info.param.nodeup_info.node_name.value,
msg->info.api_info.param.nodeup_info.node_name.length);
+
+ memset(&msg->info.api_info.param.nodeup_info.user_data, 0, sizeof(SaNameT));
+    p8 = ncs_dec_flatten_space(uba, local_data, 2);
+    if (p8 != NULL) {
+        total_bytes += clmsv_decodeSaNameT(
+            uba, &(msg->info.api_info.param.nodeup_info.user_data));
+        TRACE("userdata %s length %d",
+ msg->info.api_info.param.nodeup_info.user_data.value,
+ msg->info.api_info.param.nodeup_info.user_data.length);
+    }
+
      p8 = ncs_dec_flatten_space(uba, local_data, 8);
      // Old protocol versions don't have the boot_time field. Use the current
      // wall clock time if boot_time isn't present in the message.
diff --git a/src/clm/clmnd/cb.h b/src/clm/clmnd/cb.h
index 9b26a9bf5..cb66ffe8b 100644
--- a/src/clm/clmnd/cb.h
+++ b/src/clm/clmnd/cb.h
@@ -37,6 +37,7 @@
  typedef struct node_detail_t {
    SaUint32T node_id;
    SaNameT node_name;
+  SaNameT user_data;
    SaTimeT boot_time;
    SaUint16T no_of_addresses;
    SaClmNodeAddressT address;
diff --git a/src/clm/clmnd/main.c b/src/clm/clmnd/main.c
index 3a8479600..3ca5a7a34 100644
--- a/src/clm/clmnd/main.c
+++ b/src/clm/clmnd/main.c
@@ -15,7 +15,7 @@
   *             Ericsson AB
   *
   */
-
+#include <stdio.h>
  #include <errno.h>
  #include <inttypes.h>
  #include <saClm.h>
@@ -389,6 +389,9 @@ static uint32_t clmna_mds_enc(struct ncsmds_callback_info *info)
              total_bytes += clmsv_encodeSaNameT(
                  uba,
&(msg->info.api_info.param.nodeup_info.node_name));
+            total_bytes += clmsv_encodeSaNameT(
+                uba,
+ &(msg->info.api_info.param.nodeup_info.user_data));
              p8 = ncs_enc_reserve_space(uba, 8);
              ncs_encode_64bit(
                  &p8,
@@ -508,10 +511,32 @@ static uint32_t clmna_mds_init(void)
      return rc;
  }
  +static void get_user_data(NODE_INFO *node)
+{
+    FILE *fp;
+
+    fp = fopen(PKGSYSCONFDIR "/user_data", "r");
+    if (fp == NULL) {
+        LOG_IN("Could not open file %s - %s", PKGSYSCONFDIR "/user_data",
+               strerror(errno));
+        return;
+    }
+
+    if (fgets((char*) node->user_data.value, sizeof(node->user_data.value), fp) != NULL) {
+        node->user_data.length = strnlen((char *)node->user_data.value,
+            sizeof(node->user_data.value));
+    } else {
+        LOG_WA("Could not read file %s", PKGSYSCONFDIR "/user_data");
+    }
+
+}
+
  static int get_node_info(NODE_INFO *node)
  {
      FILE *fp;
  +    get_user_data(node);
+
      fp = fopen(PKGSYSCONFDIR "/node_name", "r");
      if (fp == NULL) {
          LOG_ER("Could not open file %s - %s", PKGSYSCONFDIR "node_name",
@@ -678,6 +703,7 @@ static void clmna_process_dummyup_msg(void)
          msg.info.api_info.param.nodeup_info.no_of_addresses =
              self_node.no_of_addresses;
          msg.info.api_info.param.nodeup_info.address = self_node.address; +        msg.info.api_info.param.nodeup_info.user_data = self_node.user_data;
          stop_scale_out_retry_tmr();
          start_scale_out_retry_tmr(CLMNA_JOIN_RETRY_TIME);
          uint32_t rc = clmna_mds_msg_send(&msg);
diff --git a/src/clm/common/clmsv_msg.h b/src/clm/common/clmsv_msg.h
index f312659f7..5619cbaf9 100644
--- a/src/clm/common/clmsv_msg.h
+++ b/src/clm/common/clmsv_msg.h
@@ -139,6 +139,7 @@ typedef struct clmsv_track_info_t {
  typedef struct {
    SaUint32T node_id;
    SaNameT node_name;
+  SaNameT user_data;
    SaTimeT boot_time;
    SaUint16T no_of_addresses;
    SaClmNodeAddressT address;



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

Reply via email to