Ack with that minor change.
Thanks
Praveen
On 02-Mar-17 1:37 PM, minh chau wrote:
> Hi Praveen,
>
> Two comments in line.
>
> Thanks,
> Minh
>
> On 02/03/17 18:06, praveen malviya wrote:
>> Hi Minh,
>>
>> With the patch opensaf stop is successful but component is not issued
>> remove callback.
>> When comp is responding for csi assignment, AMFND is not sending
>> remove callback to the component (attached amfnd traces). In
>> avnd_comp_csi_assign_done(), call to avnd_su_si_oper_done() marks the
>> SI REMOVED and the same function start clean up of all the component.
>> Since SI is marked REMOVED, AMFND does not issue remove callback even
>> though pending_removal flag is true.
>> Minor correction on the patch makes it successful:
>> diff --git a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc
>> --- a/src/amf/amfnd/comp.cc
>> +++ b/src/amf/amfnd/comp.cc
>> @@ -1614,7 +1614,7 @@ uint32_t avnd_comp_csi_assign_done(AVND_
>>
>> if (find_unassigned_csi_at_rank(csi->si, rank)
>> != nullptr) {
>> rc = assign_all_csis_at_rank(csi->si,
>> rank, true);
>> - } else {
>> + } else if (csi->pending_removal == false) {
>> /* all csis belonging to the si are
>> assigned */
>> rc = avnd_su_si_oper_done(cb,
>> comp->su, m_AVND_SU_IS_ALL_SI(comp->su) ? nullptr : csi->si);
> [Minh]: Thanks, will add this correction to the patch and test again
>> }
>>
>> Also, what if we move the call to the function
>> avnd_comp_cbq_csi_rec_del() in the first if-else block where CSI(s)
>> is/are marked REMOVED if it/they is/are in REMOVING state instead of
>> second if-else block.
> [Minh]: The change is only calling avnd_comp_cbq_csi_rec_del() if it's
> not in ASSIGNING state, moving this change to first block that means
> this change will affect on "if (AVSV_SUSI_ACT_DEL ==
> csi->single_csi_add_rem_in_si)", so it will not be equivalent to
> existing logic, unless making another check of @single_csi_add_rem_in_si
> in the first if-else block
>>
>> Thanks,
>> Praveen
>>
>>
>> On 28-Feb-17 8:19 AM, Minh Hon Chau wrote:
>>> src/amf/amfnd/comp.cc | 13 ++++++++++---
>>> 1 files changed, 10 insertions(+), 3 deletions(-)
>>>
>>>
>>> If node is shutting down, and csi is in assigning state, amfnd
>>> deletes cbk info which
>>> is waiting for response of pending callback. When component responds
>>> and cbk is deleted,
>>> amfnd can not remove csi assignment. Thus, amfnd gets stuck in
>>> shutting down.
>>>
>>> Patch makes amfnd not delete cbk info if csi is in assigning state,
>>> respectively variable
>>> pending_removal is true, and keep cbk info deleted in all other cases.
>>>
>>> diff --git a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc
>>> --- a/src/amf/amfnd/comp.cc
>>> +++ b/src/amf/amfnd/comp.cc
>>> @@ -1727,7 +1727,7 @@ uint32_t avnd_comp_csi_remove_done(AVND_
>>> osafassert(m_AVND_SU_IS_PREINSTANTIABLE(comp->su));
>>>
>>> /* delete any pending cbk rec for csi assignment / removal */
>>> - avnd_comp_cbq_csi_rec_del(cb, comp, (csi) ? csi->name : "");
>>> +
>>>
>>> /* ok, time to reset CSi_ALL flag */
>>> if (!csi && m_AVND_COMP_IS_ALL_CSI(comp)) {
>>> @@ -1755,6 +1755,7 @@ uint32_t avnd_comp_csi_remove_done(AVND_
>>> */
>>> if (csi) {
>>> if (AVSV_SUSI_ACT_DEL == csi->single_csi_add_rem_in_si) {
>>> + avnd_comp_cbq_csi_rec_del(cb, comp, csi->name);
>>> /* csi belonging to the si are removed */
>>> rc = avnd_su_si_oper_done(cb, comp->su, csi->si);
>>>
>>> @@ -1762,6 +1763,11 @@ uint32_t avnd_comp_csi_remove_done(AVND_
>>> goto done;
>>> }
>>> else {
>>> + /* Delete cbk info if csi is not ASSIGNING state,
>>> @pending_removal will be true */
>>> + if (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNING(csi)
>>> == false) {
>>> + avnd_comp_cbq_csi_rec_del(cb, comp, csi->name);
>>> + }
>>> +
>>> for (curr_csi = (AVND_COMP_CSI_REC
>>> *)m_NCS_DBLIST_FIND_LAST(&csi->si->csi_list);
>>> curr_csi;
>>> curr_csi = (AVND_COMP_CSI_REC
>>> *)m_NCS_DBLIST_FIND_PREV(&curr_csi->si_dll_node)) {
>>> @@ -1770,7 +1776,7 @@ uint32_t avnd_comp_csi_remove_done(AVND_
>>> else if
>>> (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_REMOVING(curr_csi))
>>> break;
>>> else if
>>> (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNING(curr_csi)) {
>>> - TRACE("'%s' is getting assigned, remove it after
>>> assignment",
>>> + LOG_WA("'%s' is getting assigned, remove it
>>> after assignment",
>>> curr_csi->name.c_str());
>>> curr_csi->pending_removal = true;
>>> break;
>>> @@ -1787,7 +1793,8 @@ uint32_t avnd_comp_csi_remove_done(AVND_
>>> rc = avnd_su_si_oper_done(cb, comp->su,
>>> m_AVND_SU_IS_ALL_SI(comp->su) ? nullptr :
>>> csi->si);
>>> }
>>> - } else {
>>> + } else {
>>> + avnd_comp_cbq_csi_rec_del(cb, comp, "");
>>> /* Issue remove callback with TARGET_ALL for CSIs belonging
>>> to prv rank.*/
>>> for (curr_csi =
>>> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_FIRST(&comp->csi_list));
>>>
>>> curr_csi;
>>>
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
