Hi Neel I have updated the patches based on your comments and will send them for re-review today
Thanks Lennart > -----Original Message----- > From: Neelakanta Reddy [mailto:[email protected]] > Sent: den 27 september 2016 14:25 > To: Lennart Lund <[email protected]>; Rafael Odzakow > <[email protected]> > Cc: [email protected] > Subject: Re: [PATCH 1 of 1] smf: Delete node group if already exist when > creating [#2049] > > 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:[email protected]] > >> Sent: den 27 september 2016 11:21 > >> To: Lennart Lund <[email protected]>; Rafael Odzakow > >> <[email protected]> > >> Cc: [email protected] > >> 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 [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
