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

2017-06-13 Thread Anders Widell 
Ensure compatibility with Python 3 by running the Python source code files
through the Automated Python 2 to 3 code translation tool "2to3". For more
information about this tool, see:

https://docs.python.org/3/library/2to3.html
---
 python/pyosaf/saEnumConst.py |  6 +++---
 python/pyosaf/utils/immoi/__init__.py|  8 +++
 python/pyosaf/utils/immoi/implementer.py | 36 
 python/pyosaf/utils/immom/__init__.py|  6 +++---
 python/pyosaf/utils/immom/ccb.py | 17 ---
 python/pyosaf/utils/immom/iterator.py|  8 +++
 python/pyosaf/utils/immom/object.py  |  2 +-
 7 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/python/pyosaf/saEnumConst.py b/python/pyosaf/saEnumConst.py
index 5c3338267..51f1efdeb 100644
--- a/python/pyosaf/saEnumConst.py
+++ b/python/pyosaf/saEnumConst.py
@@ -50,14 +50,14 @@ class Enumeration(object):
 
i = 0
for node in enumlist:
-   if type(node) is tuple:
+   if isinstance(node, tuple):
try:
node, i = node
except ValueError:
raise EnumException('%r:' % (node,))
-   if type(node) is not str:
+   if not isinstance(node, str):
raise EnumException('Enum name not a string: 
%r' % (node,))
-   if type(i) is not int:
+   if not isinstance(i, int):
raise EnumException('Enum value not integer: 
%r' % (node,))
if node in lookup:
raise EnumException('Enum name not unique: %r' 
% (node,))
diff --git a/python/pyosaf/utils/immoi/__init__.py 
b/python/pyosaf/utils/immoi/__init__.py
index c70f843e9..02ad89b90 100644
--- a/python/pyosaf/utils/immoi/__init__.py
+++ b/python/pyosaf/utils/immoi/__init__.py
@@ -126,7 +126,7 @@ def create_rt_object(class_name, parent_name, obj):
 
 c_attr_values = []
 
-for name, (c_attr_type, values) in obj.attrs.iteritems():
+for name, (c_attr_type, values) in obj.attrs.items():
 
 if values == None:
 values = []
@@ -175,10 +175,10 @@ def update_rt_object(dn, attributes):
 # Create and marshall attribute modifications
 attr_mods = []
 
-for name, values in attributes.iteritems():
+for name, values in attributes.items():
 
 if values is None:
-print "WARNING: Received no values for %s in %s" % (name, dn)
+print("WARNING: Received no values for %s in %s" % (name, dn))
 continue
 
 if not isinstance(values, list):
@@ -356,7 +356,7 @@ def create_non_existing_imm_object(class_name, parent_name, 
attributes):
 
 obj = ImmObject(class_name=class_name, dn=dn)
 
-for name, values in attributes.iteritems():
+for name, values in attributes.items():
 obj.__setattr__(name, values)
 
 obj.__setattr__('SaImmAttrClassName', class_name)
diff --git a/python/pyosaf/utils/immoi/implementer.py 
b/python/pyosaf/utils/immoi/implementer.py
index 865db9ba0..b183905a6 100755
--- a/python/pyosaf/utils/immoi/implementer.py
+++ b/python/pyosaf/utils/immoi/implementer.py
@@ -113,8 +113,8 @@ def _collect_full_transaction(ccb_id):
 affected_instances = [i for i in all_objects_now if i.dn == dn]
 
 if len(affected_instances) == 0:
-print ('ERROR: Failed to find object %s affected by modify 
'
-   'operation' % dn)
+print(('ERROR: Failed to find object %s affected by modify 
'
+   'operation' % dn))
 else:
 affected_instance = affected_instances[0]
 
@@ -198,8 +198,8 @@ def admin_operation(oi_handle, c_invocation_id, c_name, 
c_operation_id, c_params
 try:
 immoi.report_admin_operation_result(invocation_id, result)
 except SafException as err:
-print "ERROR: Failed to report that %s::%s returned %s (%s)" % \
-(name, invocation_id, result, err.msg)
+print("ERROR: Failed to report that %s::%s returned %s (%s)" % \
+(name, invocation_id, result, err.msg))
 
 def abort_ccb(oi_handle, ccb_id):
 ''' Callback for aborted CCBs.
@@ -275,7 +275,7 @@ def delete_added(oi_handle, ccb_id, c_name):
  eSaImmValueTypeT.SA_IMM_ATTR_SANAMET)
 
 # Create a new CCB in the cache if needed
-if not ccb_id in CCBS.keys():
+if not ccb_id in list(CCBS.keys()):
 CCBS[ccb_id] = []
 
 # Cache the operation
@@ -318,7 +318,7 @@ def modify_added(oi_handle, c_ccb_id, c_name, 
c_attr_modification):
 implementer_objection = result
 
 # Create a new CCB in the cache if needed
-if not ccb_id in CCBS.keys():
+if not ccb_id in

[devel] [PATCH 0/1] Review Request for pyosaf: Ensure compatibility with Python 3 [#2492]

2017-06-13 Thread Anders Widell 
Summary: pyosaf: Ensure compatibility with Python 3 [#2492]
Review request for Ticket(s): 2492
Peer Reviewer(s): Srikanth, Hans
Pull request to: 
Affected branch(es): develop
Development branch: ticket-2492
Base revision: db1965d634eac2f375f455b7b7d3e9f70ff0c47c
Personal repository: git://git.code.sf.net/u/anders-w/review


Impacted area   Impact y/n

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


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

revision 312674e5ca8bb4ec50a1d2d843d3db3bcbddfe02
Author: Anders Widell 
Date:   Tue, 13 Jun 2017 15:05:42 +0200

pyosaf: Ensure compatibility with Python 3 [#2492]

Ensure compatibility with Python 3 by running the Python source code files
through the Automated Python 2 to 3 code translation tool "2to3". For more
information about this tool, see:

https://docs.python.org/3/library/2to3.html



Complete diffstat:
--
 python/pyosaf/saEnumConst.py |  6 +++---
 python/pyosaf/utils/immoi/__init__.py|  8 +++
 python/pyosaf/utils/immoi/implementer.py | 36 
 python/pyosaf/utils/immom/__init__.py|  6 +++---
 python/pyosaf/utils/immom/ccb.py | 17 ---
 python/pyosaf/utils/immom/iterator.py|  8 +++
 python/pyosaf/utils/immom/object.py  |  2 +-
 7 files changed, 42 insertions(+), 41 deletions(-)


Testing Commands:
-
Run regression tests for the Python bindings using both Python 2 and Python 3


Testing, Expected Results:
--
Regression tests shall pass both when using Python 2 and Python 3


Conditions of Submission:
-
Ack from reviewer(s)


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

[devel] [PATCH 2/3] amfnd: Refactor AVND_SU to class and comp_list to std::vector [#1945]

2017-06-13 Thread Hans Nordeback 
---
 src/amf/amfnd/avnd_comp.h |  16 +---
 src/amf/amfnd/avnd_err.h  |   6 +-
 src/amf/amfnd/avnd_su.h   | 119 --
 src/amf/amfnd/avnd_tmr.h  |  10 +--
 src/amf/amfnd/avnd_util.h |   3 +
 src/amf/amfnd/clc.cc  |   7 +-
 src/amf/amfnd/comp.cc |   4 +-
 src/amf/amfnd/compdb.cc   |  32 ++-
 src/amf/amfnd/err.cc  |  29 ++-
 src/amf/amfnd/su.cc   | 100 --
 src/amf/amfnd/sudb.cc |  30 ++-
 src/amf/amfnd/susm.cc | 206 +++---
 src/amf/amfnd/tmr.cc  |   6 +-
 src/amf/amfnd/util.cc |  10 +++
 14 files changed, 221 insertions(+), 357 deletions(-)

diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h
index 68de4cc8e..a2fc22691 100644
--- a/src/amf/amfnd/avnd_comp.h
+++ b/src/amf/amfnd/avnd_comp.h
@@ -36,7 +36,7 @@
 
 struct avnd_cb_tag;
 struct avnd_su_si_rec;
-struct avnd_su_tag;
+class AVND_SU;
 struct avnd_srm_req_tag;
 
 /***
@@ -323,9 +323,6 @@ enum UsedComptypeAttrs {
 
 class AVND_COMP {
  public:
-  // TODO(uabhano) replace the NCS_DB_LINK_LIST_NODE with C++ STL. Now 
su_dll_node must be first in AVND_COMP
-  // as the macro m_AVND_COMP_SU_DLL_NODE_OFFSET depends on the offset. 
offsetof is to be avoided in classes.
-  NCS_DB_LINK_LIST_NODE su_dll_node {}; /* su dll node (key is inst-level) */
   AVND_COMP() {}
   ~AVND_COMP() {}
 
@@ -386,7 +383,7 @@ class AVND_COMP {
 
   NCS_DB_LINK_LIST csi_list {}; /* csi list */
 
-  struct avnd_su_tag *su {}; /* back ptr to parent SU */
+  AVND_SU *su {}; /* back ptr to parent SU */
 
   AVND_COMP *pxy_comp {}; /* ptr to the proxy comp (if any) */
 
@@ -627,13 +624,6 @@ class AVND_COMP {
 /* macro to determine if the pre-configured proxied comp has any proxy comp */
 #define m_AVND_COMP_IS_PROXIED(x) ((x)->proxy_comp)
 
-/* macro to retrieve component ptr from su dll node ptr */
-/* TODO(uabhano) remove these macros */
-#define m_AVND_COMP_SU_DLL_NODE_OFFSET 0
-
-#define m_AVND_COMP_FROM_SU_DLL_NODE_GET(x) \
-  ((x) ? ((AVND_COMP *)(((uint8_t *)(x)) - m_AVND_COMP_SU_DLL_NODE_OFFSET)) : 
0)
-
 /* macro to retrieve csi ptr from comp-csi dll node ptr */
 #define m_AVND_CSI_COMP_DLL_NODE_OFFSET  \
   ((uint8_t *)&(AVND_COMP_CSI_REC_NULL->comp_dll_node) - \
@@ -987,7 +977,7 @@ extern uint32_t avnd_comp_oper_req(struct avnd_cb_tag *cb,
AVSV_PARAM_INFO *param);
 extern uint32_t avnd_comptype_oper_req(struct avnd_cb_tag *cb,
AVSV_PARAM_INFO *param);
-extern unsigned int avnd_comp_config_get_su(struct avnd_su_tag *su);
+extern unsigned int avnd_comp_config_get_su(AVND_SU *su);
 extern int avnd_comp_config_reinit(AVND_COMP *comp);
 extern void avnd_comp_delete(AVND_COMP *comp);
 extern void avnd_comp_pres_state_set(const struct avnd_cb_tag *cb,
diff --git a/src/amf/amfnd/avnd_err.h b/src/amf/amfnd/avnd_err.h
index 17e236d80..76f968927 100644
--- a/src/amf/amfnd/avnd_err.h
+++ b/src/amf/amfnd/avnd_err.h
@@ -114,11 +114,11 @@ typedef struct avnd_err_tag {
 
 struct avnd_cb_tag;
 class AVND_COMP;
-struct avnd_su_tag;
+class AVND_SU;
 
 extern uint32_t avnd_err_process(struct avnd_cb_tag *, AVND_COMP *,
  AVND_ERR_INFO *);
-extern uint32_t avnd_err_su_repair(struct avnd_cb_tag *, struct avnd_su_tag *);
-extern bool is_no_assignment_due_to_escalations(struct avnd_su_tag *);
+extern uint32_t avnd_err_su_repair(struct avnd_cb_tag *, AVND_SU *);
+extern bool is_no_assignment_due_to_escalations(AVND_SU *);
 
 #endif  // AMF_AMFND_AVND_ERR_H_
diff --git a/src/amf/amfnd/avnd_su.h b/src/amf/amfnd/avnd_su.h
index 69124ed95..c7ba25718 100644
--- a/src/amf/amfnd/avnd_su.h
+++ b/src/amf/amfnd/avnd_su.h
@@ -34,6 +34,12 @@
 #ifndef AMF_AMFND_AVND_SU_H_
 #define AMF_AMFND_AVND_SU_H_
 
+#include 
+#include 
+
+#include "avnd_comp.h"
+#include "avnd_util.h"
+
 struct avnd_cb_tag;
 
 /***
@@ -53,7 +59,7 @@ typedef AVSV_D2N_COMPCSI_ASSIGN_MSG_INFO 
AVND_COMP_CSI_PARAMS_INFO;
 
 /* declaration clc event handler */
 typedef uint32_t (*AVND_SU_PRES_FSM_FN)(struct avnd_cb_tag *,
-struct avnd_su_tag *, AVND_COMP *);
+AVND_SU *, AVND_COMP *);
 
 /* su presence state fsm events */
 typedef enum avnd_su_pres_fsm_ev {
@@ -97,7 +103,7 @@ typedef struct avnd_su_si_rec {
   NCS_DB_LINK_LIST csi_list; /* ordered csi list (based on csi rank) */
 
   /* links to other entities */
-  struct avnd_su_tag *su; /* bk ptr to su */
+  AVND_SU *su; /* bk ptr to su */
   std::string su_name;/* For checkpointing su name */
   AVSV_SUSI_ACT single_csi_add_rem_in_si; /* To detect whether single csi
  addition/removal is going on.*/
@@ -115,37 +121,40 @@

[devel] [PATCH 0/3] Review Request for amfnd: Refactor AVND_COMP for simpler cmd argument handling V3 [#1945]

2017-06-13 Thread Hans Nordeback 
Summary: amfnd: Refactor AVND_COMP for simpler environment variable handling 
[#1945]
Review request for Ticket(s): 1945
Peer Reviewer(s): Praveen, Gary, Nagu
Pull request to: 
Affected branch(es): develop
Development branch: ticket-1945
Base revision: db1965d634eac2f375f455b7b7d3e9f70ff0c47c
Personal repository: git://git.code.sf.net/u/hansnordeback/review


Impacted area   Impact y/n

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

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

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

revision 3f616b75b74f8543d0dffcda3eea884b3c52a5e9
Author: Hans Nordeback 
Date:   Tue, 13 Jun 2017 12:49:30 +0200

amfnd: Refactor AVND_COMP for simpler cmd argument handling V3 [#1945]



revision 531df2512152b16e655fc5a73f591d72486212f1
Author: Hans Nordeback 
Date:   Tue, 13 Jun 2017 10:12:29 +0200

amfnd: Refactor AVND_SU to class and comp_list to std::vector [#1945]



revision 140b3afa67e866c8fc30037748eb4af045266edd
Author: Hans Nordeback 
Date:   Tue, 13 Jun 2017 10:12:29 +0200

amfnd: Refactor AVND_COMP for simpler environment variable handling [#1945]



Complete diffstat:
--
 src/amf/amfnd/avnd_comp.h | 211 --
 src/amf/amfnd/avnd_err.h  |  11 +--
 src/amf/amfnd/avnd_hc.h   |   3 +-
 src/amf/amfnd/avnd_proc.h |  11 +--
 src/amf/amfnd/avnd_su.h   | 120 --
 src/amf/amfnd/avnd_tmr.h  |  17 ++--
 src/amf/amfnd/avnd_util.h |   6 +-
 src/amf/amfnd/cam.cc  |   3 +-
 src/amf/amfnd/chc.cc  |   3 +-
 src/amf/amfnd/clc.cc  | 167 
 src/amf/amfnd/comp.cc |  88 ++-
 src/amf/amfnd/compdb.cc   | 181 +++
 src/amf/amfnd/err.cc  |  30 ++-
 src/amf/amfnd/proxydb.cc  |   3 +-
 src/amf/amfnd/su.cc   | 101 --
 src/amf/amfnd/sudb.cc |  33 ++--
 src/amf/amfnd/susm.cc | 208 +++--
 src/amf/amfnd/tmr.cc  |   7 +-
 src/amf/amfnd/util.cc |  11 +++
 19 files changed, 517 insertions(+), 697 deletions(-)


Testing Commands:
-
*** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES ***


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


Conditions of Submission:
-
*** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC ***


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

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

2017-06-13 Thread Hans Nordeback 
---
 src/amf/amfnd/avnd_comp.h |  77 ---
 src/amf/amfnd/avnd_err.h  |   1 +
 src/amf/amfnd/avnd_hc.h   |   1 +
 src/amf/amfnd/avnd_proc.h |   1 +
 src/amf/amfnd/avnd_su.h   |   1 +
 src/amf/amfnd/avnd_tmr.h  |   7 +--
 src/amf/amfnd/avnd_util.h |   1 +
 src/amf/amfnd/cam.cc  |   3 +-
 src/amf/amfnd/chc.cc  |   3 +-
 src/amf/amfnd/clc.cc  |  35 --
 src/amf/amfnd/comp.cc |  51 -
 src/amf/amfnd/compdb.cc   | 113 +++---
 src/amf/amfnd/err.cc  |   1 +
 src/amf/amfnd/proxydb.cc  |   1 +
 src/amf/amfnd/su.cc   |   1 +
 src/amf/amfnd/sudb.cc |   3 +-
 src/amf/amfnd/susm.cc |   2 +-
 src/amf/amfnd/tmr.cc  |   1 +
 src/amf/amfnd/util.cc |   1 +
 19 files changed, 167 insertions(+), 137 deletions(-)

diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h
index a2fc22691..4b735138e 100644
--- a/src/amf/amfnd/avnd_comp.h
+++ b/src/amf/amfnd/avnd_comp.h
@@ -1,6 +1,7 @@
 /*  -*- OpenSAF  -*-
  *
  * (C) Copyright 2008 The OpenSAF Foundation
+ * (C) Copyright 2017 Ericsson AB - 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
@@ -33,11 +34,14 @@
 #include 
 #include 
 #include 
+#include 
+#include "avnd_tmr.h"
 
 struct avnd_cb_tag;
 struct avnd_su_si_rec;
 class AVND_SU;
 struct avnd_srm_req_tag;
+class AVND_COMP;
 
 /***
  **  S T R U C T U R E / E N U M  D E F I N I T I O N S  ***
@@ -90,16 +94,42 @@ typedef enum avnd_comp_clc_cmd_type {
 } AVND_COMP_CLC_CMD_TYPE;
 
 /* clc command parameter definition */
-typedef struct avnd_comp_clc_param {
-  char cmd[SAAMF_CLC_LEN]; /* cmd ascii string */
-  SaTimeT timeout; /* cmd timeout value */
-  uint32_t len;/* cmd len */
-} AVND_COMP_CLC_CMD_PARAM;
+struct CompClcCmdParam {
+  void init_clc_cli_command(const char *clc_cmd, char **clc_cmd_argv,
+const SaImmAttrValuesT_2 **attributes,
+const char *attr_name);
+  std::string cmd;
+  std::vector cmd_argv;  /* cmd argv */
+  SaTimeT timeout;/* cmd timeout value */
+};
 
 /* clc info definition (top level wrapper structure) */
-typedef struct avnd_comp_clc_info {
-  /* clc commands (indexed by cmd type) */
-  AVND_COMP_CLC_CMD_PARAM cmds[AVND_COMP_CLC_CMD_TYPE_MAX - 1];
+struct CompClcInfo {
+  std::map cmds;
+  std::string get_cmd(AVND_COMP_CLC_CMD_TYPE cmd_type);
+
+  template
+  void create_argv(std::array , uint32_t , 
AVND_COMP_CLC_CMD_TYPE cmd_type) {
+std::string delimiter {' '};
+argc = 0;
+std::string str = saAmfNodeSwBundlePathPrefix + cmds[cmd_type].cmd;
+size_t start = 0;
+size_t end = 0;
+
+while (end != std::string::npos) {
+  if (argc >= argv.size()) {
+LOG_WA("Too many arguments given, max %zu arguments are supported", 
argv.size());
+break;
+  }
+
+  end = str.find(delimiter, start);
+  std::string tmp = str.substr(start, (end == std::string::npos) ? 
std::string::npos : end - start);
+  argv[argc++] = strdup(tmp.data());
+  start = ((end > (std::string::npos - delimiter.size())) ? 
std::string::npos : end + delimiter.size());
+}
+  }
+
+  std::string saAmfNodeSwBundlePathPrefix;
 
   uint32_t inst_retry_max; /* configured no of instantiate retry attempts */
   uint32_t inst_retry_cnt; /* curr no of instantiate retry attempts */
@@ -123,7 +153,7 @@ typedef struct avnd_comp_clc_info {
   uint32_t inst_code_rcvd; /* Store the error value
   received from the instantiate script */
 
-} AVND_COMP_CLC_INFO;
+};
 
 /*##
 COMPONENT CALLBACK DEFINITIONS
@@ -341,7 +371,7 @@ class AVND_COMP {
   bool is_hc_cmd_configured {};
 
   /* clc info */
-  AVND_COMP_CLC_INFO clc_info {};
+  CompClcInfo clc_info {};
 
   /* Update received flag, which will normally be false and will be
* true if updates are received from the AVD on fail-over.*/
@@ -684,33 +714,6 @@ class AVND_COMP {
 void m_AVND_COMP_OPER_STATE_AVD_SYNC(struct avnd_cb_tag *cb,
  const AVND_COMP *comp, uint32_t _rc);
 
-/* macro to parse the clc cmd string */
-#define m_AVND_COMP_CLC_STR_PARSE(st, sc, ac, av, tav)   \
-  {  \
-char str[SAAMF_CLC_LEN], *tok = nullptr; \
-/* copy the str as strtok modifies the original str */   \
-strcpy(str, st); \
-ac = 0;  \
-if (nullptr != (tok = strtok(str, " "))) {   \
-  strncpy(sc, tok, SAAMF_CLC_LEN

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

2017-06-13 Thread Hans Nordeback 
---
 src/amf/amfnd/avnd_comp.h | 134 ++
 src/amf/amfnd/avnd_err.h  |   4 +-
 src/amf/amfnd/avnd_hc.h   |   2 +-
 src/amf/amfnd/avnd_proc.h |  10 ++--
 src/amf/amfnd/avnd_util.h |   2 +-
 src/amf/amfnd/clc.cc  | 125 --
 src/amf/amfnd/comp.cc |  35 
 src/amf/amfnd/compdb.cc   |  36 +++--
 src/amf/amfnd/proxydb.cc  |   2 +-
 9 files changed, 138 insertions(+), 212 deletions(-)

diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h
index 611e90e11..68de4cc8e 100644
--- a/src/amf/amfnd/avnd_comp.h
+++ b/src/amf/amfnd/avnd_comp.h
@@ -31,6 +31,8 @@
 #define AMF_AMFND_AVND_COMP_H_
 
 #include 
+#include 
+#include 
 
 struct avnd_cb_tag;
 struct avnd_su_si_rec;
@@ -57,7 +59,7 @@ struct avnd_srm_req_tag;
 
 /* clc event handler declaration */
 typedef uint32_t (*AVND_COMP_CLC_FSM_FN)(struct avnd_cb_tag *,
- struct avnd_comp_tag *);
+ AVND_COMP *);
 
 /* clc fsm events */
 typedef enum avnd_comp_clc_pres_fsm_ev {
@@ -141,7 +143,7 @@ typedef struct avnd_cbk_tag {
   AVSV_AMF_CBK_INFO *cbk_info; /* callbk info */
 
   /* link to other elements */
-  struct avnd_comp_tag *comp; /* bk ptr to the comp */
+  AVND_COMP *comp; /* bk ptr to the comp */
   struct avnd_cbk_tag *next;
   std::string comp_name; /* For checkpointing */
 } AVND_COMP_CBK;
@@ -182,7 +184,7 @@ typedef struct avnd_comp_csi_rec {
wrt prv ha state */
 
   /* links to other entities */
-  struct avnd_comp_tag *comp; /* bk ptr to the comp */
+  AVND_COMP *comp; /* bk ptr to the comp */
   struct avnd_su_si_rec *si;  /* bk ptr to the si record */
   std::string comp_name;  /* For Checkpointing */
   std::string si_name;/* For Checkpointing */
@@ -256,7 +258,7 @@ typedef struct avnd_hc_rec_tag {
   uint32_t opq_hdl; /* hdl returned by hdl-mngr (used during tmr expiry) */
   AVND_COMP_HC_STATUS status; /* indicates status of hc rec */
 
-  struct avnd_comp_tag *comp; /* back ptr to the comp */
+  AVND_COMP *comp; /* back ptr to the comp */
   struct avnd_hc_rec_tag *next;
   std::string comp_name; /* For checkpoiting */
 } AVND_COMP_HC_REC;
@@ -278,7 +280,7 @@ typedef struct avnd_pm_rec {
   } rec_rcvr;
 
   /* links to other entities */
-  struct avnd_comp_tag *comp; /* back ptr to the comp */
+  AVND_COMP *comp; /* back ptr to the comp */
 } AVND_COMP_PM_REC;
 
 /*##
@@ -288,7 +290,7 @@ typedef struct avnd_pm_rec {
 /* proxied info */
 typedef struct avnd_pxied_rec {
   NCS_DB_LINK_LIST_NODE comp_dll_node; /* node in the comp-pxied dll  */
-  struct avnd_comp_tag *pxied_comp;/* ptr to the proxied comp */
+  AVND_COMP *pxied_comp;/* ptr to the proxied comp */
 } AVND_COMP_PXIED_REC;
 
 #define AVND_COMP_TYPE_LOCAL_NODE 0x0001
@@ -319,100 +321,115 @@ enum UsedComptypeAttrs {
   NumAttrs
 };
 
-typedef struct avnd_comp_tag {
-  NCS_DB_LINK_LIST_NODE su_dll_node; /* su dll node (key is inst-level) */
+class AVND_COMP {
+ public:
+  // TODO(uabhano) replace the NCS_DB_LINK_LIST_NODE with C++ STL. Now 
su_dll_node must be first in AVND_COMP
+  // as the macro m_AVND_COMP_SU_DLL_NODE_OFFSET depends on the offset. 
offsetof is to be avoided in classes.
+  NCS_DB_LINK_LIST_NODE su_dll_node {}; /* su dll node (key is inst-level) */
+  AVND_COMP() {}
+  ~AVND_COMP() {}
 
   std::string name; /* comp name */
   std::string saAmfCompType;
-  uint32_t numOfCompCmdEnv;   /* number of comp command environment variables 
*/
-  SaStringT *saAmfCompCmdEnv; /* comp command environment variables */
-  uint32_t inst_level;/* comp instantiation level */
 
-  uint32_t comp_hdl; /* hdl returned by hdl-mngr */
+  uint32_t inst_level {};/* comp instantiation level */
+
+  uint32_t comp_hdl {}; /* hdl returned by hdl-mngr */
 
   /* component attributes */
-  uint32_t flag;  /* comp attributes */
-  bool is_restart_en; /* flag to indicate if comp-restart is allowed */
-  SaAmfCompCapabilityModelT cap; /* comp capability model */
-  bool is_am_en;
-  bool is_hc_cmd_configured;
+  uint32_t flag {};  /* comp attributes */
+  bool is_restart_en {}; /* flag to indicate if comp-restart is allowed */
+  SaAmfCompCapabilityModelT cap {}; /* comp capability model */
+  bool is_am_en {};
+  bool is_hc_cmd_configured {};
 
   /* clc info */
-  AVND_COMP_CLC_INFO clc_info;
+  AVND_COMP_CLC_INFO clc_info {};
 
   /* Update received flag, which will normally be false and will be
* true if updates are received from the AVD on fail-over.*/
-  bool avd_updt_flag;
+  bool avd_updt_flag {};
 
   /* component registration info */
-  SaAmfHandleT reg_hdl; /* registered handle value */
-  MDS_DEST reg_dest;/* mds dest of the registering prc */
+  SaAmfHandleT reg_hdl

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

2017-06-13 Thread Hans Nordebäck 

Hi Praveen,

good catch, I'll update. /Thanks HansN


On 06/13/2017 08:53 AM, praveen malviya wrote:

Hi Hans,

One comment on this patch inline with [Praveen].

Thanks,
Praveen
On 18-May-17 3:32 PM, Hans Nordeback wrote:

---
  src/amf/amfnd/avnd_comp.h | 134 
++

  src/amf/amfnd/avnd_err.h  |   4 +-
  src/amf/amfnd/avnd_hc.h   |   2 +-
  src/amf/amfnd/avnd_proc.h |  10 ++--
  src/amf/amfnd/avnd_util.h |   2 +-
  src/amf/amfnd/clc.cc  | 125 
--

  src/amf/amfnd/comp.cc |  35 
  src/amf/amfnd/compdb.cc   |  36 +++--
  src/amf/amfnd/proxydb.cc  |   2 +-
  9 files changed, 138 insertions(+), 212 deletions(-)

diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h
index 611e90e11..68de4cc8e 100644
--- a/src/amf/amfnd/avnd_comp.h
+++ b/src/amf/amfnd/avnd_comp.h
@@ -31,6 +31,8 @@
  #define AMF_AMFND_AVND_COMP_H_
#include 
+#include 
+#include 
struct avnd_cb_tag;
  struct avnd_su_si_rec;
@@ -57,7 +59,7 @@ struct avnd_srm_req_tag;
/* clc event handler declaration */
  typedef uint32_t (*AVND_COMP_CLC_FSM_FN)(struct avnd_cb_tag *,
- struct avnd_comp_tag *);
+ AVND_COMP *);
/* clc fsm events */
  typedef enum avnd_comp_clc_pres_fsm_ev {
@@ -141,7 +143,7 @@ typedef struct avnd_cbk_tag {
AVSV_AMF_CBK_INFO *cbk_info; /* callbk info */
  /* link to other elements */
-  struct avnd_comp_tag *comp; /* bk ptr to the comp */
+  AVND_COMP *comp; /* bk ptr to the comp */
struct avnd_cbk_tag *next;
std::string comp_name; /* For checkpointing */
  } AVND_COMP_CBK;
@@ -182,7 +184,7 @@ typedef struct avnd_comp_csi_rec {
 wrt prv ha state */
  /* links to other entities */
-  struct avnd_comp_tag *comp; /* bk ptr to the comp */
+  AVND_COMP *comp; /* bk ptr to the comp */
struct avnd_su_si_rec *si;  /* bk ptr to the si 
record */

std::string comp_name;  /* For Checkpointing */
std::string si_name;/* For Checkpointing */
@@ -256,7 +258,7 @@ typedef struct avnd_hc_rec_tag {
uint32_t opq_hdl; /* hdl returned by hdl-mngr (used during tmr 
expiry) */

AVND_COMP_HC_STATUS status; /* indicates status of hc rec */
  -  struct avnd_comp_tag *comp; /* back ptr to the comp */
+  AVND_COMP *comp; /* back ptr to the comp */
struct avnd_hc_rec_tag *next;
std::string comp_name; /* For checkpoiting */
  } AVND_COMP_HC_REC;
@@ -278,7 +280,7 @@ typedef struct avnd_pm_rec {
} rec_rcvr;
  /* links to other entities */
-  struct avnd_comp_tag *comp; /* back ptr to the comp */
+  AVND_COMP *comp; /* back ptr to the comp */
  } AVND_COMP_PM_REC;
/*##
@@ -288,7 +290,7 @@ typedef struct avnd_pm_rec {
  /* proxied info */
  typedef struct avnd_pxied_rec {
NCS_DB_LINK_LIST_NODE comp_dll_node; /* node in the comp-pxied 
dll  */

-  struct avnd_comp_tag *pxied_comp;/* ptr to the proxied comp */
+  AVND_COMP *pxied_comp;/* ptr to the proxied comp */
  } AVND_COMP_PXIED_REC;
#define AVND_COMP_TYPE_LOCAL_NODE 0x0001
@@ -319,100 +321,115 @@ enum UsedComptypeAttrs {
NumAttrs
  };
  -typedef struct avnd_comp_tag {
-  NCS_DB_LINK_LIST_NODE su_dll_node; /* su dll node (key is 
inst-level) */

+class AVND_COMP {
+ public:
+  // TODO(uabhano) replace the NCS_DB_LINK_LIST_NODE with C++ STL. 
Now su_dll_node must be first in AVND_COMP
+  // as the macro m_AVND_COMP_SU_DLL_NODE_OFFSET depends on the 
offset. offsetof is to be avoided in classes.
+  NCS_DB_LINK_LIST_NODE su_dll_node {}; /* su dll node (key is 
inst-level) */

+  AVND_COMP() {}
+  ~AVND_COMP() {}
  std::string name; /* comp name */
std::string saAmfCompType;
-  uint32_t numOfCompCmdEnv;   /* number of comp command environment 
variables */

-  SaStringT *saAmfCompCmdEnv; /* comp command environment variables */
-  uint32_t inst_level;/* comp instantiation level */
  -  uint32_t comp_hdl; /* hdl returned by hdl-mngr */
+  uint32_t inst_level {};/* comp instantiation level */
+
+  uint32_t comp_hdl {}; /* hdl returned by hdl-mngr */
  /* component attributes */
-  uint32_t flag;  /* comp attributes */
-  bool is_restart_en; /* flag to indicate if comp-restart is allowed */
-  SaAmfCompCapabilityModelT cap; /* comp capability model */
-  bool is_am_en;
-  bool is_hc_cmd_configured;
+  uint32_t flag {};  /* comp attributes */
+  bool is_restart_en {}; /* flag to indicate if comp-restart is 
allowed */

+  SaAmfCompCapabilityModelT cap {}; /* comp capability model */
+  bool is_am_en {};
+  bool is_hc_cmd_configured {};
  /* clc info */
-  AVND_COMP_CLC_INFO clc_info;
+  AVND_COMP_CLC_INFO clc_info {};
  /* Update received flag, which will normally be false and will be
 *

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

2017-06-13 Thread Hans Nordebäck 

Hi Praveen,

The intention with V2 was to correct the case when a space is needed in 
the argument, but as you say it is not backward


compatible. V1 of this patch is backward compatible, I'll put it back 
and send it out for a new review.


/Thanks HansN

On 06/13/2017 08:53 AM, praveen malviya wrote:

Hi Hans,

One comment on this patch inline with [Praveen].

Thanks,
Praveen

On 18-May-17 3:32 PM, Hans Nordeback wrote:

---
  src/amf/amfnd/avnd_comp.h |  71 ++---
  src/amf/amfnd/avnd_tmr.h  |   6 +--
  src/amf/amfnd/cam.cc  |   2 +-
  src/amf/amfnd/chc.cc  |   2 +-
  src/amf/amfnd/clc.cc  |  34 --
  src/amf/amfnd/comp.cc |  45 ++
  src/amf/amfnd/compdb.cc   | 113 
+++---

  src/amf/amfnd/susm.cc |   2 +-
  8 files changed, 140 insertions(+), 135 deletions(-)

diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h
index a2fc22691..52bf84e47 100644
--- a/src/amf/amfnd/avnd_comp.h
+++ b/src/amf/amfnd/avnd_comp.h
@@ -33,11 +33,14 @@
  #include 
  #include 
  #include 
+#include 
+#include "avnd_tmr.h"
struct avnd_cb_tag;
  struct avnd_su_si_rec;
  class AVND_SU;
  struct avnd_srm_req_tag;
+class AVND_COMP;
/***
   **  S T R U C T U R E / E N U M  D E F I N I T I O N S  
***

@@ -90,16 +93,37 @@ typedef enum avnd_comp_clc_cmd_type {
  } AVND_COMP_CLC_CMD_TYPE;
/* clc command parameter definition */
-typedef struct avnd_comp_clc_param {
-  char cmd[SAAMF_CLC_LEN]; /* cmd ascii string */
-  SaTimeT timeout; /* cmd timeout value */
-  uint32_t len;/* cmd len */
-} AVND_COMP_CLC_CMD_PARAM;
+struct CompClcCmdParam {
+  void init_clc_cli_command(const char *clc_cmd, char **clc_cmd_argv,
+const SaImmAttrValuesT_2 **attributes,
+const char *attr_name);
+  std::string cmd;
+  std::vector cmd_argv;  /* cmd argv */
+  SaTimeT timeout;/* cmd timeout value */
+};
/* clc info definition (top level wrapper structure) */
-typedef struct avnd_comp_clc_info {
-  /* clc commands (indexed by cmd type) */
-  AVND_COMP_CLC_CMD_PARAM cmds[AVND_COMP_CLC_CMD_TYPE_MAX - 1];
+struct CompClcInfo {
+  std::map cmds;
+  std::string get_cmd(AVND_COMP_CLC_CMD_TYPE cmd_type);
+
+  template
+  void create_argv(std::array , uint32_t , 
AVND_COMP_CLC_CMD_TYPE cmd_type) {

+argc = 0;
+
+std::string tmp = saAmfNodeSwBundlePathPrefix + cmds[cmd_type].cmd;
+argv[argc++] = strdup(tmp.data());
+
+for (auto str : cmds[cmd_type].cmd_argv) {
+   if (argc >= argv.size()) {
+LOG_WA("Too many arguments given, max %zu arguments are 
supported", argv.size());

+break;
+  }
+  argv[argc++] = strdup(str.data());
+}
+  }
+
+  std::string saAmfNodeSwBundlePathPrefix;
  uint32_t inst_retry_max; /* configured no of instantiate retry 
attempts */

uint32_t inst_retry_cnt; /* curr no of instantiate retry attempts */
@@ -123,7 +147,7 @@ typedef struct avnd_comp_clc_info {
uint32_t inst_code_rcvd; /* Store the error value
received from the instantiate script */
  -} AVND_COMP_CLC_INFO;
+};
/*##
  COMPONENT CALLBACK DEFINITIONS
@@ -341,7 +365,7 @@ class AVND_COMP {
bool is_hc_cmd_configured {};
  /* clc info */
-  AVND_COMP_CLC_INFO clc_info {};
+  CompClcInfo clc_info {};
  /* Update received flag, which will normally be false and will be
 * true if updates are received from the AVD on fail-over.*/
@@ -684,33 +708,6 @@ class AVND_COMP {
  void m_AVND_COMP_OPER_STATE_AVD_SYNC(struct avnd_cb_tag *cb,
   const AVND_COMP *comp, 
uint32_t _rc);

  -/* macro to parse the clc cmd string */
-#define m_AVND_COMP_CLC_STR_PARSE(st, sc, ac, av, tav)   \
-  {  \
-char str[SAAMF_CLC_LEN], *tok = nullptr; \
-/* copy the str as strtok modifies the original str */   \
-strcpy(str, st); \
-ac = 0;  \
-if (nullptr != (tok = strtok(str, " "))) {   \
-  strncpy(sc, tok, SAAMF_CLC_LEN - 1);   \
-  av[ac] = sc;   \
-}\
-ac++;\
-while ((nullptr != (tok = strtok(nullptr, " "))) &&  \
-   (ac < (AVND_COMP_CLC_PARAM_MAX + 1))) { \
-  if (strlen(tok) > AVND_COMP_CLC_PARAM_SIZE_MAX) break; \
-  strcpy(tav[ac], tok);  \
-  av[ac] = tav[ac];

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

2017-06-13 Thread praveen malviya 

Hi Hans,

One comment on this patch inline with [Praveen].

Thanks,
Praveen

On 18-May-17 3:32 PM, Hans Nordeback wrote:

---
  src/amf/amfnd/avnd_comp.h |  71 ++---
  src/amf/amfnd/avnd_tmr.h  |   6 +--
  src/amf/amfnd/cam.cc  |   2 +-
  src/amf/amfnd/chc.cc  |   2 +-
  src/amf/amfnd/clc.cc  |  34 --
  src/amf/amfnd/comp.cc |  45 ++
  src/amf/amfnd/compdb.cc   | 113 +++---
  src/amf/amfnd/susm.cc |   2 +-
  8 files changed, 140 insertions(+), 135 deletions(-)

diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h
index a2fc22691..52bf84e47 100644
--- a/src/amf/amfnd/avnd_comp.h
+++ b/src/amf/amfnd/avnd_comp.h
@@ -33,11 +33,14 @@
  #include 
  #include 
  #include 
+#include 
+#include "avnd_tmr.h"
  
  struct avnd_cb_tag;

  struct avnd_su_si_rec;
  class AVND_SU;
  struct avnd_srm_req_tag;
+class AVND_COMP;
  
  /***

   **  S T R U C T U R E / E N U M  D E F I N I T I O N S  ***
@@ -90,16 +93,37 @@ typedef enum avnd_comp_clc_cmd_type {
  } AVND_COMP_CLC_CMD_TYPE;
  
  /* clc command parameter definition */

-typedef struct avnd_comp_clc_param {
-  char cmd[SAAMF_CLC_LEN]; /* cmd ascii string */
-  SaTimeT timeout; /* cmd timeout value */
-  uint32_t len;/* cmd len */
-} AVND_COMP_CLC_CMD_PARAM;
+struct CompClcCmdParam {
+  void init_clc_cli_command(const char *clc_cmd, char **clc_cmd_argv,
+const SaImmAttrValuesT_2 **attributes,
+const char *attr_name);
+  std::string cmd;
+  std::vector cmd_argv;  /* cmd argv */
+  SaTimeT timeout;/* cmd timeout value */
+};
  
  /* clc info definition (top level wrapper structure) */

-typedef struct avnd_comp_clc_info {
-  /* clc commands (indexed by cmd type) */
-  AVND_COMP_CLC_CMD_PARAM cmds[AVND_COMP_CLC_CMD_TYPE_MAX - 1];
+struct CompClcInfo {
+  std::map cmds;
+  std::string get_cmd(AVND_COMP_CLC_CMD_TYPE cmd_type);
+
+  template
+  void create_argv(std::array , uint32_t , 
AVND_COMP_CLC_CMD_TYPE cmd_type) {
+argc = 0;
+
+std::string tmp = saAmfNodeSwBundlePathPrefix + cmds[cmd_type].cmd;
+argv[argc++] = strdup(tmp.data());
+
+for (auto str : cmds[cmd_type].cmd_argv) {
+   if (argc >= argv.size()) {
+LOG_WA("Too many arguments given, max %zu arguments are supported", 
argv.size());
+break;
+  }
+  argv[argc++] = strdup(str.data());
+}
+  }
+
+  std::string saAmfNodeSwBundlePathPrefix;
  
uint32_t inst_retry_max; /* configured no of instantiate retry attempts */

uint32_t inst_retry_cnt; /* curr no of instantiate retry attempts */
@@ -123,7 +147,7 @@ typedef struct avnd_comp_clc_info {
uint32_t inst_code_rcvd; /* Store the error value
received from the instantiate script */
  
-} AVND_COMP_CLC_INFO;

+};
  
  /*##

  COMPONENT CALLBACK DEFINITIONS
@@ -341,7 +365,7 @@ class AVND_COMP {
bool is_hc_cmd_configured {};
  
/* clc info */

-  AVND_COMP_CLC_INFO clc_info {};
+  CompClcInfo clc_info {};
  
/* Update received flag, which will normally be false and will be

 * true if updates are received from the AVD on fail-over.*/
@@ -684,33 +708,6 @@ class AVND_COMP {
  void m_AVND_COMP_OPER_STATE_AVD_SYNC(struct avnd_cb_tag *cb,
   const AVND_COMP *comp, uint32_t _rc);
  
-/* macro to parse the clc cmd string */

-#define m_AVND_COMP_CLC_STR_PARSE(st, sc, ac, av, tav)   \
-  {  \
-char str[SAAMF_CLC_LEN], *tok = nullptr; \
-/* copy the str as strtok modifies the original str */   \
-strcpy(str, st); \
-ac = 0;  \
-if (nullptr != (tok = strtok(str, " "))) {   \
-  strncpy(sc, tok, SAAMF_CLC_LEN - 1);   \
-  av[ac] = sc;   \
-}\
-ac++;\
-while ((nullptr != (tok = strtok(nullptr, " "))) &&  \
-   (ac < (AVND_COMP_CLC_PARAM_MAX + 1))) {   \
-  if (strlen(tok) > AVND_COMP_CLC_PARAM_SIZE_MAX) break; \
-  strcpy(tav[ac], tok);  \
-  av[ac] = tav[ac];  \
-  ac++;  \
-}\
-if (nullptr != tok) {\
-  sc[0] = (char)(long)nullptr;   \
-  av[0] =

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

2017-06-13 Thread praveen malviya 

Hi Hans,

One comment on this patch inline with [Praveen].

Thanks,
Praveen
On 18-May-17 3:32 PM, Hans Nordeback wrote:

---
  src/amf/amfnd/avnd_comp.h | 134 ++
  src/amf/amfnd/avnd_err.h  |   4 +-
  src/amf/amfnd/avnd_hc.h   |   2 +-
  src/amf/amfnd/avnd_proc.h |  10 ++--
  src/amf/amfnd/avnd_util.h |   2 +-
  src/amf/amfnd/clc.cc  | 125 --
  src/amf/amfnd/comp.cc |  35 
  src/amf/amfnd/compdb.cc   |  36 +++--
  src/amf/amfnd/proxydb.cc  |   2 +-
  9 files changed, 138 insertions(+), 212 deletions(-)

diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h
index 611e90e11..68de4cc8e 100644
--- a/src/amf/amfnd/avnd_comp.h
+++ b/src/amf/amfnd/avnd_comp.h
@@ -31,6 +31,8 @@
  #define AMF_AMFND_AVND_COMP_H_
  
  #include 

+#include 
+#include 
  
  struct avnd_cb_tag;

  struct avnd_su_si_rec;
@@ -57,7 +59,7 @@ struct avnd_srm_req_tag;
  
  /* clc event handler declaration */

  typedef uint32_t (*AVND_COMP_CLC_FSM_FN)(struct avnd_cb_tag *,
- struct avnd_comp_tag *);
+ AVND_COMP *);
  
  /* clc fsm events */

  typedef enum avnd_comp_clc_pres_fsm_ev {
@@ -141,7 +143,7 @@ typedef struct avnd_cbk_tag {
AVSV_AMF_CBK_INFO *cbk_info; /* callbk info */
  
/* link to other elements */

-  struct avnd_comp_tag *comp; /* bk ptr to the comp */
+  AVND_COMP *comp; /* bk ptr to the comp */
struct avnd_cbk_tag *next;
std::string comp_name; /* For checkpointing */
  } AVND_COMP_CBK;
@@ -182,7 +184,7 @@ typedef struct avnd_comp_csi_rec {
 wrt prv ha state */
  
/* links to other entities */

-  struct avnd_comp_tag *comp; /* bk ptr to the comp */
+  AVND_COMP *comp; /* bk ptr to the comp */
struct avnd_su_si_rec *si;  /* bk ptr to the si record */
std::string comp_name;  /* For Checkpointing */
std::string si_name;/* For Checkpointing */
@@ -256,7 +258,7 @@ typedef struct avnd_hc_rec_tag {
uint32_t opq_hdl; /* hdl returned by hdl-mngr (used during tmr expiry) */
AVND_COMP_HC_STATUS status; /* indicates status of hc rec */
  
-  struct avnd_comp_tag *comp; /* back ptr to the comp */

+  AVND_COMP *comp; /* back ptr to the comp */
struct avnd_hc_rec_tag *next;
std::string comp_name; /* For checkpoiting */
  } AVND_COMP_HC_REC;
@@ -278,7 +280,7 @@ typedef struct avnd_pm_rec {
} rec_rcvr;
  
/* links to other entities */

-  struct avnd_comp_tag *comp; /* back ptr to the comp */
+  AVND_COMP *comp; /* back ptr to the comp */
  } AVND_COMP_PM_REC;
  
  /*##

@@ -288,7 +290,7 @@ typedef struct avnd_pm_rec {
  /* proxied info */
  typedef struct avnd_pxied_rec {
NCS_DB_LINK_LIST_NODE comp_dll_node; /* node in the comp-pxied dll  */
-  struct avnd_comp_tag *pxied_comp;/* ptr to the proxied comp */
+  AVND_COMP *pxied_comp;/* ptr to the proxied comp */
  } AVND_COMP_PXIED_REC;
  
  #define AVND_COMP_TYPE_LOCAL_NODE 0x0001

@@ -319,100 +321,115 @@ enum UsedComptypeAttrs {
NumAttrs
  };
  
-typedef struct avnd_comp_tag {

-  NCS_DB_LINK_LIST_NODE su_dll_node; /* su dll node (key is inst-level) */
+class AVND_COMP {
+ public:
+  // TODO(uabhano) replace the NCS_DB_LINK_LIST_NODE with C++ STL. Now 
su_dll_node must be first in AVND_COMP
+  // as the macro m_AVND_COMP_SU_DLL_NODE_OFFSET depends on the offset. 
offsetof is to be avoided in classes.
+  NCS_DB_LINK_LIST_NODE su_dll_node {}; /* su dll node (key is inst-level) */
+  AVND_COMP() {}
+  ~AVND_COMP() {}
  
std::string name; /* comp name */

std::string saAmfCompType;
-  uint32_t numOfCompCmdEnv;   /* number of comp command environment variables 
*/
-  SaStringT *saAmfCompCmdEnv; /* comp command environment variables */
-  uint32_t inst_level;/* comp instantiation level */
  
-  uint32_t comp_hdl; /* hdl returned by hdl-mngr */

+  uint32_t inst_level {};/* comp instantiation level */
+
+  uint32_t comp_hdl {}; /* hdl returned by hdl-mngr */
  
/* component attributes */

-  uint32_t flag;  /* comp attributes */
-  bool is_restart_en; /* flag to indicate if comp-restart is allowed */
-  SaAmfCompCapabilityModelT cap; /* comp capability model */
-  bool is_am_en;
-  bool is_hc_cmd_configured;
+  uint32_t flag {};  /* comp attributes */
+  bool is_restart_en {}; /* flag to indicate if comp-restart is allowed */
+  SaAmfCompCapabilityModelT cap {}; /* comp capability model */
+  bool is_am_en {};
+  bool is_hc_cmd_configured {};
  
/* clc info */

-  AVND_COMP_CLC_INFO clc_info;
+  AVND_COMP_CLC_INFO clc_info {};
  
/* Update received flag, which will normally be false and will be

 * true if updates are received from the AVD on fail-over.*/
-  bool avd_updt_flag;
+

Re: [devel] [PATCH 1/1] amf: Replace osafassert by log_warning to avoid cyclic reboot [#2477]

2017-06-13 Thread Nagendra Kumar 
Hi Minh,

Sorry, my email was a bit confusing.

>> I still don't get how these osafassert(s) are helpful as a part of SG logic.

In certain situation, it may crash somewhere else. That is the reason, the 
logic of checking weird SUSIs should be kept intact along with assert.

 

Even we are not using avd_sg_validate_headless_cached_rta(), we can use it for 
temporary place holder.

In the function, avd_process_state_info_queue(), when 
avd_sg_validate_headless_cached_rta() is called, we have SU1 and SU4 having 
SUSI as Act.

So, two SUs in the same SG has Act assignments. This check should suffice.

So, we can invalidate it here.

 

Thanks

-Nagu

 

From: minh chau [mailto:minh.c...@dektech.com.au] 
Sent: 13 June 2017 09:53
To: Nagendra Kumar
Cc: hans.nordeb...@ericsson.com; Praveen Malviya; gary@dektech.com.au; 
opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] amf: Replace osafassert by log_warning to avoid cyclic 
reboot [#2477]

 

Hi Nagu,

I seem to remember now, the function avd_sg_validate_headless_cached_rta() was 
written in essence it is just debugging only, invalidating sg is not a 
solution. There is a function header comment

/**
 * @brief  Validate all cached RTAs read from IMM after headless.
   This validation is necessary. If AMFD doesn't have this
   validation routine and the cached RTAs are invalid,
   that would lead into *unpredictably* wrong states, which
   is hard to debug (harder if no trace)
 * @param  Control block (AVD_CL_CB).
 * @Return true if valid, false otherwise.
*/
bool avd_sg_validate_headless_cached_rta(AVD_CL_CB *cb) {

I think we are talking about "2 SUSI", more precisely it is problem of 2 
different SU(s) being assigned ACTIVE

In the log, SU4 has 3 active SUSIs from 3 different SI

Jun  2 16:05:14.082046 osafamfd [285:285:src/amf/amfd/siass.cc:0440] >> 
avd_susi_create: safSu=SU4,safSg=AmfDemoTwon,safApp=AmfDemoTwon 
safSi=AmfDemoTwonDep2,safApp=AmfDemoTwon state=1
Jun  2 16:05:14.082080 osafamfd [285:285:src/amf/amfd/siass.cc:0440] >> 
avd_susi_create: safSu=SU4,safSg=AmfDemoTwon,safApp=AmfDemoTwon 
safSi=AmfDemoTwonDep1,safApp=AmfDemoTwon state=1
Jun  2 16:05:14.082108 osafamfd [285:285:src/amf/amfd/siass.cc:0440] >> 
avd_susi_create: safSu=SU4,safSg=AmfDemoTwon,safApp=AmfDemoTwon 
safSi=AmfDemoTwon,safApp=AmfDemoTwon state=1

As you pointed that SU1 also has 3 active SUSIs, that we read from IMM as a 
helper to perform node_fail(). In this case, 3 SUSIs of SU1 must be removed and 
amfd continues to create 3 standby SUSIs for another SU. When amfd completes to 
remove 3 SUSIs of SU1, amfd will be able to create another 3 standby SUSIs as 
of existing logic of SG. 

The osafassert(s) are there just to prevent amfd to remove SUSIs of SU1 and 
continue to create standby SUSIs, it even causes cyclic reboot.
I still don't get how these osafassert(s) are helpful as a part of SG logic. 
The patch actually does not change any core logic of sg, it removes the assert 
to avoid reboot, and make sure 
avd_sg_2n_act_susi() always returns active/standby if found (do not return 2 
active SU). The sg logic is still there where sg fsm calls avd_sg_2n_act_susi() 
and handling the result of this function.

Thanks,
Minh



On 12/06/17 17:11, Nagendra Kumar wrote:

Hi Minh,
I could see in your trace that 3 SUSI with Act assignments(state'1' 
means Act) exists:
 
Jun  2 16:05:26.441156 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:3379] TEST 
>> node_fail: 'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon', 1
Jun  2 16:05:26.441160 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4095] >> 
avd_su_state_determine: SU 'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon'
Jun  2 16:05:26.441163 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4120] TR 
Assigned su'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon', 
si'safSi=AmfDemoTwon,safApp=AmfDemoTwon', state'1'
Jun  2 16:05:26.441167 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4120] TR 
Assigned su'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon', 
si'safSi=AmfDemoTwonDep1,safApp=AmfDemoTwon', state'1'
Jun  2 16:05:26.441170 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4120] TR 
Assigned su'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon', 
si'safSi=AmfDemoTwonDep2,safApp=AmfDemoTwon', state'1'
 
So, my suggestion is use this information and invalidate the SG as it has more 
than one SUSI Act.
 
 
Thanks
-Nagu
 

-Original Message-
From: minh chau [mailto:minh.c...@dektech.com.au]
Sent: 09 June 2017 16:58
To: Nagendra Kumar
Cc: HYPERLINK "mailto:hans.nordeb...@ericsson.com"hans.nordeb...@ericsson.com; 
Praveen Malviya;
HYPERLINK "mailto:gary@dektech.com.au"gary@dektech.com.au; HYPERLINK 
"mailto:opensaf-devel@lists.sourceforge.net"opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] amf: Replace osafassert by log_warning to avoid
cyclic reboot [#2477]
 
Hi Nagu,
 
We may have to create 2 active susi(s) so that we can know the sg invalid. A
question is when