Hi Neel,

Besides Zoran's comment, I have 2 more comments inline.

BR,

Hung Nguyen - DEK Technologies


--------------------------------------------------------------------------------
From: Neelakanta Reddy reddy.neelaka...@oracle.com
Sent: Wednesday, November 30, 2016 12:39PM
To: Zoran Milinkovic, Hung Nguyen
     zoran.milinko...@ericsson.com, hung.d.ngu...@dektech.com.au
Cc: Opensaf-devel
     opensaf-devel@lists.sourceforge.net
Subject: [PATCH 1 of 1] imm:allow augumentCcbInit with ROF as false in 
completed callback[#1956] V2


  osaf/libs/agents/saf/imma/imma_cb.h     |   5 +
  osaf/libs/agents/saf/imma/imma_db.c     |  99 
+++++++++++++++++++++++++++++++++
  osaf/libs/agents/saf/imma/imma_oi_api.c |  60 +++++++------------
  osaf/libs/agents/saf/imma/imma_proc.c   |  64 ++-------------------
  4 files changed, 133 insertions(+), 95 deletions(-)


Two new variables objectName and adminOwnerName added to imma_oi_ccb_record. 
For CCB create operation adminOwnerName will be added.
For CCB delete/modify operation objectName will be added

diff --git a/osaf/libs/agents/saf/imma/imma_cb.h 
b/osaf/libs/agents/saf/imma/imma_cb.h
--- a/osaf/libs/agents/saf/imma/imma_cb.h
+++ b/osaf/libs/agents/saf/imma/imma_cb.h
@@ -36,6 +36,10 @@ struct imma_oi_ccb_record {
        struct imma_callback_info* ccbCallback;
        SaImmHandleT privateAugOmHandle;
        SaImmAdminOwnerHandleT privateAoHandle;
+       SaNameT objectName;/* The  Object name from the modif/delete ccb 
operationi. The objectName is used to obtain
+                       adminOnerName when saImmOiAugmentCcbInitialize is 
called in completed-callback*/
+       SaNameT adminOwnerName; /* adminowner name of the ccb id, assigned at 
ccb-create-callback.The adminOwnerName is used
+                        saImmOiAugmentCcbInitialize is called in 
completed-callback*/
  };
  
  typedef struct imma_client_node {
@@ -197,6 +201,7 @@ int imma_oi_ccb_record_set_critical(IMMA
  int imma_oi_ccb_record_terminate(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT 
ccbId);
  int imma_oi_ccb_record_abort(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId);
  int imma_oi_ccb_record_exists(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId);
+struct imma_oi_ccb_record * imma_oi_ccb_record_find(IMMA_CLIENT_NODE *cl_node, 
SaImmOiCcbIdT ccbId);
  int imma_oi_ccb_record_set_error(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT 
ccbId, const SaStringT errorString);
  SaStringT imma_oi_ccb_record_get_error(IMMA_CLIENT_NODE *cl_node, 
SaImmOiCcbIdT ccbId);
  void imma_oi_ccb_allow_error_string(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT 
ccbId);
diff --git a/osaf/libs/agents/saf/imma/imma_db.c 
b/osaf/libs/agents/saf/imma/imma_db.c
--- a/osaf/libs/agents/saf/imma/imma_db.c
+++ b/osaf/libs/agents/saf/imma/imma_db.c
@@ -24,6 +24,8 @@
  *****************************************************************************/
  
  #include "imma.h"
+#include "osaf_extended_name.h"
+
  /****************************************************************************
    Name          : imma_client_tree_init
    Description   : This routine is used to initialize the client tree
@@ -273,6 +275,9 @@ int imma_oi_ccb_record_delete(IMMA_CLIEN
                        to_delete->mCcbErrorString = NULL;
                }
  
+               osaf_extended_name_free(&(to_delete->objectName));
+               osaf_extended_name_free(&(to_delete->adminOwnerName));
+
                free(to_delete);
                TRACE_LEAVE();
                return 1;
@@ -541,6 +546,11 @@ int imma_oi_ccb_record_note_callback(IMM
         */
  
        int rs = 0;
+       const SaImmAttrNameT admoNameAttr = SA_IMM_ATTR_ADMIN_OWNER_NAME;
+       SaImmAttrValuesT_2 **attr, **attributes = NULL;
+       SaImmAttrValuesT_2 *attrVal = NULL;
+       size_t attrDataSize = 0;
+
        struct imma_oi_ccb_record *tmp = imma_oi_ccb_record_find(cl_node, 
ccbId);
  
        if(tmp && !(tmp->isAborted)) {
@@ -553,7 +563,96 @@ int imma_oi_ccb_record_note_callback(IMM
                        rs = 1;
                }
        }
+       if(callback){
  
+
+               int noOfAttributes = 0;
+
+               /* NOTE: The code below is practically a copy of the code
+                  in immOm searchNext, for serving the attrValues structure.
+                  This code should be factored out into some common function.
+                */
+               IMMSV_ATTR_VALUES_LIST *p = callback->attrValues;
+               int i = 0;
+               while (p) {
+                       ++noOfAttributes;
+                       p = p->next;
+               }
+
+               p = callback->attrValues;
+
+
+               if(cl_node->isImmA2fCbk) {
+                       if(noOfAttributes) {
+                               p = p->next;  // Skip RDN. RDN is the first 
attribute
+                               noOfAttributes--;
+                       }
+               }
+
+               attrDataSize = sizeof(SaImmAttrValuesT_2 *) * (noOfAttributes + 
1);
+               attr = calloc(1, attrDataSize); /*alloc-1 */
+               for (; i < noOfAttributes && p; i++, p = p->next) {
+                       IMMSV_ATTR_VALUES *q = &(p->n);
+                       attr[i] = calloc(1, sizeof(SaImmAttrValuesT_2));        
/*alloc-2 */
+                       attr[i]->attrName = malloc(q->attrName.size + 1);       
/*alloc-3 */
+                       strncpy(attr[i]->attrName, (const char 
*)q->attrName.buf, q->attrName.size + 1);
+                       attr[i]->attrName[q->attrName.size] = 0;        
/*redundant. */
+                       attr[i]->attrValuesNumber = q->attrValuesNumber;
+                       attr[i]->attrValueType = 
(SaImmValueTypeT)q->attrValueType;
+                       if (q->attrValuesNumber) {
+                               attr[i]->attrValues =
+                                       calloc(q->attrValuesNumber, 
sizeof(SaImmAttrValueT));   /*alloc-4 */
+                               /*alloc-5 */
+                               attr[i]->attrValues[0] =
+                                       imma_copyAttrValue3(q->attrValueType, 
&(q->attrValue));
+
+                               if (q->attrValuesNumber > 1) {
+                                       int ix;
+                                       IMMSV_EDU_ATTR_VAL_LIST *r = 
q->attrMoreValues;
+                                       for (ix = 1; ix < q->attrValuesNumber; 
++ix) {
+                                               osafassert(r);
+                                               attr[i]->attrValues[ix] = 
/*alloc-5 */
+                                                       
imma_copyAttrValue3(q->attrValueType, &(r->n));
+                                               r = r->next;
+                                       }//for
+                               }//if
+                       }//if
+               }//for
+               attr[i] = NULL; /*redundant */
+
+
+               /* Save a copy of the attrvals pointer in the callback to allow
+                  saImmOiAugmentCcbInitialize to get access to the 
ccbCreateContext.
+                  We cant use callback->attrValues because it is partially 
stolen
+                  from (caused to be incomplete) in the loop above (see 
imma_copyAttrValue3).
+                */
+
+               callback->attrValsForCreateUc = (const SaImmAttrValuesT_2 
**)attr;
+
+               if(callback->type == IMMA_CALLBACK_OI_CCB_CREATE && 
osaf_is_extended_name_empty(&(tmp->adminOwnerName))) {      
+                       attributes = (SaImmAttrValuesT_2 **) 
callback->attrValsForCreateUc;
+                       int i=0;
+                       while((attrVal = attributes[i++]) != NULL) {
+                               if(strcmp(admoNameAttr, attrVal->attrName)==0) {
+                                       TRACE("Found %s attribute in object 
create upcall", admoNameAttr);
+                                       break;
+                               }
+                       }
+
+                       if(!attrVal || strcmp(attrVal->attrName, admoNameAttr) 
|| (attrVal->attrValuesNumber!=1) ||
+                                       (attrVal->attrValueType != 
SA_IMM_ATTR_SASTRINGT)) {
+                               
LOG_ER("imma_oi_ccb_record_note_callback:Missmatch on attribute %s", 
admoNameAttr);
+                               abort();
+                       }
+                       osaf_extended_name_alloc(*(SaStringT*) 
attrVal->attrValues[0], &(tmp->adminOwnerName));
+
+               } else if (((callback->type == IMMA_CALLBACK_OI_CCB_DELETE) || 
(callback->type == IMMA_CALLBACK_OI_CCB_MODIFY))
+                               && 
osaf_is_extended_name_empty(&(tmp->objectName))){
+                       SaConstStringT tmpStr = 
osaf_extended_name_borrow(&(callback->name));
+                       osaf_extended_name_alloc(tmpStr, &(tmp->objectName));
+               }
+       }
+               

[Hung] If it's IMMA_CALLBACK_OI_CCB_COMPLETED, we can update 
'callback->name' with 'tmp->objectName' right here. That way, we can 
avoid adding extra param 'object' to getAdmoName(). In getAdmoName(), 
'cbi->name' will be always usable (even when it's a CcbCompleted callback).

        return rs;
  }
  
diff --git a/osaf/libs/agents/saf/imma/imma_oi_api.c 
b/osaf/libs/agents/saf/imma/imma_oi_api.c
--- a/osaf/libs/agents/saf/imma/imma_oi_api.c
+++ b/osaf/libs/agents/saf/imma/imma_oi_api.c
@@ -3565,46 +3565,25 @@ extern void immsv_om_handle_finalize(
                                 SaImmHandleT privateOmHandle) 
__attribute__((weak));
  
  static SaAisErrorT
-getAdmoName(SaImmHandleT privateOmHandle, IMMA_CALLBACK_INFO * cbi, SaNameT* 
admoNameOut)
+getAdmoName(SaImmHandleT privateOmHandle, IMMA_CALLBACK_INFO * cbi, SaNameT* 
object, SaNameT* admoNameOut)
  {
      SaAisErrorT rc = SA_AIS_OK;
      const SaImmAttrNameT admoNameAttr = SA_IMM_ATTR_ADMIN_OWNER_NAME;
-    SaImmAttrValuesT_2 **attributes = NULL;
-    SaImmAttrValuesT_2 *attrVal = NULL;
+
      TRACE_ENTER();
  
-    if(cbi->type == IMMA_CALLBACK_OI_CCB_CREATE) {
-           /* Admo attribute for created object should be in attr list.*/
-           int i=0;
-           attributes = (SaImmAttrValuesT_2 **) cbi->attrValsForCreateUc;
-           while((attrVal = attributes[i++]) != NULL) {
-                   if(strcmp(admoNameAttr, attrVal->attrName)==0) {
-                           TRACE("Found %s attribute in object create upcall", 
admoNameAttr);
-                           break;
-                   }
-           }
-
-           if(!attrVal || strcmp(attrVal->attrName, admoNameAttr) || 
(attrVal->attrValuesNumber!=1) ||
-                   (attrVal->attrValueType != SA_IMM_ATTR_SASTRINGT)) {
-                   LOG_ER("Missmatch on attribute %s", admoNameAttr);
-                   abort();
-           }
-
-           osaf_extended_name_alloc(*(SaStringT*) attrVal->attrValues[0], 
admoNameOut);
-
-    } else {
-           /* modify or delete => fetch admo attribute for object from server. 
*/
-           if((cbi->type != IMMA_CALLBACK_OI_CCB_DELETE) &&
-                   (cbi->type != IMMA_CALLBACK_OI_CCB_MODIFY)) {
-                   LOG_ER("Inconsistency in callback type:%u", cbi->type);
-                   abort();
-           }
-
-           osafassert(immsv_om_augment_ccb_get_admo_name);
-           rc = immsv_om_augment_ccb_get_admo_name(privateOmHandle, 
&(cbi->name), admoNameOut);
-           if(rc == SA_AIS_ERR_LIBRARY) {
-                   LOG_ER("Missmatch on attribute %s for delete or modify", 
admoNameAttr);
-           }
+    /* modify or delete => fetch admo attribute for object from server. */
+    if((cbi->type != IMMA_CALLBACK_OI_CCB_DELETE) &&
+                   (cbi->type != IMMA_CALLBACK_OI_CCB_MODIFY) &&
+                   (cbi->type != IMMA_CALLBACK_OI_CCB_COMPLETED)) {
+           LOG_ER("Inconsistency in callback type:%u", cbi->type);
+           abort();
+    }
+
+    osafassert(immsv_om_augment_ccb_get_admo_name);
+    rc = immsv_om_augment_ccb_get_admo_name(privateOmHandle, &(cbi->name), 
admoNameOut);
+    if(rc == SA_AIS_ERR_LIBRARY) {
+           LOG_ER("Missmatch on attribute %s for delete or modify", 
admoNameAttr);
      }
  
      /* attrVal found either in create callback, or fetched from server. */
@@ -3734,6 +3713,7 @@ SaAisErrorT saImmOiAugmentCcbInitialize(
                goto done;
        }
  
+       struct imma_oi_ccb_record *ccb_oi_record = 
imma_oi_ccb_record_find(cl_node, ccbId);
        cbi = imma_oi_ccb_record_ok_augment(cl_node, ccbId, &privateOmHandle, 
&privateAoHandle);
        if(!cbi) {
                TRACE_2("ERR_BAD_OPERATION: Ccb %u, is not in a state that "
@@ -3848,11 +3828,17 @@ SaAisErrorT saImmOiAugmentCcbInitialize(
  
                if(!privateAoHandle) {
                        TRACE("AugCcbinit: Admo has ReleaseOnFinalize FALSE "
-                               "=> init separate admo => must fetch admo-name 
first");
+                                       "=> init separate admo => must fetch 
admo-name first");
                        osafassert(locked == false);
                        SaNameT admName;/* Used to get admo string name copied 
to stack.*/
  
-                       rc = getAdmoName(privateOmHandle, cbi, &admName);
+                       if((cbi->type == IMMA_CALLBACK_OI_CCB_CREATE)|| 
((cbi->type == IMMA_CALLBACK_OI_CCB_COMPLETED) &&
+                               
!osaf_is_extended_name_empty(&(ccb_oi_record->adminOwnerName)))){
+                               admName = ccb_oi_record->adminOwnerName;
+                       } else {
+                               rc = getAdmoName(privateOmHandle, cbi, 
&(ccb_oi_record->objectName),&admName);
+                       }
+
[Hung] In case we already have a CcbCreate callback before the 
CcbMod/Del callback (a ccb can have many operations), we already got the 
admo name, we don't have to use the objectName to retrieve the admo name 
from the server. So I think the if statement should be if 
(!osaf_is_extended_name_empty(&(ccb_oi_record->adminOwnerName))) { 
admName = ccb_oi_record->adminOwnerName; } else { rc = getAdmoName(...); }


                        if(rc != SA_AIS_OK) {
                                TRACE("ERR_TRY_AGAIN: failed to obtain 
SaImmAttrAdminOwnerName %u", rc);
                                if(rc != SA_AIS_ERR_TRY_AGAIN) {
diff --git a/osaf/libs/agents/saf/imma/imma_proc.c 
b/osaf/libs/agents/saf/imma/imma_proc.c
--- a/osaf/libs/agents/saf/imma/imma_proc.c
+++ b/osaf/libs/agents/saf/imma/imma_proc.c
@@ -2285,7 +2285,6 @@ static bool imma_process_callback_info(I
                                IMMSV_EVT ccbObjCrRpl;
                                bool locked = false;
                                SaImmAttrValuesT_2 **attr = NULL;
-                               size_t attrDataSize = 0;
                                int i = 0;
                                /* No need to check for 
o.iCallbkA2f.saImmOiCcbObjectCreateCallback
                                 * "o" is union and 
o.iCallbk.saImmOiCcbObjectCreateCallback is same
@@ -2308,62 +2307,11 @@ static bool imma_process_callback_info(I
  
                                        const SaImmClassNameT className = 
callback->className;  /*0 */
                                        callback->className = NULL;
-                                       int noOfAttributes = 0;
-
-                                       /* NOTE: The code below is practically 
a copy of the code
-                                          in immOm searchNext, for serving the 
attrValues structure.
-                                          This code should be factored out 
into some common function.
-                                       */
-                                       IMMSV_ATTR_VALUES_LIST *p = 
callback->attrValues;
-                                       while (p) {
-                                               ++noOfAttributes;
-                                               p = p->next;
-                                       }
-
-                                       p = callback->attrValues;
-
-                                       if(cl_node->isImmA2fCbk) {
-                                               if(noOfAttributes) {
-                                                       p = p->next;  // Skip 
RDN. RDN is the first attribute
-                                                       noOfAttributes--;
-                                               } else {
-                                                       /* There must be at 
least one attribute value */
-                                                       localEr = 
SA_AIS_ERR_BAD_OPERATION;
-                                                       clientCapable = false;
-                                               }
-                                       }
-
-                                       if(localEr == SA_AIS_OK) {
-                                               attrDataSize = 
sizeof(SaImmAttrValuesT_2 *) * (noOfAttributes + 1);
-                                               attr = calloc(1, attrDataSize); 
/*alloc-1 */
-                                               for (; i < noOfAttributes && p; 
i++, p = p->next) {
-                                                       IMMSV_ATTR_VALUES *q = 
&(p->n);
-                                                       attr[i] = calloc(1, 
sizeof(SaImmAttrValuesT_2));        /*alloc-2 */
-                                                       attr[i]->attrName = 
malloc(q->attrName.size + 1);       /*alloc-3 */
-                                                       
strncpy(attr[i]->attrName, (const char *)q->attrName.buf, q->attrName.size + 1);
-                                                       
attr[i]->attrName[q->attrName.size] = 0;        /*redundant. */
-                                                       
attr[i]->attrValuesNumber = q->attrValuesNumber;
-                                                       attr[i]->attrValueType 
= (SaImmValueTypeT)q->attrValueType;
-                                                       if 
(q->attrValuesNumber) {
-                                                               
attr[i]->attrValues =
-                                                                       
calloc(q->attrValuesNumber, sizeof(SaImmAttrValueT));   /*alloc-4 */
-                                                               /*alloc-5 */
-                                                               
attr[i]->attrValues[0] =
-                                                                       
imma_copyAttrValue3(q->attrValueType, &(q->attrValue));
-
-                                                               if 
(q->attrValuesNumber > 1) {
-                                                                       int ix;
-                                                                       
IMMSV_EDU_ATTR_VAL_LIST *r = q->attrMoreValues;
-                                                                       for (ix 
= 1; ix < q->attrValuesNumber; ++ix) {
-                                                                               
osafassert(r);
-                                                                               
attr[i]->attrValues[ix] = /*alloc-5 */
-                                                                               
        imma_copyAttrValue3(q->attrValueType, &(r->n));
-                                                                               
r = r->next;
-                                                                       }//for
-                                                               }//if
-                                                       }//if
-                                               }//for
-                                               attr[i] = NULL; /*redundant */
+
+                                       if(cl_node->isImmA2fCbk && 
!callback->attrValsForCreateUc) {
+                                               /* There must be at least one 
attribute value */
+                                               localEr = 
SA_AIS_ERR_BAD_OPERATION;
+                                               clientCapable = false;
                                        }
  
                                        if(localEr == SA_AIS_OK && 
cl_node->isImmA2fCbk) {
@@ -2388,7 +2336,6 @@ static bool imma_process_callback_info(I
                                           We cant use callback->attrValues 
because it is partially stolen
                                           from (caused to be incomplete) in 
the loop above (see imma_copyAttrValue3).
                                         */
-                                       callback->attrValsForCreateUc = (const 
SaImmAttrValuesT_2 **)attr;
  
                                        if (localEr == SA_AIS_OK
                                                        && 
!osaf_is_extended_names_enabled()
@@ -2457,6 +2404,7 @@ static bool imma_process_callback_info(I
  
                                        free(className);        /*free-0 */
                                        free(objectName);       /*free-6 */
+                                       attr = (SaImmAttrValuesT_2 **) 
callback->attrValsForCreateUc;
                                        for (i = 0; attr[i]; ++i) {
                                                free(attr[i]->attrName);        
/*free-3 */
                                                attr[i]->attrName = 0;


------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to