Hi Minh,
Thanks for your comment.
I update as Thuan's comment. So we can ignore your comment.
-----Original Message-----
From: Minh Hon Chau <minh.c...@dektech.com.au>
Sent: Monday, March 2, 2020 10:03 AM
To: Thang Duc Nguyen <thang.d.ngu...@dektech.com.au>; Thuan Tran
<thuan.t...@dektech.com.au>; Gary Lee <gary....@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] amfnd: handle comp instantiate event with SU in
terminating presence state [#3160]
Hi Thang,
Please see comments below.
Thanks
Minh
On 27/2/20 5:19 pm, thang.d.nguyen wrote:
> - When component is instantiated with SU in terminating presence
> state, need trigger SU fsm with instantiate event if all components
> are either uninstantiated or instantiated.
> - Need informing error recovery when amfnd fails to send callback
> parameters to component when component already exited.
> ---
> src/amf/amfnd/avnd_su.h | 19 +++++++++++++++++++
> src/amf/amfnd/comp.cc | 23 ++++++++++++++++++++++-
> src/amf/amfnd/susm.cc | 6 ++++--
> 3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/src/amf/amfnd/avnd_su.h b/src/amf/amfnd/avnd_su.h index
> 45effd6c9..eb708dce0 100644
> --- a/src/amf/amfnd/avnd_su.h
> +++ b/src/amf/amfnd/avnd_su.h
> @@ -256,6 +256,25 @@ typedef struct avnd_su_tag {
> } \
> } \
> }
> +/* macro to determine if all the pi comps in an su
> + are instantiated or uninstantiated */
> +#define m_AVND_SU_IS_INSTANTIATED_UNINSTANTIATED(su, is) \
> + { \
> + AVND_COMP *curr = 0; \
> + (is) = true; \
> + for (curr = m_AVND_COMP_FROM_SU_DLL_NODE_GET( \
> + m_NCS_DBLIST_FIND_FIRST(&su->comp_list)); \
> + curr; curr = m_AVND_COMP_FROM_SU_DLL_NODE_GET( \
> + m_NCS_DBLIST_FIND_NEXT(&curr->su_dll_node))) { \
> + if (m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(curr) && \
> + (!m_AVND_COMP_PRES_STATE_IS_INSTANTIATED(curr) && \
> + !m_AVND_COMP_PRES_STATE_IS_UNINSTANTIATED(curr))) { \
> + (is) = false; \
> + break; \
> + } \
> + } \
> + }
>
[M]: There are su_all_pi_comps_instantiated() and
pi_su_all_comps_uninstantiated(), can we reuse them? (The naming of these two
functions seems not consistent, though).
> /* macros to manage the presence state */
> #define m_AVND_SU_PRES_STATE_IS_INSTANTIATED(x) \ diff --git
> a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc index
> 8a11d75fb..cef94a0d4 100644
> --- a/src/amf/amfnd/comp.cc
> +++ b/src/amf/amfnd/comp.cc
> @@ -2031,6 +2031,7 @@ uint32_t avnd_comp_cbk_send(AVND_CB *cb, AVND_COMP
> *comp,
> AVND_COMP_CSI_REC *csi_rec) {
> SaAmfCSIDescriptorT csi_desc;
> SaAmfCSIFlagsT csi_flag;
> + AVND_ERR_INFO err_info;
> std::string csi_name;
> AVSV_AMF_CBK_INFO *cbk_info = 0;
> AVND_COMP_CSI_REC *curr_csi = 0;
> @@ -2039,6 +2040,7 @@ uint32_t avnd_comp_cbk_send(AVND_CB *cb, AVND_COMP
> *comp,
> SaAmfHandleT hdl = 0;
> SaTimeT per = 0;
> uint32_t rc = NCSCC_RC_SUCCESS;
> + uint32_t ret = NCSCC_RC_SUCCESS;
>
> TRACE_ENTER2("'%s' %u", comp->name.c_str(), type);
> /*
> @@ -2201,7 +2203,26 @@ uint32_t avnd_comp_cbk_send(AVND_CB *cb, AVND_COMP
> *comp,
> rc = avnd_comp_cbq_send(cb, comp, dest, hdl, cbk_info, per);
>
> done:
> - if ((NCSCC_RC_SUCCESS != rc) && cbk_info)
> avsv_amf_cbk_free(cbk_info);
> + if ((NCSCC_RC_SUCCESS != rc) && cbk_info) {
> + avsv_amf_cbk_free(cbk_info);
> + LOG_ER("avnd_comp_cbk_send failed for comp: %s, type: %d",
> + comp->name.c_str(), type);
> + if (type == AVSV_AMF_CSI_SET) {
> + err_info.src = AVND_ERR_SRC_CBK_CSI_SET_FAILED;
> + } else if (type == AVSV_AMF_COMP_TERM) {
> + err_info.src = AVND_ERR_SRC_CMD_FAILED;
> + } else {
> + // For another callback types
> + err_info.src = AVND_ERR_SRC_MAX;
> + }
> + err_info.rec_rcvr.avsv_ext =
> + static_cast<AVSV_ERR_RCVR>(comp->err_info.def_rec);
> + ret = avnd_err_process(cb, comp, &err_info);
> + if (NCSCC_RC_SUCCESS != ret) {
> + LOG_ER("process error failed for comp: %s",
> + comp->name.c_str());
> + }
> + }
>
> TRACE_LEAVE2("%u", rc);
> return rc;
> diff --git a/src/amf/amfnd/susm.cc b/src/amf/amfnd/susm.cc index
> 86811f1e4..162f65625 100644
> --- a/src/amf/amfnd/susm.cc
> +++ b/src/amf/amfnd/susm.cc
> @@ -2897,7 +2897,7 @@ uint32_t avnd_su_pres_terming_compinst_hdler(AVND_CB
> *cb, AVND_SU *su,
> su->name.c_str(), compname.c_str());
>
> if (m_AVND_SU_IS_PREINSTANTIABLE(su)) {
> - bool is;
> + bool is = false;
> osafassert(comp != nullptr);
> if (m_AVND_COMP_IS_FAILED(comp)) {
> m_AVND_COMP_FAILED_RESET(comp); @@ -2908,8 +2908,10 @@
> uint32_t avnd_su_pres_terming_compinst_hdler(AVND_CB *cb, AVND_SU *su,
> if (true == is) {
> avnd_su_pres_state_set(cb, su, SA_AMF_PRESENCE_INSTANTIATED);
> }
> +
> if (m_AVND_SU_IS_RESTART(su)) {
> - if (su->admin_op_Id == SA_AMF_ADMIN_RESTART)
> + m_AVND_SU_IS_INSTANTIATED_UNINSTANTIATED(su, is)
> + if ((su->admin_op_Id == SA_AMF_ADMIN_RESTART) || (true == is))
> /*This can happen when SU has both restartable and non restartable
> comps.Go for further instantiation.*/
> rc = avnd_su_pres_fsm_run(cb, su, 0,
> AVND_SU_PRES_FSM_EV_INST);
[M] If m_AVND_SU_IS_INSTANTIATED_UNINSTANTIATED() is true, which means all
comps may be un-instantiated, so is it ok to trigger EV_INST to SU?
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
