Ack with comments inline.

I prefer we keep setting the PEND state (calling m_AVND_SU_ASSIGN_PEND_SET) in 
one place. Below I have proposed a way to 
solve that.

Thanks,
Hans

On 10/15/2013 08:21 AM, [email protected] wrote:
>   osaf/services/saf/amf/amfnd/di.cc   |  31 +++++++++++++++++++++++++++++--
>   osaf/services/saf/amf/amfnd/susm.cc |   2 ++
>   osaf/services/saf/amf/amfnd/term.cc |   1 +
>   3 files changed, 32 insertions(+), 2 deletions(-)
>
>
> When responding to AMFD for any assignment, AMFND should call 
> m_AVND_SU_ASSIGN_PEND_RESET(su) only
> after checking the assignment states of all SIs in SU. if any SI is still in 
> assigning/removing state then
> flag should not be reset. Change of this patch is needed in situation when 
> AMFND gets per susi level
> assignments from AMFD like SI dep related configuration or during fresh 
> assignments.
>
> diff --git a/osaf/services/saf/amf/amfnd/di.cc 
> b/osaf/services/saf/amf/amfnd/di.cc
> --- a/osaf/services/saf/amf/amfnd/di.cc
> +++ b/osaf/services/saf/amf/amfnd/di.cc
> @@ -505,6 +505,32 @@ uint32_t avnd_di_oper_send(AVND_CB *cb,
>       return rc;
>   }
>
> +/*
> +* @brief       Checks if any SI assigned to SU is in assigning/removing 
> state.

Check if all SI assignments for this SU are in stable state (not 
assigning/removing).

> +*
> +* @param [in]  ptr to SU
> +*
> +* @returns     true/false
> +*/
> +
> +static bool avnd_su_all_sis_in_assigned_state(const AVND_SU *su)

change name to su_assign_state_is_stable

> +{
> +     AVND_SU_SI_REC *curr_si;
> +     bool all_csi_assigned = true;

bool not needed, return direct instead

> +
> +     for (curr_si = (AVND_SU_SI_REC *)m_NCS_DBLIST_FIND_FIRST(&su->si_list);
> +                     curr_si && all_csi_assigned;

remove all_csi_assigned from condition, not needed.

> +                     curr_si = (AVND_SU_SI_REC 
> *)m_NCS_DBLIST_FIND_NEXT(&curr_si->su_dll_node)) {
> +             if (m_AVND_SU_SI_CURR_ASSIGN_STATE_IS_ASSIGNING(curr_si) ||
> +                             
> m_AVND_SU_SI_CURR_ASSIGN_STATE_IS_REMOVING(curr_si)) {
> +                     all_csi_assigned = false;
> +                     break;

return instead of break

> +             }
> +     }
> +
> +     return all_csi_assigned;
> +}

Something like:


static bool su_assign_state_is_stable(const AVND_SU *su)
{
        const AVND_SU_SI_REC *si;

        for (si = (AVND_SU_SI_REC *)m_NCS_DBLIST_FIND_FIRST(&su->si_list);
                        si;
                        si = (AVND_SU_SI_REC 
*)m_NCS_DBLIST_FIND_NEXT(&si->su_dll_node)) {
                if (m_AVND_SU_SI_CURR_ASSIGN_STATE_IS_ASSIGNING(si) ||
                                m_AVND_SU_SI_CURR_ASSIGN_STATE_IS_REMOVING(si)) 
{
                        return false;
                        break;
                }
        }

        return true;
}


> +
>   
> /****************************************************************************
>     Name          : avnd_di_susi_resp_send
>
> @@ -529,7 +555,7 @@ uint32_t avnd_di_susi_resp_send(AVND_CB
>       memset(&msg, 0, sizeof(AVND_MSG));

add an if statement here instead:

        if (cb->term_state == AVND_TERM_STATE_OPENSAF_SHUTDOWN_STARTED)
                return rc;

>
>       // should be in assignment pending state to be here
> -     osafassert(m_AVND_SU_ASSIGN_PEND_SET(su));
> +     osafassert(m_AVND_SU_IS_ASSIGN_PEND(su));
>
>       /* get the curr-si */
>       curr_si = (si) ? si : (AVND_SU_SI_REC 
> *)m_NCS_DBLIST_FIND_FIRST(&su->si_list);
> @@ -593,7 +619,8 @@ uint32_t avnd_di_susi_resp_send(AVND_CB
>                       msg.info.avd = 0;
>
>               /* we have completed the SU SI msg processing */
> -             m_AVND_SU_ASSIGN_PEND_RESET(su);
> +             if (avnd_su_all_sis_in_assigned_state(su))
> +                     m_AVND_SU_ASSIGN_PEND_RESET(su);
>               m_AVND_SU_ALL_SI_RESET(su);
>               m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, 
> AVND_CKPT_SU_FLAG_CHANGE);
>       } else
> 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
> @@ -957,6 +957,7 @@ uint32_t avnd_su_si_oper_done(AVND_CB *c
>                               uint32_t sirank = tmp->rank;
>
>                               for (; tmp && (tmp->rank == sirank); tmp = 
> avnd_silist_getprev(tmp)) {
> +                                     m_AVND_SU_ASSIGN_PEND_SET(tmp->su);

removed with above change

>                                       uint32_t rc = avnd_su_si_remove(cb, 
> tmp->su, tmp);
>                                       osafassert(rc == NCSCC_RC_SUCCESS);
>                               }
> @@ -1025,6 +1026,7 @@ uint32_t avnd_su_si_oper_done(AVND_CB *c
>                       uint32_t sirank = tmp->rank;
>
>                       for (; tmp && (tmp->rank == sirank); tmp = 
> avnd_silist_getprev(tmp)) {
> +                             m_AVND_SU_ASSIGN_PEND_SET(tmp->su);

removed with above change

>                               uint32_t rc = avnd_su_si_remove(cb, tmp->su, 
> tmp);
>                               osafassert(rc == NCSCC_RC_SUCCESS);
>                       }
> diff --git a/osaf/services/saf/amf/amfnd/term.cc 
> b/osaf/services/saf/amf/amfnd/term.cc
> --- a/osaf/services/saf/amf/amfnd/term.cc
> +++ b/osaf/services/saf/amf/amfnd/term.cc
> @@ -168,6 +168,7 @@ uint32_t avnd_evt_last_step_term_evh(AVN
>               if (currsi->curr_assign_state == 
> AVND_SU_SI_ASSIGN_STATE_REMOVED)
>                       continue;
>
> +             m_AVND_SU_ASSIGN_PEND_SET(currsi->su);

removed with above change

>               uint32_t rc = avnd_su_si_remove(cb, currsi->su, currsi);
>               osafassert(rc == NCSCC_RC_SUCCESS);
>               si_removed = true;
>
>

------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135031&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to