Re: [devel] [PATCH 1 of 1] amfd: assign SUs on unlocked nodes when locked NG is deleted [#1435]

[Minh H C. <min...@de...>]

so ack from me, and thanks for your explanation /Minh

On 8/17/2015 3:15 PM, praveen malviya wrote:
>
>
> On 17-Aug-15 7:44 AM, Minh H Chau wrote:
>> Hi Praveen,
>>
>> I have tested the patch, it can solve the problem reported. Just 2 minor
>> comments as below:
>>
>> I think this problem can be solved by either ways:
>> @a- Individually unlock a LOCKED node under LOCKED node-group will also
>> start app assignment on that node
>> @b- Or start app assignment on UNLOCKED node if all relevant ng(s) is
>> deleted
>>
> This patch for #1435 is based on approach @b. And this will followed 
> in future also.
>> As documented in PR: "After completion of nodegroup SHUTDOWN or LOCK
>> operation, a user can unlock either the individual nodes by performing
>> unlock on that node or by performing unlock operation on that 
>> nodegorup."
>> So unlock a single node here means the app assignments of that node will
>> be brought into in-service too, not only just set admin state of the
>> node to UNLOCK?
> Here intention was only to set the admin state to unlocked for 
> individual nodes and not assignment on them because NG is locked. I 
> will add update PR doc.
>
>> Since there's a symmetric case, that individually lock an UNLOCKED node
>> under UNLOCKED ng removes app assignment on that node.
>>
> It is like locking a SU when SG is unlocked. In that case also we 
> remove assignments. But if a SG is locked and a user unlocks a SU of 
> it then SU will not be assigned.
>> The 2nd comment is (if we'll still agree on @b), there's another
>> node_ccb_completed_delete_hdlr named in node.cc already, not sure if
>> ng_ccb_apply_delete_hdlr sounds ok? (we do apply callback for delete op)
>>
> I will change the name from node_ccb_completed_delete_hdlr to 
> ng_ccb_completed_delete_hdlr, before pushing the patch.
>
> Thanks,
> Praveen
>> Thanks,
>> Minh
>> On 8/14/2015 3:21 PM, pra...@or... wrote:
>>> osaf/services/saf/amf/amfd/nodegroup.cc |  27
>>> +++++++++++++++++++++++----
>>>   1 files changed, 23 insertions(+), 4 deletions(-)
>>>
>>>
>>> A nodegroup is deleted in locked state when some of the nodes are kept
>>> in admin
>>> unlocked state. Application remains in unassigned state.
>>>
>>> In CCB apply for deletion of NG, AMF is not going for new assignments
>>> on the
>>> nodes of NG. Since this is not documented and a valid use case, the
>>> problem
>>> needs a fix.
>>>
>>> Patch fixes the problem by calling the assignment logic in CCB apply 
>>> for
>>> deletion of NG.
>>>
>>> diff --git a/osaf/services/saf/amf/amfd/nodegroup.cc
>>> b/osaf/services/saf/amf/amfd/nodegroup.cc
>>> --- a/osaf/services/saf/amf/amfd/nodegroup.cc
>>> +++ b/osaf/services/saf/amf/amfd/nodegroup.cc
>>> @@ -26,6 +26,9 @@
>>>   AmfDb<std::string, AVD_AMF_NG> *nodegroup_db = 0;
>>>   static AVD_AMF_NG *ng_create(SaNameT *dn, const SaImmAttrValuesT_2
>>> **attributes);
>>> +//TODO: Make  below function members.
>>> +static void ng_admin_unlock_inst(AVD_AMF_NG *ng);
>>> +static void ng_unlock(AVD_AMF_NG *ng);
>>>   /**
>>>    * Lookup object in db using dn
>>> @@ -556,6 +559,24 @@ static void ng_ccb_apply_modify_hdlr(Ccb
>>>       TRACE_LEAVE();
>>>   }
>>> +static void node_ccb_completed_delete_hdlr(CcbUtilOperationData_t
>>> *opdata)
>>> +{
>>> +    TRACE_ENTER();
>>> +    AVD_AMF_NG *ng = avd_ng_get(&opdata->objectName);
>>> +    //Temporarily keep NG in UNLOCKED state to assign SUs.
>>> +    ng->saAmfNGAdminState = SA_AMF_ADMIN_UNLOCKED;
>>> +    ng_unlock(ng);
>>> +
>>> +    //Since AMF will delete NG, clear its pointers in node.
>>> +    for (std::set<std::string>::const_iterator iter =
>>> ng->saAmfNGNodeList.begin();
>>> +            iter != ng->saAmfNGNodeList.end(); ++iter) {
>>> +        AVD_AVND *node = avd_node_get(*iter);
>>> +        node->admin_ng = NULL;
>>> +    }
>>> +    ng->node_oper_list.clear();
>>> +    ng_delete(ng);
>>> +    TRACE_LEAVE2("deleted %s", opdata->objectName.value);
>>> +}
>>>   /**
>>>    * Callback for CCB apply
>>>    * @param opdata
>>> @@ -576,9 +597,7 @@ static void ng_ccb_apply_cb(CcbUtilOpera
>>>           ng_ccb_apply_modify_hdlr(opdata);
>>>           break;
>>>       case CCBUTIL_DELETE:
>>> -        ng = avd_ng_get(&opdata->objectName);
>>> -        ng_delete(ng);
>>> -        TRACE("deleted %s", opdata->objectName.value);
>>> +        node_ccb_completed_delete_hdlr(opdata);
>>>           break;
>>>       default:
>>>           osafassert(0);
>>> @@ -778,7 +797,7 @@ void ng_node_lock_and_shutdown(AVD_AVND
>>>    *         on this node.
>>>    * @param       ptr to Nodegroup (AVD_AMF_NG).
>>>    */
>>> -void ng_unlock(AVD_AMF_NG *ng)
>>> +static void ng_unlock(AVD_AMF_NG *ng)
>>>   {
>>>       TRACE_ENTER2("'%s'",ng->name.value);
>>>       for (std::set<std::string>::const_iterator iter =
>>> ng->saAmfNGNodeList.begin();
>>>
>>
>