Hi zoran,
I overlooked the "if" condition.
Thanks, for explaining.
Reviewed the patch.
Ack.
/Neel.
On Thursday 29 October 2015 05:48 PM, Neelakanta Reddy wrote:
> Hi zoran,
>
> Ideally, the errorstrings should capture all errors, that are the result
> of ccb apply.
> i.e : if the validation error is coming from more than one implementer
> then these errors needs to be captured and sent to OM.
> In the present patch, only one validation abort is added.
>
> README:
>
> Multiple error strings may however be set from different OIs
> in different callbacks related to the same OM downcall (e.g.
> saImmOmCcbApply).
>
> /Neel.
>
> On Monday 26 October 2015 03:57 PM, Zoran Milinkovic wrote:
>> osaf/services/saf/immsv/immnd/ImmModel.cc | 103
>> +++++++++--------------------
>> 1 files changed, 31 insertions(+), 72 deletions(-)
>>
>>
>> The patch add a precedence of validation error string over resource abort
>> error string.
>> If resource abort error string exists, it will be overwritten by validation
>> abort error string.
>> If at least one validation abort error string exist, new resource abort
>> error string will not be adda.
>>
>> Since the new check of abort error strings is done in setCcbErrorString, the
>> patch removes earlier code for ignoring several abort error strings.
>>
>> 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
>> @@ -5185,22 +5185,7 @@ ImmModel::ccbApply(SaUint32T ccbId,
>> osafassert(reqConn==0 || (ccb->mOriginatingConn == reqConn));
>>
>> if(!ccb->isOk()) {
>> - /* At this point, CCB might already have error string with CCB
>> abort reason.
>> - * If none CCB abort reason can be found in error strings,
>> - * then it's a resource abort.
>> - */
>> - ImmsvAttrNameList *errStr = ccb->mErrorStrings;
>> - while(errStr) {
>> - if(strstr(errStr->name.buf, IMM_VALIDATION_ABORT) ==
>> errStr->name.buf
>> - || strstr(errStr->name.buf, IMM_RESOURCE_ABORT) ==
>> errStr->name.buf) {
>> - break;
>> - }
>> - errStr = errStr->next;
>> - }
>> - if(!errStr) {
>> - setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an
>> error state");
>> - }
>> -
>> + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> err = SA_AIS_ERR_FAILED_OPERATION;
>> } else if((sImmNodeState == IMM_NODE_LOADING) &&
>> !sMissingParents.empty()) {
>> MissingParentsMap::iterator mpm;
>> @@ -6254,20 +6239,7 @@ ImmModel::ccbAugmentInit(immsv_oi_ccb_up
>> if(ccb->mVeto != SA_AIS_OK) {
>> TRACE("Ccb %u is already in an error state %u, can not accept
>> augmentation",
>> ccbId, ccb->mVeto);
>> -
>> - /* ccb->mVeto != SA_AIS_OK, error string with abort reason can
>> already be set */
>> - ImmsvAttrNameList *errStr = ccb->mErrorStrings;
>> - while(errStr) {
>> - if(strstr(errStr->name.buf, IMM_VALIDATION_ABORT) ==
>> errStr->name.buf
>> - || strstr(errStr->name.buf, IMM_RESOURCE_ABORT) ==
>> errStr->name.buf) {
>> - break;
>> - }
>> - errStr = errStr->next;
>> - }
>> - if(!errStr) {
>> - setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> - }
>> -
>> + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> err = SA_AIS_ERR_FAILED_OPERATION; /*ccb->mVeto;*/
>> goto done;
>> }
>> @@ -7020,21 +6992,8 @@ SaAisErrorT ImmModel::ccbObjectCreate(Im
>> if(!ccb->isOk()) {
>> LOG_NO("ERR_FAILED_OPERATION: ccb %u is in an error state "
>> "rejecting ccbObjectCreate operation ", ccbId);
>> -
>> - /* !ccb->isOk(), error string with abort reason can already be set
>> */
>> - ImmsvAttrNameList *errStr = ccb->mErrorStrings;
>> - while(errStr) {
>> - if(strstr(errStr->name.buf, IMM_VALIDATION_ABORT) ==
>> errStr->name.buf
>> - || strstr(errStr->name.buf, IMM_RESOURCE_ABORT) ==
>> errStr->name.buf) {
>> - break;
>> - }
>> - errStr = errStr->next;
>> - }
>> - if(!errStr) {
>> - setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> - }
>> -
>> err = SA_AIS_ERR_FAILED_OPERATION;
>> + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> goto ccbObjectCreateExit;
>> }
>>
>> @@ -8192,21 +8151,8 @@ ImmModel::ccbObjectModify(const ImmsvOmC
>> if(!ccb->isOk()) {
>> LOG_NO("ERR_FAILED_OPERATION: ccb %u is in an error state "
>> "rejecting ccbObjectModify operation ", ccbId);
>> -
>> - /* !ccb->isOk(), error string with abort reason can already be set
>> */
>> - ImmsvAttrNameList *errStr = ccb->mErrorStrings;
>> - while(errStr) {
>> - if(strstr(errStr->name.buf, IMM_VALIDATION_ABORT) ==
>> errStr->name.buf
>> - || strstr(errStr->name.buf, IMM_RESOURCE_ABORT) ==
>> errStr->name.buf) {
>> - break;
>> - }
>> - errStr = errStr->next;
>> - }
>> - if(!errStr) {
>> - setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> - }
>> -
>> err = SA_AIS_ERR_FAILED_OPERATION;
>> + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> goto ccbObjectModifyExit;
>> }
>>
>> @@ -9154,21 +9100,8 @@ ImmModel::ccbObjectDelete(const ImmsvOmC
>> if(!ccb->isOk()) {
>> LOG_NO("ERR_FAILED_OPERATION: ccb %u is in an error state "
>> "rejecting ccbObjectDelete operation ", ccbId);
>> -
>> - /* !ccb->isOk(), error string with abort reason can already be set
>> */
>> - ImmsvAttrNameList *errStr = ccb->mErrorStrings;
>> - while(errStr) {
>> - if(strstr(errStr->name.buf, IMM_VALIDATION_ABORT) ==
>> errStr->name.buf
>> - || strstr(errStr->name.buf, IMM_RESOURCE_ABORT) ==
>> errStr->name.buf) {
>> - break;
>> - }
>> - errStr = errStr->next;
>> - }
>> - if(!errStr) {
>> - setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> - }
>> -
>> err = SA_AIS_ERR_FAILED_OPERATION;
>> + setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
>> state");
>> goto ccbObjectDeleteExit;
>> }
>>
>> @@ -9648,6 +9581,7 @@ ImmModel::setCcbErrorString(CcbInfo *ccb
>> char *fmtError = (char *)malloc(errLen);
>> int len;
>> va_list args;
>> + int isValidationErrString = 0;
>>
>> va_copy(args, vl);
>> len = vsnprintf(fmtError, errLen, errorString, args);
>> @@ -9660,6 +9594,12 @@ ImmModel::setCcbErrorString(CcbInfo *ccb
>> osafassert(vsnprintf(fmtError, len, errorString, vl) >= 0);
>> }
>>
>> + if(strstr(errorString, IMM_VALIDATION_ABORT) == errorString) {
>> + isValidationErrString = 1;
>> + } else if(strstr(errorString, IMM_RESOURCE_ABORT) == errorString) {
>> + isValidationErrString = 2;
>> + }
>> +
>> unsigned int ix=0;
>> ImmsvAttrNameList* errStr = ccb->mErrorStrings;
>> ImmsvAttrNameList** errStrTail = &(ccb->mErrorStrings);
>> @@ -9670,6 +9610,25 @@ ImmModel::setCcbErrorString(CcbInfo *ccb
>> free(fmtError);
>> return;
>> }
>> + if(isValidationErrString) {
>> + // Precendence of validation abort over resource abort error
>> string
>> + if(isValidationErrString == 1) {
>> + // Validation abort error string will replace resource
>> abort error string
>> + if(strstr(errStr->name.buf, IMM_RESOURCE_ABORT) ==
>> errStr->name.buf) {
>> + free(errStr->name.buf);
>> + errStr->name.buf = fmtError;
>> + errStr->name.size = len;
>> + return;
>> + }
>> + } else if(strstr(errStr->name.buf, IMM_RESOURCE_ABORT) ==
>> errStr->name.buf
>> + || strstr(errStr->name.buf, IMM_VALIDATION_ABORT) ==
>> errStr->name.buf) {
>> + // If validation abort or resource abort error string exist,
>> + // new resource abort string will not be added.
>> + // It can exists more validation abort error strings
>> + // or only the first resource abort error string
>> + return;
>> + }
>> + }
>> ++ix;
>> errStrTail = &(errStr->next);
>> errStr = errStr->next;
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Opensaf-devel mailing list
> Opensaf-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
