ACK with previous comments
On 09/23/2016 10:48 AM, Lennart Lund wrote: > My own comments: > > 1) > Do not set m_errno in object delete method. Not used, not needed. Instead use > internal ais_rc where needed > > See also [Lennart] inline > > /Lennart > >> -----Original Message----- >> From: Lennart Lund [mailto:[email protected]] >> Sent: den 22 september 2016 15:34 >> To: Rafael Odzakow <[email protected]>; >> [email protected] >> Cc: [email protected] >> Subject: [devel] [PATCH 1 of 1] smf: Recreate IMM handles if bad handle >> when deleting node group [#2046] >> >> osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc | 49 >> +++++++++++++++++-------- >> 1 files changed, 33 insertions(+), 16 deletions(-) >> >> >> Fix the deleteNodeGroup() method must be so that if the delete operation >> fails >> with bad handles new handles are created so that the node group can be >> deleted >> >> diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc >> b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc >> --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc >> +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc >> @@ -3580,22 +3580,36 @@ bool SmfAdminOperation::deleteNodeGroup( >> &nodeGroupName); >> >> TRACE("\t Deleting nodeGroup '%s'", nodeGroupName_s.c_str()); >> - >> - m_errno = immutil_saImmOmCcbObjectDelete(m_ccbHandle, >> - &nodeGroupName); >> - if (m_errno != SA_AIS_OK) { >> - LOG_NO("%s: saImmOmCcbObjectDelete '%s' Fail %s", >> - __FUNCTION__, nodeGroupName_s.c_str(), >> - saf_error(m_errno)); >> - rc = false; >> - } else { >> - m_errno = saImmOmCcbApply(m_ccbHandle); >> - if (m_errno != SA_AIS_OK) { >> - LOG_NO("%s: saImmOmCcbApply() Fail '%s'", >> - __FUNCTION__, saf_error(m_errno)); >> - rc = false; >> - } >> - } >> + while (true) { >> + m_errno = immutil_saImmOmCcbObjectDelete(m_ccbHandle, >> + &nodeGroupName); >> + if (m_errno == SA_AIS_ERR_BAD_HANDLE) { >> + // Handles may have been invalidated/timed out >> + (void)immutil_saImmOmFinalize(m_omHandle); >> + bool rc = getAllImmHandles(); > [Lennart] Incorrect. Is mixed up with merthod return code. Separate method > return code and this internal return code. Set method return code if > getAllImmHandles() fail. > >> + if (rc == false) { >> + LOG_NO("%s: getAllImmHandles Fail", >> + __FUNCTION__); >> + break; >> + } >> + continue; >> + } >> + if (m_errno != SA_AIS_OK) { >> + LOG_NO("%s: saImmOmCcbObjectDelete '%s' Fail %s", >> + __FUNCTION__, nodeGroupName_s.c_str(), >> + saf_error(m_errno)); >> + rc = false; >> + break; >> + } else { >> + m_errno = saImmOmCcbApply(m_ccbHandle); >> + if (m_errno != SA_AIS_OK) { >> + LOG_NO("%s: saImmOmCcbApply() Fail '%s'", >> + __FUNCTION__, saf_error(m_errno)); >> + rc = false; >> + } >> + break; >> + } >> + } >> >> TRACE_LEAVE(); >> return rc; >> @@ -3633,6 +3647,9 @@ bool SmfAdminOperation::nodeGroupAdminOp >> &nodeGroupName, 0, adminOp, params, >> &oi_rc, smfd_cb->adminOpTimeout); >> >> + if (m_errno == SA_AIS_OK && oi_rc == SA_AIS_OK) >> + break; >> + >> if (retry <= 0) { >> LOG_NO("Fail to invoke admin operation, too many >> OI " >> "TRY_AGAIN, giving up. %s", >> __FUNCTION__); >> >> ------------------------------------------------------------------------------ >> _______________________________________________ >> Opensaf-devel mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
