In ntfimcn the OM handle shall have a short lifespan. Change from creating a
handle once when ntfimcn process starts to create a handle each time it is
needed and finalize when no longer needed.

Change start handling of ntfimcn (in ntf process) so the ntfimcn process is
started on the active node only since the ntfimcn process is not doing
anything on the standby node. Refactor/simplify code accordingly.
---
 src/ntf/ntfd/ntfs_amf.c         |   8 +--
 src/ntf/ntfd/ntfs_imcnutil.c    | 105 +++++++++++++++++++++--------------
 src/ntf/ntfimcnd/ntfimcn_imm.c  | 119 ++++++++++++++++++++++++----------------
 src/ntf/ntfimcnd/ntfimcn_imm.h  |  20 ++++++-
 src/ntf/ntfimcnd/ntfimcn_main.h |   1 -
 5 files changed, 158 insertions(+), 95 deletions(-)

diff --git a/src/ntf/ntfd/ntfs_amf.c b/src/ntf/ntfd/ntfs_amf.c
index cda35d6f9..e9c521d4a 100644
--- a/src/ntf/ntfd/ntfs_amf.c
+++ b/src/ntf/ntfd/ntfs_amf.c
@@ -263,12 +263,10 @@ response:
                checkNotificationList();
        }
 done:
-       /* Kills the osafntfimcnd process if current state is Active or Standby
-        * and the process is not already running in this state. The process
-        * will be restarted by the process surveillance thread. This function
-        * will not return until the process is terminated.
+       /* Start the osafntfimcn process if becoming active or stop the process
+        * if leaving active state
         */
-       handle_state_ntfimcn(ntfs_cb->ha_state);
+       handle_state_ntfimcn(new_haState);
 
        TRACE_LEAVE();
 }
diff --git a/src/ntf/ntfd/ntfs_imcnutil.c b/src/ntf/ntfd/ntfs_imcnutil.c
index dd27a255c..3add5db5a 100644
--- a/src/ntf/ntfd/ntfs_imcnutil.c
+++ b/src/ntf/ntfd/ntfs_imcnutil.c
@@ -41,13 +41,13 @@
 #include "osaf/configmake.h"
 
 typedef struct {
-       SaAmfHAStateT ha_state;
        pid_t pid;
        pthread_t thread;
+       bool ntfimcn_on;
 } init_params_t;
 
