Additional patch to be merged with this one. avnd_evt_clc_resp_evh() now 
consists of a simple switch(cmd_type)
/Hans



On 08/08/2013 11:02 AM, Hans Feldt wrote:
I am OK with patch except for the change mentioned below. But
avnd_evt_clc_resp_evh() can be simplified and made more elegant now when
cmd_type is available. I will send an additional patch that should be
merged with this one.
/Hans

On 08/07/2013 05:03 PM, Hans Feldt wrote:
On 7 August 2013 14:14,  <praveen.malv...@oracle.com> wrote:
   osaf/services/saf/avsv/avnd/avnd_clc.c         |  93 
+++++++++++--------------
   osaf/services/saf/avsv/avnd/include/avnd_evt.h |   2 -
   2 files changed, 40 insertions(+), 55 deletions(-)


At present when clc response event comes AMFND searches whole component
database and matches the context. This context does not include component
name. Thus AMFND can associate clc response with wrong component. The patch
ensures that AMFND gets component name in the clc response event and thus
solves the problem by associating clc response event with the component.

diff --git a/osaf/services/saf/avsv/avnd/avnd_clc.c 
b/osaf/services/saf/avsv/avnd/avnd_clc.c
--- a/osaf/services/saf/avsv/avnd/avnd_clc.c
+++ b/osaf/services/saf/avsv/avnd/avnd_clc.c
@@ -70,7 +70,6 @@ static uint32_t avnd_comp_clc_orph_resta

   uint32_t avnd_comp_clc_st_chng_prc(AVND_CB *, AVND_COMP *, 
SaAmfPresenceStateT, SaAmfPresenceStateT);

-static uint32_t avnd_comp_clc_resp(NCS_OS_PROC_EXECUTE_TIMED_CB_INFO *);
   static uint32_t avnd_instfail_su_failover(AVND_CB *, AVND_SU *, AVND_COMP *);

   /***************************************************************************
@@ -311,33 +310,20 @@ uint32_t avnd_evt_clc_resp_evh(AVND_CB *
          AVND_CLC_EVT *clc_evt = &evt->info.clc;
          AVND_COMP *comp = 0;
          uint32_t rc = NCSCC_RC_SUCCESS;
-       uint32_t amcmd = 0;
          TRACE_ENTER();

-       /* get the comp */
-       if (clc_evt->exec_ctxt) {
-               /* => cmd successfully launched (scan the entire comp-database) 
*/
-               for (comp = (AVND_COMP *)ncs_patricia_tree_getnext(&cb->compdb,
-                                                                  (uint8_t 
*)0); comp;
-                    comp = (AVND_COMP *)ncs_patricia_tree_getnext(&cb->compdb, 
(uint8_t *)&comp->name)) {
-                       if (comp->clc_info.cmd_exec_ctxt == clc_evt->exec_ctxt)
-                               break;
-                       if (comp->clc_info.am_cmd_exec_ctxt == 
clc_evt->exec_ctxt) {
-                               amcmd = 1;
-                               break;
-                       }
-               }
-       } else
-               /* => cmd did not launch successfully (comp is available) */
-               comp = m_AVND_COMPDB_REC_GET(cb->compdb, clc_evt->comp_name);
+       comp = m_AVND_COMPDB_REC_GET(cb->compdb, clc_evt->comp_name);

          /* either other cmd is currently executing or the comp is deleted */
-       if (!comp)
-               goto done;
+       if (comp == NULL) {
+                LOG_WA("%s: could not find comp '%s'", __FUNCTION__,
+                                clc_evt->comp_name.value);
+                goto done;
+       }

          TRACE("'%s'", comp->name.value);

