Hi Nagu, Thanks for testing. Please test as much as you want. I'm worried if the patch causes degradation since it touches the assignment sequence, which is not a simple logic because of recursion in amfnd code.
Thanks, Minh On 8/11/2015 9:32 PM, Nagendra Kumar wrote: > Hi Minh, > Thanks for the patch. > > I tested the following scenarios and it worked fine (Configurations: 3 Comps > and 3 CSIs with CSI dep configured) : > > 1. Unlock SU and report error from comp1 in Act assignment. > 2. Unlock SI and report error from comp1 in Act assignment. > > I also tested some basic tests with error report like SU lock/unlock, SI > lock/unlock. > > I will commit it after running some more tests. > > Ack. > > Thanks > -Nagu > >> -----Original Message----- >> From: Minh Hon Chau [mailto:[email protected]] >> Sent: 06 August 2015 10:23 >> To: [email protected]; Nagendra Kumar; Praveen Malviya; >> [email protected] >> Cc: [email protected] >> Subject: [PATCH 1 of 1] AMFND: Comp receives QUIESCED csi when it has not >> received ACTIVE csi [#1386] V4 >> >> osaf/services/saf/amf/amfnd/comp.cc | 47 ++++++++++++++++++--- >> osaf/services/saf/amf/amfnd/include/avnd_comp.h | 9 ++++ >> osaf/services/saf/amf/amfnd/sidb.cc | 2 +- >> osaf/services/saf/amf/amfnd/susm.cc | 55 >> +++++++++++++++++++----- >> 4 files changed, 92 insertions(+), 21 deletions(-) >> >> >> If su-si active assignment is on-going, some csi(s) may get assigned but >> other >> csi(s) could be still in unassigned state. At this point, assigning component >> issues saAmfErrorReport(COMPONENT_FAILOVER), AMFND receives QUIESCED >> assignment from AMFD. The unassigned yet component also receives the >> quiesced csi callback that should not happen >> >> Adding new flag suspending_assignment, which indicates the unassigned csi(s) >> will not involve in QUIESCED/QUIESCING assignment loop. Once su-si >> QUIESCED/QUIESCING assignment is done, this flag reset to false. >> >> 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 >> @@ -906,6 +906,7 @@ uint32_t avnd_comp_csi_assign(AVND_CB *c >> /* flags to indicate the prv & curr inst states of an npi comp */ >> bool npi_prv_inst = true, npi_curr_inst = true; >> AVND_COMP_CSI_REC *curr_csi = 0; >> + AVND_COMP_CSI_REC *t_csi = 0; >> AVND_COMP_CLC_PRES_FSM_EV comp_ev = >> AVND_COMP_CLC_PRES_FSM_EV_MAX; >> bool mark_csi = false; >> uint32_t rc = NCSCC_RC_SUCCESS; >> @@ -1000,11 +1001,29 @@ uint32_t avnd_comp_csi_assign(AVND_CB *c >> if (NCSCC_RC_SUCCESS != rc) >> goto done; >> } else { >> - /* active/standby can be directly >> assigned */ >> - rc = avnd_comp_cbk_send(cb, comp, >> AVSV_AMF_CSI_SET, 0, 0); >> - if (NCSCC_RC_SUCCESS != rc) >> - goto done; >> - mark_csi = true; >> + if (curr_csi->curr_assign_state == >> AVND_COMP_CSI_ASSIGN_STATE_UNASSIGNED && >> + curr_csi->prv_assign_state == >> AVND_COMP_CSI_ASSIGN_STATE_UNASSIGNED) { >> + // Mark >> suspending_assignment for all unassigned csi(s) which are going >> + // to be assigned to *curr_csi- >>> comp* >> + for (t_csi = >> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_FIRST( >> &curr_csi->comp->csi_list)); >> + t_csi; >> + t_csi = >> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_NEXT( >> &t_csi->comp_dll_node))) { >> + if (t_csi- >>> curr_assign_state == AVND_COMP_CSI_ASSIGN_STATE_UNASSIGNED && >> + t_csi- >>> prv_assign_state == AVND_COMP_CSI_ASSIGN_STATE_UNASSIGNED) { >> + t_csi- >>> suspending_assignment = true; >> + } >> + } >> + // just generate csi-oper done >> for unassigned csi >> + rc = >> avnd_comp_csi_assign_done(cb, comp, 0); >> + if (NCSCC_RC_SUCCESS != rc) >> + goto done; >> + } else { >> + /* active/standby can be >> directly assigned */ >> + rc = avnd_comp_cbk_send(cb, >> comp, AVSV_AMF_CSI_SET, 0, 0); >> + if (NCSCC_RC_SUCCESS != rc) >> + goto done; >> + mark_csi = true; >> + } >> } >> } else { >> /* assign the csi as the comp is aware of >> atleast one csi */ @@ -1222,7 +1241,16 @@ uint32_t >> avnd_comp_csi_remove(AVND_CB *c >> curr_csi; >> curr_csi = >> >> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_NEXT( >> &curr_csi->comp_dll_node))) { >> - if >> ((m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNED(curr_csi)) >> + >> + // silently removing csi, don't issue callback >> + if >> (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_UNASSIGNED(curr_csi)) { >> + >> m_AVND_COMP_CSI_CURR_ASSIGN_STATE_SET(curr_csi, >> + >> AVND_COMP_CSI_ASSIGN_STATE_REMOVING); >> + >> m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, curr_csi, >> + >> AVND_CKPT_COMP_CSI_CURR_ASSIGN_STATE); >> + rc = avnd_comp_csi_remove_done(cb, >> comp, csi); >> + goto done; >> + } else if >> +((m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNED(curr_csi)) >> || >> (m_AVND_COMP_CSI_PRV_ASSIGN_STATE_IS_ASSIGNED(curr_csi)) >> || >> (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNING(curr_csi))) { >> if >> (!csi_of_same_si_in_removing_state(curr_csi->comp,curr_csi->si)) >> @@ -1404,6 +1432,10 @@ static bool all_csis_at_rank_assigned(st >> >> LOG_IN("Ignoring Failed/Unreg Comp'%s' comp pres >> state=%u, comp flag %x, su pres state %u", >> csi->comp->name.value, csi->comp->pres, csi- >>> comp->flag, csi->comp->su->pres); >> + } else if >> (m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(csi->comp) && >> + csi->suspending_assignment) { >> + LOG_IN("Ignoring quiescing/quiesced >> assignment on unassigned comp'%s'", >> + csi->comp->name.value); >> } else { >> TRACE_LEAVE2("false"); >> return false; >> @@ -1627,6 +1659,7 @@ bool all_csis_in_removed_state(const AVN >> AVND_COMP_CSI_REC *curr_csi; >> AVND_SU_SI_REC *curr_si; >> bool all_csi_removed = true; >> + TRACE_ENTER2("'%s'",su->name.value); >> >> for (curr_si = (AVND_SU_SI_REC *)m_NCS_DBLIST_FIND_FIRST(&su- >>> si_list); >> curr_si && all_csi_removed; >> @@ -1639,7 +1672,7 @@ bool all_csis_in_removed_state(const AVN >> } >> } >> } >> - >> + TRACE_LEAVE2("%u", all_csi_removed); >> return all_csi_removed; >> } >> >> /**************************************************************** >> ************ >> diff --git a/osaf/services/saf/amf/amfnd/include/avnd_comp.h >> b/osaf/services/saf/amf/amfnd/include/avnd_comp.h >> --- a/osaf/services/saf/amf/amfnd/include/avnd_comp.h >> +++ b/osaf/services/saf/amf/amfnd/include/avnd_comp.h >> @@ -192,6 +192,15 @@ typedef struct avnd_comp_csi_rec { >> removal callback only after CSI gets assigned. >> AMFND will mark this flag >> so that it can start removal after CSI is >> assigned. >> */ >> + bool suspending_assignment; /* This flag is used when su-si active >> assignment is on-going, >> + some other csi may get assigned but *this* csi >> is still in >> + unassigned state. At this point, AMFND >> receives QUIESCED or >> + QUIESCING assignment from AMFD. This csi >> has never got active >> + assignment so the quiesced csi callback >> should not be issued. >> + This flag is true, which indicates this csi is >> not >> involved >> + in QUIESCED/QUIESCING assignment loop. >> Once su-si QUIESCED/ >> + QUIESCING assignment is done, this flag reset >> to false. */ >> + >> } AVND_COMP_CSI_REC; >> >> #define AVND_COMP_CSI_REC_NULL ((AVND_COMP_CSI_REC *)0) diff --git >> a/osaf/services/saf/amf/amfnd/sidb.cc b/osaf/services/saf/amf/amfnd/sidb.cc >> --- a/osaf/services/saf/amf/amfnd/sidb.cc >> +++ b/osaf/services/saf/amf/amfnd/sidb.cc >> @@ -424,7 +424,7 @@ AVND_COMP_CSI_REC *avnd_su_si_csi_rec_ad >> csi_rec->si_name = si_rec->name; >> csi_rec->su_name = su->name; >> csi_rec->pending_removal = false; >> - >> + csi_rec->suspending_assignment = false; >> return csi_rec; >> >> err: >> diff --git a/osaf/services/saf/amf/amfnd/susm.cc >> b/osaf/services/saf/amf/amfnd/susm.cc >> --- a/osaf/services/saf/amf/amfnd/susm.cc >> +++ b/osaf/services/saf/amf/amfnd/susm.cc >> @@ -428,16 +428,24 @@ static bool csi_of_same_si_in_assigning_ { >> AVND_COMP_CSI_REC *curr_csi; >> bool all_csi_assigned = true; >> - >> + TRACE_ENTER(); >> for (curr_csi = >> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_FIRST( >> &cmp->csi_list)); >> curr_csi; >> curr_csi = >> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_NEXT( >> &curr_csi->comp_dll_node))) { >> + TRACE("curr_csi:'%s', prv_assign_state:%u >> curr_assign_state:%u, suspending_assignment:%u", >> + curr_csi->name.value, >> + curr_csi->prv_assign_state, >> + curr_csi->curr_assign_state, >> + curr_csi->suspending_assignment); >> if ((curr_csi->si == si) && >> !m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNING(curr_csi)) { >> - all_csi_assigned = false; >> - break; >> + //ignore suspending_assignment >> + if (!curr_csi->suspending_assignment) { >> + all_csi_assigned = false; >> + break; >> + } >> } >> } >> - >> + TRACE_LEAVE(); >> return all_csi_assigned; >> } >> /** >> @@ -521,14 +529,17 @@ static uint32_t assign_si_to_su(AVND_SU_ >> } >> /*if ((!single_csi) && (false == >> curr_csi- >>> comp->assigned_flag) && >> (m_AVND_COMP_CSI_PRV_ASSIGN_STATE_IS_ASSIGNED(curr_csi)))*/ >> - if ((false == curr_csi->comp- >>> assigned_flag) && >> - >> (m_AVND_COMP_CSI_PRV_ASSIGN_STATE_IS_ASSIGNED(curr_csi))) { >> - if >> (csi_of_same_si_in_assigning_state(curr_csi->comp, si)) >> - curr_csi- >>> comp->assigned_flag = true; >> + if (false == curr_csi->comp- >>> assigned_flag) { >> + if >> (csi_of_same_si_in_assigning_state(curr_csi->comp, si)) { >> + >> curr_csi->comp->assigned_flag = true; >> + } >> } >> } else { >> - >> m_AVND_COMP_CSI_CURR_ASSIGN_STATE_SET(curr_csi, >> AVND_COMP_CSI_ASSIGN_STATE_ASSIGNED); >> - >> m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, curr_csi, >> AVND_CKPT_COMP_CSI_CURR_ASSIGN_STATE); >> + // Don't move csi to ASSIGNED >> state if its assignment's suspending >> + if (!curr_csi- >>> suspending_assignment) { >> + >> m_AVND_COMP_CSI_CURR_ASSIGN_STATE_SET(curr_csi, >> AVND_COMP_CSI_ASSIGN_STATE_ASSIGNED); >> + >> m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, curr_csi, >> AVND_CKPT_COMP_CSI_CURR_ASSIGN_STATE); >> + } >> } >> } >> } >> @@ -828,12 +839,14 @@ static bool susi_operation_in_progress(A >> AVND_COMP_CSI_REC *curr_csi, *t_csi; >> bool opr_done = true; >> >> + TRACE_ENTER2("'%s' '%s'", su->name.value, si ? si->name.value : >> NULL); >> + >> for (curr_si = (si) ? si : (AVND_SU_SI_REC >> *)m_NCS_DBLIST_FIND_FIRST(&su->si_list); >> curr_si && opr_done; curr_si = (si) ? 0 : >> (AVND_SU_SI_REC *)m_NCS_DBLIST_FIND_NEXT(&curr_si->su_dll_node)) { >> for (curr_csi = (AVND_COMP_CSI_REC >> *)m_NCS_DBLIST_FIND_FIRST(&curr_si->csi_list); >> curr_csi; curr_csi = (AVND_COMP_CSI_REC >> *)m_NCS_DBLIST_FIND_NEXT(&curr_csi->si_dll_node)) { >> if >> (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNING(curr_csi) || >> m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_REMOVING(curr_csi)) { >> - opr_done = false; >> + opr_done = false; >> break; >> } else if (m_AVND_COMP_IS_FAILED(curr_csi->comp)){ >> for (t_si = (si) ? si : (AVND_SU_SI_REC >> *)m_NCS_DBLIST_FIND_FIRST(&su->si_list); >> @@ -847,8 +860,11 @@ static bool susi_operation_in_progress(A >> else if >> (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNING(t_csi) || >> >> m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_REMOVING(t_csi) || >> >> m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_UNASSIGNED(t_csi)) { >> - opr_done = false; >> - break; >> + >> + if (!t_csi- >>> suspending_assignment) { >> + opr_done = >> false; >> + break; >> + } >> } >> } >> } >> @@ -856,6 +872,19 @@ static bool susi_operation_in_progress(A >> } >> } >> >> + // reset suspending_assignment flag if opr_done >> + if (opr_done) { >> + for (curr_si = (si) ? si : (AVND_SU_SI_REC >> *)m_NCS_DBLIST_FIND_FIRST(&su->si_list); >> + curr_si; curr_si = (si) ? 0 : (AVND_SU_SI_REC >> *)m_NCS_DBLIST_FIND_NEXT(&curr_si->su_dll_node)) { >> + for (curr_csi = (AVND_COMP_CSI_REC >> *)m_NCS_DBLIST_FIND_FIRST(&curr_si->csi_list); >> + curr_csi; curr_csi = >> (AVND_COMP_CSI_REC *)m_NCS_DBLIST_FIND_NEXT(&curr_csi->si_dll_node)) >> { >> + if (curr_csi->suspending_assignment) >> + curr_csi->suspending_assignment = >> false; >> + } >> + } >> + } >> + >> + TRACE_LEAVE2("%u", opr_done); >> return opr_done; >> } >> ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
