comments one comment under [rafael] tag.
On 09/22/2016 03:33 PM, Lennart Lund wrote:
> 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) {
[rafael] The while loop is here to retry CcbObjectDelete only one time.
A much clearer way is to call CcbObjectDelete twice. As it looks now I
needed to reread this section before I understood the logic. By trying
to avoid writing extra code you have complicated a simple function. Call
it twice instead of having a while loop that will retry after BAD_HANDLE
endless amounts of times. A possibility for an infinite loop is not a
good idea. After the second CcbObjectDelete you can fail normally.
Please explain in comments or remove this while loop.
> + 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();
> + 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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
