I have a similar problem and solution in my pending patch for 
http://sourceforge.net/p/opensaf/tickets/601/

I don't think we should return TIMEOUT, this can cause upgrades to fail. I mean 
the purpose of the LOCK operation (to 
un-assign the service) has been fulfilled albeit with some errors. So AMF 
should return OK to the IMM admin operation.

Another comment, if you move the node_complete_admin_op up you can remove the 
forward declaration and make it static 
(file local).

Will you send another patch with the above comments?

Thanks,
Hans

On 01/09/2014 07:39 AM, [email protected] wrote:
>   osaf/services/saf/amf/amfd/include/proc.h |   1 +
>   osaf/services/saf/amf/amfd/sgproc.cc      |  31 
> +++++++++++++++++++++++++++++++
>   2 files changed, 32 insertions(+), 0 deletions(-)
>
>
> When admin operation like lock and shutdown are performed on Node and this 
> leads to sufailover,
> AMFD is not clearing the operation related flag in node pointer. This patch 
> ensures if sufailover
> gets escalated when admin operation is performed on the node, then AMF will 
> clear operation related flag.
> Also AMF will respond to IMM for the completion of operation.
>
> diff --git a/osaf/services/saf/amf/amfd/include/proc.h 
> b/osaf/services/saf/amf/amfd/include/proc.h
> --- a/osaf/services/saf/amf/amfd/include/proc.h
> +++ b/osaf/services/saf/amf/amfd/include/proc.h
> @@ -161,6 +161,7 @@ extern AVD_SU *get_other_su_from_oper_li
>   extern void su_complete_admin_op(AVD_SU *su, SaAisErrorT result);
>   extern void comp_complete_admin_op(AVD_COMP *comp, SaAisErrorT result);
>   extern void su_disable_comps(AVD_SU *su, SaAisErrorT result);
> +extern void node_complete_admin_op(AVD_AVND *node, SaAisErrorT result);
>   extern bool cluster_su_instantiation_done(AVD_CL_CB *cb, AVD_SU *su);
>   extern void cluster_startup_expiry_event_generate(AVD_CL_CB *cb);
>
> diff --git a/osaf/services/saf/amf/amfd/sgproc.cc 
> b/osaf/services/saf/amf/amfd/sgproc.cc
> --- a/osaf/services/saf/amf/amfd/sgproc.cc
> +++ b/osaf/services/saf/amf/amfd/sgproc.cc
> @@ -277,6 +277,20 @@ static uint32_t sg_su_failover_func(AVD_
>       avd_su_readiness_state_set(su, SA_AMF_READINESS_OUT_OF_SERVICE);
>       su_complete_admin_op(su, SA_AIS_ERR_TIMEOUT);
>       su_disable_comps(su, SA_AIS_ERR_TIMEOUT);
> +     if (su->su_on_node->admin_node_pend_cbk.invocation != 0) {
> +
> +             /* Node level operation is going on the node hosting the SU for 
> which
> +                sufailover got escalated. Sufailover event will always come 
> after the
> +                initiation of node level operation on the list of SUs. So if 
> this SU has
> +                list of SUSIs, AMF would have sent assignment as a part of 
> node level operation.
> +                Now SUSIs in this SU are going to be deleted so we can 
> decrement the counter in node.
> +              */
> +             su->su_on_node->su_cnt_admin_oper--;
> +
> +             /* If node level operation is finished on all the SUs, reply to 
> imm.*/
> +             if (su->su_on_node->su_cnt_admin_oper == 0)
> +                     node_complete_admin_op(su->su_on_node, 
> SA_AIS_ERR_TIMEOUT);
> +     }
>
>       /*If the AvD is in AVD_APP_STATE then reassign all the SUSI assignments 
> for this SU */
>       if (avd_cb->init_state == AVD_APP_STATE) {
> @@ -2116,3 +2130,20 @@ void su_disable_comps(AVD_SU *su, SaAisE
>               m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(avd_cb, comp, 
> AVSV_CKPT_AVD_COMP_CONFIG);
>       }
>   }
> +
> +/**
> + * @brief    This function completes admin operation on node.
> + *              It responds IMM with the result of admin operation on node.
> + * @param    ptr to node
> + * @param    result
> + *
> + */
> +void node_complete_admin_op(AVD_AVND *node, SaAisErrorT result)
> +{
> +     if (node->admin_node_pend_cbk.invocation != 0) {
> +             avd_saImmOiAdminOperationResult(avd_cb->immOiHandle,
> +                             node->admin_node_pend_cbk.invocation, result);
> +             node->admin_node_pend_cbk.invocation = 0;
> +             node->admin_node_pend_cbk.admin_oper = 
> static_cast<SaAmfAdminOperationIdT>(0);
> +     }
> +}
>
>

------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to