Hi Neel, Please see my comments inline.
BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Neelakanta Reddy [email protected] Sent: Wednesday, November 18, 2015 8:28PM To: Hung Nguyen, Zoran Milinkovic [email protected], [email protected] Cc: Opensaf-devel [email protected] Subject: [PATCH 1 of 1] imm:Allow deletion of dangling ref object, when the object is modified by clearing NO_DANGLING attribute in same CCB [#1599] osaf/services/saf/immsv/immnd/ImmModel.cc | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-) If the NO_DANGLING reference is modified(removed) in the same CCB, then deletion of the NO_DANGLING object can be allowed because, we are in apply and the CCBcommit will modify sReverseRefsNoDanglingMMap. diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc b/osaf/services/saf/immsv/immnd/ImmModel.cc --- a/osaf/services/saf/immsv/immnd/ImmModel.cc +++ b/osaf/services/saf/immsv/immnd/ImmModel.cc @@ -5050,6 +5050,24 @@ bool ImmModel::validateNoDanglingRefsDel if(!(ommi->second->mObjFlags & IMM_DELETE_LOCK)) { std::string objName; getObjectName(ommi->second, objName); + if(ommi->second->mObjFlags & IMM_NO_DANGLING_FLAG){ // IMM_NO_DANGLING_FLAG will be set in CcbModify + /* If the NO_DANGLING reference is modified(removed) in the same CCB, + then deletion of the NO_DANGLING object can be allowed because, we are in apply + and the CCBcommit will modify sReverseRefsNoDanglingMMap*/ + ObjectMutationMap::iterator omit = ccb->mMutations.find(objName); + if(omit != ccb->mMutations.end()){ + ObjectInfo * afim = omit->second->mAfterImage; + ObjectNameSet afimRefs; + ObjectNameSet::iterator it; + collectNoDanglingRefs(afim, afimRefs); + if(afimRefs.find(omit->first.c_str()) == afimRefs.end()){ + TRACE("Delete of object %s is removed from NO_DANGLING reference in the same Ccb%u" + "from object %s", omit->first.c_str(), ccb->mId, objName.c_str()); + break; [Hung] Here we break the while() loop but there might be more objects that have no-dangling ref to 'omi->second'. This would leave those objects with dangling refs, which will make the cluster fail to reboot. So the while() shouldn't break here and continue the check. And even when 'ommi->second' passes the deletion/modification check, it must also pass the in-the-same-ccb check. Personally I think that ccb check is a prerequisite. We should do it first. That check should be moved above 'if(!(ommi->second->mObjFlags & IMM_DELETE_LOCK))'. And the afimRefs check can be something like this: if(afimRefs.find(omit->first.c_str()) == afimRefs.end()){ TRACE("..."); ++ommi; continue; /* This is safe because the ccb check is moved above, we will not miss the ccb check */ } And if you move the ccb check above, this 'if' should be an assertion. if(omit != ccb->mMutations.end()) This is just a suggestion from me, mainly to show that we might miss the ccb-check and other objects that also have refs to the to-be-deleted object. I think there's also other alternative solutions. Like, we can even remove that 'if(ommi->second->mCcbId != ccb->mId)', and use 'omit != ccb->mMutations.end()' as ccb check. If it is in the same ccb, we will surely find it in the mutation map. + } + } + } + LOG_WA("ERR_FAILED_OPERATION: Delete of object %s would violate NO_DANGLING reference " "from object %s, not scheduled for delete by this Ccb:%u", omit->first.c_str(), objName.c_str(), ccb->mId); ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
