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
