ack, code review only/Thanks HansN
On 03/07/2016 06:31 AM, Gary Lee wrote:
> osaf/services/saf/amf/amfd/ckpt_dec.cc | 25 ++++++++++++++++++++-----
> osaf/services/saf/amf/amfd/ckpt_updt.cc | 6 +++---
> osaf/services/saf/amf/amfd/su.cc | 8 ++++++--
> 3 files changed, 29 insertions(+), 10 deletions(-)
>
>
> Sometimes a mbc checkpoint may arrive after an object has been deleted, since
> IMM
> callbacks and mbc checkpoints are sent over different channels. Fix crashes
> in standby amfd by not asserting in these cases.
>
> Also fix a crash in su_ccb_apply_delete_hdlr() on standby amfd.
> The saAmfSUAdminState of an SU may change around the same time as
> the SU is deleted. If this state change is not synced
> (new state: SA_AMF_ADMIN_LOCKED_INSTANTIATION) to the standby with mbc,
> before su_ccb_completed_delete_hdlr() is called, then userData
> will be NULL and cause an assert when su_ccb_apply_delete_hdlr() is called.
>
> 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
> @@ -2186,7 +2186,10 @@
> for (count = 0; count < num_of_obj; count++) {
> decode_app(&dec->i_uba, &dec_app);
> status = avd_ckpt_app(cb, &dec_app, dec->i_action);
> - osafassert(status == NCSCC_RC_SUCCESS);
> + if (status != NCSCC_RC_SUCCESS) {
> + // perhaps this ckpt came late, after app was deleted
> + LOG_WA("'%s' update failed", dec_app.name.value);
> + }
> }
>
> TRACE_LEAVE2("status %u, count %u", status, count);
> @@ -2217,7 +2220,10 @@
> for (count = 0; count < num_of_obj; count++) {
> decode_sg(&dec->i_uba, &dec_sg);
> status = avd_ckpt_sg(cb, &dec_sg, dec->i_action);
> - osafassert(status == NCSCC_RC_SUCCESS);
> + if (status != NCSCC_RC_SUCCESS) {
> + // perhaps this ckpt came late, after sg was deleted
> + LOG_WA("'%s' update failed", dec_sg.name.value);
> + }
> }
>
> TRACE_LEAVE2("status %u, count %u", status, count);
> @@ -2248,7 +2254,10 @@
> for (unsigned i = 0; i < num_of_obj; i++) {
> decode_su(&dec->i_uba, &su, dec->i_peer_version);
> status = avd_ckpt_su(cb, &su, dec->i_action);
> - osafassert(status == NCSCC_RC_SUCCESS);
> + if (status != NCSCC_RC_SUCCESS) {
> + // perhaps this ckpt came late, after a su delete
> + LOG_WA("'%s' update failed", su.name.value);
> + }
> }
>
> TRACE_LEAVE2("status '%u'", status);
> @@ -2282,7 +2291,10 @@
> for (unsigned i = 0; i < num_of_obj; i++) {
> decode_si(&dec->i_uba, &si, dec->i_peer_version);
> status = avd_ckpt_si(cb, &si, dec->i_action);
> - osafassert(status == NCSCC_RC_SUCCESS);
> + if (status != NCSCC_RC_SUCCESS) {
> + // perhaps this ckpt came late, after a si delete
> + LOG_WA("'%s' update failed", si.name.value);
> + }
> }
>
> TRACE_LEAVE2("status '%u'", status);
> @@ -2772,7 +2784,10 @@
> for (count = 0; count < num_of_obj; count++) {
> decode_comp_cs_type_config(&dec->i_uba, &comp_cs_type);
> status = avd_ckpt_compcstype(cb, &comp_cs_type, dec->i_action);
> - osafassert(status == NCSCC_RC_SUCCESS);
> + if (status != NCSCC_RC_SUCCESS) {
> + // perhaps this ckpt came late, after a delete
> + LOG_WA("'%s' update failed", comp_cs_type.name.value);
> + }
> }
> TRACE_LEAVE2("status '%u'", status);
> return status;
> diff --git a/osaf/services/saf/amf/amfd/ckpt_updt.cc
> b/osaf/services/saf/amf/amfd/ckpt_updt.cc
> --- a/osaf/services/saf/amf/amfd/ckpt_updt.cc
> +++ b/osaf/services/saf/amf/amfd/ckpt_updt.cc
> @@ -145,7 +145,7 @@
> osafassert (action == NCS_MBCSV_ACT_UPDATE);
>
> if (nullptr == (sg = sg_db->find(Amf::to_string(&ckpt_sg->name)))) {
> - LOG_ER("sg_db->find() FAILED for '%s'", ckpt_sg->name.value);
> + LOG_WA("sg_db->find() FAILED for '%s'", ckpt_sg->name.value);
> rc = NCSCC_RC_FAILURE;
> goto done;
> }
> @@ -188,7 +188,7 @@
> osafassert (action == NCS_MBCSV_ACT_UPDATE);
>
> if (nullptr == (su = su_db->find(Amf::to_string(&ckpt_su->name)))) {
> - LOG_ER("su_db->find FAILED for '%s'", ckpt_su->name.value);
> + LOG_WA("su_db->find FAILED for '%s'", ckpt_su->name.value);
> rc = NCSCC_RC_FAILURE;
> goto done;
> }
> @@ -588,7 +588,7 @@
> osafassert (action == NCS_MBCSV_ACT_UPDATE);
>
> if (nullptr == (ccst = compcstype_db->find(Amf::to_string(dn)))) {
> - LOG_ER("compcstype_db->find()FAILED for '%s'", dn->value);
> + LOG_WA("compcstype_db->find()FAILED for '%s'", dn->value);
> goto done;
> }
> ccst->saAmfCompNumCurrActiveCSIs =
> ckpt_compcstype->saAmfCompNumCurrActiveCSIs;
> diff --git a/osaf/services/saf/amf/amfd/su.cc
> b/osaf/services/saf/amf/amfd/su.cc
> --- a/osaf/services/saf/amf/amfd/su.cc
> +++ b/osaf/services/saf/amf/amfd/su.cc
> @@ -1481,7 +1481,9 @@
> if (su->sg_of_su->saAmfSGAdminState ==
> SA_AMF_ADMIN_LOCKED_INSTANTIATION)
> goto done_ok;
>
> - if (su->saAmfSUAdminState != SA_AMF_ADMIN_LOCKED_INSTANTIATION)
> {
> + // skip this check if this is the standby director
> + if (su->saAmfSUAdminState != SA_AMF_ADMIN_LOCKED_INSTANTIATION
> &&
> + avd_cb->avail_state_avd != SA_AMF_HA_STANDBY) {
> report_ccb_validation_error(opdata,
> "Admin state is not locked instantiation
> required for deletion");
> goto done;
> @@ -1495,9 +1497,11 @@
>
> done_ok:
> rc = SA_AIS_OK;
> - opdata->userData = su;
>
> done:
> + // store su in all cases. saAmfSUAdminState may change
> + // between ccb complete and ccb apply, if this is the standby
> + opdata->userData = su;
> TRACE_LEAVE();
> return rc;
> }
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
