Guess the idea when we have error strings is that we should not have syslogs :-) /Hans
> -----Original Message----- > From: Anders Björnerstedt > Sent: den 7 april 2014 15:01 > To: Hans Feldt; Zoran Milinkovic; reddy.neelaka...@oracle.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: RE: [devel] [PATCH 1 of 2] IMM: add error strings for CCB operations > for ERR_NOT_EXIST error [#459] > > Yes its OK, the only thought I had here was that perhaps there could be an > advantage if the error string returned over the API was identical to the > related > syslog message. > > /AndersBj > > -----Original Message----- > From: Hans Feldt [mailto:hans.fe...@ericsson.com] > Sent: den 7 april 2014 13:49 > To: Zoran Milinkovic; reddy.neelaka...@oracle.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 2] IMM: add error strings for CCB operations > for ERR_NOT_EXIST error [#459] > > Some feedback on the strings produced: > > instead of: > "ERR_NOT_EXIST: object '%s' does not have an implementer and flag > SA_IMM_CCB_REGISTERED_OI is set" > > we could have: > "ERR_NOT_EXIST: object '%s' exist but no implementer (which is required)" > > and: > "ERR_NOT_EXIST: object '%s' does not exist" > > Any comments? > /Hans > > On 04/02/2014 03:36 PM, Zoran Milinkovic wrote: > > osaf/services/saf/immsv/immnd/ImmModel.cc | 60 > > +++++++++++++++++++++++++++++++ > > osaf/services/saf/immsv/immnd/ImmModel.hh | 5 ++ > > osaf/services/saf/immsv/immnd/immnd_evt.c | 27 ++++++++++++- > > 3 files changed, 89 insertions(+), 3 deletions(-) > > > > > > When an implementer is detached from an object/class, CCB operations return > > ERR_NOT_EXIST error code. > > The patch should give more information regarding the error code in the > > error string. > > > > 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 > > @@ -6950,6 +6950,10 @@ SaAisErrorT ImmModel::ccbObjectCreate(Im > > TRACE_7("ERR_NOT_EXIST: object '%s' does not have an " > > "implementer and flag SA_IMM_CCB_REGISTERED_OI is > > set", > > objectName.c_str()); > > + setCcbErrorString(ccb, > > + "ERR_NOT_EXIST: object '%s' does not have an " > > + "implementer and flag SA_IMM_CCB_REGISTERED_OI > > is set", > > + objectName.c_str()); > > err = SA_AIS_ERR_NOT_EXIST; > > } > > } else { /* SA_IMM_CCB_REGISTERED_OI NOT set */ @@ > > -7790,6 +7794,10 @@ ImmModel::ccbObjectModify(const ImmsvOmC > > TRACE_7("ERR_NOT_EXIST: object '%s' does not have an " > > "implementer and flag SA_IMM_CCB_REGISTERED_OI is > > set", > > objectName.c_str()); > > + setCcbErrorString(ccb, > > + "ERR_NOT_EXIST: object '%s' does not have an " > > + "implementer and flag SA_IMM_CCB_REGISTERED_OI is > > set", > > + objectName.c_str()); > > err = SA_AIS_ERR_NOT_EXIST; > > } > > } else { /* SA_IMM_CCB_REGISTERED_OI NOT set */ @@ -8201,6 > > +8209,10 @@ ImmModel::deleteObject(ObjectMap::iterat > > TRACE_7("ERR_NOT_EXIST: object '%s' does not have an > > implementer " > > "and flag SA_IMM_CCB_REGISTERED_OI is set", > > oi->first.c_str()); > > + setCcbErrorString(ccb, > > + "ERR_NOT_EXIST: object '%s' does not have an > > implementer " > > + "and flag SA_IMM_CCB_REGISTERED_OI is set", > > + oi->first.c_str()); > > return SA_AIS_ERR_NOT_EXIST; > > } > > } else { /* SA_IMM_CCB_REGISTERED_OI NOT set */ @@ -8370,6 > > +8382,54 @@ ImmModel::deleteObject(ObjectMap::iterat > > return SA_AIS_OK; > > } > > > > +void > > +ImmModel::setCcbErrorString(CcbInfo *ccb, const char *errorString, > > +...) { > > + int errLen = strlen(errorString) + 1; > > + char *fmtError = (char *)malloc(errLen); > > + int len; > > + va_list vl; > > + > > + va_start(vl, errorString); > > + len = vsnprintf(fmtError, errLen, errorString, vl); > > + va_end(vl); > > + > > + osafassert(len >= 0); > > + len++; /* Reserve one byte for null-terminated sign '\0' */ > > + if(len > errLen) { > > + fmtError = (char *)realloc(fmtError, len); > > + va_start(vl, errorString); > > + osafassert(vsnprintf(fmtError, len, errorString, vl) >= 0); > > + va_end(vl); > > + } > > + > > + unsigned int ix=0; > > + ImmsvAttrNameList* errStr = ccb->mErrorStrings; > > + ImmsvAttrNameList** errStrTail = &(ccb->mErrorStrings); > > + while(errStr) { > > + if(!strncmp(fmtError, errStr->name.buf, len)) { > > + TRACE_5("Discarding duplicate error string '%s' for ccb id %u", > > + fmtError, ccb->mId); > > + free(fmtError); > > + return; > > + } > > + ++ix; > > + errStrTail = &(errStr->next); > > + errStr = errStr->next; > > + } > > + > > + if(ix >= IMMSV_MAX_ATTRIBUTES) { > > + TRACE_5("Discarding error string '%s' for ccb id %u (too many)", > > + fmtError, ccb->mId); > > + free(fmtError); > > + } else { > > + (*errStrTail) = (ImmsvAttrNameList *) > > malloc(sizeof(ImmsvAttrNameList)); > > + (*errStrTail)->next = NULL; > > + (*errStrTail)->name.size = len; > > + (*errStrTail)->name.buf = fmtError; > > + } > > +} > > + > > bool > > ImmModel::hasLocalClassAppliers(ClassInfo* classInfo) > > { > > diff --git a/osaf/services/saf/immsv/immnd/ImmModel.hh > > b/osaf/services/saf/immsv/immnd/ImmModel.hh > > --- a/osaf/services/saf/immsv/immnd/ImmModel.hh > > +++ b/osaf/services/saf/immsv/immnd/ImmModel.hh > > @@ -259,6 +259,11 @@ public: > > IdVector& continuations, > > unsigned int pbeIsLocal); > > > > + void setCcbErrorString( > > + CcbInfo *ccb, > > + const char *errorString, > > + ...); > > + > > bool hasLocalClassAppliers(ClassInfo* classInfo); > > bool hasLocalObjAppliers(const std::string& objName); > > > > 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 > > @@ -5732,11 +5732,18 @@ static void immnd_evt_proc_object_create > > memset(&send_evt, '\0', sizeof(IMMSV_EVT)); > > send_evt.type = IMMSV_EVT_TYPE_IMMA; > > send_evt.info.imma.info.errRsp.error = err; > > - send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > > + send_evt.info.imma.info.errRsp.errStrings = > > +immModel_ccbGrabErrStrings(cb, evt->info.objCreate.ccbId); > > + > > + if(send_evt.info.imma.info.errRsp.errStrings) { > > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; > > + } else { > > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > > + } > > > > if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != > > NCSCC_RC_SUCCESS) { > > LOG_WA("Failed to send result to Agent over MDS"); > > } > > + > > +immsv_evt_free_attrNames(send_evt.info.imma.info.errRsp.errStrings); > > } > > TRACE_LEAVE(); > > } > > @@ -5957,11 +5964,18 @@ static void immnd_evt_proc_object_modify > > memset(&send_evt, '\0', sizeof(IMMSV_EVT)); > > send_evt.type = IMMSV_EVT_TYPE_IMMA; > > send_evt.info.imma.info.errRsp.error = err; > > - send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > > + send_evt.info.imma.info.errRsp.errStrings = > > +immModel_ccbGrabErrStrings(cb, evt->info.objModify.ccbId); > > + > > + if(send_evt.info.imma.info.errRsp.errStrings) { > > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; > > + } else { > > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > > + } > > > > if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != > > NCSCC_RC_SUCCESS) { > > LOG_WA("Failed to send result to Agent over MDS"); > > } > > + > > +immsv_evt_free_attrNames(send_evt.info.imma.info.errRsp.errStrings); > > } > > > > done: > > @@ -6715,11 +6729,18 @@ static void immnd_evt_proc_object_delete > > memset(&send_evt, '\0', sizeof(IMMSV_EVT)); > > send_evt.type = IMMSV_EVT_TYPE_IMMA; > > send_evt.info.imma.info.errRsp.error = err; > > - send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > > + send_evt.info.imma.info.errRsp.errStrings = > > +immModel_ccbGrabErrStrings(cb, evt->info.objDelete.ccbId); > > + > > + if(send_evt.info.imma.info.errRsp.errStrings) { > > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; > > + } else { > > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > > + } > > > > if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != > > NCSCC_RC_SUCCESS) { > > LOG_WA("Failed to send result to Agent over MDS"); > > } > > + > > +immsv_evt_free_attrNames(send_evt.info.imma.info.errRsp.errStrings); > > } > > } > > > > > > ---------------------------------------------------------------------- > > -------- _______________________________________________ > > Opensaf-devel mailing list > > Opensaf-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > > > > > ------------------------------------------------------------------------------ > Put Bad Developers to Shame > Dominate Development with Jenkins Continuous Integration Continuously > Automate Build, Test & Deployment Start a new project > now. Try Jenkins in the cloud. > http://p.sf.net/sfu/13600_Cloudbees_APR > _______________________________________________ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Put Bad Developers to Shame Dominate Development with Jenkins Continuous Integration Continuously Automate Build, Test & Deployment Start a new project now. Try Jenkins in the cloud. http://p.sf.net/sfu/13600_Cloudbees_APR _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
