Ack, code review only. Some minor comments below. /Thanks HansN
-----Original Message-----
From: praveen.malv...@oracle.com [mailto:praveen.malv...@oracle.com]
Sent: den 6 januari 2015 06:35
To: Hans Feldt; nagendr...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: [devel] [PATCH 1 of 1] amfd: assign only one csi per NPI component for
PI SU [#1237]
osaf/services/saf/amf/amfd/comp.cc | 23 +++++++++++++++++++++++
osaf/services/saf/amf/amfd/include/comp.h | 2 +-
osaf/services/saf/amf/amfd/sgproc.cc | 3 ++-
osaf/services/saf/amf/amfd/su.cc | 7 ++++++-
4 files changed, 32 insertions(+), 3 deletions(-)
Unlock of SU having NPI comp hangs, if more than one CSIs are configured for
NPI component.
AMFD creates compcsi object for all the CSIs of same SI or different SI which
can go to the single component. When AMFND gets the assignment messages from
AMFD, it assigns one CSI to NPI component. Since other CSIs remain unassigned,
AMFND never responds to AMFD.
This is the reason of hanging of unlock operation.
While creating comp-csi objects, AMFD should not assign more than one CSis to a
single NPI component.
diff --git a/osaf/services/saf/amf/amfd/comp.cc
b/osaf/services/saf/amf/amfd/comp.cc
--- a/osaf/services/saf/amf/amfd/comp.cc
+++ b/osaf/services/saf/amf/amfd/comp.cc
@@ -1574,3 +1574,26 @@ bool comp_is_preinstantiable(const AVD_C
(category ==
AVSV_COMP_TYPE_PROXIED_LOCAL_PRE_INSTANTIABLE) ||
(category == AVSV_COMP_TYPE_EXTERNAL_PRE_INSTANTIABLE));
}
+
+/**
+ * @brief Returns true if the component is assigned any CSI.
+ Note:comp->assign_flag is not always reliable to
+ * check if a component is assigned or not as this flag
+ is always reset before going for new assignments.
+ * @param comp.
+ * @return true/false.
+ */
+
+bool is_comp_assigned_any_csi(AVD_COMP *comp) {
+ for (AVD_SI *si = comp->su->sg_of_su->list_of_si; si != NULL; si =
si->sg_list_of_si_next) {
+ for (AVD_CSI *csi = si->list_of_csi; csi; csi =
csi->si_list_of_csi_next) {
+ for (AVD_COMP_CSI_REL *compcsi = csi->list_compcsi;
compcsi; compcsi = compcsi->csi_csicomp_next) {
+ if (compcsi->comp == comp)
+ return true;
+ }
+ }
+ }
+ return false;
+
+}
diff --git a/osaf/services/saf/amf/amfd/include/comp.h
b/osaf/services/saf/amf/amfd/include/comp.h
--- a/osaf/services/saf/amf/amfd/include/comp.h
+++ b/osaf/services/saf/amf/amfd/include/comp.h
@@ -237,5 +237,5 @@ extern AVD_COMPCS_TYPE * avd_compcstype_ extern void
avd_compcstype_constructor(void); extern AVD_COMP
*avd_comp_get_or_create(const SaNameT *dn); bool comp_is_preinstantiable(const
AVD_COMP *comp);
-
+extern bool is_comp_assigned_any_csi(AVD_COMP *comp);
#endif
diff --git a/osaf/services/saf/amf/amfd/sgproc.cc
b/osaf/services/saf/amf/amfd/sgproc.cc
--- a/osaf/services/saf/amf/amfd/sgproc.cc
+++ b/osaf/services/saf/amf/amfd/sgproc.cc
@@ -137,7 +137,8 @@ uint32_t avd_new_assgn_susi(AVD_CL_CB *c
/* Assign to only those comps, which have assignment.
Those comps, which could not have assignment
before, cann't find compcsi here also.*/
while (l_comp != NULL) {
- if (true == l_comp->assign_flag) {
+ AVD_COMP_TYPE *comptype =
comptype_db->find(Amf::to_string(&l_comp->saAmfCompType));
[HansN] check comptype for 0, or osafassert(comptype)
+ if ((true == l_comp->assign_flag) &&
(comptype->saAmfCtCompCategory
+!= SA_AMF_COMP_LOCAL)) {
if (NULL != (cst =
avd_compcstype_find_match(&l_csi->saAmfCSType, l_comp))) {
if (SA_AMF_HA_ACTIVE ==
ha_state) {
if
(cst->saAmfCompNumCurrActiveCSIs < cst->saAmfCompNumMaxActiveCSIs) { 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
@@ -2043,7 +2043,12 @@ void AVD_SU::reset_all_comps_assign_flag
AVD_COMP *AVD_SU::find_unassigned_comp_that_provides_cstype(const SaNameT
*cstype) {
AVD_COMP *l_comp = list_of_comp;
while (l_comp != NULL) {
- if (l_comp->assign_flag == false) {
+ bool npi_not_assigned = true;
[HansN] maybe bool npi_is_assigned = false is easier to read?
+ AVD_COMP_TYPE *comptype =
comptype_db->find(Amf::to_string(&l_comp->saAmfCompType));
[HansN] check comptype for 0, or osafassert(comptype)
+ if ((comptype->saAmfCtCompCategory == SA_AMF_COMP_LOCAL) &&
is_comp_assigned_any_csi(l_comp))
+ npi_not_assigned = false;
+
+ if ((l_comp->assign_flag == false) && (npi_not_assigned ==
true)) {
AVD_COMPCS_TYPE *cst =
avd_compcstype_find_match(cstype, l_comp);
if (cst != NULL)
break;
------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
