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