Hi Gary,
One comment on this patch inline with [Praveen].
For patch 1, declaration of avsv_edp_ckpt_msg_siass() needs to be
deleted from ckpt_edu.h.
I think patch series can be pushed after incorporating the comments.
Thanks,
Praveen
On 04-Dec-15 11:14 AM, Gary Lee wrote:
> osaf/services/saf/amf/amfd/ckpt_dec.cc | 68
> ++++++------------
> osaf/services/saf/amf/amfd/ckpt_edu.cc | 55 ---------------
> osaf/services/saf/amf/amfd/ckpt_enc.cc | 63
> ++++++-----------
> osaf/services/saf/amf/amfd/include/ckpt.h | 3 +
> osaf/services/saf/amf/amfd/include/ckpt_edu.h | 3 -
> osaf/services/saf/amf/amfd/tests/test_ckpt_enc_dec.cc | 40 ++++++++++-
> 6 files changed, 88 insertions(+), 144 deletions(-)
>
>
> diff --git a/osaf/services/saf/amf/amfd/ckpt_dec.cc
> b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> --- a/osaf/services/saf/amf/amfd/ckpt_dec.cc
> +++ b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> @@ -557,6 +557,16 @@
> return status;
> }
>
> +void decode_si_trans(NCS_UBAID *ub,
> + AVSV_SI_TRANS_CKPT_MSG *msg,
> + const uint16_t peer_version)
> +{
> + osaf_decode_sanamet(ub, &msg->sg_name);
> + osaf_decode_sanamet(ub, &msg->si_name);
> + osaf_decode_sanamet(ub, &msg->min_su_name);
> + osaf_decode_sanamet(ub, &msg->max_su_name);
> +}
> +
> /**************************************************************************
> * @brief decodes the si transfer parameters
> * @param[in] cb
> @@ -564,45 +574,30 @@
> *************************************************************************/
> static uint32_t dec_si_trans(AVD_CL_CB *cb, NCS_MBCSV_CB_DEC *dec)
> {
> - uint32_t status = NCSCC_RC_SUCCESS;
> - AVSV_SI_TRANS_CKPT_MSG *si_trans_ckpt;
> - AVSV_SI_TRANS_CKPT_MSG dec_si_trans_ckpt;
> - EDU_ERR ederror = static_cast<EDU_ERR>(0);
> + AVSV_SI_TRANS_CKPT_MSG si_trans_ckpt;
>
> TRACE_ENTER2("i_action '%u'", dec->i_action);
>
> - si_trans_ckpt = &dec_si_trans_ckpt;
> -
> switch (dec->i_action) {
> case NCS_MBCSV_ACT_ADD:
> - status = m_NCS_EDU_VER_EXEC(&cb->edu_hdl,
> avsv_edp_ckpt_msg_si_trans,
> - &dec->i_uba, EDP_OP_TYPE_DEC, (AVSV_SI_TRANS_CKPT_MSG
> **)&si_trans_ckpt,
> - &ederror, dec->i_peer_version);
> + decode_si_trans(&dec->i_uba, &si_trans_ckpt,
> dec->i_peer_version);
> break;
>
> case NCS_MBCSV_ACT_RMV:
> /* Send only key information */
> - status = ncs_edu_exec(&cb->edu_hdl, avsv_edp_ckpt_msg_si_trans,
> &dec->i_uba,
> - EDP_OP_TYPE_DEC, (AVSV_SI_TRANS_CKPT_MSG
> **)&si_trans_ckpt, &ederror, 1, 1);
> + osaf_decode_sanamet(&dec->i_uba, &si_trans_ckpt.sg_name);
> break;
>
> default:
> osafassert(0);
> }
>
> - if (status != NCSCC_RC_SUCCESS) {
> - LOG_ER("%s: decode failed, ederror=%u", __FUNCTION__, ederror);
> - return status;
> - }
> -
> - avd_ckpt_si_trans(cb, si_trans_ckpt, dec->i_action);
> -
> - /* If update is successful, update async update count */
> - if (NCSCC_RC_SUCCESS == status)
> - cb->async_updt_cnt.si_trans_updt++;
> -
> - TRACE_LEAVE2("status '%u'", status);
> - return status;
> + avd_ckpt_si_trans(cb, &si_trans_ckpt, dec->i_action);
> +
> + cb->async_updt_cnt.si_trans_updt++;
> +
> + TRACE_LEAVE();
> + return NCSCC_RC_SUCCESS;
> }
>
> void decode_siass(NCS_UBAID *ub,
> @@ -2444,33 +2439,18 @@
> *****************************************************************/
> static uint32_t dec_cs_si_trans(AVD_CL_CB *cb, NCS_MBCSV_CB_DEC *dec,
> uint32_t num_of_obj)
> {
> - uint32_t status = NCSCC_RC_SUCCESS;
> - AVSV_SI_TRANS_CKPT_MSG *si_trans_ckpt;
> - AVSV_SI_TRANS_CKPT_MSG dec_si_trans_ckpt;
> - EDU_ERR ederror = static_cast<EDU_ERR>(0);
> + AVSV_SI_TRANS_CKPT_MSG si_trans_ckpt;
> uint32_t count = 0;
>
> TRACE_ENTER();
>
> - si_trans_ckpt = &dec_si_trans_ckpt;
> -
> for (count = 0; count < num_of_obj; count++) {
> - status = m_NCS_EDU_VER_EXEC(&cb->edu_hdl,
> avsv_edp_ckpt_msg_si_trans,
> - &dec->i_uba, EDP_OP_TYPE_DEC,
> (AVSV_SI_TRANS_CKPT_MSG **)&si_trans_ckpt,
> - &ederror, dec->i_peer_version);
> - if (status != NCSCC_RC_SUCCESS) {
> - LOG_ER("%s: decode failed, ederror=%u", __FUNCTION__,
> ederror);
> - }
> -
> - status = avd_ckpt_si_trans(cb, si_trans_ckpt, dec->i_action);
> -
> - if (status != NCSCC_RC_SUCCESS) {
> - return NCSCC_RC_FAILURE;
> - }
> + decode_si_trans(&dec->i_uba, &si_trans_ckpt,
> dec->i_peer_version);
> + avd_ckpt_si_trans(cb, &si_trans_ckpt, dec->i_action);
> }
>
> - TRACE_LEAVE2("status '%u'", status);
> - return status;
> + TRACE_LEAVE();
> + return NCSCC_RC_SUCCESS;
>
> }
>
> diff --git a/osaf/services/saf/amf/amfd/ckpt_edu.cc
> b/osaf/services/saf/amf/amfd/ckpt_edu.cc
> --- a/osaf/services/saf/amf/amfd/ckpt_edu.cc
> +++ b/osaf/services/saf/amf/amfd/ckpt_edu.cc
> @@ -73,10 +73,6 @@
> if (rc != NCSCC_RC_SUCCESS)
> goto error;
>
> - rc = m_NCS_EDU_COMPILE_EDP(&cb->edu_hdl, avsv_edp_ckpt_msg_si_trans,
> &err);
> - if (rc != NCSCC_RC_SUCCESS)
> - goto error;
> -
> return rc;
>
> error:
> @@ -288,8 +284,6 @@
> {EDU_EXEC, ncs_edp_uns32, 0, 0, 0,
> (long)&((AVSV_ASYNC_UPDT_CNT *)0)->compcstype_updt, 0,
> nullptr},
> {EDU_EXEC, ncs_edp_uns32, 0, 0, 0,
> - (long)&((AVSV_ASYNC_UPDT_CNT *)0)->si_trans_updt, 0, nullptr},
> - {EDU_EXEC, ncs_edp_uns32, 0, 0, 0,
[Praveen] I think this change (deletion of above two lines) should not
be pushed. Because of this change, standby amfd will go out of sync as
it will not decode count for si_trans_updt and will lead to crash during
warm sync. Its effect can be observed only when application uses equal
distribution feature in NWay, NplusM or Nway active model.
> (long)&((AVSV_ASYNC_UPDT_CNT *)0)->ng_updt, 0, nullptr},
>
> {EDU_END, 0, 0, 0, 0, 0, 0, nullptr},
> @@ -417,52 +411,3 @@
> ptr_data_len, buf_env, op, o_err);
> return rc;
> }
> -
> -/******************************************************************
> - * @brief encode/decode rules for si transfer parameters
> - * @param[in] hdl
> - * @param[in] edu_tkn
> - * @param[in] ptr
> - * @param[in] ptr_data_len
> - * @param[in] buf_env
> - * @param[in] op
> - * @param[out] o_err
> - ******************************************************************/
> -uint32_t avsv_edp_ckpt_msg_si_trans(EDU_HDL *hdl, EDU_TKN *edu_tkn,
> - NCSCONTEXT ptr, uint32_t *ptr_data_len,
> - EDU_BUF_ENV *buf_env, EDP_OP_TYPE op, EDU_ERR
> *o_err)
> -{
> - uint32_t rc = NCSCC_RC_SUCCESS;
> - AVSV_SI_TRANS_CKPT_MSG *struct_ptr = nullptr, **d_ptr = nullptr;
> -
> - EDU_INST_SET avsv_ckpt_msg_si_trans_rules[] = {
> - {EDU_START, avsv_edp_ckpt_msg_si_trans, 0, 0, 0,
> - sizeof(AVSV_SI_TRANS_CKPT_MSG), 0, nullptr},
> -
> - {EDU_EXEC, ncs_edp_sanamet, 0, 0, 0,
> (long)&((AVSV_SI_TRANS_CKPT_MSG *)0)->sg_name, 0, nullptr},
> - {EDU_EXEC, ncs_edp_sanamet, 0, 0, 0,
> (long)&((AVSV_SI_TRANS_CKPT_MSG *)0)->si_name, 0, nullptr},
> - {EDU_EXEC, ncs_edp_sanamet, 0, 0, 0,
> (long)&((AVSV_SI_TRANS_CKPT_MSG *)0)->min_su_name, 0, nullptr},
> - {EDU_EXEC, ncs_edp_sanamet, 0, 0, 0,
> (long)&((AVSV_SI_TRANS_CKPT_MSG *)0)->max_su_name, 0, nullptr},
> -
> - {EDU_END, 0, 0, 0, 0, 0, 0, nullptr},
> - };
> -
> - if (op == EDP_OP_TYPE_ENC) {
> - struct_ptr = (AVSV_SI_TRANS_CKPT_MSG *)ptr;
> - } else if (op == EDP_OP_TYPE_DEC) {
> - d_ptr = (AVSV_SI_TRANS_CKPT_MSG **)ptr;
> - if (*d_ptr == nullptr) {
> - *o_err = EDU_ERR_MEM_FAIL;
> - return NCSCC_RC_FAILURE;
> - }
> - memset(*d_ptr, '\0', sizeof(AVSV_SI_TRANS_CKPT_MSG));
> - struct_ptr = *d_ptr;
> - } else {
> - struct_ptr = static_cast<AVSV_SI_TRANS_CKPT_MSG*>(ptr);
> - }
> -
> - rc = m_NCS_EDU_RUN_RULES(hdl, edu_tkn, avsv_ckpt_msg_si_trans_rules,
> struct_ptr,
> - ptr_data_len, buf_env, op, o_err);
> -
> - return rc;
> -}
> diff --git a/osaf/services/saf/amf/amfd/ckpt_enc.cc
> b/osaf/services/saf/amf/amfd/ckpt_enc.cc
> --- a/osaf/services/saf/amf/amfd/ckpt_enc.cc
> +++ b/osaf/services/saf/amf/amfd/ckpt_enc.cc
> @@ -596,6 +596,16 @@
> return status;
> }
>
> +void encode_si_trans(NCS_UBAID *ub,
> + const AVD_SG *sg,
> + const uint16_t peer_version)
> +{
> + osaf_encode_sanamet(ub, &sg->name);
> + osaf_encode_sanamet(ub, &sg->si_tobe_redistributed->name);
> + osaf_encode_sanamet(ub, &sg->min_assigned_su->name);
> + osaf_encode_sanamet(ub, &sg->max_assigned_su->name);
> +}
> +
> /*********************************************************************
> * @brief encodes si transfer parameters
> * @param[in] cb
> @@ -603,39 +613,26 @@
> ********************************************************************/
> static uint32_t enc_si_trans(AVD_CL_CB *cb, NCS_MBCSV_CB_ENC *enc)
> {
> - uint32_t status = NCSCC_RC_SUCCESS;
> - AVSV_SI_TRANS_CKPT_MSG si_trans_ckpt;
> - EDU_ERR ederror = static_cast<EDU_ERR>(0);
> TRACE_ENTER2("io_action '%u'", enc->io_action);
>
> - memset(&si_trans_ckpt, 0, sizeof(AVSV_SI_TRANS_CKPT_MSG));
> - si_trans_ckpt.sg_name = ((AVD_SG
> *)(NCS_INT64_TO_PTR_CAST(enc->io_reo_hdl)))->name;
> + const AVD_SG *sg = (AVD_SG *)enc->io_reo_hdl;
>
> switch (enc->io_action) {
> case NCS_MBCSV_ACT_ADD:
> - si_trans_ckpt.si_name = ((AVD_SG
> *)(NCS_INT64_TO_PTR_CAST(enc->io_reo_hdl)))->si_tobe_redistributed->name;
> - si_trans_ckpt.min_su_name = ((AVD_SG
> *)(NCS_INT64_TO_PTR_CAST(enc->io_reo_hdl)))->min_assigned_su->name;
> - si_trans_ckpt.max_su_name = ((AVD_SG
> *)(NCS_INT64_TO_PTR_CAST(enc->io_reo_hdl)))->max_assigned_su->name;
> - status = m_NCS_EDU_VER_EXEC(&cb->edu_hdl,
> avsv_edp_ckpt_msg_si_trans,
> - &enc->io_uba, EDP_OP_TYPE_ENC, &si_trans_ckpt,
> &ederror, enc->i_peer_version);
> + encode_si_trans(&enc->io_uba, sg, enc->i_peer_version);
> break;
>
> case NCS_MBCSV_ACT_RMV:
> /* Send only key information */
> - status = m_NCS_EDU_SEL_VER_EXEC(&cb->edu_hdl,
> avsv_edp_ckpt_msg_si_trans, &enc->io_uba,
> - EDP_OP_TYPE_ENC, &si_trans_ckpt, &ederror,
> enc->i_peer_version, 1, 1);
> + osaf_encode_sanamet(&enc->io_uba, &sg->name);
> break;
>
> default:
> osafassert(0);
> }
>
> - if (status != NCSCC_RC_SUCCESS) {
> - LOG_ER("%s: encode failed, ederror=%u", __FUNCTION__, ederror);
> - }
> -
> - TRACE_LEAVE2("status '%u'", status);
> - return status;
> + TRACE_LEAVE();
> + return NCSCC_RC_SUCCESS;
> }
>
> void encode_siass(NCS_UBAID *ub,
> @@ -2211,35 +2208,19 @@
> *******************************************************************/
> static uint32_t enc_cs_si_trans(AVD_CL_CB *cb, NCS_MBCSV_CB_ENC *enc,
> uint32_t *num_of_obj)
> {
> - uint32_t status = NCSCC_RC_SUCCESS;
> - AVSV_SI_TRANS_CKPT_MSG si_trans_ckpt;
> - EDU_ERR ederror = static_cast<EDU_ERR>(0);
> -
> TRACE_ENTER();
>
> for (std::map<std::string, AVD_SG*>::const_iterator it = sg_db->begin();
> it != sg_db->end(); it++) {
> AVD_SG *sg = it->second;
> - if (sg->si_tobe_redistributed != nullptr) {
> - si_trans_ckpt.sg_name = sg->name;
> - si_trans_ckpt.si_name = sg->si_tobe_redistributed->name;
> - si_trans_ckpt.min_su_name = sg->min_assigned_su->name;
> - si_trans_ckpt.max_su_name = sg->max_assigned_su->name;
> + if (sg->si_tobe_redistributed != nullptr) {
> + encode_si_trans(&enc->io_uba, sg, enc->i_peer_version);
> + (*num_of_obj)++;
> + }
> + }
>
> - status = m_NCS_EDU_VER_EXEC(&cb->edu_hdl,
> avsv_edp_ckpt_msg_si_trans,
> - &enc->io_uba, EDP_OP_TYPE_ENC, &si_trans_ckpt,
> &ederror,
> - enc->i_peer_version);
> -
> - if (status != NCSCC_RC_SUCCESS) {
> - LOG_ER("%s: encode failed, ederror=%u", __FUNCTION__,
> ederror);
> - return status;
> - }
> -
> - (*num_of_obj)++;
> - }
> - }
> - TRACE_LEAVE2("status '%u'", status);
> - return status;
> + TRACE_LEAVE();
> + return NCSCC_RC_SUCCESS;
> }
>
>
> /****************************************************************************\
> diff --git a/osaf/services/saf/amf/amfd/include/ckpt.h
> b/osaf/services/saf/amf/amfd/include/ckpt.h
> --- a/osaf/services/saf/amf/amfd/include/ckpt.h
> +++ b/osaf/services/saf/amf/amfd/include/ckpt.h
> @@ -49,6 +49,7 @@
> struct cl_cb_tag;
> class AVD_APP;
> class AVD_COMP;
> +class AVD_SG;
> /*
> * SU SI Relationship checkpoint encode/decode message structure..
> */
> @@ -160,5 +161,7 @@
>
> void encode_siass(NCS_UBAID *ub, const struct avd_su_si_rel_tag *susi,
> const uint16_t peer_version);
> void decode_siass(NCS_UBAID *ub, struct avsv_su_si_rel_ckpt_msg *susi,
> const uint16_t peer_version);
> +void encode_si_trans(NCS_UBAID *ub, const AVD_SG *sg, const uint16_t
> peer_version);
> +void decode_si_trans(NCS_UBAID *ub, AVSV_SI_TRANS_CKPT_MSG *msg, const
> uint16_t peer_version);
>
> #endif
> diff --git a/osaf/services/saf/amf/amfd/include/ckpt_edu.h
> b/osaf/services/saf/amf/amfd/include/ckpt_edu.h
> --- a/osaf/services/saf/amf/amfd/include/ckpt_edu.h
> +++ b/osaf/services/saf/amf/amfd/include/ckpt_edu.h
> @@ -61,7 +61,4 @@
> NCSCONTEXT ptr, uint32_t
> *ptr_data_len,
> EDU_BUF_ENV *buf_env, EDP_OP_TYPE
> op, EDU_ERR *o_err);
>
> -uint32_t avsv_edp_ckpt_msg_si_trans(EDU_HDL *hdl, EDU_TKN *edu_tkn,
> - NCSCONTEXT ptr, uint32_t
> *ptr_data_len,
> - EDU_BUF_ENV *buf_env, EDP_OP_TYPE
> op, EDU_ERR *o_err);
> #endif
> diff --git a/osaf/services/saf/amf/amfd/tests/test_ckpt_enc_dec.cc
> b/osaf/services/saf/amf/amfd/tests/test_ckpt_enc_dec.cc
> --- a/osaf/services/saf/amf/amfd/tests/test_ckpt_enc_dec.cc
> +++ b/osaf/services/saf/amf/amfd/tests/test_ckpt_enc_dec.cc
> @@ -208,4 +208,42 @@
> ASSERT_EQ(susi_ckpt.csi_add_rem, static_cast<SaBoolT>(0));
> ASSERT_EQ(Amf::to_string(&susi_ckpt.comp_name), "comp_name");
> ASSERT_EQ(Amf::to_string(&susi_ckpt.csi_name), "csi_name");
> -}
> \ No newline at end of file
> +}
> +
> +TEST_F(CkptEncDecTest, testEncDecAvdSiTrans) {
> + int rc = 0;
> + SG_2N sg;
> + AVD_SI tobe_redistributed;
> + AVD_SU min_assigned_su;
> + AVD_SU max_assigned_su;
> + std::string sg_name("sg_name");
> + std::string si_name("si_name");
> + std::string min_name("min_name");
> + std::string max_name("max_name");
> + AVSV_SI_TRANS_CKPT_MSG msg;
> +
> + min_assigned_su.name = *(asSaNameT(min_name));
> + max_assigned_su.name = *(asSaNameT(max_name));
> + tobe_redistributed.name = *(asSaNameT(si_name));
> + sg.name = *(asSaNameT(sg_name));
> + sg.si_tobe_redistributed = &tobe_redistributed;
> + sg.min_assigned_su = &min_assigned_su;
> + sg.max_assigned_su = &max_assigned_su;
> +
> + rc = ncs_enc_init_space(&enc.io_uba);
> + ASSERT_TRUE(rc == NCSCC_RC_SUCCESS);
> +
> + enc.io_msg_type = NCS_MBCSV_MSG_ASYNC_UPDATE;
> + enc.io_action = NCS_MBCSV_ACT_UPDATE;
> + enc.io_reo_hdl = (MBCSV_REO_HDL)&sg;
> + enc.io_reo_type = AVSV_CKPT_AVD_SI_TRANS;
> + enc.i_peer_version = AVD_MBCSV_SUB_PART_VERSION_4;
> +
> + encode_si_trans(&enc.io_uba, static_cast<AVD_SG*>(&sg),
> enc.i_peer_version);
> + decode_si_trans(&enc.io_uba, &msg, enc.i_peer_version);
> +
> + ASSERT_EQ(Amf::to_string(&msg.sg_name), "sg_name");
> + ASSERT_EQ(Amf::to_string(&msg.si_name), "si_name");
> + ASSERT_EQ(Amf::to_string(&msg.min_su_name), "min_name");
> + ASSERT_EQ(Amf::to_string(&msg.max_su_name), "max_name");
> +}
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