-static init_params_t ipar;
-pthread_mutex_t ntfimcn_mutex;
+static init_params_t ipar = {0, 0, false};
+pthread_mutex_t ntfimcn_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 /**
  * Kill the osafntfimcn child process using previously saved Pid
@@ -190,6 +190,14 @@ done:
        TRACE_LEAVE();
 }
 
+/**
+ * Create a ntfimcnd process
+ * Note: The process can run on both active and standby node. It must be given
+ *       a start argument with this information.
+ *
+ * @param ha_state[in] Current HA state
+ * @return PID for the created process
+ */
 static pid_t create_imcnprocess(SaAmfHAStateT ha_state)
 {
        char *start_args[3];
@@ -226,6 +234,11 @@ static pid_t create_imcnprocess(SaAmfHAStateT ha_state)
  * Start the imcn process and wait for process to exit.
  * If the process exit then restart it.
  *
+ * TODO: (Lennart #2506) The ntfimcn process is no longer started on the
+ *       standby node. Handling of HA state in the ntfimcn process can 
therefore
+ *       be removed. This also means that giving a start argument would no
+ *       longer be needed.
+ *
  * @param _init_params[in]
  * @return
  */
@@ -240,13 +253,18 @@ static void *cnsurvail_thread(void *_init_params)
 
        while (1) {
                osaf_mutex_lock_ordie(&ntfimcn_mutex);
-               pid = create_imcnprocess(ipar->ha_state);
+               /* Note: Always give HA state SA_AMF_HA_ACTIVE to the imcn
+                * process since ntfimcn is no longer started on the standby
+                * node */
+               pid = create_imcnprocess(SA_AMF_HA_ACTIVE);
                ipar->pid = pid;
                osaf_mutex_unlock_ordie(&ntfimcn_mutex);
 
                /* Wait for child process to exit */
                do {
                        rc = waitpid(ipar->pid, &status, 0);
+                       TRACE("%s: waitpid() released. rc = %d",
+                               __FUNCTION__, rc);
                } while ((rc == -1) && (errno == EINTR));
 
                if ((rc == -1) && (errno == ECHILD)) {
@@ -267,29 +285,26 @@ static void *cnsurvail_thread(void *_init_params)
                TRACE("osafntfimcnd process terminated reason %s (%d)",
                      exit_rc ? "exit" : "other", exit_stat);
        }
+       TRACE_LEAVE();
 }
 
 /**
- * Start the imcn process surveillance thread
- *
- * @param ha_state[in]
+ * Start the imcn process surveillance thread.
+ * The surveillance thread starts the imcn process
+ * Note: This shall be done only if we are active
  */
-static void start_cnprocess(SaAmfHAStateT ha_state)
+static void start_ntfimcn(void)
 {
        int rc;
 
        TRACE_ENTER();
 
-       rc = pthread_mutex_init(&ntfimcn_mutex, NULL);
+       rc = pthread_create(
+               &ipar.thread, NULL, cnsurvail_thread, (void *)&ipar);
        if (rc != 0)
                osaf_abort(rc);
 
-       ipar.ha_state = ha_state;
-
-       rc =
-           pthread_create(&ipar.thread, NULL, cnsurvail_thread, (void *)&ipar);
-       if (rc != 0)
-               osaf_abort(rc);
+       ipar.ntfimcn_on = true;
 
        TRACE_LEAVE();
 }
@@ -297,34 +312,45 @@ static void start_cnprocess(SaAmfHAStateT ha_state)
 /**
  * Initialize the configuration change notifier
  *
+ * Start ntfimcn if we are active
+ *
  * @param ha_state[in]
  */
-void init_ntfimcn(SaAmfHAStateT ha_state) { start_cnprocess(ha_state); }
+void init_ntfimcn(SaAmfHAStateT ha_state)
+{
+       TRACE_ENTER();
+       if (ha_state == SA_AMF_HA_ACTIVE) {
+               start_ntfimcn();
+       }
+       TRACE_LEAVE();
+}
 
 /**
- * Kill the imcn process. Use the pid saved when the process was started
- * This will cause the imcn process to be restarted in the given state.
- * This is done only if the current state is Active or Standby and if
- * the state has changed
+ * Start or stop ntfimcn when changing HA state
  *
- * @param ha_state[in]
+ * The ntfimcn process and the surveillance thread (see cnsurvail_thread())
+ * is stopped or started based on given HA state we are changing to
+ * ntfimcn shall be started if we are becoming active else it shall be stopped
+ * Note1: The ntfimcn process is started by the surveillance thread
+ * Note2: When stopping ntfimcn also the surveillance thread has to be
+ * terminated or else it will start the ntfimcn process again.
+ *
+ * @param ha_state[in] The HA state we are changing to
  */
-void handle_state_ntfimcn(SaAmfHAStateT ha_state)
+void handle_state_ntfimcn(SaAmfHAStateT to_ha_state)
 {
-       TRACE_ENTER();
-       if ((ha_state == SA_AMF_HA_ACTIVE) || (ha_state == SA_AMF_HA_STANDBY)) {
-               osaf_mutex_lock_ordie(&ntfimcn_mutex);
-               if (ha_state != ipar.ha_state) {
-                       ipar.ha_state = ha_state;
-                       TRACE("%s: Terminating osafntfimcnd process",
-                             __FUNCTION__);
-                       timedwait_imcn_exit();
-               } else {
-                       TRACE("%s: osafntfimcnd process not restarted. "
-                             "Already in correct state",
-                             __FUNCTION__);
-               }
-               osaf_mutex_unlock_ordie(&ntfimcn_mutex);
+       TRACE_ENTER2("to_ha_state = %d", to_ha_state);
+       /* Note: ntfimcnd process still needs HA state even if it only is
+        * started on the active node. See TODO in cnsurvail_thread() */
+       if ((to_ha_state == SA_AMF_HA_ACTIVE) &&
+               (ipar.ntfimcn_on == false)) {
+               /* We are becoming active and ntfimcn is not started.
+                * Start ntfimcn */
+               start_ntfimcn();
+       } else if ((ipar.ntfimcn_on == true) &&
+               (to_ha_state != SA_AMF_HA_ACTIVE)) {
+               /* Always stop ntfimcn if we are leaving active state */
+               stop_ntfimcn();
        }
        TRACE_LEAVE();
 }
@@ -342,7 +368,7 @@ int stop_ntfimcn(void)
        int rc = 0;
        TRACE_ENTER();
 
-       if (ipar.ha_state != 0) {
+       if (ipar.ntfimcn_on == true) {
                TRACE("%s: Cancel the imcn surveillance thread", __FUNCTION__);
                rc = pthread_cancel(ipar.thread);
                if (rc != 0)
@@ -350,12 +376,11 @@ int stop_ntfimcn(void)
                rc = pthread_join(ipar.thread, &join_ret);
                if (rc != 0)
                        osaf_abort(rc);
-               rc = pthread_mutex_destroy(&ntfimcn_mutex);
-               if (rc != 0)
-                       osaf_abort(rc);
 
                TRACE("%s: Terminating osafntfimcnd process", __FUNCTION__);
                timedwait_imcn_exit();
+
+               ipar.ntfimcn_on = false;
        }
 
        TRACE_LEAVE();
diff --git a/src/ntf/ntfimcnd/ntfimcn_imm.c b/src/ntf/ntfimcnd/ntfimcn_imm.c
index 4f62741d2..d751a724c 100644
--- a/src/ntf/ntfimcnd/ntfimcn_imm.c
+++ b/src/ntf/ntfimcnd/ntfimcn_imm.c
@@ -32,6 +32,7 @@
 #include "base/saf_error.h"
 #include "base/ncsgl_defs.h"
 #include "base/osaf_extended_name.h"
+#include "base/osaf_time.h"
 
 #include "imm/saf/saImmOm.h"
 #include "imm/saf/saImmOi.h"
@@ -45,9 +46,9 @@
  */
 /* Release code, major version, minor version */
 static SaVersionT imm_version = {'A', 2, 12};
-static const unsigned int sleep_delay_ms = 500;
-static const unsigned int max_waiting_time_7s = (7 * 1000);   /* 7 sec */
-static const unsigned int max_waiting_time_60s = (60 * 1000); /* 60 sec */
+static const uint64_t sleep_delay_ms = 500;
+static const uint64_t max_waiting_time_7s = (7 * 1000);   /* 7 sec */
+static const uint64_t max_waiting_time_60s = (60 * 1000); /* 60 sec */
 static const SaImmOiImplementerNameT applier_nameA =
     (SaImmOiImplementerNameT) "@OpenSafImmReplicatorA";
 static const SaImmOiImplementerNameT applier_nameB =
@@ -86,7 +87,6 @@ extern ntfimcn_cb_t ntfimcn_cb;
  * Get name of rdn attribute from IMM
  *
  * Note:
- * A valid ntf_cb.immOmHandle must exist
  * Uses in file global struct s_get_rdn_attr_name
  *
  * @param className[in]
@@ -118,15 +118,22 @@ static char *get_rdn_attr_name(const SaImmClassNameT 
className)
        memcpy(s_get_rdn_attr_name.saved_className, className,
               strlen(className) + 1);
 
+       /* Get an IMM OM Handle */
+       SaImmHandleT immOmHandle = 0;
+       if (getImmOmHandle(&immOmHandle) == false) {
+               LOG_ER("getImmOmHandle() Fail");
+               goto error;
+       }
+
        /* Get class description */
        msecs_waited = 0;
-       rc = saImmOmClassDescriptionGet_2(ntfimcn_cb.immOmHandle, className,
+       rc = saImmOmClassDescriptionGet_2(immOmHandle, className,
                                          &classCategory, &attrDescr);
        while (((rc == SA_AIS_ERR_TRY_AGAIN) || (rc == SA_AIS_ERR_TIMEOUT)) &&
               (msecs_waited < max_waiting_time_7s)) {
                usleep(sleep_delay_ms * 1000);
                msecs_waited += sleep_delay_ms;
-               rc = saImmOmClassDescriptionGet_2(ntfimcn_cb.immOmHandle,
+               rc = saImmOmClassDescriptionGet_2(immOmHandle,
                                                  className, &classCategory,
                                                  &attrDescr);
        }
@@ -153,7 +160,7 @@ static char *get_rdn_attr_name(const SaImmClassNameT 
className)
        }
 
        /* Free memory allocated for attribute descriptions */
-       rc = saImmOmClassDescriptionMemoryFree_2(ntfimcn_cb.immOmHandle,
+       rc = saImmOmClassDescriptionMemoryFree_2(immOmHandle,
                                                 attrDescr);
        if (rc != SA_AIS_OK) {
                LOG_ER("saImmOmClassDescriptionMemoryFree_2 failed %s",
@@ -161,6 +168,9 @@ static char *get_rdn_attr_name(const SaImmClassNameT 
className)
                goto error;
        }
 
+       /* Release the OM Handle */
+       finalizeImmOmHandle(immOmHandle);
+
 done:
        TRACE_LEAVE();
        return s_get_rdn_attr_name.attrName;
@@ -690,15 +700,6 @@ static const SaImmOiCallbacksT_2 callbacks = {
 static const SaImmCallbacksT omCallbacks = {
     .saImmOmAdminOperationInvokeCallback = NULL};
 
-/**
- * Initialize the OI interface, get a selection object and become applier
- *
- * @global_param max_waiting_time_ms: Wait max time for each operation.
- * @global_param applier_name: The name of the "configuration change" applier
- * @param *cb[out]
- *
- * @return (-1) if init fail
- */
 int ntfimcn_imm_init(ntfimcn_cb_t *cb)
 {
        SaAisErrorT rc;
@@ -841,41 +842,63 @@ int ntfimcn_imm_init(ntfimcn_cb_t *cb)
                break;
        }
 
-       /*
-        * Initialize the IMM OM API
-        * -------------------------
-        */
-       msecs_waited = 0;
-       cb->immOmHandle = 0;
-       rc = saImmOmInitialize(&cb->immOmHandle, &omCallbacks, &imm_version);
-       while ((rc == SA_AIS_ERR_TRY_AGAIN || rc == SA_AIS_ERR_TIMEOUT) &&
-              msecs_waited < max_waiting_time_60s) {
-               if (rc == SA_AIS_ERR_TIMEOUT) {
-                       LOG_WA("%s saImmOmInitialize() returned %s",
-                              __FUNCTION__, saf_error(rc));
-               }
-               usleep(sleep_delay_ms * 1000);
-               msecs_waited += sleep_delay_ms;
-               if (rc == SA_AIS_ERR_TIMEOUT && cb->immOmHandle != 0) {
-                       while (saImmOmFinalize(cb->immOmHandle) ==
-                                  SA_AIS_ERR_TRY_AGAIN &&
-                              msecs_waited < max_waiting_time_60s) {
-                               usleep(sleep_delay_ms * 1000);
-                               msecs_waited += sleep_delay_ms;
-                       }
+done:
+       TRACE_LEAVE();
+       return internal_rc;
+}
+
+/**
+ * Initialize an IMM OM Handle
+ *
+ * Note: Writes to syslog if error
+ *
+ * @param immOmHandle[out] Set to OM Handle or 0 if Fail
+ * @return false if Fail
+ */
+bool getImmOmHandle(SaImmOiHandleT* immOmHandle) {
+       struct timespec timeout_ts;
+       struct timespec delay_ts;
+       SaAisErrorT ais_rc;
+       bool internal_rc = true;
+
+       osaf_millis_to_timespec(sleep_delay_ms, &delay_ts);
+       osaf_set_millis_timeout(max_waiting_time_60s, &timeout_ts);
+
+       while (osaf_is_timeout(&timeout_ts) == false) {
+               ais_rc = saImmOmInitialize(immOmHandle,
+                       &omCallbacks, &imm_version);
+               if (ais_rc != SA_AIS_ERR_TRY_AGAIN) {
+                       break;
                }
-               cb->immOmHandle = 0;
-               rc = saImmOmInitialize(&cb->immOmHandle, &omCallbacks,
-                                      &imm_version);
+               osaf_nanosleep(&delay_ts);
        }
-       if (rc != SA_AIS_OK) {
+
+       if (ais_rc != SA_AIS_OK) {
                LOG_ER("%s saImmOmInitialize failed %s", __FUNCTION__,
-                      saf_error(rc));
-               internal_rc = NTFIMCN_INTERNAL_ERROR;
-               goto done;
+                      saf_error(ais_rc));
+               internal_rc = false;
        }
-
-done:
-       TRACE_LEAVE();
        return internal_rc;
 }
+
+void finalizeImmOmHandle(SaImmHandleT immOmHandle) {
+       struct timespec timeout_ts;
+       struct timespec delay_ts;
+       SaAisErrorT ais_rc;
+
+       osaf_millis_to_timespec(sleep_delay_ms, &delay_ts);
+       osaf_set_millis_timeout(max_waiting_time_60s, &timeout_ts);
+
+       while (osaf_is_timeout(&timeout_ts) == false) {
+               ais_rc = saImmOmFinalize(immOmHandle);
+               if (ais_rc != SA_AIS_ERR_TRY_AGAIN) {
+                       break;
+               }
+               osaf_nanosleep(&delay_ts);
+       }
+
+       if (ais_rc != SA_AIS_OK) {
+               LOG_ER("%s saImmOmInitialize failed %s", __FUNCTION__,
+                      saf_error(ais_rc));
+       }
+}
\ No newline at end of file
diff --git a/src/ntf/ntfimcnd/ntfimcn_imm.h b/src/ntf/ntfimcnd/ntfimcn_imm.h
index 4d58fa907..86825a590 100644
--- a/src/ntf/ntfimcnd/ntfimcn_imm.h
+++ b/src/ntf/ntfimcnd/ntfimcn_imm.h
@@ -19,13 +19,31 @@
 #define NTF_NTFIMCND_NTFIMCN_IMM_H_
 
 #include "ntfimcn_main.h"
+#include <stdbool.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+/**
+ * Initialize the OI interface, get a selection object and become applier
+ *
+ * @global_param max_waiting_time_ms: Wait max time for each operation.
+ * @global_param applier_name: The name of the "configuration change" applier
+ * @param *cb[out]
+ *
+ * @return (-1) if init fail
+ */
 int ntfimcn_imm_init(ntfimcn_cb_t *cb);
-void ntfimcn_special_applier_clear(void);
+
+/**
+ * Initialize an IMM OM Handle
+ *
+ * @param immOmHandle[out] Set to OM Handle or 0 if Fail
+ * @return false if Fail
+ */
+bool getImmOmHandle(SaImmOiHandleT* immOmHandle);
+void finalizeImmOmHandle(SaImmOiHandleT immOmHandle);
 
 #ifdef __cplusplus
 }
diff --git a/src/ntf/ntfimcnd/ntfimcn_main.h b/src/ntf/ntfimcnd/ntfimcn_main.h
index c1a950264..c8a207377 100644
--- a/src/ntf/ntfimcnd/ntfimcn_main.h
+++ b/src/ntf/ntfimcnd/ntfimcn_main.h
@@ -41,7 +41,6 @@ extern "C" {
 
 typedef struct {
   SaImmOiHandleT immOiHandle; /* Handle from IMM OI initialize */
-  SaImmHandleT immOmHandle;   /* Handle from IMM OM initialize */
   SaSelectionObjectT
       immSelectionObject;  /* Selection Object to wait for IMM events */
   SaNtfHandleT ntf_handle; /* Handle from NTF initialize */
-- 
2.13.0


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