ack, code review only. Minor comments below/Thanks HansN

On 11/04/2015 07:11 AM, [email protected] wrote:
>   osaf/services/saf/amf/amfnd/comp.cc             |  22 ++++++++++++++++++++++
>   osaf/services/saf/amf/amfnd/include/avnd_comp.h |   2 ++
>   osaf/services/saf/amf/amfnd/include/avnd_su.h   |   1 +
>   osaf/services/saf/amf/amfnd/su.cc               |   9 +++++++++
>   osaf/services/saf/amf/amfnd/susm.cc             |  17 +++++++++++++++++
>   5 files changed, 51 insertions(+), 0 deletions(-)
>
>
> First user performs restart admin operation on SU.
> After this shutdown operation is performed which left SG in unstable state.
>
> Configuration used in the issue configures compinstantiationlevel for a
> NPI component and next instantiantionlevel belogns to a PI comp. Since a NPI
> component is instantiated when it is assigned some work load.
> Issue is reproducible when a configuration meets following criteria:
> -Atleast one non-restartable component is present in the configuration.
> -All non restartable component should be unassigned.
> -NPI comp should be restartable.
> -CompInstantiationlevel is configured in such a way that last
> instantiationlevel belongs to a PI component which is non-restartable and
> unassigned.
>
> When SU restart operation is invoked, AMFND picks the last
> component which is an unassigned non-restartable PI comp and starts terminaing
> it (avnd_su_pres_inst_surestart_hdler). After successful termination, next
> component is a NPI componenti.AMFND skips it and starts terminating next PI 
> comp
> (avnd_su_pres_terming_compuninst_hdler). When SU moves to Instantiated state 
> and
> AMF tries to reassgin SIs, SI assigned to NPI comp remains in Assigning state 
> as
> AMFND could not instantiate the component (it was not terminated).
> After this when user tries shutdown operation on SU and AMFND sends quiescing
> state to AMFND, this assignment gets postponed as AMFND waits infinitely for
> previous assignment because of surestart is still pending.
>
> Patch fixes the problem by terminating the NPI comp in a non-restartable SU
> during RESTART admin op on SU.
>
> 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
> @@ -2908,3 +2908,25 @@ void clear_error_report_alarm(AVND_COMP
>               comp->error_report_sent = false;
>       }
>   }
> +
> +/**
> + * @brief  Checks if comp is nonrestartable (DisableRestart=1).
> + * @param  comp
> + * @return true/false
> + */
[HansN] Use const|| qualifier for the function
> +bool nonrestartable(const AVND_COMP *comp)
> +{
> +     TRACE("comp nonrestartable:%u",m_AVND_COMP_IS_RESTART_DIS(comp));
> +     return (m_AVND_COMP_IS_RESTART_DIS(comp));
> +}
> +
> +/**
> + * @brief  Count no. of CSIs this comp is handling.
> + * @param  comp
> + * @return uint32_t
> + */
[HansN] Use const|| qualifier for the function
> +uint32_t csi_count(const AVND_COMP *comp)
> +{
> +        return (comp->csi_list.n_nodes);
> +}
> +
> 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
> @@ -930,5 +930,7 @@ bool comp_has_quiesced_assignment(const
>   extern uint32_t comp_restart_initiate(AVND_COMP *comp);
>   extern void comp_reset_restart_count(AVND_COMP *comp);
>   extern void clear_error_report_alarm(AVND_COMP *comp);
> +bool nonrestartable(const AVND_COMP *comp);
> +uint32_t csi_count(const AVND_COMP *comp);
>   
>   #endif   /* !AVND_COMP_H */
> diff --git a/osaf/services/saf/amf/amfnd/include/avnd_su.h 
> b/osaf/services/saf/amf/amfnd/include/avnd_su.h
> --- a/osaf/services/saf/amf/amfnd/include/avnd_su.h
> +++ b/osaf/services/saf/amf/amfnd/include/avnd_su.h
> @@ -420,4 +420,5 @@ bool pi_su_all_comps_uninstantiated (con
>   bool is_any_non_restartable_comp_assigned(const AVND_SU& su);
>   bool su_all_pi_comps_instantiated(const AVND_SU *su);
>   bool all_csis_in_assigned_state(const AVND_SU *su);
[HansN] Use const|| qualifier for the function. Perhaps a better name is 
isAdminRestarted(const AVND_SU *su) const?
> +bool restart_admin_op(const AVND_SU *su);
>   #endif
> diff --git a/osaf/services/saf/amf/amfnd/su.cc 
> b/osaf/services/saf/amf/amfnd/su.cc
> --- a/osaf/services/saf/amf/amfnd/su.cc
> +++ b/osaf/services/saf/amf/amfnd/su.cc
> @@ -849,3 +849,12 @@ bool su_all_pi_comps_instantiated(const
>       TRACE("All PI comps instantiated :'%u'",su_is_instantiated);
>       return su_is_instantiated;
>   }
> +/**
> + * @brief Checks if RESTART admin op is going on SU.
> + * @return true/false
> + */
> +bool restart_admin_op(const AVND_SU *su)
> +{
> +     return (su->admin_op_Id == SA_AMF_ADMIN_RESTART);
> +}
> +
> 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
> @@ -2876,6 +2876,23 @@ uint32_t avnd_su_pres_terming_compuninst
>                                       if (NCSCC_RC_SUCCESS != rc)
>                                               goto done;
>                                       break;
> +                             } else {
> +                                     /*
> +                                        For a NPI comp in PI SU, component 
> FSM is always triggered
> +                                        at the time of assignments. If this 
> component is non-restartable
> +                                        then start reassginment from the 
> whole SU now, it will take care
> +                                        if its termination/clean up.
> +                                      */
> +                                     if ((nonrestartable(curr_comp) == true) 
> && (csi_count(curr_comp) > 0)) {
> +                                             TRACE("Start reassignment to 
> different SU as '%s' is"
> +                                                             " not 
> restartable",curr_comp->name.value);
> +                                             
> su_send_suRestart_recovery_msg(su);
> +                                             goto done;
> +                                     } else  if (restart_admin_op(su) == 
> true) {
> +                                             rc = avnd_comp_clc_fsm_run(cb, 
> curr_comp, AVND_COMP_CLC_PRES_FSM_EV_RESTART);
> +                                             if (curr_comp->pres == 
> SA_AMF_PRESENCE_RESTARTING)
> +                                                     goto done;
> +                                     }
>                               }
>                       }               
>                       if (pi_su_all_comps_uninstantiated(*su) == true)

------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to