Some minor comments inline.
If we agree on them I can push.
Thanks,
Hans
On 31 March 2014 06:32, Gary Lee <[email protected]> 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
> [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
