My comment was wrong. In the BAD_HANDLE case avd_imm_reinit_bg() must
be triggered and this is how we do it now. My concern was if the admin
resp job should remain in the queue or be discarded in this particular
case.
/Hans
On 11 April 2014 09:14, Hans Feldt <osafde...@gmail.com> wrote:
> Some minor comments inline.
> If we agree on them I can push.
> Thanks,
> Hans
>
> On 31 March 2014 06:32, Gary Lee <gary....@dektech.com.au> wrote:
>> osaf/services/saf/amf/amfd/imm.cc | 40
>> ++++++++++++++++++++++++++-----
>> osaf/services/saf/amf/amfd/include/imm.h | 16 ++++++++++++
>> 2 files changed, 49 insertions(+), 7 deletions(-)
>>
>>
>> Currently, admin operation responses from amfd are not synchronised with
>> IMM changes that may be performed as part of the admin operation.
>> That is, an admin operation may return before the corresponding
>> IMM changes have been performed. IMM changes by amfd are typically placed
>> into
>> a FIFO queue to be done at a later time.
>>
>> This patch puts admin operation responses into the same queue. Therefore
>> guaranteeing that when an admin operation returns, the corresponding
>> IMM changes have already been done.
>>
>> diff --git a/osaf/services/saf/amf/amfd/imm.cc
>> b/osaf/services/saf/amf/amfd/imm.cc
>> --- a/osaf/services/saf/amf/amfd/imm.cc
>> +++ b/osaf/services/saf/amf/amfd/imm.cc
>> @@ -228,6 +228,35 @@ ImmObjDelete::~ImmObjDelete()
>> }
>>
>> //
>> +AvdJobDequeueResultT ImmAdminResponse::exec(const SaImmOiHandleT handle) {
>> + SaAisErrorT rc;
>> + AvdJobDequeueResultT res;
>> +
>> + TRACE_ENTER();
>
> this should be a TRACE_ENTER2 printing "adm resp inv:%llu res:%u"
>
>> +
>> + rc = saImmOiAdminOperationResult(handle, this->invocation_,
>> this->result_);
>> +
>> + if (rc == SA_AIS_OK) {
>> + delete Fifo::dequeue();
>> + res = JOB_EXECUTED;
>> + } else if (rc == SA_AIS_ERR_TRY_AGAIN) {
>> + TRACE("TRY-AGAIN");
>> + res = JOB_ETRYAGAIN;
>> + } else if (rc == SA_AIS_ERR_BAD_HANDLE) {
>
> I don't think we should handle this (BAD_HANDLE) case, just let it
> slip through to the else branch
>
>> + TRACE("BADHANDLE");
>> + avd_imm_reinit_bg();
>> + res = JOB_ETRYAGAIN;
>> + } else {
>> + delete Fifo::dequeue();
>> + LOG_ER("Admin op failed for invocation: %llu, result %u",
>> + this->invocation_, this->result_);
>> + res = JOB_ERR;
>> + }
>> +
>> + TRACE_LEAVE2("%u", res);
>> + return res;
>> +}
>> +
>> Job* Fifo::peek()
>> {
>> Job* tmp;
>> @@ -1719,15 +1748,12 @@ void avd_saImmOiAdminOperationResult(SaI
>>
>> SaInvocationT invocation,
>>
>> SaAisErrorT result)
>> {
>> - SaAisErrorT error;
>> + TRACE_ENTER2("inv:%llu, res:%u", invocation, result);
>>
>> - TRACE_ENTER2("inv:%llu, res:%u", invocation, result);
>> - saflog(LOG_NOTICE, amfSvcUsrName, "Admin op done for invocation:
>> %llu, result %u",
>> - invocation, result);
>> + ImmAdminResponse *job = new ImmAdminResponse(invocation, result);
>> + Fifo::queue(job);
>>
>> - error = immutil_saImmOiAdminOperationResult(immOiHandle, invocation,
>> result);
>> - if (error != SA_AIS_OK)
>> - LOG_NO("saImmOiAdminOperationResult for %llu failed %u",
>> invocation, error);
>> + TRACE_LEAVE();
>> }
>>
>> /**
>> diff --git a/osaf/services/saf/amf/amfd/include/imm.h
>> b/osaf/services/saf/amf/amfd/include/imm.h
>> --- a/osaf/services/saf/amf/amfd/include/imm.h
>> +++ b/osaf/services/saf/amf/amfd/include/imm.h
>> @@ -91,6 +91,22 @@ public:
>> ~ImmObjDelete();
>> };
>>
>> +class ImmAdminResponse : public Job {
>> + public:
>> + ImmAdminResponse(const SaInvocationT invocation,
>> + const SaAisErrorT result) {
>> + this->invocation_ = invocation;
>> + this->result_ = result;
>> + }
>> + AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle);
>> +
>> + ~ImmAdminResponse() {}
>> + private:
>> + ImmAdminResponse();
>> + SaInvocationT invocation_;
>> + SaAisErrorT result_;
>> +};
>> +
>> //
>> class Fifo {
>> public:
>>
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> 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
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
