I have tested and the patch is good but:
1) the same problem as for Node LOCK exist for SU LOCK, this means that 
su_complete_admin_op() should also be called with OK and not TIMEOUT
I think it is better to fix this in this ticket rather than creating a new one 
since it touches the same logic and are related.

2) I don't think we should always return OK the admin op. We should just do it 
in the case of admin state LOCKED (and perhaps SHUTTING-DOWN) and other cases 
(UNLOCKED) we should probably return TIMEOUT. If you do SU unlock and get oper 
state DISABLED back we probably want to reflect that back to the admin op 
issuer.

Something like:

If (su->adm_state == LOCKED)
        su_complete_admin_op(su, OK)
else
        su_complete_admin_op(su, TMO)

If you agree can you send out a new patch?

Thanks,
Hans


> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> Sent: den 9 januari 2014 13:17
> To: Hans Feldt; [email protected]
> Cc: [email protected]
> Subject: [PATCH 1 of 1] amfd : clear node level admin op related flags during 
> sufailover [#663]
> 
>  osaf/services/saf/amf/amfd/sgproc.cc |  31 +++++++++++++++++++++++++++++++
>  1 files changed, 31 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/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
> @@ -256,6 +256,23 @@ static void su_try_repair(const AVD_SU *
>  }
> 
>  /**
> + * @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
> + *
> + */
> +static 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);
> +     }
> +}
> +
> +/**
>   * @brief    Perform failover of SU as a single entity and free all SUSIs 
> associated with this SU.
>   * @param    su
>   *
> @@ -277,6 +294,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_OK);
> +     }
> 
>       /*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) {

------------------------------------------------------------------------------
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