Hi
I will push this tomorrow (1 Apr) if there are no more comments.
Thanks
Gary
On 11/03/2016, 7:19 PM, "Hans Nordebäck" <hans.nordeb...@ericsson.com> wrote:
>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=278785471&iu=/4140
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
