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

Reply via email to