Hi Leenart, returning of ERR_EXIST is very rare, and not as frequent as BAD_HANDLE. Still, not satisfied with infinite loop, without finite re-trys.
Ack from me. Thanks, Neel. On 2016/09/27 05:14 PM, Lennart Lund wrote: > Hi Neel > > See my comments/answer tagged [Lennart] > > Thanks > Lennart > >> -----Original Message----- >> From: Neelakanta Reddy [mailto:reddy.neelaka...@oracle.com] >> Sent: den 27 september 2016 11:21 >> To: Lennart Lund <lennart.l...@ericsson.com>; Rafael Odzakow >> <rafael.odza...@ericsson.com> >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: Re: [PATCH 1 of 1] smf: Delete node group if already exist when >> creating [#2049] >> >> Hi Lennart, >> >> Reviewed the patch. Following are the comments: >> >> 1. Do not use infinite loops like while(true), In this case while loop >> is not required. > [Lennart] I have chosen to use a loop since > immutil_saImmOmCcbObjectCreate_2() may have to be called more than once > See also my answers for ticket #2046. The condition for this loop to continue > is SA_AIS_ERR_EXIST (in #2046 it's SA_AIS_ERR_BAD_HANDLE) otherwise the > mechanism is the same. > >> 2. comments inline. >> >> Thanks, >> Neel. >> >> >> On 2016/09/22 08:34 PM, Lennart Lund wrote: >>> 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) { >>> + 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(); >>> + if (rc == false) { >>> + LOG_NO("%s: deleteNodeGroup() Fail", >>> + __FUNCTION__); >>> + break; >>> + } >> in the else codition, if deleteNodeGroup is true call >> immutil_saImmOmCcbObjectCreate_2. In deleteNodeGroup there are >> retries >> for deleting. >> No need to have while loop, > [Lennart] I have chosen to use a loop instead of a sequence that has a call > of immutil_saImmOmCcbObjectCreate_2() in two places. > This solution also encapsulates all saImmOmCcbObjectCreate_2 handling > including deleting if needed and the error handling >>> + 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 Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
