Hi Minh,
Thanks for the review.
>> Do you think it's a problem? If not, then the patch is ok to me.
I also found the same thing. And this is fine and this is the purpose of
sending TRY_AGAIN that
if the previous command has failed and got TRY_AGAIN, then the next command
will succeed
if no fault has happened.
If you could check the trace, then in the first admin op, the admin state would
be UNLOCKED.
But after TRY_AGAIN, since the admin command succeeds, the admin state will be
locked.
Thanks
-Nagu
> -----Original Message-----
> From: minh chau [mailto:minh.c...@dektech.com.au]
> Sent: 20 February 2017 10:55
> To: Nagendra Kumar; hans.nordeb...@ericsson.com; Praveen Malviya;
> gary....@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] [PATCH 1 of 1] amfd: return TRY_AGAIN on rollback of
> shutdown admin op [#2133]
>
> Hi Nagu,
>
> When testing the patch, I notice one thing.
> My test scenario:
> - Setup 2N application, SU4 hosted in PL4, SU5 hosted in PL5, SU4/SU5 has
> Active/Standby assignment respectively.
> - Run amf-adm shutdown <SI> in SC1
> - delay csi quiescing callback for component of SU4
> - reboot PL4
>
> Now, when amf-adm is returned, the AdminState of <SI> is not rolled back to
> UNLOCKED, it now becomes to LOCKED.
> The reason is when OM receives TRY_AGAIN, the admin si shutdown is
> silently started again. At second time, the shutdown command completes.
>
> From user's perspective, the eventual result has changed comparing to the
> previous one. Do you think it's a problem? If not, then the patch is ok to me.
>
> Thanks,
> Minh
>
> On 14/02/17 16:21, Nagendra Kumar wrote:
> > Review please :)
> >
> >> -----Original Message-----
> >> From: Nagendra Kumar
> >> Sent: 07 February 2017 11:53
> >> To: hans.nordeb...@ericsson.com; Praveen Malviya;
> >> minh.c...@dektech.com.au; gary....@dektech.com.au
> >> Cc: opensaf-devel@lists.sourceforge.net
> >> Subject: [devel] [PATCH 1 of 1] amfd: return TRY_AGAIN on rollback of
> >> shutdown admin op [#2133]
> >>
> >> src/amf/amfd/sg_2n_fsm.cc | 17 +++++++++++++++++
> >> src/amf/amfd/sgproc.cc | 13 ++++++++++---
> >> 2 files changed, 27 insertions(+), 3 deletions(-)
> >>
> >>
> >> When shutdown operation on SI is issued and if there are some faults
> >> (component failover, SU failover or node failover) on components on
> >> SU getting quiescing csi cbk, then in the following situation
> >> TRY_AGAIN is returned and admin state is rolled back to unlock:
> >> For SI having only one SUSI:
> >> Without SI Dep : a.) in comp fo
> >> For SI having two SUSI:
> >> Without SI Dep : a.) in node fo, su fo, comp fo
> >> With SI Dep : a.) node fo, su fo
> >> In other cases below, the admin state is marked locked :
> >> For SI having only one SUSI:
> >> Without SI Dep : a.) node fo, su fo
> >> With SI Dep : a.) node fo, su fo, comp fo
> >> For SI having two SUSI:
> >> With SI Dep : a.) Comp fo
> >>
> >> diff --git a/src/amf/amfd/sg_2n_fsm.cc b/src/amf/amfd/sg_2n_fsm.cc
> >> --- a/src/amf/amfd/sg_2n_fsm.cc
> >> +++ b/src/amf/amfd/sg_2n_fsm.cc
> >> @@ -1,6 +1,7 @@
> >> /* -*- OpenSAF -*-
> >> *
> >> * (C) Copyright 2008 The OpenSAF Foundation
> >> + * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
> >> *
> >> * This program is distributed in the hope that it will be useful, but
> >> * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> MERCHANTABILITY @@ -1050,9 +1051,17 @@ uint32_t
> >> SG_2N::su_fault_si_oper(AVD_SU
> >> su->sg_of_su->admin_si-
> >>> set_admin_state(SA_AMF_ADMIN_LOCKED);
> >> else
> >> su->sg_of_su->admin_si-
> >>> set_admin_state(SA_AMF_ADMIN_UNLOCKED);
> >> + AVD_SI *si_tmp = su->sg_of_su->admin_si;
> >> m_AVD_CLEAR_SG_ADMIN_SI(cb, (su-
> >>> sg_of_su));
> >> avd_sg_su_oper_list_add(cb, su, false);
> >> su->sg_of_su-
> >>> set_fsm_state(AVD_SG_FSM_SU_OPER);
> >> + if ((si_tmp->invocation != 0) && (si_tmp-
> >>> saAmfSIAdminState == SA_AMF_ADMIN_UNLOCKED)) {
> >> + TRACE("Admin operation fails on
> >> SI:'%s'", si_tmp->name.c_str());
> >> +
> >> avd_saImmOiAdminOperationResult(avd_cb->immOiHandle,
> >> + si_tmp->invocation,
> >> SA_AIS_ERR_TRY_AGAIN);
> >> + si_tmp->invocation = 0;
> >> + }
> >> +
> >> } else {
> >> /* The SU has standby assignments. Change
> the SI admin state to
> >> * unlock. Remove the SI from the SI admin
> pointer.
> >> @@ -3150,8 +3159,16 @@ void SG_2N::node_fail_si_oper(AVD_SU *su
> >> }
> >>
> >> su->sg_of_su->admin_si-
> >>> set_admin_state(SA_AMF_ADMIN_UNLOCKED);
> >> + AVD_SI *si_tmp = su->sg_of_su->admin_si;
> >> m_AVD_CLEAR_SG_ADMIN_SI(cb, (su-
> >>> sg_of_su));
> >> su->delete_all_susis();
> >> + if (si_tmp->invocation != 0) {
> >> + TRACE("Admin operation fails on
> >> SI:'%s'", si_tmp->name.c_str());
> >> +
> >> avd_saImmOiAdminOperationResult(avd_cb->immOiHandle,
> >> + si_tmp->invocation,
> >> SA_AIS_ERR_TRY_AGAIN);
> >> + si_tmp->invocation = 0;
> >> + }
> >> +
> >> } /* if (s_susi != AVD_SU_SI_REL_NULL) */
> >> else {
> >> su->sg_of_su->admin_si-
> >>> set_admin_state(SA_AMF_ADMIN_LOCKED);
> >> diff --git a/src/amf/amfd/sgproc.cc b/src/amf/amfd/sgproc.cc
> >> --- a/src/amf/amfd/sgproc.cc
> >> +++ b/src/amf/amfd/sgproc.cc
> >> @@ -1,6 +1,7 @@
> >> /* -*- OpenSAF -*-
> >> *
> >> * (C) Copyright 2008 The OpenSAF Foundation
> >> + * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
> >> *
> >> * This program is distributed in the hope that it will be useful, but
> >> * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> MERCHANTABILITY @@ -518,9 +519,15 @@ static uint32_t
> >> sg_su_failover_func(AVD_
> >>
> >> /* Reply to IMM for admin operation on SI */
> >> if (susi->si->invocation != 0) {
> >> - avd_saImmOiAdminOperationResult(avd_cb-
> >>> immOiHandle,
> >> - susi->si->invocation,
> >> SA_AIS_OK);
> >> - susi->si->invocation = 0;
> >> + if ((susi->su->sg_of_su->admin_si != nullptr)
> >> &&
> >> + (susi->su->sg_of_su-
> >>> admin_si->saAmfSIAdminState == SA_AMF_ADMIN_SHUTTING_DOWN)
> &&
> >> + (susi->su->sg_of_su-
> >>> sg_redundancy_model == SA_AMF_2N_REDUNDANCY_MODEL)) {
> >> + TRACE("Do nothing.");
> >> + } else {
> >> +
> >> avd_saImmOiAdminOperationResult(avd_cb->immOiHandle,
> >> + susi->si->invocation,
> >> SA_AIS_OK);
> >> + susi->si->invocation = 0;
> >> + }
> >> }
> >> }
> >> su->sg_of_su->node_fail(avd_cb, su);
> >>
> >> ---------------------------------------------------------------------
> >> --------- 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
> >> Opensaf-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
------------------------------------------------------------------------------
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
