Hi Gary,
ack, code review only. Minor comments below.
/Thanks HansN
On 09/06/2017 06:19 AM, Gary Lee wrote:
It is possible for an object to be deleted in IMM, before
a standby SC finishes initilization. Now, if the related
callbacks are processed by the standby late, then unnecessary
assertions or null pointer accesses may occur.
---
src/amf/amfd/cb.h | 1 +
src/amf/amfd/comp.cc | 4 ++++
src/amf/amfd/compcstype.cc | 6 ++++++
src/amf/amfd/cstype.cc | 8 ++++++++
src/amf/amfd/hlt.cc | 7 +++++++
src/amf/amfd/node.cc | 11 +++++++++++
src/amf/amfd/nodeswbundle.cc | 3 +++
src/amf/amfd/sg.cc | 10 ++++++++++
src/amf/amfd/sgtype.cc | 10 ++++++++++
src/amf/amfd/si.cc | 10 ++++++++++
src/amf/amfd/si_dep.cc | 4 ++++
src/amf/amfd/sirankedsu.cc | 4 ++++
src/amf/amfd/su.cc | 14 ++++++++++++--
src/amf/amfd/sutype.cc | 7 +++++++
src/amf/amfd/svctype.cc | 8 ++++++++
src/amf/amfd/svctypecstypes.cc | 5 +++++
16 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/src/amf/amfd/cb.h b/src/amf/amfd/cb.h
index 6cdf9944f..6140258db 100644
--- a/src/amf/amfd/cb.h
+++ b/src/amf/amfd/cb.h
@@ -136,6 +136,7 @@ typedef struct cl_cb_tag {
SaAmfHAStateT avail_state_avd; /* the redundancy state for
* Availability director
*/
[HansN] bool is_active() const {return avail_state_avd ==
SA_AMF_HA_ACTIVE;}
+ bool is_active() {return avail_state_avd == SA_AMF_HA_ACTIVE;};
MDS_HDL vaddr_pwe_hdl; /* The pwe handle returned when
* vdest address is created.
diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc
index 4f2d16ac9..2e2c5b3ed 100644
--- a/src/amf/amfd/comp.cc
+++ b/src/amf/amfd/comp.cc
@@ -1752,6 +1752,10 @@ void comp_ccb_apply_delete_hdlr(struct
CcbUtilOperationData *opdata) {
TRACE_ENTER();
AVD_COMP *comp =
comp_db->find(Amf::to_string(&opdata->objectName));
+ if (comp == nullptr && avd_cb->is_active() == false) {
[HansN] TRACE_LEAVE is not necessary to add when c++ is used, (it is
done by trace via RAII)
+ TRACE_LEAVE();
+ return;
+ }
/* comp should be found in the database even if it was
* due to parent su delete the changes are applied in
* bottom up order so all the component deletes are applied
diff --git a/src/amf/amfd/compcstype.cc b/src/amf/amfd/compcstype.cc
index f45cb9fd9..b88bd801a 100644
--- a/src/amf/amfd/compcstype.cc
+++ b/src/amf/amfd/compcstype.cc
@@ -358,6 +358,10 @@ static SaAisErrorT
compcstype_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
AVD_COMP_CSI_REL *compcsi;
cst =
compcstype_db->find(Amf::to_string(&opdata->objectName));
+ if (cst == nullptr && avd_cb->is_active() == false) {
+ rc = SA_AIS_OK;
+ break;
+ }
osafassert(cst);
avsv_sanamet_init(Amf::to_string(&opdata->objectName), comp_name,
"safComp=");
@@ -380,6 +384,7 @@ static SaAisErrorT
compcstype_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
break;
}
done:
[HansN] TRACE_LEAVE is not necessary to add when c++ is used, (it is
done by trace via RAII)
+ TRACE_LEAVE();
return rc;
}
@@ -407,6 +412,7 @@ static void
compcstype_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
osafassert(0);
break;
}
[HansN] TRACE_LEAVE is not necessary to add when c++ is used, (it is
done by trace via RAII)
+ TRACE_LEAVE();
}
static SaAisErrorT compcstype_rt_attr_callback(
diff --git a/src/amf/amfd/cstype.cc b/src/amf/amfd/cstype.cc
index a72aeecf2..cadc6dfa0 100644
--- a/src/amf/amfd/cstype.cc
+++ b/src/amf/amfd/cstype.cc
@@ -200,6 +200,11 @@ static SaAisErrorT
cstype_ccb_completed_hdlr(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
cst = cstype_db->find(object_name);
+ if (cst == nullptr && avd_cb->is_active() == false) {
+ rc = SA_AIS_OK;
+ opdata->userData = nullptr;
+ break;
+ }
if (cst->list_of_csi != nullptr) {
/* check whether there exists a delete operation for
* each of the CSI in the cs_type list in the current CCB
@@ -247,6 +252,9 @@ static void
cstype_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
cstype_add_to_model(cst);
break;
case CCBUTIL_DELETE:
+ if (opdata->userData == nullptr && avd_cb->is_active() ==
false) {
+ break;
+ }
cstype_delete(static_cast<AVD_CS_TYPE *>(opdata->userData));
break;
default:
diff --git a/src/amf/amfd/hlt.cc b/src/amf/amfd/hlt.cc
index f0fd0c2b1..27863db6e 100644
--- a/src/amf/amfd/hlt.cc
+++ b/src/amf/amfd/hlt.cc
@@ -71,6 +71,10 @@ static SaAisErrorT
ccb_completed_delete_hdlr(CcbUtilOperationData_t *opdata) {
avsv_sanamet_init(Amf::to_string(&opdata->objectName), comp_name,
"safComp=");
comp = comp_db->find(comp_name);
+ if (comp == nullptr && avd_cb->is_active() == false) {
+ opdata->userData = nullptr;
+ goto done;
+ }
for (curr_susi = comp->su->list_of_susi; curr_susi != nullptr;
curr_susi = curr_susi->su_next)
for (compcsi = curr_susi->list_of_csicomp; compcsi;
@@ -160,6 +164,9 @@ static void
ccb_apply_modify_hdlr(CcbUtilOperationData_t *opdata) {
static void ccb_apply_delete_hdlr(CcbUtilOperationData_t *opdata) {
AVSV_PARAM_INFO param = {0};
+ if (opdata->userData == nullptr && avd_cb->is_active() == false) {
+ return;
+ }
AVD_AVND *node = static_cast<AVD_AVND *>(opdata->userData);
param.class_id = AVSV_SA_AMF_HEALTH_CHECK;
diff --git a/src/amf/amfd/node.cc b/src/amf/amfd/node.cc
index 9521a0978..d23c31ffe 100644
--- a/src/amf/amfd/node.cc
+++ b/src/amf/amfd/node.cc
@@ -532,6 +532,12 @@ static SaAisErrorT node_ccb_completed_delete_hdlr(
TRACE_ENTER2("'%s'",
osaf_extended_name_borrow(&opdata->objectName));
+ if (node == nullptr && avd_cb->is_active() == false) {
+ opdata->userData = nullptr;
+ goto done;
+ }
+ osafassert(node != nullptr);
+
if (node->node_info.member) {
report_ccb_validation_error(opdata, "Node '%s' is still cluster
member",
osaf_extended_name_borrow(&opdata->objectName));
@@ -783,6 +789,11 @@ static SaAisErrorT
node_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
}
static void node_ccb_apply_delete_hdlr(AVD_AVND *node) {
+ if (node == nullptr && avd_cb->is_active() == false) {
+ TRACE_ENTER();
[HansN] TRACE_LEAVE is not necessary to add when c++ is used, (it is
done by trace via RAII)
+ TRACE_LEAVE();
+ return;
+ }
TRACE_ENTER2("'%s'", node->name.c_str());
avd_node_delete_nodeid(node);
avd_node_delete(node);
diff --git a/src/amf/amfd/nodeswbundle.cc b/src/amf/amfd/nodeswbundle.cc
index 16df1b432..4ab79f71c 100644
--- a/src/amf/amfd/nodeswbundle.cc
+++ b/src/amf/amfd/nodeswbundle.cc
@@ -122,6 +122,9 @@ static int is_swbdl_delete_ok(const std::string
&bundle_dn,
/* Check if any comps are referencing this bundle */
avsv_sanamet_init(bundle_dn, node_dn, "safAmfNode=");
node = avd_node_get(node_dn);
+ if (node == nullptr && avd_cb->is_active() == false) {
+ return 1;
+ }
if (!is_swbdl_delete_ok_for_node(bundle_dn, node_dn,
node->list_of_ncs_su,
opdata))
diff --git a/src/amf/amfd/sg.cc b/src/amf/amfd/sg.cc
index 98cccc6bb..c0e7d7320 100644
--- a/src/amf/amfd/sg.cc
+++ b/src/amf/amfd/sg.cc
@@ -1624,6 +1624,12 @@ static SaAisErrorT
sg_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
sg = sg_db->find(Amf::to_string(&opdata->objectName));
+ if (sg == nullptr && avd_cb->is_active() == false) {
+ rc = SA_AIS_OK;
+ opdata->userData = nullptr;
+ break;
+ }
+ osafassert(sg != nullptr);
if (sg->list_of_si.empty() == false) {
/* check whether there is parent app delete */
const SaNameTWrapper app_name(sg->app->name);
@@ -1680,6 +1686,10 @@ static void
sg_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
sg_add_to_model(sg);
break;
case CCBUTIL_DELETE:
+ if (opdata->userData == nullptr && avd_cb->is_active() ==
false) {
+ break;
+ }
+ osafassert(opdata->userData != nullptr);
avd_sg_delete(static_cast<AVD_SG *>(opdata->userData));
break;
case CCBUTIL_MODIFY:
diff --git a/src/amf/amfd/sgtype.cc b/src/amf/amfd/sgtype.cc
index e97321eb1..25cfd7c20 100644
--- a/src/amf/amfd/sgtype.cc
+++ b/src/amf/amfd/sgtype.cc
@@ -382,6 +382,12 @@ static SaAisErrorT
sgtype_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
sgt = sgtype_db->find(Amf::to_string(&opdata->objectName));
+ if (sgt == nullptr && avd_cb->is_active() == false) {
+ rc = SA_AIS_OK;
+ opdata->userData = nullptr;
+ goto done;
+ }
+ osafassert(sgt != nullptr);
for (const auto &sg : sgt->list_of_sg) {
SaNameTWrapper sg_name(sg->name);
@@ -492,6 +498,10 @@ static void
sgtype_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
sgtype_ccb_apply_modify_hdlr(opdata);
break;
case CCBUTIL_DELETE:
+ if (opdata->userData == nullptr && avd_cb->is_active() ==
false) {
+ break;
+ }
+ osafassert(opdata->userData != nullptr);
sgtype_delete(static_cast<AVD_AMF_SG_TYPE *>(opdata->userData));
break;
default:
diff --git a/src/amf/amfd/si.cc b/src/amf/amfd/si.cc
index 27245339c..74b465314 100644
--- a/src/amf/amfd/si.cc
+++ b/src/amf/amfd/si.cc
@@ -1079,6 +1079,12 @@ static SaAisErrorT
si_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
si = avd_si_get(Amf::to_string(&opdata->objectName));
+ if (si == nullptr && avd_cb->is_active () == false) {
+ rc = SA_AIS_OK;
+ opdata->userData = nullptr;
+ goto done;
+ }
+ osafassert(si != nullptr);
if (nullptr != si->list_of_sisu) {
report_ccb_validation_error(opdata, "SaAmfSI is in use '%s'",
si->name.c_str());
@@ -1293,6 +1299,10 @@ static void
si_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
si->si_add_to_model();
break;
case CCBUTIL_DELETE:
+ if (opdata->userData == nullptr && avd_cb->is_active() ==
false) {
+ break;
+ }
+ osafassert(opdata->userData != nullptr);
avd_si_delete(static_cast<AVD_SI *>(opdata->userData));
break;
case CCBUTIL_MODIFY:
diff --git a/src/amf/amfd/si_dep.cc b/src/amf/amfd/si_dep.cc
index 2772be2b8..55fcb28cb 100644
--- a/src/amf/amfd/si_dep.cc
+++ b/src/amf/amfd/si_dep.cc
@@ -1359,6 +1359,10 @@ static void
sidep_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
case CCBUTIL_DELETE:
avd_sidep_indx_init(Amf::to_string(&opdata->objectName), &tmp_sidep);
sidep = sidep_db_find(tmp_sidep.spons_name, tmp_sidep.dep_name);
+ if (sidep == nullptr && avd_cb->is_active() == false) {
+ break;
+ }
+ osafassert(sidep != nullptr);
dep_si = avd_si_get(sidep->dep_name);
osafassert(dep_si != nullptr);
/* If SI is in tolerance timer running state because of this
diff --git a/src/amf/amfd/sirankedsu.cc b/src/amf/amfd/sirankedsu.cc
index 0b060a09f..64cde49f5 100644
--- a/src/amf/amfd/sirankedsu.cc
+++ b/src/amf/amfd/sirankedsu.cc
@@ -340,6 +340,10 @@ static void
sirankedsu_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
+ if (opdata->userData == nullptr && avd_cb->is_active () ==
false) {
+ break;
+ }
+ osafassert(opdata->userData != nullptr);
/* delete and free the structure */
avd_sirankedsu_del_si_list(
avd_cb, static_cast<AVD_SUS_PER_SI_RANK
*>(opdata->userData));
diff --git a/src/amf/amfd/su.cc b/src/amf/amfd/su.cc
index 3726a71fb..9ade12f97 100644
--- a/src/amf/amfd/su.cc
+++ b/src/amf/amfd/su.cc
@@ -1661,6 +1661,11 @@ static SaAisErrorT su_ccb_completed_delete_hdlr(
is_app_su = 0;
su = su_db->find(Amf::to_string(&opdata->objectName));
+ if (su == nullptr && avd_cb->is_active() == false) {
+ opdata->userData = nullptr;
[HansN] TRACE_LEAVE is not necessary to add when c++ is used, (it is
done by trace via RAII)
+ TRACE_LEAVE();
+ return SA_AIS_OK;
+ }
osafassert(su != nullptr);
if (is_app_su) {
@@ -2008,10 +2013,15 @@ static void su_ccb_apply_modify_hdlr(struct
CcbUtilOperationData *opdata) {
* @param su
*/
void su_ccb_apply_delete_hdlr(struct CcbUtilOperationData *opdata) {
- AVD_SU *su = static_cast<AVD_SU *>(opdata->userData);
AVD_AVND *su_node_ptr;
AVSV_PARAM_INFO param;
- AVD_SG *sg = su->sg_of_su;
+ AVD_SG *sg;
+
+ if (opdata->userData == nullptr && avd_cb->is_active() == false) {
+ return;
+ }
+ AVD_SU *su = static_cast<AVD_SU *>(opdata->userData);
+ sg = su->sg_of_su;
TRACE_ENTER2("'%s'", su->name.c_str());
diff --git a/src/amf/amfd/sutype.cc b/src/amf/amfd/sutype.cc
index 344b94332..e3fe7b3d3 100644
--- a/src/amf/amfd/sutype.cc
+++ b/src/amf/amfd/sutype.cc
@@ -281,6 +281,9 @@ static void
sutype_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
sut = sutype_db->find(Amf::to_string(&opdata->objectName));
+ if (sut == nullptr && avd_cb->is_active() == false) {
+ break;
+ }
sutype_db->erase(sut->name);
sutype_delete(&sut);
break;
@@ -373,6 +376,10 @@ static SaAisErrorT
sutype_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
sut = sutype_db->find(Amf::to_string(&opdata->objectName));
+ if (sut == nullptr && avd_cb->is_active() == false) {
+ rc = SA_AIS_OK;
+ break;
+ }
/* check whether there exists a delete operation for
* each of the SU in the su_type list in the current CCB
diff --git a/src/amf/amfd/svctype.cc b/src/amf/amfd/svctype.cc
index d576542bd..c7e597e80 100644
--- a/src/amf/amfd/svctype.cc
+++ b/src/amf/amfd/svctype.cc
@@ -155,6 +155,11 @@ static SaAisErrorT
svctype_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
break;
case CCBUTIL_DELETE:
svc_type =
svctype_db->find(Amf::to_string(&opdata->objectName));
+ if (svc_type == nullptr && avd_cb->is_active() == false) {
+ rc = SA_AIS_OK;
+ opdata->userData = nullptr;
+ break;
+ }
/* check whether there exists a delete operation for
* each of the SI in the svc_type list in the current CCB
@@ -200,6 +205,9 @@ static void
svctype_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
svctype_db_add(svc_type);
break;
case CCBUTIL_DELETE:
+ if (opdata->userData == nullptr && avd_cb->is_active() ==
false) {
+ break;
+ }
svctype_delete(static_cast<AVD_SVC_TYPE *>(opdata->userData));
break;
default:
diff --git a/src/amf/amfd/svctypecstypes.cc
b/src/amf/amfd/svctypecstypes.cc
index f1f8e439a..381dc3980 100644
--- a/src/amf/amfd/svctypecstypes.cc
+++ b/src/amf/amfd/svctypecstypes.cc
@@ -157,6 +157,11 @@ static SaAisErrorT svctypecstypes_ccb_completed_cb(
case CCBUTIL_DELETE:
svctypecstype =
svctypecstypes_db->find(Amf::to_string(&opdata->objectName));
+ if (svctypecstype == nullptr && avd_cb->is_active() == false) {
+ rc = SA_AIS_OK;
+ opdata->userData = nullptr;
+ break;
+ }
if (svctypecstype->curr_num_csis == 0) {
rc = SA_AIS_OK;
opdata->userData = svctypecstype;
