Hi Anders, Adding new mutations in completed callback will be rejected at IMMND. That also results in aborting the whole CCB.
isCcbAugOk is needed when we want to prevent adding new mutations at library level. But in the patches sent out for review, that is not implemented. Type of isCcbAugOk is still 'bool'. The ccb api (ccb_object_create_common/ccb_object_modify_common/ccb_object_delete_common) is also not implemented to check 'isCcbAugOk'. That leaves me confused whether we are going to implement the check at library level or just simply let IMMND reject the new mutation and abort the ccb. BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Anders Bjornerstedt [email protected] Sent: Tuesday, March 29, 2016 3:18PM To: Hung Nguyen, Neelakanta Reddy, Zoran Milinkovic [email protected], [email protected], [email protected] Cc: Opensaf-devel [email protected] Subject: Re: [devel] [PATCH 1 of 3] imm: add support to the library for transactional safe read [#48] Hi Hung, The three valued logic for isCcbAugOk is needed because only safe reads are allowed in the completed callback. Create, Delete and Modify augmentations are only allowed inside create/deleet/modify callbacks. The reason for this is that the CCB must not augment new mutations to the CCB when it has entered validation (completed callbacks). Remember that there may be more than one OI involved and validation proceeds concurrently at the various OIs for a CCB. SafeReads can be added during validation (completed callback) by an OI because safe-reads do not disturb the base for the validation for the other OIs. Thanks AndersBj > ----Ursprungligt meddelande---- > Från : [email protected] > Datum : 2016-03-29 - 09:57 (CEST) > Till : [email protected], [email protected] > Kopia : [email protected] > Ämne : Re: [devel] [PATCH 1 of 3] imm: add support to the library for > transactional safe read [#48] > > Hi Zoran, > > Please see my comments inline. > > BR, > > Hung Nguyen - DEK Technologies > > > -------------------------------------------------------------------------------- > From: Zoran Milinkovic [email protected] > Sent: Wednesday, March 23, 2016 11:51PM > To: Neelakanta Reddy > [email protected] > Cc: Opensaf-devel > [email protected] > Subject: [devel] [PATCH 1 of 3] imm: add support to the library for > transactional safe read [#48] > > > osaf/libs/agents/saf/imma/imma_cb.h | 3 +- > osaf/libs/agents/saf/imma/imma_db.c | 14 +- > osaf/libs/agents/saf/imma/imma_oi_api.c | 2 +- > osaf/libs/agents/saf/imma/imma_om_api.c | 415 > ++++++++++++++++++++++- > osaf/libs/agents/saf/imma/imma_proc.c | 48 ++- > osaf/libs/common/immsv/immsv_evt.c | 24 +- > osaf/libs/common/immsv/include/immsv_evt.h | 2 + > osaf/libs/common/immsv/include/immsv_evt_model.h | 1 + > osaf/libs/saf/include/saImmOm_A_2_16.h | 2 + > osaf/libs/saf/include/saImmOm_A_2_17.h | 6 + > 10 files changed, 490 insertions(+), 27 deletions(-) > > > The patch contains code for the library part of transactional safe read > > 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 > @@ -96,6 +96,7 @@ typedef struct imma_ccb_node { > SaImmCcbHandleT ccb_hdl; /* locally generated handle */ > SaImmHandleT mImmHandle; /* The immOm handle */ > SaImmAdminOwnerHandleT mAdminOwnerHdl; > + SaImmAccessorHandleT mCcbObjectReadAccessorHandle; > SaImmCcbFlagsT mCcbFlags; > SaUint32T mCcbId; /* Om client uses 32 bit ccbId. */ > SaStringT* mErrorStrings; > @@ -200,7 +201,7 @@ int imma_oi_ccb_record_set_error(IMMA_CL > 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); > int imma_oi_ccb_record_note_callback(IMMA_CLIENT_NODE *cl_node, > SaImmOiCcbIdT ccbId, > - struct imma_callback_info *callback); > + struct imma_callback_info *callback, SaUint8T augPermission); > struct imma_callback_info * imma_oi_ccb_record_ok_augment(IMMA_CLIENT_NODE > *cl_node, > SaImmOiCcbIdT ccbId, SaImmHandleT *privateOmHandle, > SaImmAdminOwnerHandleT *privateAoHandle); > void imma_oi_ccb_record_augment(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT > ccbId, SaImmHandleT privateOmHandle, > 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 > @@ -344,8 +344,8 @@ int imma_oi_ccb_record_ok_for_critical(I > TRACE_5("op-count matches with inv:%u", inv); > } > } > - tmp->isCcbAugOk = false; /* not allowed to augment ccb in > completed UC */ > - tmp->ccbCallback = NULL; > + tmp->isCcbAugOk = 1; /* only allowed to augment ccb with > safe-read in completed UC */ > + //tmp->ccbCallback = NULL; > > *[Hung][isCcbAugOk] There's a problem with 'isCcbAugOk' that I'm not > really sure why new values (0, 1 and 2) are introduced for this > variable, but the type is still 'bool'. I think we don't need those > values. The current code does same thing for value of 1 and 2. so the > above code should be tmp->isCcbAugOk = true; //tmp->ccbCallback = NULL; * > > } else { > LOG_NO("Record for ccb 0x%llx not found or found aborted in > ok_for_critical", ccbId); > } > @@ -365,7 +365,7 @@ int imma_oi_ccb_record_set_critical(IMMA > tmp->isCritical = true; > rs = 1; > tmp->isCcbErrOk = false; > - tmp->isCcbAugOk = false; > + tmp->isCcbAugOk = 0; > > *[Hung][isCcbAugOk] Should be 'false', not '0'. * > tmp->ccbCallback = NULL; > if(tmp->opCount) { > if(!(cl_node->isPbe || cl_node->isApplier)) { > @@ -510,7 +510,7 @@ int imma_oi_ccb_record_close_augment(IMM > struct imma_oi_ccb_record *tmp = imma_oi_ccb_record_find(cl_node, > ccbId); > > if(tmp) { > - tmp->isCcbAugOk = false; > + tmp->isCcbAugOk = 0; > > *[Hung][isCcbAugOk] Should be 'false', not '0'. * > if(privateOmHandle) { > *privateOmHandle = tmp->privateAugOmHandle; > if(ccbDone) { > @@ -529,7 +529,7 @@ int imma_oi_ccb_record_close_augment(IMM > } > > int imma_oi_ccb_record_note_callback(IMMA_CLIENT_NODE *cl_node, > SaImmOiCcbIdT ccbId, > - IMMA_CALLBACK_INFO *callback) > + IMMA_CALLBACK_INFO *callback, SaUint8T augPermission) > > *[Hung][isCcbAugOk] We don't need new arg for this function. See my > comments in imma_proc_ccbaug_setup() below. * { > /* Stores pointer to callback record in oi_ccb_record, but should only > be done for ccb create/delete/modify upcalls and only for main OI > @@ -546,7 +546,9 @@ int imma_oi_ccb_record_note_callback(IMM > if(tmp && !(tmp->isAborted)) { > tmp->ccbCallback = callback; > if(callback) { > - tmp->isCcbAugOk = true; > + tmp->isCcbAugOk = augPermission; /* 0, 1, or 2 */ > + /* see: imma_oi_ccb_record in imma_cb.h*/ > > *[Hung][isCcbAugOk] **Should be 'true', not 'augPermission'. See my > comments in imma_proc_ccbaug_setup() below. *+ > osafassert(!(tmp->isCritical)); > osafassert(!(cl_node->isApplier)); > rs = 1; > 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 > @@ -3655,7 +3655,7 @@ SaAisErrorT saImmOiAugmentCcbInitialize( > IMMA_CALLBACK_INFO * cbi=NULL; > SaImmHandleT privateOmHandle = 0LL; > SaImmAdminOwnerHandleT privateAoHandle = 0LL; > - SaVersionT version = {'A', 2, 11}; > + SaVersionT version = {'A', 2, 17}; > SaUint32T adminOwnerId = 0; > SaUint32T ccbId = 0; > > 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 > @@ -3463,7 +3463,8 @@ SaAisErrorT imma_applyCcb(SaImmCcbHandle > Ccb handle is still valid. > > SA_AIS_ERR_VERSION - Not allowed for IMM API version > below A.2.14. > - SA_AIS_ERR_BAD_OPERATION - saImmOmCcbAbort not allowed on > augmented ccb. > + SA_AIS_ERR_BAD_OPERATION - saImmOmCcbAbort not allowed in > ccb > + augmentation > > Remaining returncodes identical to saImmOmFinalize. > > @@ -3488,7 +3489,7 @@ SaAisErrorT saImmOmCcbAbort(SaImmCcbHand > such as some problem with the PBE (persistent back end), > or a crash > of some vital IMM process. > > - Thus after a successfull saImmOmCcbVAlidate, the ccb is > in a state > + Thus after a successfull saImmOmCcbValidate, the ccb is > in a state > that only accepts one of: > > - saImmOmCcbApply(ccbHandle); - Only applicable if > saImmOmCcbValidate > @@ -3513,6 +3514,8 @@ SaAisErrorT saImmOmCcbAbort(SaImmCcbHand > SA_AIS_ERR_FAILED_OPERATION - Ccb is aborted possibly > due to failed > validation. > > + > + > > > ******************************************************************************/ > SaAisErrorT saImmOmCcbValidate(SaImmCcbHandleT ccbHandle) > @@ -3520,8 +3523,6 @@ SaAisErrorT saImmOmCcbValidate(SaImmCcbH > return imma_applyCcb(ccbHandle, true); > } > > - > - > > /**************************************************************************** > Name : saImmOmAdminOperationInvoke_2/_o2/_o3 > > @@ -5705,7 +5706,8 @@ static SaAisErrorT accessor_get_common(S > SaConstStringT objectName, > const SaImmAttrNameT *attributeNames, > SaImmAttrValuesT_2 ***attributes, > - bool bUseString); > + bool bUseString, > + SaUint32T ccbId); > > SaAisErrorT saImmOmAccessorGet_2(SaImmAccessorHandleT accessorHandle, > const SaNameT *objectName, > @@ -5732,7 +5734,7 @@ SaAisErrorT saImmOmAccessorGet_2(SaImmAc > } > } > > - rc = accessor_get_common(accessorHandle, objectNameStr, attributeNames, > attributes, false); > + rc = accessor_get_common(accessorHandle, objectNameStr, attributeNames, > attributes, false, 0); > > if(freeMemory == SA_TRUE) { > free(objectNameStr); > @@ -5745,14 +5747,15 @@ SaAisErrorT saImmOmAccessorGet_o3(SaImmA > SaConstStringT objectName, > const SaImmAttrNameT *attributeNames, > SaImmAttrValuesT_2 ***attributes) > { > - return accessor_get_common(accessorHandle, objectName, attributeNames, > attributes, true); > + return accessor_get_common(accessorHandle, objectName, attributeNames, > attributes, true, 0); > } > > static SaAisErrorT accessor_get_common(SaImmAccessorHandleT accessorHandle, > SaConstStringT objectName, > const SaImmAttrNameT *attributeNames, > SaImmAttrValuesT_2 ***attributes, > - bool bUseString) > + bool bUseString, > + SaUint32T ccbId) > { > SaAisErrorT rc = SA_AIS_OK; > uint32_t proc_rc; > @@ -5765,6 +5768,9 @@ static SaAisErrorT accessor_get_common(S > SaUint32T timeout; > > TRACE_ENTER(); > + if(ccbId) { > + TRACE_2("This is a SAFE read, ccbId:%u", ccbId); > + } > > if (cb->sv_id == 0) { > TRACE_2("ERR_BAD_HANDLE: No initialized handle exists!"); > @@ -5861,11 +5867,12 @@ static SaAisErrorT accessor_get_common(S > > memset(&evt, 0, sizeof(IMMSV_EVT)); > evt.type = IMMSV_EVT_TYPE_IMMND; > - evt.info.immnd.type = IMMND_EVT_A2ND_ACCESSOR_GET; > + evt.info.immnd.type = > (ccbId)?IMMND_EVT_A2ND_OBJ_SAFE_READ:IMMND_EVT_A2ND_ACCESSOR_GET; > IMMSV_OM_SEARCH_INIT *req = &(evt.info.immnd.info.searchInit); > req->client_hdl = immHandle; > req->rootName.size = strlen(objectName) + 1; > req->rootName.buf = (char *)objectName; > + req->ccbId = ccbId; /* Zero if not safe read. */ > > req->scope = SA_IMM_ONE; > if(osaf_is_extended_names_enabled()) { > @@ -6163,6 +6170,384 @@ static unsigned int get_obj_size(const I > return obj_size; > } > > +/**************************************************************************** > + Name : saImmOmCcbObjectRead > + > + Description : Adds support for an OM CCB client to read-access a config > object > + transactionally. The API works exactly the same way as > + saImmOmAccessorGet, except that > + > + a) The API takes a SaImmCcbHandleT instead of a > SaImmAccessorHandleT > + > + b) The values returned for the objects config attributes > are from > + a version of the object consistent with the > CCB/transaction. > + This means either the latest applied version or a > newer version > + created in the same CCB but not yet applied. > + > + c) Access to an object that has been deleted in the same > CCB but > + not applied is rejected with ERR_NOT_EXIST. > + > + d) Access to an object that has been created in the same > CCB but > + not applied is allowed, providing the latest version of > + the config attributes in that CCB. > + > + e) Safe read is not allowed using a runtime object as > target. > + > + Runtime attributes residing in a config object are > handled exactly > + the same as for saImmOmAccessorGet. The reason a > safe-read call > + is not allowed on a runtime *object* is that a runtime > object *only* > + contains runtime attributes. Performing a safe-read on a > runtime > + object makes no sense. > + > + > + Arguments : ccbHandle - Ccb Handle > + > + Return Values : > + Same return values as saImmOmAccessorGet_2 with > adjustments to exceptions > + in stated in the description > + > + Refer to SAI-AIS IMM A.2.1 specification for the other > return values > + and to the OpenSAF_IMMSv_PR (programmers reference) for > implementation > + specific details. > + > +******************************************************************************/ > +SaAisErrorT saImmOmCcbObjectRead(SaImmCcbHandleT ccbHandle, SaConstStringT > objectName, > + const SaImmAttrNameT *attributeNames, SaImmAttrValuesT_2 > ***attributes) > +{ > + SaAisErrorT rc = SA_AIS_OK; > + IMMA_CB *cb = &imma_cb; > + bool locked = false; > + SaImmAccessorHandleT accessorHandle=0LL; // handle value will be copied > from ccb-node > + IMMA_ADMIN_OWNER_NODE *ao_node = NULL; > + IMMA_CCB_NODE *ccb_node = NULL; > + SaImmHandleT immHandle=0LL; > + IMMA_CLIENT_NODE *cl_node = NULL; > + SaUint32T ccbId = 0; > + SaUint32T adminOwnerId = 0; > + //SaStringT *newErrorStrings = NULL; > + TRACE_ENTER(); > + > + /* Library checks below follows same "pattern" as > ccb_object_modify_common */ > + if (cb->sv_id == 0) { > + TRACE_2("ERR_BAD_HANDLE: No initialized handle exists!"); > + return SA_AIS_ERR_BAD_HANDLE; > + } > + > + if (objectName == NULL) { > + TRACE_2("ERR_INVALID_PARAM: objectName is NULL"); > + TRACE_LEAVE(); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > + if (!objectName[0]) { > + TRACE_2("ERR_INVALID_PARAM: objectName is empty"); > + TRACE_LEAVE(); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > + if (cb->is_immnd_up == false) { > + TRACE_3("ERR_TRY_AGAIN: IMMND is DOWN"); > + /* Any on going ccb has been aborted and this will be detected > + in resurrect processing. But this could be the first > operation > + attempted for a new ccb-id, i.e. there is no on-going ccb-id, > + (remember that a ccb-handle is associated with a chain of > actual > + ccb-ids). In that case resurrection may succeed. > + */ > + return SA_AIS_ERR_TRY_AGAIN; > + } > + > + /* get the CB Lock */ > + if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != NCSCC_RC_SUCCESS) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: Lock failed"); > + goto lock_fail; > + } > + locked = true; > + > + /* Get the CCB info */ > + imma_ccb_node_get(&cb->ccb_tree, &ccbHandle, &ccb_node); > + if (!ccb_node) { > + rc = SA_AIS_ERR_BAD_HANDLE; > + TRACE_2("ERR_BAD_HANDLE: Ccb handle not valid"); > + goto done; > + } > + > + if (ccb_node->mExclusive) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + TRACE_3("ERR_TRY_AGAIN: Ccb-id %u being created or in critical > phase, in another thread", > + ccb_node->mCcbId); > + goto done; > + } > + > + if (ccb_node->mAborted) { > + TRACE_2("ERR_FAILED_OPERATION: CCB %u has already been aborted", > + ccb_node->mCcbId); > + rc = SA_AIS_ERR_FAILED_OPERATION; > + goto done; > + } > + > + immHandle = ccb_node->mImmHandle; > + > + > + /* Free string from previous ccb-op */ > + imma_free_errorStrings(ccb_node->mErrorStrings); > + ccb_node->mErrorStrings = NULL; > + > + /*Look up client node also, to verify that the client handle > + is still active. */ > + imma_client_node_get(&cb->client_tree, &immHandle, &cl_node); > + if (!(cl_node && cl_node->isOm)) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: No valid SaImmHandleT associated with > Ccb"); > + goto done; > + } > + > + if(!cl_node->isImmA2x11) { > + rc = SA_AIS_ERR_VERSION; > + TRACE_2("ERR_VERSION: saImmOmCcbObjectRead only supported for " > + "A.02.17 and above"); > + goto done; > + } > + > + /* Stale check and possible resurrect done here for the complete > ccb-handle level. > + The stale check later done in the call via accessorGet wil be > redundant. But > + the handle should never be stale there if call arrived from here. > + */ > + if (cl_node->stale) { > + TRACE_1("IMM Handle %llx is stale", immHandle); > + > + if(!(ccb_node->mApplied)) { > + TRACE_3("ERR_FAILED_OPERATION: IMMND DOWN discards ccb " > + "in active but non-critical state"); > + ccb_node->mAborted = true; > + rc = SA_AIS_ERR_FAILED_OPERATION; > + /* We drop the resurrect task since this ccb is doomed. > */ > + goto done; > + } > + > + /* Why do we bother trying resurrect? See ##1## above. */ > + > + bool resurrected = imma_om_resurrect(cb, cl_node, &locked); > + cl_node = NULL; > + ccb_node =NULL; > + > + if (!locked && m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != > NCSCC_RC_SUCCESS) { > + TRACE_4("ERR_LIBRARY: Lock failed"); > + rc = SA_AIS_ERR_LIBRARY; > + goto done; > + } > + locked = true; > + > + imma_client_node_get(&cb->client_tree, &immHandle, &cl_node); > + > + if (!resurrected || !cl_node || !(cl_node->isOm) || > cl_node->stale) { > + TRACE_3("ERR_BAD_HANDLE: Reactive ressurect of handle > %llx failed", immHandle); > + if (cl_node && cl_node->stale) {cl_node->exposed = > true;} > + rc = SA_AIS_ERR_BAD_HANDLE; > + goto done; > + } > + > + TRACE_1("Reactive resurrect of handle %llx succeeded", > immHandle); > + > + /* Look up ccb_node again */ > + imma_ccb_node_get(&cb->ccb_tree, &ccbHandle, &ccb_node); > + if (!ccb_node) { > + rc = SA_AIS_ERR_BAD_HANDLE; > + TRACE_3("ERR_BAD_HANDLE: Ccb handle not valid after > successful resurrect"); > + goto done; > + } > + > + if (ccb_node->mExclusive) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + TRACE_3("ERR_TRY_AGAIN: Ccb-id %u being created or in > critical phase, " > + "in another thread", ccb_node->mCcbId); > + goto done; > + } > + > + if (ccb_node->mAborted) { > + TRACE_3("ERR_FAILED_OPERATION: Ccb-id %u was aborted", > ccb_node->mCcbId); > + rc = SA_AIS_ERR_FAILED_OPERATION; > + goto done; > + } > + } > + > + /* Get the Admin Owner info Although not really interested in admo for > safe-read. */ > + imma_admin_owner_node_get(&cb->admin_owner_tree, > &(ccb_node->mAdminOwnerHdl), &ao_node); > + if (!ao_node) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: No Amin-Owner associated with Ccb"); > + goto done; > + } > + > + osafassert(ccb_node->mImmHandle == ao_node->mImmHandle); > + adminOwnerId = ao_node->mAdminOwnerId; > + ao_node=NULL; > + > + if (ccb_node->mApplied) { /* Current ccb-id is closed, get a new one.*/ > + if((rc = imma_proc_increment_pending_reply(cl_node, true)) != > SA_AIS_OK) { > + TRACE_4("ERR_LIBRARY: Overlapping use of IMM handle by > multiple threads"); > + goto done; > + } > + rc = imma_newCcbId(cb, ccb_node, adminOwnerId, &locked, > cl_node->syncr_timeout); > + cl_node = NULL; > + if(rc == SA_AIS_ERR_LIBRARY) {goto done;} > + /* ccb_node still valid if rc == SA_AIS_OK. */ > + if(rc == SA_AIS_OK) { > + osafassert(!(ccb_node->mExclusive)); > + osafassert(locked); > + } > + > + if (!locked) { > + if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != > NCSCC_RC_SUCCESS) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: Lock failed"); > + goto done; > + } > + locked = true; > + } > + > + imma_client_node_get(&cb->client_tree, &immHandle, &cl_node); > + if (!(cl_node && cl_node->isOm)) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: No client associated with Admin > Owner"); > + goto done; > + } > + > + imma_proc_decrement_pending_reply(cl_node, true); > + > + if (rc != SA_AIS_OK) { > + goto done; > + } > + > + /* successfully obtained new ccb-id */ > + > + if (cl_node->stale) { > + /* Became stale AFTER we successfully obtained new > ccb-id ! */ > + TRACE_3("ERR_FAILED_OPERATION: IMM Handle %llx became > stale", immHandle); > + > + rc = SA_AIS_ERR_FAILED_OPERATION; > + /* We know the ccb WAS terminated*/ > + ccb_node->mCcbId = 0; > + ccb_node->mAborted = true; > + goto done; > + } > + } > + > + osafassert(locked); > + osafassert(cl_node); > + osafassert(ccb_node); > + > + ccbId = ccb_node->mCcbId; > + accessorHandle = ccb_node->mCcbObjectReadAccessorHandle; > + > + m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); > + locked = false; > + > + if(accessorHandle == 0LL) { > + rc = saImmOmAccessorInitialize(immHandle, &accessorHandle); > + if (rc != SA_AIS_OK) { > + TRACE_2("Failed to initialize accessor-handle"); > + goto done; > + } > + TRACE_2("Initialized CCB safe-read accessor handle %llu", > accessorHandle); > + > + /* get the CB Lock */ > + if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != > NCSCC_RC_SUCCESS) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: Lock failed"); > + goto lock_fail; > + } > + locked = true; > + > + /* Get the CCB info */ > + imma_ccb_node_get(&cb->ccb_tree, &ccbHandle, &ccb_node); > + if (!ccb_node) { > + rc = SA_AIS_ERR_BAD_HANDLE; > + TRACE_2("ERR_BAD_HANDLE: Ccb handle not valid"); > + goto done; > + } > > *[Hung] 'ccb_node' is already retrived above, we don't need to call > imma_ccb_node_get() again. *+ > + ccb_node->mCcbObjectReadAccessorHandle = accessorHandle; > + > + m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); > + locked = false; > + } > + > + TRACE("Optimistically invoking accessor_get_common with ccbid:%u", > ccbId); > + rc = accessor_get_common(accessorHandle, objectName, attributeNames, > attributes, true, ccbId); > + if(rc == SA_AIS_ERR_INTERRUPT) { > + TRACE("CcbObjectRead: Object was not locked. Get global shared > read lock"); > + IMMSV_EVT evt; > + IMMSV_EVT *out_evt = NULL; > + > + /* get the CB Lock */ > + if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != > NCSCC_RC_SUCCESS) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: Lock failed"); > + goto lock_fail; > + } > + locked = true; > + > + imma_client_node_get(&cb->client_tree, &immHandle, &cl_node); > + if (!(cl_node && cl_node->isOm)) { > + rc = SA_AIS_ERR_LIBRARY; > + TRACE_4("ERR_LIBRARY: No client associated with Admin > Owner"); > + goto done; > + } > + > + /* Obtain a safe-read CCB lock for this the object over fevs. */ > + memset(&evt, 0, sizeof(IMMSV_EVT)); > + evt.type = IMMSV_EVT_TYPE_IMMND; > + /* This is a hack. Re-using the SAFE_READ msg type here only to > ackquire lock. */ > + evt.info.immnd.type = IMMND_EVT_A2ND_OBJ_SAFE_READ; > + IMMSV_OM_SEARCH_INIT *req = &(evt.info.immnd.info.searchInit); > + req->client_hdl = immHandle; > + req->rootName.size = strlen(objectName) + 1; > + req->rootName.buf = (char *)objectName; > + req->ccbId = ccbId; > + req->scope = SA_IMM_ONE; > + if(osaf_is_extended_names_enabled()) { > + req->searchParam.present = > ImmOmSearchParameter_PR_NOTHING; > + } else { > + req->searchParam.present = > ImmOmSearchParameter_PR_nonExtendedName_NOTHING; > + } > + req->searchOptions = SA_IMM_SEARCH_ONE_ATTR | > SA_IMM_SEARCH_GET_ALL_ATTR; > + req->searchOptions |= SA_IMM_SEARCH_NO_RDN; > > *[Hung] When using this hack, only 'req->ccbId' and 'req->rootName' are > used. 'req->searchParam.present' is also needed for bypassing > immsv_evt_enc_sublevels(). We don't have to set value for other fields.* > > + > + rc = imma_evt_fake_evs(cb, &evt, &out_evt, > cl_node->syncr_timeout, cl_node->handle, &locked, false); > + if(rc != SA_AIS_OK ) { > + goto done; > + } > + > + rc = out_evt->info.imma.info.errRsp.error; > + if(rc != SA_AIS_OK ) { > + goto done; > + } > + > + if (locked) { > + m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); > + locked = false; > + } > + TRACE("Ccb %u Obtained shared safe-read lock for object '%s' > over fevs", > + ccbId, objectName); > + > + rc = accessor_get_common(accessorHandle, objectName, > attributeNames, attributes, true, ccbId); > + } > + > + done: > + //imma_free_errorStrings(newErrorStrings); /* In case of failed > resurrect only */ > + > + if (locked) { > + m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); > + locked = false; > + } > + > + lock_fail: > + > + TRACE_LEAVE(); > + return rc; > + > +} > > > SaAisErrorT immsv_sync(SaImmHandleT immHandle, const SaImmClassNameT > className, > @@ -8429,6 +8814,7 @@ static SaAisErrorT imma_finalizeCcb(SaIm > SaImmAdminOwnerHandleT adminOwnerHdl = 0LL; > SaUint32T adminOwnerId = 0; > SaUint32T timeout = 0; > + SaImmAccessorHandleT safeReadAccessorHandle=0LL; // Copied from > ccb_node later. > TRACE_ENTER(); > > if (cb->sv_id == 0) { > @@ -8476,10 +8862,13 @@ static SaAisErrorT imma_finalizeCcb(SaIm > goto done; > } > > + > + > immHandle = ccb_node->mImmHandle; > adminOwnerHdl = ccb_node->mAdminOwnerHdl; > ccbActive = ccb_node->mCcbId && !(ccb_node->mApplied); > ccbId = ccb_node->mCcbId; > + safeReadAccessorHandle = ccb_node->mCcbObjectReadAccessorHandle; /* > Safe read handle */ > > ccb_node->mExclusive = true; > > @@ -8647,8 +9036,14 @@ static SaAisErrorT imma_finalizeCcb(SaIm > } > > done: > - if (locked) > + if (locked) { > m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); > + } > + > + if(safeReadAccessorHandle && !keepCcbHandleOpen) { > + TRACE_3("Finalizing CCB safe-read accessor handle %llu", > safeReadAccessorHandle); > + saImmOmAccessorFinalize(safeReadAccessorHandle); > + } > > lock_fail: > > 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 > @@ -1880,14 +1880,21 @@ static void imma_proc_ccbaug_setup(IMMA_ > case IMMA_CALLBACK_OI_CCB_CREATE: > case IMMA_CALLBACK_OI_CCB_DELETE: > case IMMA_CALLBACK_OI_CCB_MODIFY: > - if(!(imma_oi_ccb_record_note_callback(cl_node, > callback->ccbID, callback))) { > + if(!(imma_oi_ccb_record_note_callback(cl_node, > callback->ccbID, callback, 2))) { > TRACE_3("Failed to note callback for ccb %u, " > "Ccb augment not possible", > callback->ccbID); > } > break; > > + case IMMA_CALLBACK_OI_CCB_COMPLETED: > + if(!(imma_oi_ccb_record_note_callback(cl_node, > callback->ccbID, callback, 1))) { > + TRACE_3("Failed to note completed callback for > ccb %u, " > + "Ccb augment not possible", > callback->ccbID); > + } > + break; > + > default: > - imma_oi_ccb_record_note_callback(cl_node, > callback->ccbID, NULL); > + imma_oi_ccb_record_note_callback(cl_node, > callback->ccbID, NULL, 0); > } > } > > *[Hung][isCcbAugOk] The above code should be: case > IMMA_CALLBACK_OI_CCB_CREATE: case IMMA_CALLBACK_OI_CCB_DELETE: case > IMMA_CALLBACK_OI_CCB_MODIFY: case IMMA_CALLBACK_OI_CCB_COMPLETED: > if(!(imma_oi_ccb_record_note_callback(cl_node, callback->ccbID, > callback))) { TRACE_3("Failed to note callback for ccb %u, " "Ccb > augment not possible", callback->ccbID); } break; default: > imma_oi_ccb_record_note_callback(cl_node, callback->ccbID, NULL); I > think we don't need new arg for ***imma_oi_ccb_record_note_callback*() > since whether 'isCcbAugOk' is 1 or 2, there's no different.* > > > @@ -2162,10 +2169,41 @@ static bool imma_process_callback_info(I > } > > if(!(cl_node->isApplier) || (isPbeOp && > cl_node->isPbe)) { > + SaImmHandleT privateAugOmHandle = 0LL; > /* Appliers dont reply on completed > except PBE slave replying on completed > for PRTO- delete. PRTO-delete means > ccb-id is in the high rannge. > So PBE slave does NOT reply on > completed for regular CCBs. > */ > + > + //Add code for closing safe-read > augmentations in completed callback. > + > if(!imma_oi_ccb_record_close_augment(cl_node, callback->ccbID, > &privateAugOmHandle, false)) { > + > TRACE_3("CcbObjectCompletedCallback: imma_oi_ccb_record_close_augment " > + "returned false for ccb > %u", callback->ccbID); > + } > + > + if(privateAugOmHandle) { > + osafassert(locked); > + > osafassert(m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE) == NCSCC_RC_SUCCESS); > + locked = false; > + > + > osafassert(immsv_om_augment_ccb_get_result); > + SaAisErrorT augResult = > immsv_om_augment_ccb_get_result(privateAugOmHandle, > + callback->ccbID); > + > + > osafassert(m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) == NCSCC_RC_SUCCESS); > + locked = true; > + > + if(augResult != SA_AIS_OK) { > + TRACE("A non-ok result > %u from an augmented CCB overrides the reply from " > + "the OI %u on > completed", augResult, > + > ccbCompletedRpl.info.immnd.info.ccbUpcallRsp.result); > + > + > ccbCompletedRpl.info.immnd.info.ccbUpcallRsp.result = augResult; > + } > > + } > + > + > + > localEr = imma_evt_fake_evs(cb, > &ccbCompletedRpl, NULL, 0, cl_node->handle, &locked, false); > if (localEr != NCSCC_RC_SUCCESS) { > /*Cant do anything but log > error and drop this reply. */ > @@ -2486,7 +2524,7 @@ static bool imma_process_callback_info(I > locked = true; > > if(augResult != SA_AIS_OK) { > - TRACE("A non-ok result > from an augmented CCB %u overrides the reply from " > + TRACE("A non-ok result > %u from an augmented CCB overrides the reply from " > "the OI %u on > create-uc", augResult, > > ccbObjCrRpl.info.immnd.info.ccbUpcallRsp.result); > > @@ -2652,7 +2690,7 @@ static bool imma_process_callback_info(I > locked = true; > > if(augResult != SA_AIS_OK) { > - TRACE("A non-ok result > from an augmented CCB %u overrides the reply from " > + TRACE("A non-ok result > %u from an augmented CCB overrides the reply from " > "the OI %u on > delete-op", augResult, > > ccbObjDelRpl.info.immnd.info.ccbUpcallRsp.result); > > @@ -2891,7 +2929,7 @@ static bool imma_process_callback_info(I > locked = true; > > if(augResult != SA_AIS_OK) { > - TRACE("A non-ok result > from an augmented CCB %u overrides the reply from " > + TRACE("A non-ok result > %u from an augmented CCB overrides the reply from " > "the OI %u on > modify-uc", augResult, > > ccbObjModRpl.info.immnd.info.ccbUpcallRsp.result); > > diff --git a/osaf/libs/common/immsv/immsv_evt.c > b/osaf/libs/common/immsv/immsv_evt.c > --- a/osaf/libs/common/immsv/immsv_evt.c > +++ b/osaf/libs/common/immsv/immsv_evt.c > @@ -181,6 +181,7 @@ static const char *immnd_evt_names[] = { > "IMMND_EVT_D2ND_IMPLSET_RSP_2", /* Implementer set reply from D with > impl id */ > "IMMND_EVT_A2ND_OBJ_CREATE_2", /* saImmOmCcbObjectCreate_o3 */ > "IMMND_EVT_A2ND_OI_OBJ_CREATE_2", /* saImmOiRtObjectCreate_o3 */ > + "IMMND_EVT_A2ND_OBJ_SAFE_READ", /* saImmOmCcbObjectRead */ > "undefined (high)" > }; > > @@ -1860,7 +1861,8 @@ static uint32_t immsv_evt_enc_sublevels( > } > } > } else if ((i_evt->info.immnd.type == > IMMND_EVT_A2ND_SEARCHINIT) || > - (i_evt->info.immnd.type == > IMMND_EVT_A2ND_ACCESSOR_GET)) { > + (i_evt->info.immnd.type == IMMND_EVT_A2ND_ACCESSOR_GET) > || > + (i_evt->info.immnd.type == > IMMND_EVT_A2ND_OBJ_SAFE_READ)) { > int depth = 0; > /*Encode the rootName */ > IMMSV_OCTET_STRING *os = > &(i_evt->info.immnd.info.searchInit.rootName); > @@ -1901,7 +1903,6 @@ static uint32_t immsv_evt_enc_sublevels( > LOG_ER("TOO MANY attribute names line:%u", > __LINE__); > return NCSCC_RC_OUT_OF_MEM; > } > - > } else if (i_evt->info.immnd.type == > IMMND_EVT_A2ND_RT_ATT_UPPD_RSP) { > int depth = 0; > /*Encode the objectName */ > @@ -2406,8 +2407,9 @@ static uint32_t immsv_evt_dec_sublevels( > ++depth; > } > } > - } else if ((o_evt->info.immnd.type == > IMMND_EVT_A2ND_SEARCHINIT) || > - (o_evt->info.immnd.type == > IMMND_EVT_A2ND_ACCESSOR_GET)) { > + } else if ((o_evt->info.immnd.type == > IMMND_EVT_A2ND_SEARCHINIT) || > + (o_evt->info.immnd.type == IMMND_EVT_A2ND_ACCESSOR_GET) > || > + (o_evt->info.immnd.type == > IMMND_EVT_A2ND_OBJ_SAFE_READ)) { > /*Decode the rootName */ > IMMSV_OCTET_STRING *os = > &(o_evt->info.immnd.info.searchInit.rootName); > if (os->size) { > @@ -3235,6 +3237,7 @@ static uint32_t immsv_evt_enc_toplevel(I > > case IMMND_EVT_A2ND_SEARCHINIT: /* SearchInitialize */ > case IMMND_EVT_A2ND_ACCESSOR_GET: /* AccessorGet */ > + case IMMND_EVT_A2ND_OBJ_SAFE_READ: /* saImmOmCcbObjectRead > */ > > IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 8); > ncs_encode_64bit(&p8, > immndevt->info.searchInit.client_hdl); > @@ -3274,6 +3277,12 @@ static uint32_t immsv_evt_enc_toplevel(I > IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 1); > ncs_encode_8bit(&p8, > (immndevt->info.searchInit.attributeNames) ? 1 : 0); > ncs_enc_claim_space(o_ub, 1); > + > + if(immndevt->type == IMMND_EVT_A2ND_OBJ_SAFE_READ) { > + IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 4); > + ncs_encode_32bit(&p8, > immndevt->info.searchInit.ccbId); > + ncs_enc_claim_space(o_ub, 4); > + } > break; > > case IMMND_EVT_A2ND_SEARCHNEXT: /* SearchNext */ > @@ -4638,6 +4647,7 @@ static uint32_t immsv_evt_dec_toplevel(N > > case IMMND_EVT_A2ND_ACCESSOR_GET: /* AccessorGet */ > case IMMND_EVT_A2ND_SEARCHINIT: /* SearchInitialize */ > + case IMMND_EVT_A2ND_OBJ_SAFE_READ: /* saImmOmCcbObjectRead */ > IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 8); > immndevt->info.searchInit.client_hdl = > ncs_decode_64bit(&p8); > ncs_dec_skip_space(i_ub, 8); > @@ -4677,6 +4687,12 @@ static uint32_t immsv_evt_dec_toplevel(N > immndevt->info.searchInit.attributeNames = > (void *)0x1; > } > ncs_dec_skip_space(i_ub, 1); > + > + if(immndevt->type == IMMND_EVT_A2ND_OBJ_SAFE_READ) { > + IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, > 4); > + immndevt->info.searchInit.ccbId = > ncs_decode_32bit(&p8); > + ncs_dec_skip_space(i_ub, 4); > + } > break; > > case IMMND_EVT_A2ND_SEARCHNEXT: /* SearchNext */ > diff --git a/osaf/libs/common/immsv/include/immsv_evt.h > b/osaf/libs/common/immsv/include/immsv_evt.h > --- a/osaf/libs/common/immsv/include/immsv_evt.h > +++ b/osaf/libs/common/immsv/include/immsv_evt.h > @@ -214,6 +214,8 @@ typedef enum immnd_evt_type { > IMMND_EVT_A2ND_OBJ_CREATE_2 = 98, /* saImmOmCcbObjectCreate_o3 */ > IMMND_EVT_A2ND_OI_OBJ_CREATE_2 = 99, /* saImmOiRtObjectCreate_o3 */ > > + IMMND_EVT_A2ND_OBJ_SAFE_READ = 100, /* saImmOmCcbObjectRead */ > + > IMMND_EVT_MAX > } IMMND_EVT_TYPE; > /* Make sure the string array in immsv_evt.c matches the IMMND_EVT_TYPE > enum. */ > diff --git a/osaf/libs/common/immsv/include/immsv_evt_model.h > b/osaf/libs/common/immsv/include/immsv_evt_model.h > --- a/osaf/libs/common/immsv/include/immsv_evt_model.h > +++ b/osaf/libs/common/immsv/include/immsv_evt_model.h > @@ -208,6 +208,7 @@ extern "C" { > SaUint64T searchOptions; > IMMSV_SEARCH_PARAM searchParam; > IMMSV_ATTR_NAME_LIST *attributeNames; > + SaUint32T ccbId; /* Only for saImmOmCcbObjectRead */ > } IMMSV_OM_SEARCH_INIT; > > typedef struct ImmsvOmSearchRemote { > diff --git a/osaf/libs/saf/include/saImmOm_A_2_16.h > b/osaf/libs/saf/include/saImmOm_A_2_16.h > --- a/osaf/libs/saf/include/saImmOm_A_2_16.h > +++ b/osaf/libs/saf/include/saImmOm_A_2_16.h > @@ -66,4 +66,6 @@ extern "C" { > > #include <saImmOm_A_2_17.h> > > +#include <saImmOm_A_2_17.h> > > *[Hung] 'saImmOm_A_2_17.h' is already included. This include is duplicated. *+ > #endif /* _SA_IMM_OM_A_2_16_H */ > diff --git a/osaf/libs/saf/include/saImmOm_A_2_17.h > b/osaf/libs/saf/include/saImmOm_A_2_17.h > --- a/osaf/libs/saf/include/saImmOm_A_2_17.h > +++ b/osaf/libs/saf/include/saImmOm_A_2_17.h > @@ -59,6 +59,12 @@ extern "C" { > #define SA_IMM_ATTR_STRONG_DEFAULT 0x0000000020000000 /* See: > https://sourceforge.net/p/opensaf/tickets/1425 > Supported in > OpenSaf 5.0 */ > > +/* 4.8.x saImmOmCcb */ > > *[Hung] It's 5.0.x * > > + > + extern SaAisErrorT > + saImmOmCcbObjectRead(SaImmCcbHandleT ccbHandle, SaConstStringT > objectName, > + const SaImmAttrNameT *attributeNames, > SaImmAttrValuesT_2 ***attributes); > + > > #ifdef __cplusplus > } > > *[Hung] Comments in these functions should be corrected as now we > support ccb augmentation for completed callback.******/* Internal > function used *only* from saImmOiAugmentCcbInitialize****Creates a > mockup ccb-node and mockup admo-node so that an****OI client can augment > a pre existing ccb with added create,****delete and modify > operations.*****/****SaAisErrorT > immsv_om_augment_ccb_initialize(******/* Internal function used *only* >from callback processing in imma_proc.c****Obtains the outcome for an OI > augmented CCB. ****The outcome should be obtained after the > create/delete/modify callback ****to the OI returns, to the callback > post processing in imma_proc.c****See > imma_process_callback_info()*****/****SaAisErrorT > immsv_om_augment_ccb_get_result(******int > imma_oi_ccb_record_note_callback(****{****/* Stores pointer to callback > record in oi_ccb_record, but should only****be done for ccb > create/delete/modify upcalls and only for main OI ****(not appliers), > since otherwise the ccb can not be augmented.****The pointer is needed > for the augmented ccb dowcalls. These ****downcalls are in essence > incremental pre-replies on an on-going****oi-upcall > (create/delete/modify). The augmenting "sub ccb" must be****terminated > before returning from the oi-upcall. *****/* > > > > > ------------------------------------------------------------------------------ > Transform Data into Opportunity. > Accelerate data analysis in your applications with > Intel Data Analytics Acceleration Library. > Click to learn more. > http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > > ------------------------------------------------------------------------------ > Transform Data into Opportunity. > Accelerate data analysis in your applications with > Intel Data Analytics Acceleration Library. > Click to learn more. > http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140 > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
