Hi Hung, A minor comment can be found inline.
Ack when the code is fixed. No need to send the patch for another review. -----Original Message----- From: Hung Nguyen [mailto:hung.d.ngu...@dektech.com.au] Sent: den 29 september 2016 11:24 To: Zoran Milinkovic <zoran.milinko...@ericsson.com>; reddy.neelaka...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: [PATCH 1 of 1] imm: Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650] osaf/services/saf/immsv/immnd/ImmModel.cc | 23 ++++++++- osaf/services/saf/immsv/immnd/immnd_evt.c | 74 +++++++----------------------- 2 files changed, 37 insertions(+), 60 deletions(-) Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION. This is to prevent om clients from retrying the operations that are already rejected by OI. 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 @@ -10497,6 +10497,11 @@ ImmModel::deleteObject(ObjectMap::iterat void ImmModel::setCcbErrorString(CcbInfo *ccb, const char *errorString, va_list vl) { + /* Only set error strings on node where the OM client resides */ + if (!ccb->mOriginatingConn && !(ccb->mAugCcbParent && ccb->mAugCcbParent->mOriginatingConn)) { + return; + } + [Zoran] The upper code should be removed. There are cases where OI set error strings. Thanks, Zoran int errLen = strlen(errorString) + 1; char *fmtError = (char *)malloc(errLen); int len; @@ -10872,7 +10877,11 @@ ImmModel::ccbObjDelContinuation(immsv_oi "implementer returned error, Ccb aborted with error: %u", rsp->result); ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION; - setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned error: ", rsp->result); + if (rsp->result == SA_AIS_ERR_BAD_OPERATION || rsp->result == SA_AIS_ERR_FAILED_OPERATION) { + setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned error: %u", rsp->result); + } else { + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer returned error: %u", rsp->result); + } //TODO: This is perhaps more drastic than the specification //demands. We are here aborting the entire Ccb, whereas the spec //seems to allow for a non-ok returnvalue from implementer @@ -11150,7 +11159,11 @@ ImmModel::ccbObjCreateContinuation(SaUin "implementer returned error, Ccb aborted with error: %u", error); ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION; - setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned error: %u", error); + if (error == SA_AIS_ERR_BAD_OPERATION || error == SA_AIS_ERR_FAILED_OPERATION) { + setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned error: %u", error); + } else { + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer returned error: %u", error); + } //TODO: This is perhaps more drastic than the specification demands. //We are here aborting the entire Ccb, whereas the spec seems to allow //for a non-ok returnvalue from implementer (in this callback) to @@ -11240,7 +11253,11 @@ ImmModel::ccbObjModifyContinuation(SaUin LOG_IN("ImmModel::ccbObjModifyContinuation: " "implementer returned error, Ccb aborted with error: %u", error); ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION; - setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned error: %u", error); + if (error == SA_AIS_ERR_BAD_OPERATION || error == SA_AIS_ERR_FAILED_OPERATION) { + setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned error: %u", error); + } else { + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer returned error: %u", error); + } //TODO: This is perhaps more drastic than the specification demands. //We are here aborting the entire Ccb, whereas the spec seems to allow //for a non-ok returnvalue from implementer (in this callback) to diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c b/osaf/services/saf/immsv/immnd/immnd_evt.c --- a/osaf/services/saf/immsv/immnd/immnd_evt.c +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c @@ -3742,41 +3742,21 @@ static void immnd_evt_proc_ccb_obj_modif send_evt.type = IMMSV_EVT_TYPE_IMMA; send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; IMMSV_ATTR_NAME_LIST strList; - IMMSV_ATTR_NAME_LIST errStrList = { { 0 }, NULL }; - + IMMSV_ATTR_NAME_LIST* errStrList = immModel_ccbGrabErrStrings(cb, evt->info.ccbUpcallRsp.ccbId); + if (evt->info.ccbUpcallRsp.result != SA_AIS_OK) { - if(evt->info.ccbUpcallRsp.result != SA_AIS_ERR_FAILED_OPERATION) { - char buf[2]; - int size; - - /* Create error string */ - size = snprintf(buf, 1, - IMM_RESOURCE_ABORT "Upcall failed with error code: %u", - evt->info.ccbUpcallRsp.result); - osafassert(size >= 0); - errStrList.name.size = ++size; - errStrList.name.buf = (char *)malloc(size); - errStrList.next = NULL; - size = snprintf(errStrList.name.buf, errStrList.name.size, - IMM_RESOURCE_ABORT "Upcall failed with error code: %u", - evt->info.ccbUpcallRsp.result); - osafassert(size >= 0); - - evt->info.ccbUpcallRsp.result = SA_AIS_ERR_FAILED_OPERATION; - } + evt->info.ccbUpcallRsp.result = SA_AIS_ERR_FAILED_OPERATION; + if (evt->info.ccbUpcallRsp.errorString.size) { osafassert(evt->type == IMMND_EVT_A2ND_CCB_OBJ_MODIFY_RSP_2); - if(errStrList.name.buf) { - strList.next = &errStrList; - } else { - strList.next = NULL; - } + + strList.next = errStrList; strList.name = evt->info.ccbUpcallRsp.errorString;/*borrow*/ send_evt.info.imma.info.errRsp.errStrings = &(strList); send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; - } else if(errStrList.name.buf) { + } else if (errStrList) { osafassert(evt->type == IMMND_EVT_A2ND_CCB_OBJ_MODIFY_RSP); - send_evt.info.imma.info.errRsp.errStrings = &(errStrList); + send_evt.info.imma.info.errRsp.errStrings = errStrList; send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; } } @@ -3789,7 +3769,7 @@ static void immnd_evt_proc_ccb_obj_modif LOG_WA("Failed to send response to agent/client over MDS rc:%u", rc); } - free(errStrList.name.buf); + immsv_evt_free_attrNames(errStrList); } TRACE_LEAVE(); @@ -3841,41 +3821,21 @@ static void immnd_evt_proc_ccb_obj_creat send_evt.type = IMMSV_EVT_TYPE_IMMA; send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; IMMSV_ATTR_NAME_LIST strList; - IMMSV_ATTR_NAME_LIST errStrList = { { 0 }, NULL }; + IMMSV_ATTR_NAME_LIST* errStrList = immModel_ccbGrabErrStrings(cb, evt->info.ccbUpcallRsp.ccbId); if (evt->info.ccbUpcallRsp.result != SA_AIS_OK) { - if(evt->info.ccbUpcallRsp.result != SA_AIS_ERR_FAILED_OPERATION) { - char buf[2]; - int size; - - /* Create error string */ - size = snprintf(buf, 1, - IMM_RESOURCE_ABORT "Upcall failed with error code: %u", - evt->info.ccbUpcallRsp.result); - osafassert(size >= 0); - errStrList.name.size = ++size; - errStrList.name.buf = (char *)malloc(size); - errStrList.next = NULL; - size = snprintf(errStrList.name.buf, errStrList.name.size, - IMM_RESOURCE_ABORT "Upcall failed with error code: %u", - evt->info.ccbUpcallRsp.result); - osafassert(size >= 0); - - evt->info.ccbUpcallRsp.result = SA_AIS_ERR_FAILED_OPERATION; - } + evt->info.ccbUpcallRsp.result = SA_AIS_ERR_FAILED_OPERATION; + if (evt->info.ccbUpcallRsp.errorString.size) { osafassert(evt->type == IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP_2); - if(errStrList.name.buf) { - strList.next = &errStrList; - } else { - strList.next = NULL; - } + + strList.next = errStrList; strList.name = evt->info.ccbUpcallRsp.errorString;/*borrow*/ send_evt.info.imma.info.errRsp.errStrings = &(strList); send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; - } else if(errStrList.name.buf) { + } else if (errStrList) { osafassert(evt->type == IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP); - send_evt.info.imma.info.errRsp.errStrings = &(errStrList); + send_evt.info.imma.info.errRsp.errStrings = errStrList; send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; } } @@ -3888,7 +3848,7 @@ static void immnd_evt_proc_ccb_obj_creat LOG_WA("Failed to send response to agent/client over MDS rc:%u", rc); } - free(errStrList.name.buf); + immsv_evt_free_attrNames(errStrList); } TRACE_LEAVE(); ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
