Hi Hans,
Can you please check whether we are in sync with the
approaches(Please check my comments inlined with [Nagu]).
Thanks
-Nagu
-----Original Message-----
From: Nagendra Kumar
Sent: 18 November 2013 12:22
To: Hans Feldt; Hans Nordebäck; Anders Björnerstedt
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1 of 1] amfd: Use correct CLC command to term/clean
component [#104]
-----Original Message-----
From: Hans Feldt [mailto:hans.fe...@ericsson.com]
Sent: 18 November 2013 12:06
To: Nagendra Kumar; Hans Nordebäck; Anders Björnerstedt
Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1 of 1] amfd: Use correct CLC command to term/clean
component [#104]
> -----Original Message-----
> From: Nagendra Kumar [mailto:nagendr...@oracle.com]
> Sent: den 18 november 2013 05:50
> To: Hans Feldt; Hans Nordebäck; Anders Björnerstedt
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1 of 1] amfd: Use correct CLC command to
> term/clean component [#104]
>
> Hi Hans/Anders/Neel/Mathi,
> To upgrade immnd service by changing the CLC
> path would be
> violated with the suggested fix. Is it ok for immsv service?
I don't understand this question. Immnd has nothing todo with the original
problem and the immnd CLC path is never changed. And why should we support
restarting immnd by admin command because some test case requires it? It is not
a real use case.
[Nagu]: As per your suggestion, keep this change in the
avnd_comp_clc_restart_termsucc_hdler():
/* Refresh the component configuration, it may have changed */
if (!m_AVND_IS_SHUTTING_DOWN(cb) && (avnd_comp_config_reinit(comp) !=
0)) {
rc = NCSCC_RC_FAILURE;
goto err;
}
Here since component is terminated and just before we are about to start the
component, we need to read the configuration in avnd_comp_config_reinit as
comp->config_is_valid is set true. comp->config_is_valid is marked false in
avnd_comp_oper_req() when 'CLC path'/comptype were changed.
Did I get it wrong ?
>
> I don't find any other way as of now to support it as we are anyway
> reading the configuration from immnd. When we stop reading the configuration
> in the future, probability suggested approach can be adopted.
Can you share the patch resulting from what I propose in the ticket and in
detail describe what the problem with it is?
Thanks,
Hans
>
> Thanks
> -Nagu
> -----Original Message-----
> From: Hans Feldt [mailto:hans.fe...@ericsson.com]
> Sent: 15 November 2013 18:19
> To: Nagendra Kumar; Hans Nordebäck
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1 of 1] amfd: Use correct CLC command to
> term/clean component [#104]
>
> These are quite intrusive changes. I don't think it has to be done
> like this. Why not do it as I suggested in the ticket and fix the "immnd
> admin restart" use case?
>
> Thanks,
> Hans
>
> > -----Original Message-----
> > From: nagendr...@oracle.com [mailto:nagendr...@oracle.com]
> > Sent: den 15 november 2013 13:21
> > To: Hans Feldt; Hans Nordebäck
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: [PATCH 1 of 1] amfd: Use correct CLC command to term/clean
> > component [#104]
> >
> > osaf/services/saf/amf/amfnd/clc.cc | 64
> > +++++++++++++++++++++++++
> > osaf/services/saf/amf/amfnd/compdb.cc | 23 ++++++--
> > osaf/services/saf/amf/amfnd/include/avnd_comp.h | 3 +
> > 3 files changed, 83 insertions(+), 7 deletions(-)
> >
> >
> > diff --git a/osaf/services/saf/amf/amfnd/clc.cc
> > b/osaf/services/saf/amf/amfnd/clc.cc
> > --- a/osaf/services/saf/amf/amfnd/clc.cc
> > +++ b/osaf/services/saf/amf/amfnd/clc.cc
> > @@ -1436,6 +1436,7 @@ uint32_t avnd_comp_clc_insting_inst_hdle
> > uint32_t avnd_comp_clc_insting_instsucc_hdler(AVND_CB *cb, AVND_COMP
> > *comp) {
> > uint32_t rc = NCSCC_RC_SUCCESS;
> > + AVND_COMP_CLC_CMD_PARAM *cmd, *dest_cmd;
> > TRACE_ENTER2("'%s': Instantiate success event in the Instantiating
> > state", comp->name.value);
> >
> > /* stop the reg tmr */
> > @@ -1447,6 +1448,27 @@ uint32_t avnd_comp_clc_insting_instsucc_
> >
> > /* transition to 'instantiated' state */
> > avnd_comp_pres_state_set(comp, SA_AMF_PRESENCE_INSTANTIATED);
> > +
> > + cmd = &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1];
> > + if (cmd->new_config == true) {
> > + dest_cmd =
> > &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1];
> > + memset(&dest_cmd->cmd, 0, sizeof(dest_cmd->cmd));
> > + strncpy(dest_cmd->cmd, cmd->cmd, cmd->len);
> > +
> > + cmd->new_config = false;
> > + dest_cmd->len = cmd->len;
> > + }
> > +
> > + cmd = &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1];
> > + if (cmd->new_config == true) {
> > + dest_cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP
> > - 1];
> > + memset(&dest_cmd->cmd, 0, sizeof(dest_cmd->cmd));
> > + strncpy(dest_cmd->cmd, cmd->cmd, cmd->len);
> > +
> > + cmd->new_config = false;
> > + dest_cmd->len = cmd->len;
> > + }
> > +
> > m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, comp, AVND_CKPT_COMP_CONFIG);
> >
> > TRACE_LEAVE();
> > @@ -2090,6 +2112,7 @@ uint32_t avnd_comp_clc_terming_cleanfail
> > uint32_t avnd_comp_clc_restart_instsucc_hdler(AVND_CB *cb, AVND_COMP
> > *comp) {
> > uint32_t rc = NCSCC_RC_SUCCESS;
> > + AVND_COMP_CLC_CMD_PARAM *cmd, *dest_cmd;
> > TRACE_ENTER2("'%s': Instantiate success event in the restarting
> > state", comp->name.value);
> >
> > /* stop the reg tmr */
> > @@ -2101,6 +2124,26 @@ uint32_t avnd_comp_clc_restart_instsucc_
> > /* just transition back to 'instantiated' state */
> > avnd_comp_pres_state_set(comp, SA_AMF_PRESENCE_INSTANTIATED);
> >
> > + cmd = &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE
> > - 1];
> > + if (cmd->new_config == true) {
> > + dest_cmd =
> > &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1];
> > + memset(&dest_cmd->cmd, 0, sizeof(dest_cmd->cmd));
> > + strncpy(dest_cmd->cmd, cmd->cmd, cmd->len);
> > +
> > + cmd->new_config = false;
> > + dest_cmd->len = cmd->len;
> > + }
> > +
> > + cmd = &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP -
> > 1];
> > + if (cmd->new_config == true) {
> > + dest_cmd =
> > &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1];
> > + memset(&dest_cmd->cmd, 0, sizeof(dest_cmd->cmd));
> > + strncpy(dest_cmd->cmd, cmd->cmd, cmd->len);
> > +
> > + cmd->new_config = false;
> > + dest_cmd->len = cmd->len;
> > + }
> > +
> > TRACE_LEAVE();
> > return rc;
> > }
> > @@ -2321,6 +2364,7 @@ uint32_t avnd_comp_clc_restart_cleanfail
> > uint32_t avnd_comp_clc_orph_instsucc_hdler(AVND_CB *cb, AVND_COMP
> > *comp) {
> > uint32_t rc = NCSCC_RC_SUCCESS;
> > + AVND_COMP_CLC_CMD_PARAM *cmd, *dest_cmd;
> > TRACE_ENTER2("'%s': Instantiate success event in the Orphaned
> > state", comp->name.value);
> >
> > /* stop the proxied registration timer */ @@ -2332,6 +2376,26 @@
> > uint32_t avnd_comp_clc_orph_instsucc_hdl
> > /* just transition to 'instantiated' state */
> > avnd_comp_pres_state_set(comp, SA_AMF_PRESENCE_INSTANTIATED);
> >
> > + cmd = &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE
> > - 1];
> > + if (cmd->new_config == true) {
> > + dest_cmd =
> > &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1];
> > + memset(&dest_cmd->cmd, 0, sizeof(dest_cmd->cmd));
> > + strncpy(dest_cmd->cmd, cmd->cmd, cmd->len);
> > +
> > + cmd->new_config = false;
> > + dest_cmd->len = cmd->len;
> > + }
> > +
> > + cmd = &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP -
> > 1];
> > + if (cmd->new_config == true) {
> > + dest_cmd =
> > &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1];
> > + memset(&dest_cmd->cmd, 0, sizeof(dest_cmd->cmd));
> > + strncpy(dest_cmd->cmd, cmd->cmd, cmd->len);
> > +
> > + cmd->new_config = false;
> > + dest_cmd->len = cmd->len;
> > + }
> > +
> > TRACE_LEAVE();
> > return rc;
> > }
> > diff --git a/osaf/services/saf/amf/amfnd/compdb.cc
> > b/osaf/services/saf/amf/amfnd/compdb.cc
> > --- a/osaf/services/saf/amf/amfnd/compdb.cc
> > +++ b/osaf/services/saf/amf/amfnd/compdb.cc
> > @@ -1181,7 +1181,7 @@ done:
> > static void init_bundle_dependent_attributes(AVND_COMP *comp,
> > const amf_comp_type_t *comptype,
> > const char *path_prefix,
> > - const SaImmAttrValuesT_2 **attributes)
> > + const SaImmAttrValuesT_2 **attributes, bool
> > re_read_config)
> > {
> > int i, j;
> > AVND_COMP_CLC_CMD_PARAM *cmd;
> > @@ -1208,8 +1208,12 @@ static void init_bundle_dependent_attrib
> > osafassert((cmd->len > 0) && (cmd->len < sizeof(cmd->cmd)));
> > TRACE("cmd=%s", cmd->cmd);
> > }
> > + if (re_read_config == true) {
> > + cmd =
> > &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1];
> > + cmd->new_config = true;
> > + } else
> > + cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE -
> > 1];
> >
> > - cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1];
> > if (comptype->saAmfCtRelPathTerminateCmd != NULL) {
> > i = 0;
> > i += snprintf(&cmd->cmd[i], sizeof(cmd->cmd) - i, "%s/%s", @@
> > -1230,7 +1234,12 @@ static void init_bundle_dependent_attrib
> > TRACE("cmd=%s", cmd->cmd);
> > }
> >
> > - cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1];
> > + if (re_read_config == true) {
> > + cmd = &comp->new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP
> > - 1];
> > + cmd->new_config = true;
> > + } else
> > + cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1];
> > +
> > if (comptype->saAmfCtRelPathCleanupCmd != NULL) {
> > i = 0;
> > i += snprintf(&cmd->cmd[i], sizeof(cmd->cmd) - i, "%s/%s", @@
> > -1347,7 +1356,7 @@ static void init_bundle_dependent_attrib
> > * @return int
> > */
> > static int comp_init(AVND_COMP *comp, const SaImmAttrValuesT_2
> > **attributes,
> > - bool bundle_missing_is_error)
> > + bool bundle_missing_is_error, bool re_read_config)
> > {
> > int res = -1;
> > AVND_COMP_CLC_CMD_PARAM *cmd;
> > @@ -1381,7 +1390,7 @@ static int comp_init(AVND_COMP *comp, co
> > &nodeswbundle_name, &path_prefix);
> >
> > if (res == 0) {
> > - init_bundle_dependent_attributes(comp, comptype, path_prefix,
> > attributes);
> > + init_bundle_dependent_attributes(comp, comptype, path_prefix,
> > +attributes, re_read_config);
> > } else {
> > if (bundle_missing_is_error) {
> > LOG_NO("%s: '%s'", __FUNCTION__, comp->name.value); @@
> > -1599,7
> > +1608,7 @@ static AVND_COMP *avnd_comp_create(const
> > error = immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCompType"),
> > attributes, 0, &comp->saAmfCompType);
> > osafassert(error == SA_AIS_OK);
> >
> > - if (comp_init(comp, attributes, false) != 0)
> > + if (comp_init(comp, attributes, false, false) != 0)
> > goto done;
> >
> > /* create the association with hdl-mngr */ @@ -1777,7 +1786,7 @@
> > int avnd_comp_config_reinit(AVND_COMP *c
> > goto done2;
> > }
> >
> > - res = comp_init(comp, attributes, true);
> > + res = comp_init(comp, attributes, true, true);
> > if (res == 0)
> > TRACE("'%s' configuration reread from IMM", comp->name.value);
> >
> > diff --git a/osaf/services/saf/amf/amfnd/include/avnd_comp.h
> > b/osaf/services/saf/amf/amfnd/include/avnd_comp.h
> > --- a/osaf/services/saf/amf/amfnd/include/avnd_comp.h
> > +++ b/osaf/services/saf/amf/amfnd/include/avnd_comp.h
> > @@ -88,6 +88,7 @@ typedef struct avnd_comp_clc_param {
> > char cmd[SAAMF_CLC_LEN]; /* cmd ascii string */
> > SaTimeT timeout; /* cmd timeout value */
> > uint32_t len; /* cmd len */
> > + bool new_config;
> > } AVND_COMP_CLC_CMD_PARAM;
> >
> > /* clc info definition (top level wrapper structure) */ @@ -359,6
> > +360,8 @@ typedef struct avnd_comp_tag {
> > SaBoolT admin_oper; /*set to true if undergoing admin operation */
> > int config_is_valid; /* Used to indicate that config has to be
> > refreshed from IMM */
> > bool assigned_flag; /* Used in finding multiple csi for a single
> > comp while csi mod.*/
> > + /* Used only for term/clean args */
> > + AVND_COMP_CLC_CMD_PARAM
> > +new_configured_cmds[AVND_COMP_CLC_CMD_TYPE_MAX - 1];
> >
> > } AVND_COMP;
> >
------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
