Hi Hung, Thanks for your comments. will re-publish the patch, shortly.
/Neel. On 2016/12/06 02:21 PM, Hung Nguyen wrote: > Hi Neel, > > Tested the patch and had coredump from pbe and appliers. > > 1. In case of applier, imma_proc_ccbaug_setup() return immediately > if(cl_node->isApplier) {return;} > so callback->attrValsForCreateUc is not populated. > > 2. When creating runtime object, the callback type is > IMMA_CALLBACK_PBE_PRT_OBJ_CREATE. > It goes to the default of the switch statement in imma_proc_ccbaug_setup(). > In imma_oi_ccb_record_note_callback(), with NULL callback, > callback->attrValsForCreateUc is not populated. > > Attached is the coredump backtrace. > > > BR, > Hung Nguyen - DEK Technologies > > -------------------------------------------------------------------------------- > From: Neelakanta reddyreddy.neelaka...@oracle.com > Sent: Monday, December 05, 2016 5:38PM > 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] V3 > > > osaf/libs/agents/saf/imma/imma_cb.h | 5 + > osaf/libs/agents/saf/imma/imma_db.c | 107 > ++++++++++++++++++++++++++++++++ > osaf/libs/agents/saf/imma/imma_oi_api.c | 102 > +++++++++++++----------------- > osaf/libs/agents/saf/imma/imma_om_api.c | 11 +- > osaf/libs/agents/saf/imma/imma_proc.c | 64 +----------------- > 5 files changed, 168 insertions(+), 121 deletions(-) > > > 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; > + SaStringT object;/* The Object string from the modify/delete ccb > operation. The object is used to obtain > + adminOwner when saImmOiAugmentCcbInitialize is called > in completed-callback*/ > + SaStringT adminOwner; /* adminowner of the ccb id, assigned at > ccb-create-callback. The adminOwner used in > + 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,15 @@ int imma_oi_ccb_record_delete(IMMA_CLIEN > to_delete->mCcbErrorString = NULL; > } > > + if(to_delete->object){ > + free(to_delete->object); > + to_delete->object = NULL; > + } > + if(to_delete->adminOwner){ > + free(to_delete->adminOwner); > + to_delete->adminOwner = NULL; > + } > + > free(to_delete); > TRACE_LEAVE(); > return 1; > @@ -541,6 +552,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 +569,98 @@ int imma_oi_ccb_record_note_callback(IMM > rs = 1; > } > } > + if(callback){ > + if(callback->type == IMMA_CALLBACK_OI_CCB_CREATE){ > + 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 && > !(tmp->adminOwner)) { > + 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(); > + } > + tmp->adminOwner = (SaStringT) > malloc(strlen(*(SaConstStringT*) attrVal->attrValues[0])+1); > + strcpy(tmp->adminOwner, *(SaConstStringT*) > attrVal->attrValues[0]); > + > + } else if (((callback->type == IMMA_CALLBACK_OI_CCB_DELETE) || > (callback->type == IMMA_CALLBACK_OI_CCB_MODIFY)) > + && !(tmp->object) && !(tmp->adminOwner)){ > + SaConstStringT tmpStr = > osaf_extended_name_borrow(&(callback->name)); > + tmp->object= (SaStringT) malloc(strlen(tmpStr)+1); > + strcpy(tmp->object, tmpStr); > + } > + } > + > 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 > @@ -3549,8 +3549,8 @@ extern SaAisErrorT immsv_om_augment_ccb_ > > extern SaAisErrorT immsv_om_augment_ccb_get_admo_name( > SaImmHandleT privateOmHandle, > - SaNameT* objectName, > - SaNameT* admoNameOut) __attribute__((weak)); > + SaStringT object, > + SaStringT * admOwnerOut) __attribute__((weak)); > > > extern SaAisErrorT immsv_om_handle_initialize(SaImmHandleT > *privateOmHandle, SaVersionT* version) __attribute__((weak)); > @@ -3565,52 +3565,30 @@ 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, > SaStringT object, SaStringT *admOwnerOut) > { > 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, object, > admOwnerOut); > + 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. */ > > if(rc == SA_AIS_OK) { > - TRACE("Obtained AdmoName:%s", > osaf_extended_name_borrow(admoNameOut)); > + TRACE("Obtained AdmoOwner:%s", *admOwnerOut); > } > TRACE_LEAVE(); > return rc; > @@ -3734,6 +3712,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,34 +3827,43 @@ 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(rc != SA_AIS_OK) { > - TRACE("ERR_TRY_AGAIN: failed to obtain > SaImmAttrAdminOwnerName %u", rc); > - if(rc != SA_AIS_ERR_TRY_AGAIN) { > - rc = SA_AIS_ERR_TRY_AGAIN; > + SaStringT adminOwner = NULL;/* Used to get admo string > name copied to stack.*/ > + > + osafassert(immsv_om_admo_handle_initialize); > + if(ccb_oi_record->adminOwner){ > + rc = > immsv_om_admo_handle_initialize(privateOmHandle, ccb_oi_record->adminOwner, > + &privateAoHandle); > + } else { > + rc = getAdmoName(privateOmHandle, cbi, > ccb_oi_record->object, &adminOwner); > + > + if(rc != SA_AIS_OK) { > + TRACE("ERR_TRY_AGAIN: failed to obtain > SaImmAttrAdminOwnerName %u", rc); > + if(rc != SA_AIS_ERR_TRY_AGAIN) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + } > + goto done; > } > - osaf_extended_name_free(&admName); > - goto done; > + /* Allocate private admowner with > ReleaseOnFinalize as TRUE */ > + rc = > immsv_om_admo_handle_initialize(privateOmHandle, adminOwner, > &privateAoHandle); > } > - TRACE("Obtaned AdminOwnerName:%s", > osaf_extended_name_borrow(&admName)); > - /* Allocate private admowner with ReleaseOnFinalize as > TRUE */ > - osafassert(immsv_om_admo_handle_initialize); > - rc = immsv_om_admo_handle_initialize(privateOmHandle, > - (SaImmAdminOwnerNameT) > osaf_extended_name_borrow(&admName), &privateAoHandle); > > if(rc != SA_AIS_OK) { > TRACE("ERR_TRY_AGAIN: failed to obtain internal > admo handle rc:%u", rc); > if(rc != SA_AIS_ERR_TRY_AGAIN) { > rc = SA_AIS_ERR_TRY_AGAIN; > } > - osaf_extended_name_free(&admName); > + if(adminOwner){ > + free(adminOwner); > + adminOwner = NULL; > + } > goto done; > } > - osaf_extended_name_free(&admName); > + if(adminOwner){ > + free(adminOwner); > + adminOwner = NULL; > + } > } > } else {TRACE("AugCcbinit: Admo has ROF == TRUE");} > > @@ -3888,8 +3876,8 @@ SaAisErrorT saImmOiAugmentCcbInitialize( > /* Now dip into the OM library & create mockup ccb-node & admo-node for > use by OI */ > osafassert(immsv_om_augment_ccb_initialize); > rc = immsv_om_augment_ccb_initialize(privateOmHandle, ccbId, > adminOwnerId, > - ccbHandle, &privateAoHandle); > - done: > + ccbHandle, &privateAoHandle); > +done: > > if (locked) { > m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); > diff --git a/osaf/libs/agents/saf/imma/imma_om_api.c > b/osaf/libs/agents/saf/imma/imma_om_api.c > --- a/osaf/libs/agents/saf/imma/imma_om_api.c > +++ b/osaf/libs/agents/saf/imma/imma_om_api.c > @@ -9690,8 +9690,8 @@ SaAisErrorT immsv_om_augment_ccb_get_res > > SaAisErrorT immsv_om_augment_ccb_get_admo_name( > SaImmHandleT privateOmHandle, > - SaNameT* objectName, > - SaNameT* admoNameOut) > + SaStringT object, > + SaStringT * admoNameOut) > > { > SaAisErrorT rc = SA_AIS_OK; > @@ -9701,12 +9701,11 @@ SaAisErrorT immsv_om_augment_ccb_get_adm > SaImmAttrValuesT_2 *attrVal = NULL; > SaImmAccessorHandleT acHdl=0LL; > TRACE_ENTER(); > - osaf_extended_name_clear(admoNameOut); > > rc = saImmOmAccessorInitialize(privateOmHandle, &acHdl); > if(rc != SA_AIS_OK) {goto done;} > > - rc = saImmOmAccessorGet_2(acHdl, objectName, attributeNames, > &attributes); > + rc = saImmOmAccessorGet_o3(acHdl, (SaConstStringT)object, > attributeNames, &attributes); > if(rc != SA_AIS_OK) {goto finalize;} > > attrVal = attributes[0]; > @@ -9714,8 +9713,8 @@ SaAisErrorT immsv_om_augment_ccb_get_adm > rc = SA_AIS_ERR_LIBRARY; > goto finalize; > } > - > - osaf_extended_name_alloc(*(SaStringT*) attrVal->attrValues[0], > admoNameOut); > + *admoNameOut = (SaStringT) malloc(strlen(*(SaStringT*) > attrVal->attrValues[0])+1); > + strcpy(*admoNameOut, *(SaStringT*) attrVal->attrValues[0]); > > finalize: > if(acHdl) { > 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; > ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today.http://sdm.link/xeonphi _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel