Ack, code review only./Thanks HansN
-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: den 15 september 2016 11:42
To: Hans Nordebäck <[email protected]>; [email protected]; Gary
Lee <[email protected]>; Minh Hon Chau <[email protected]>
Cc: [email protected]
Subject: [PATCH 1 of 1] amf: fix activeCompName in csiStateDescriptor used in
CSI SET cbk [#2021]
osaf/libs/agents/saf/amfa/ava_hdl.cc | 28 +++++++++++++++++++---------
osaf/libs/common/amf/n2avamsg.c | 14 ++++++--------
osaf/services/saf/amf/amfd/util.cc | 2 +-
osaf/services/saf/amf/amfnd/comp.cc | 1 -
4 files changed, 26 insertions(+), 19 deletions(-)
In the reported problem, active compname is improperly as \'sa\'
in CSI SET callback for HA state.
After setting activeCompName in standby descriptor, AMFND is wrongly calling
osaf_extended_name_clear() for active descriptor. Since
SaAmfCSIStateDescriptorT is a union, it clears the filled value.
Along with the above problems there are other minor problems like:
-AMFND is copying activecompName using osaf_extended_name_alloc() in agent
message, only when it is an extended name.
osaf_extended_name_alloc() should be done irrespective of short or long dn.
-Agent should perform validation check on dn based on HA state of the CSI SET
callback, as activecompname is not populated for all the HA states.
-When above problems are fixed, one more problem pops up:
a standby component gets its own name in activeCompName in CSI set callback
for standby.
There is a minor regression at AMFD.
Patch fixes all these problems.
diff --git a/osaf/libs/agents/saf/amfa/ava_hdl.cc
b/osaf/libs/agents/saf/amfa/ava_hdl.cc
--- a/osaf/libs/agents/saf/amfa/ava_hdl.cc
+++ b/osaf/libs/agents/saf/amfa/ava_hdl.cc
@@ -624,21 +624,31 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
AVSV_AMF_CSI_SET_PARAM *csi_set = &info->param.csi_set;
if (!ava_sanamet_is_valid(&csi_set->csi_desc.csiName) ||
- !ava_sanamet_is_valid(&csi_set->comp_name) ||
-
!ava_sanamet_is_valid(&csi_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName)
||
-
!ava_sanamet_is_valid(&csi_set->csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName))
{
+ !ava_sanamet_is_valid(&csi_set->comp_name)) {
rc = SA_AIS_ERR_NAME_TOO_LONG;
}
+ bool actv_or_stdby = true;
+ if ((csi_set->ha == SA_AMF_HA_ACTIVE) &&
+
(csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor !=
SA_AMF_CSI_NEW_ASSIGN)) {
+ actv_or_stdby =
ava_sanamet_is_valid(&csi_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName);
+ } else if (csi_set->ha == SA_AMF_HA_STANDBY) {
+ actv_or_stdby =
ava_sanamet_is_valid(&csi_set->csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName);
+ }
+ if (!actv_or_stdby)
+ rc = SA_AIS_ERR_NAME_TOO_LONG;
if (rc == SA_AIS_OK && reg_cbk->saAmfCSISetCallback) {
TRACE("CSISet: CSIName = %s, CSIFlags = %d, HA
state = %d",
osaf_extended_name_borrow(&csi_set->csi_desc.csiName),csi_set->csi_desc.csiFlags,csi_set->ha);
- TRACE("CSISet: Active Transition Descriptor =
%u, Active Component Name = %s",
-
csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor,
-
osaf_extended_name_borrow(&csi_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName));
- TRACE("CSISet: ActiveCompName = %s, StandbyRank
= %u",
-
osaf_extended_name_borrow(&csi_set->csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName),
-
csi_set->csi_desc.csiStateDescriptor.standbyDescriptor.standbyRank);
+ if ((csi_set->ha == SA_AMF_HA_ACTIVE) &&
+
(csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor !=
SA_AMF_CSI_NEW_ASSIGN))
+ TRACE("CSISet: Active Transition
Descriptor = %u, Active Component Name = %s",
+
csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor,
+
osaf_extended_name_borrow(&csi_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName));
+ if (csi_set->ha == SA_AMF_HA_STANDBY)
+ TRACE("CSISet: ActiveCompName = %s,
StandbyRank = %u",
+
osaf_extended_name_borrow(&csi_set->csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName),
+
+csi_set->csi_desc.csiStateDescriptor.standbyDescriptor.standbyRank);
TRACE("Invoking component's
saAmfCSISetCallback: InvocationId = %llx, component name = %s",
info->inv,osaf_extended_name_borrow(&csi_set->comp_name));
reg_cbk->saAmfCSISetCallback(info->inv,
diff --git a/osaf/libs/common/amf/n2avamsg.c b/osaf/libs/common/amf/n2avamsg.c
--- a/osaf/libs/common/amf/n2avamsg.c
+++ b/osaf/libs/common/amf/n2avamsg.c
@@ -333,17 +333,15 @@ uint32_t avsv_amf_cbk_copy(AVSV_AMF_CBK_
/* Copy csi state description */
if (scbk->param.csi_set.ha == SA_AMF_HA_ACTIVE) {
- if
(osaf_is_an_extended_name(&scbk->param.csi_set.csi_desc.csiStateDescriptor.activeDescriptor.activeCompName))
{
-
osaf_extended_name_alloc(osaf_extended_name_borrow(&scbk->param.csi_set.csi_desc.csiStateDescriptor.activeDescriptor.activeCompName),
-
&(*o_dcbk)->param.csi_set.csi_desc.csiStateDescriptor.activeDescriptor.activeCompName);
- }
+ //osaf_extended_name_alloc() takes care of long and
short dn.
+
osaf_extended_name_alloc(osaf_extended_name_borrow(&scbk->param.csi_set.csi_desc.csiStateDescriptor.activeDescriptor.activeCompName),
+
+&(*o_dcbk)->param.csi_set.csi_desc.csiStateDescriptor.activeDescriptor.
+activeCompName);
}
if (scbk->param.csi_set.ha == SA_AMF_HA_STANDBY) {
- if
(osaf_is_an_extended_name(&scbk->param.csi_set.csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName))
{
-
osaf_extended_name_alloc(osaf_extended_name_borrow(&scbk->param.csi_set.csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName),
-
&(*o_dcbk)->param.csi_set.csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName);
- }
+ //osaf_extended_name_alloc() takes care of long and
short dn.
+
osaf_extended_name_alloc(osaf_extended_name_borrow(&scbk->param.csi_set.csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName),
+
+&(*o_dcbk)->param.csi_set.csi_desc.csiStateDescriptor.standbyDescriptor
+.activeCompName);
}
/* copy the amf csi attr list */
diff --git a/osaf/services/saf/amf/amfd/util.cc
b/osaf/services/saf/amf/amfd/util.cc
--- a/osaf/services/saf/amf/amfd/util.cc
+++ b/osaf/services/saf/amf/amfd/util.cc
@@ -957,7 +957,7 @@ uint32_t avd_snd_susi_msg(AVD_CL_CB *cb,
}
}
} else {
-
osaf_extended_name_alloc(osaf_extended_name_borrow(&l_compcsi->csi->list_compcsi->comp->comp_info.name),
+
+osaf_extended_name_alloc(osaf_extended_name_borrow(&l_compcsi->csi_csic
+omp_next->comp->comp_info.name),
&compcsi_info->active_comp_name);
if ((trans_dsc == SA_AMF_CSI_QUIESCED)
&& diff --git a/osaf/services/saf/amf/amfnd/comp.cc
b/osaf/services/saf/amf/amfnd/comp.cc
--- a/osaf/services/saf/amf/amfnd/comp.cc
+++ b/osaf/services/saf/amf/amfnd/comp.cc
@@ -2027,7 +2027,6 @@ uint32_t avnd_comp_cbk_send(AVND_CB *cb,
&csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName);
csi_desc.csiStateDescriptor.standbyDescriptor.standbyRank =
curr_csi->standby_rank;
-
osaf_extended_name_clear(&csi_desc.csiStateDescriptor.activeDescriptor.activeCompName);
}
/* copy the attributes */
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