-       if (amcmd || clc_evt->cmd_type == AVND_COMP_CLC_CMD_TYPE_AMSTART ||
+       if (clc_evt->cmd_type == AVND_COMP_CLC_CMD_TYPE_AMSTART ||
              clc_evt->cmd_type == AVND_COMP_CLC_CMD_TYPE_AMSTOP) {
                  TRACE("CLC command 
type:'%s'",clc_cmd_type[clc_evt->cmd_type]);
                  switch (comp->clc_info.am_exec_cmd) {
@@ -431,7 +417,7 @@ uint32_t avnd_evt_clc_resp_evh(AVND_CB *
                          break;

                  default:
-                       osafassert(0);
+                       goto done;

Why this change? It will silently discard the event.
This is a typical case of assert (not osafassert) isn't it?
/Hans



------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead.
Download for free and get started troubleshooting in minutes.
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


# HG changeset patch
# Parent 33b144490e7ce8f51566f63936bbf52133b8ed9c

diff --git a/osaf/services/saf/avsv/avnd/avnd_cam.c 
b/osaf/services/saf/avsv/avnd/avnd_cam.c
--- a/osaf/services/saf/avsv/avnd/avnd_cam.c
+++ b/osaf/services/saf/avsv/avnd/avnd_cam.c
@@ -121,10 +121,6 @@ uint32_t avnd_comp_amstart_clc_res_proce
 {
        uint32_t rc = NCSCC_RC_SUCCESS;
 
-       /* reset the cmd exec context params */
-       comp->clc_info.am_exec_cmd = AVND_COMP_CLC_CMD_TYPE_MAX;
-       comp->clc_info.am_cmd_exec_ctxt = 0;
-
        if (NCS_OS_PROC_EXIT_NORMAL == value) {
                /* do nothing, just reset the count */
                comp->clc_info.am_start_retry_cnt = 0;
@@ -157,10 +153,6 @@ uint32_t avnd_comp_amstop_clc_res_proces
 
        memset(&err, '\0', sizeof(AVND_ERR_INFO));
 
-       /* reset the cmd exec context params */
-       comp->clc_info.am_exec_cmd = AVND_COMP_CLC_CMD_TYPE_MAX;
-       comp->clc_info.am_cmd_exec_ctxt = 0;
-
        if (NCS_OS_PROC_EXIT_NORMAL == value) {
                /* check for state, if inst state, call am start, else do 
nothing */
                if (m_AVND_COMP_PRES_STATE_IS_INSTANTIATED(comp) && 
comp->clc_info.am_start_retry_cnt != 0)
diff --git a/osaf/services/saf/avsv/avnd/avnd_clc.c 
b/osaf/services/saf/avsv/avnd/avnd_clc.c
--- a/osaf/services/saf/avsv/avnd/avnd_clc.c
+++ b/osaf/services/saf/avsv/avnd/avnd_clc.c
@@ -314,118 +314,101 @@ uint32_t avnd_evt_clc_resp_evh(AVND_CB *
 
        comp = m_AVND_COMPDB_REC_GET(cb->compdb, clc_evt->comp_name);
 
-       /* either other cmd is currently executing or the comp is deleted */
+       /* the comp has been deleted? */
        if (comp == NULL) {
                 LOG_WA("%s: could not find comp '%s'", __FUNCTION__,
                                 clc_evt->comp_name.value);
                 goto done;
        }
 
-       TRACE("'%s'", comp->name.value);
-
-       if (clc_evt->cmd_type == AVND_COMP_CLC_CMD_TYPE_AMSTART ||
-           clc_evt->cmd_type == AVND_COMP_CLC_CMD_TYPE_AMSTOP) {
-               TRACE("CLC command type:'%s'",clc_cmd_type[clc_evt->cmd_type]);
-               switch (comp->clc_info.am_exec_cmd) {
-               case AVND_COMP_CLC_CMD_TYPE_AMSTART:
-                       rc = avnd_comp_amstart_clc_res_process(cb, comp, 
clc_evt->exec_stat.value);
-                       break;
-               case AVND_COMP_CLC_CMD_TYPE_AMSTOP:
-                       rc = avnd_comp_amstop_clc_res_process(cb, comp, 
clc_evt->exec_stat.value);
-                       break;
-
-               default:
-                       osafassert(0);
+       TRACE("'%s', command type:%s", comp->name.value,
+                       clc_cmd_type[clc_evt->cmd_type]);
+
+       switch (clc_evt->cmd_type) {
+       case AVND_COMP_CLC_CMD_TYPE_INSTANTIATE:
+               /*
+                * note that inst-succ evt is generated for the fsm only
+                * when the comp is registered too
+                */
+               if (NCS_OS_PROC_EXIT_NORMAL == clc_evt->exec_stat.value) {
+                       /* mark that the inst cmd has been successfully 
executed */
+                       m_AVND_COMP_INST_CMD_SUCC_SET(comp);
+                       m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_FLAG_CHANGE);
+
+                       if (!m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(comp) || 
m_AVND_COMP_IS_REG(comp)) {
+                               /* all set... proceed with inst-succ evt for 
the fsm */
+                               ev = AVND_COMP_CLC_PRES_FSM_EV_INST_SUCC;
+                               TRACE("Comp '%s' Inst. flag '%u'", 
comp->name.value, comp->flag);
+                       } else {
+                               /* start the comp-reg timer */
+                               m_AVND_TMR_COMP_REG_START(cb, *comp, rc);
+                               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_CLC_REG_TMR);
+                       }
+               } else {
+                       ev = AVND_COMP_CLC_PRES_FSM_EV_INST_FAIL;
+
+                       if (NCS_OS_PROC_EXIT_WITH_CODE == 
clc_evt->exec_stat.value) {
+                               comp->clc_info.inst_code_rcvd =
+                                               
clc_evt->exec_stat.info.exit_with_code.exit_code;
+                               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_INST_CODE_RCVD);
+                       }
+
+                       LOG_NO("Instantiation of '%s' failed", 
comp->name.value);
+                       log_failed_exec(&clc_evt->exec_stat, comp, 
clc_evt->cmd_type);
                }
-       } else {
-               TRACE("CLC command 
type:'%s'",clc_cmd_type[comp->clc_info.exec_cmd]);
-               /* determine the event for the fsm */
-               switch (comp->clc_info.exec_cmd) {
-               case AVND_COMP_CLC_CMD_TYPE_INSTANTIATE:
-                       /* 
-                        * note that inst-succ evt is generated for the fsm 
only 
-                        * when the comp is registered too 
-                        */
-                       if (NCS_OS_PROC_EXIT_NORMAL == 
clc_evt->exec_stat.value) {
-                               /* mark that the inst cmd has been successfully 
executed */
-                               m_AVND_COMP_INST_CMD_SUCC_SET(comp);
-                               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_FLAG_CHANGE);
-
-                               if (!m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(comp) 
|| m_AVND_COMP_IS_REG(comp)) {
-                                       /* all set... proceed with inst-succ 
evt for the fsm */
-                                       ev = 
AVND_COMP_CLC_PRES_FSM_EV_INST_SUCC;
-                                       TRACE("Comp '%s' Inst. flag '%u'", 
comp->name.value, comp->flag);
-                               }
-                               else {
-                                       /* start the comp-reg timer */
-                                       m_AVND_TMR_COMP_REG_START(cb, *comp, 
rc);
-                                       m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, 
comp, AVND_CKPT_COMP_CLC_REG_TMR);
-                               }
-                       } else {
-                               ev = AVND_COMP_CLC_PRES_FSM_EV_INST_FAIL;
-
-                               if (NCS_OS_PROC_EXIT_WITH_CODE == 
clc_evt->exec_stat.value) {
-                                       comp->clc_info.inst_code_rcvd =
-                                           
clc_evt->exec_stat.info.exit_with_code.exit_code;
-                                       m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, 
comp, AVND_CKPT_COMP_INST_CODE_RCVD);
-                               }
-
-                               LOG_NO("Instantiation of '%s' failed", 
comp->name.value);
-                               log_failed_exec(&clc_evt->exec_stat, comp, 
comp->clc_info.exec_cmd);
-                       }
-                       break;
-
-               case AVND_COMP_CLC_CMD_TYPE_TERMINATE:
-
-                       if (NCS_OS_PROC_EXIT_NORMAL == 
clc_evt->exec_stat.value) {
-                               ev = AVND_COMP_CLC_PRES_FSM_EV_TERM_SUCC;
-                               TRACE("Comp '%s' Term.", comp->name.value);
-                       }
-                       else {
-                               ev = AVND_COMP_CLC_PRES_FSM_EV_CLEANUP;
-                               m_AVND_COMP_TERM_FAIL_SET(comp);
-                               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_FLAG_CHANGE);
-                               LOG_NO("Termination of '%s' failed", 
comp->name.value);
-                               log_failed_exec(&clc_evt->exec_stat, comp, 
comp->clc_info.exec_cmd);
-                       }
-
-                       break;
-
-               case AVND_COMP_CLC_CMD_TYPE_CLEANUP:
-
-                       if (NCS_OS_PROC_EXIT_NORMAL == 
clc_evt->exec_stat.value) {
-                               ev = AVND_COMP_CLC_PRES_FSM_EV_CLEANUP_SUCC;
-                               TRACE("Comp '%s' Cleanup.", comp->name.value);
-                       } else {
-                               ev = AVND_COMP_CLC_PRES_FSM_EV_CLEANUP_FAIL;
-                               LOG_NO("Cleanup of '%s' failed", 
comp->name.value);
-                               log_failed_exec(&clc_evt->exec_stat, comp, 
comp->clc_info.exec_cmd);
-                       }
-                       break;
-               case AVND_COMP_CLC_CMD_TYPE_HC:
-                       if (NCS_OS_PROC_EXIT_NORMAL == 
clc_evt->exec_stat.value) {
-                               avnd_comp_hc_cmd_restart(comp);
-                       } else {
-                               AVND_ERR_INFO err_info;
-                               LOG_NO("Healthcheck failed for '%s'", 
comp->name.value);
-                               log_failed_exec(&clc_evt->exec_stat, comp, 
comp->clc_info.exec_cmd);
-                               err_info.src = AVND_ERR_SRC_HC;
-                               err_info.rec_rcvr.avsv_ext = 
comp->err_info.def_rec;
-                               rc = avnd_err_process(cb, comp, &err_info);
-                               goto done;
-                       }
-                       break;
-
-               default:
+               break;
+
+       case AVND_COMP_CLC_CMD_TYPE_TERMINATE:
+               if (NCS_OS_PROC_EXIT_NORMAL == clc_evt->exec_stat.value) {
+                       ev = AVND_COMP_CLC_PRES_FSM_EV_TERM_SUCC;
+                       TRACE("Comp '%s' Term.", comp->name.value);
+               } else {
+                       ev = AVND_COMP_CLC_PRES_FSM_EV_CLEANUP;
+                       m_AVND_COMP_TERM_FAIL_SET(comp);
+                       m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_FLAG_CHANGE);
+                       LOG_NO("Termination of '%s' failed", comp->name.value);
+                       log_failed_exec(&clc_evt->exec_stat, comp, 
clc_evt->cmd_type);
+               }
+
+               break;
+
+       case AVND_COMP_CLC_CMD_TYPE_AMSTART:
+               rc = avnd_comp_amstart_clc_res_process(cb, comp, 
clc_evt->exec_stat.value);
+               break;
+
+       case AVND_COMP_CLC_CMD_TYPE_AMSTOP:
+               rc = avnd_comp_amstop_clc_res_process(cb, comp, 
clc_evt->exec_stat.value);
+               break;
+
+       case AVND_COMP_CLC_CMD_TYPE_CLEANUP:
+               if (NCS_OS_PROC_EXIT_NORMAL == clc_evt->exec_stat.value) {
+                       ev = AVND_COMP_CLC_PRES_FSM_EV_CLEANUP_SUCC;
+                       TRACE("Comp '%s' Cleanup.", comp->name.value);
+               } else {
+                       ev = AVND_COMP_CLC_PRES_FSM_EV_CLEANUP_FAIL;
+                       LOG_NO("Cleanup of '%s' failed", comp->name.value);
+                       log_failed_exec(&clc_evt->exec_stat, comp, 
clc_evt->cmd_type);
+               }
+               break;
+
+       case AVND_COMP_CLC_CMD_TYPE_HC:
+               if (NCS_OS_PROC_EXIT_NORMAL == clc_evt->exec_stat.value) {
+                       avnd_comp_hc_cmd_restart(comp);
+               } else {
+                       AVND_ERR_INFO err_info;
+                       LOG_NO("Healthcheck failed for '%s'", comp->name.value);
+                       log_failed_exec(&clc_evt->exec_stat, comp, 
clc_evt->cmd_type);
+                       err_info.src = AVND_ERR_SRC_HC;
+                       err_info.rec_rcvr.avsv_ext = comp->err_info.def_rec;
+                       rc = avnd_err_process(cb, comp, &err_info);
                        goto done;
                }
-
-               /* reset the cmd exec context params */
-               comp->clc_info.exec_cmd = AVND_COMP_CLC_CMD_TYPE_MAX;
-               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_EXEC_CMD);
-               comp->clc_info.cmd_exec_ctxt = 0;
-               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_CMD_EXEC_CTXT);
-       }                       /* if */
+               break;
+
+       default:
+               osafassert(0);
+               break;
+       }
 
        /* run the fsm */
        if (AVND_COMP_CLC_PRES_FSM_EV_MAX != ev)
@@ -2509,12 +2492,6 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_
                if (AVND_COMP_CLC_CMD_TYPE_CLEANUP == cmd_type) {
                        clc_evt->exec_stat.value = NCS_OS_PROC_EXIT_NORMAL;
 
-                       clc_info->exec_cmd = cmd_type;
-                       m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_EXEC_CMD);
-
-                       clc_info->cmd_exec_ctxt = 0;
-                       m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_CMD_EXEC_CTXT);
-
                        /* create the event */
                        evt = avnd_evt_create(cb, AVND_EVT_CLC_RESP, 0, 0, 0, 
(void *)clc_evt, 0);
                        if (!evt) {
@@ -2701,20 +2678,7 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_
                free(clc_evt);
        } else {
                TRACE_2("The CLC CLI command execution success");
-               if (cmd_type == AVND_COMP_CLC_CMD_TYPE_AMSTART || cmd_type == 
AVND_COMP_CLC_CMD_TYPE_AMSTOP)
-                       clc_info->am_cmd_exec_ctxt = 
(NCS_EXEC_HDL)cmd_info.i_usr_hdl;
-               else {
-                       clc_info->cmd_exec_ctxt = 
(NCS_EXEC_HDL)cmd_info.i_usr_hdl;
-                       m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_CMD_EXEC_CTXT);
-               }
-       }
-
-       /* irrespective of the command execution status, set the current cmd */
-       if (cmd_type == AVND_COMP_CLC_CMD_TYPE_AMSTART || cmd_type == 
AVND_COMP_CLC_CMD_TYPE_AMSTOP)
-               clc_info->am_exec_cmd = cmd_type;
-       else {
-               clc_info->exec_cmd = cmd_type;
-               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, 
AVND_CKPT_COMP_EXEC_CMD);
+               // outcome of command is reported in comp_clc_resp_callback()
        }
 
        TRACE_LEAVE2("success");
diff --git a/osaf/services/saf/avsv/avnd/include/avnd_comp.h 
b/osaf/services/saf/avsv/avnd/include/avnd_comp.h
--- a/osaf/services/saf/avsv/avnd/include/avnd_comp.h
+++ b/osaf/services/saf/avsv/avnd/include/avnd_comp.h
@@ -105,15 +105,12 @@ typedef struct avnd_comp_clc_info {
        uint32_t saAmfCompNumMaxAmStopAttempts;
 
        /* 
-        * current command execution info (note that a comp has only 
-        * one outstanding command in execution other than AM related)
+        * current command execution info
+        * TODO: not used anymore, should be removed completely
         */
        AVND_COMP_CLC_CMD_TYPE exec_cmd;        /* command in execution */
        NCS_EXEC_HDL cmd_exec_ctxt;     /* command execution context */
 
-       AVND_COMP_CLC_CMD_TYPE am_exec_cmd;     /* command in execution */
-       NCS_EXEC_HDL am_cmd_exec_ctxt;  /* command execution context */
-
        /* comp reg tmr info */
        SaTimeT inst_cmd_ts;    /* instantiate cmd start timestamp */
        AVND_TMR clc_reg_tmr;   /* comp reg tmr */
------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to