ACK, with comments
On 09/23/2016 10:55 AM, Lennart Lund wrote: > My own review comments: > > See [Lennart] inline > > /Lennart > >> -----Original Message----- >> From: Lennart Lund [mailto:[email protected]] >> Sent: den 22 september 2016 17:04 >> To: Rafael Odzakow <[email protected]>; >> [email protected] >> Cc: [email protected] >> Subject: [devel] [PATCH 1 of 1] smf: Delete node group if already exist when >> creating [#2049] >> >> osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc | 51 >> ++++++++++++++++--------- >> 1 files changed, 33 insertions(+), 18 deletions(-) >> >> >> If a node group for admin operation already exist when create is done the >> existing group shall be deleted so that the new group can be created >> >> 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 >> @@ -3537,24 +3537,39 @@ bool SmfAdminOperation::createNodeGroup( >> >> // ------------------------------------ >> // Create the node group >> - m_errno = immutil_saImmOmCcbObjectCreate_2( >> - m_ccbHandle, >> - className, >> - &nodeGroupParentDn, >> - attrValues); >> - >> - if (m_errno != SA_AIS_OK) { >> - LOG_NO("%s: saImmOmCcbObjectCreate_2() '%s' Fail %s", >> - __FUNCTION__, nGnodeName, >> 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] suggestion is to not use while loop, instead call ObjectCreate twice >> + m_errno = immutil_saImmOmCcbObjectCreate_2( >> + m_ccbHandle, >> + className, >> + &nodeGroupParentDn, >> + attrValues); >> + >> + if (m_errno == SA_AIS_ERR_EXIST) { >> + // A node group with the same name already exist >> + // May happen if delete after usage has failed >> + bool rc = deleteNodeGroup(); > [Lennart] Incorrect scope of rc > >> + if (rc == false) { >> + LOG_NO("%s: deleteNodeGroup() Fail", >> + __FUNCTION__); >> + break; >> + } >> + continue; >> + } >> + if (m_errno != SA_AIS_OK) { >> + LOG_NO("%s: saImmOmCcbObjectCreate_2() '%s' Fail >> %s", >> + __FUNCTION__, nGnodeName, >> 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; >> + } >> + } >> >> if (nodeName != NULL) >> free(nodeName); >> >> ------------------------------------------------------------------------------ >> _______________________________________________ >> 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
