Hi Praveen Ack. This comment can be removed.
//Give atleast one try for of Send() and Free() API. Thanks Gary > On 24 Oct. 2016, at 5:46 pm, [email protected] wrote: > > osaf/services/saf/amf/amfd/imm.cc | 35 +++--- > osaf/services/saf/amf/amfd/include/imm.h | 26 +++- > osaf/services/saf/amf/amfd/include/ntf.h | 1 + > osaf/services/saf/amf/amfd/main.cc | 6 +- > osaf/services/saf/amf/amfd/ntf.cc | 154 ++++++++++++++++++++---------- > osaf/services/saf/amf/amfd/sgproc.cc | 2 +- > 6 files changed, 145 insertions(+), 79 deletions(-) > > > V2 version: Includes suggestions given by Gary. > > Currently if AMFD gets TRY_AGAIN or TIMEOUT it logs notification and frees it. > > AMFD will push the notification in the existing job queue. Job queue already > handles > TRY_AGAIN and TIMEOUT. > > 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 > @@ -134,11 +134,11 @@ Job::~Job() > } > > // > -AvdJobDequeueResultT ImmObjCreate::exec(SaImmOiHandleT immOiHandle) > +AvdJobDequeueResultT ImmObjCreate::exec(const AVD_CL_CB *cb) > { > SaAisErrorT rc; > AvdJobDequeueResultT res; > - > + const SaImmOiHandleT immOiHandle = cb->immOiHandle; > TRACE_ENTER2("Create %s", parentName_.c_str()); > > const SaNameTWrapper parent_name(parentName_); > @@ -200,11 +200,12 @@ ImmObjCreate::~ImmObjCreate() > } > > // > -AvdJobDequeueResultT ImmObjUpdate::exec(SaImmOiHandleT immOiHandle) > +AvdJobDequeueResultT ImmObjUpdate::exec(const AVD_CL_CB *cb) > { > SaAisErrorT rc; > AvdJobDequeueResultT res; > - > + const SaImmOiHandleT immOiHandle = cb->immOiHandle; > + > SaImmAttrModificationT_2 attrMod; > const SaImmAttrModificationT_2 *attrMods[] = {&attrMod, nullptr}; > SaImmAttrValueT attrValues[] = {value_}; > @@ -253,10 +254,11 @@ ImmObjUpdate::~ImmObjUpdate() > } > > // > -AvdJobDequeueResultT ImmObjDelete::exec(SaImmOiHandleT immOiHandle) > +AvdJobDequeueResultT ImmObjDelete::exec(const AVD_CL_CB *cb) > { > SaAisErrorT rc; > AvdJobDequeueResultT res; > + const SaImmOiHandleT immOiHandle = cb->immOiHandle; > > TRACE_ENTER2("Delete %s", dn.c_str()); > > @@ -290,9 +292,10 @@ ImmObjDelete::~ImmObjDelete() > } > > // > -AvdJobDequeueResultT ImmAdminResponse::exec(const SaImmOiHandleT handle) { > +AvdJobDequeueResultT ImmAdminResponse::exec(const AVD_CL_CB *cb) { > SaAisErrorT rc; > AvdJobDequeueResultT res; > + const SaImmOiHandleT handle = cb->immOiHandle; > > TRACE_ENTER2("Admin resp inv:%llu res:%u", invocation_, result_); > > @@ -331,10 +334,10 @@ Job* Fifo::peek() > { > Job* tmp; > > - if (imm_job_.empty()) { > + if (job_.empty()) { > tmp = 0; > } else { > - tmp = imm_job_.front(); > + tmp = job_.front(); > } > > return tmp; > @@ -343,7 +346,7 @@ Job* Fifo::peek() > // > void Fifo::queue(Job* job) > { > - imm_job_.push(job); > + job_.push(job); > } > > // > @@ -351,11 +354,11 @@ Job* Fifo::dequeue() > { > Job* tmp; > > - if (imm_job_.empty()) { > + if (job_.empty()) { > tmp = 0; > } else { > - tmp = imm_job_.front(); > - imm_job_.pop(); > + tmp = job_.front(); > + job_.pop(); > } > > return tmp; > @@ -376,7 +379,7 @@ void check_and_flush_job_queue_standby_a > } > > // > -AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle) > +AvdJobDequeueResultT Fifo::execute(const AVD_CL_CB *cb) > { > Job *ajob; > AvdJobDequeueResultT ret; > @@ -395,7 +398,7 @@ AvdJobDequeueResultT Fifo::execute(SaImm > > TRACE_ENTER(); > > - ret = ajob->exec(immOiHandle); > + ret = ajob->exec(cb); > > TRACE_LEAVE2("%d", ret); > > @@ -418,11 +421,11 @@ void Fifo::empty() > > uint32_t Fifo::size() > { > - return imm_job_.size(); > + return job_.size(); > } > > // > -std::queue<Job*> Fifo::imm_job_; > +std::queue<Job*> Fifo::job_; > // > > extern struct ImmutilWrapperProfile immutilWrapperProfile; > 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 > @@ -24,6 +24,7 @@ > #ifndef AVD_IMM_H > #define AVD_IMM_H > > +#include "cb.h" > #include <immutil.h> > #include <queue> > #include <string> > @@ -50,9 +51,10 @@ typedef enum { > } AvdJobDequeueResultT; > > // TODO HANO Write comments > +// @todo move this into job.h > class Job { > public: > - virtual AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle) = 0; > + virtual AvdJobDequeueResultT exec(const AVD_CL_CB *cb) = 0; > virtual ~Job() = 0; > }; > > @@ -63,7 +65,7 @@ public: > std::string parentName_; > const SaImmAttrValuesT_2 **attrValues_; > > - AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle); > + AvdJobDequeueResultT exec(const AVD_CL_CB *cb); > > ~ImmObjCreate(); > }; > @@ -76,7 +78,7 @@ public: > SaImmValueTypeT attrValueType_; > void *value_; > > - AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle); > + AvdJobDequeueResultT exec(const AVD_CL_CB *cb); > > ~ImmObjUpdate(); > }; > @@ -86,7 +88,7 @@ class ImmObjDelete : public Job { > public: > std::string dn; > > - AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle); > + AvdJobDequeueResultT exec(const AVD_CL_CB *cb); > > ~ImmObjDelete(); > }; > @@ -98,7 +100,7 @@ class ImmAdminResponse : public Job { > this->invocation_ = invocation; > this->result_ = result; > } > - AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle); > + AvdJobDequeueResultT exec(const AVD_CL_CB *cb); > > ~ImmAdminResponse() {} > private: > @@ -107,6 +109,16 @@ class ImmAdminResponse : public Job { > SaAisErrorT result_; > }; > > +class NtfSend : public Job { > + public: > + NtfSend() : already_sent(false) {} > + AvdJobDequeueResultT exec(const AVD_CL_CB *cb); > + SaNtfNotificationsT myntf; > + bool already_sent; > + ~NtfSend(); > +}; > + > +// @todo move this into job.h > // > class Fifo { > public: > @@ -116,13 +128,13 @@ public: > > static Job* dequeue(); > > - static AvdJobDequeueResultT execute(SaImmOiHandleT immOiHandle); > + static AvdJobDequeueResultT execute(const AVD_CL_CB *cb); > > static void empty(); > > static uint32_t size(); > private: > - static std::queue<Job*> imm_job_; > + static std::queue<Job*> job_; > }; > // > > diff --git a/osaf/services/saf/amf/amfd/include/ntf.h > b/osaf/services/saf/amf/amfd/include/ntf.h > --- a/osaf/services/saf/amf/amfd/include/ntf.h > +++ b/osaf/services/saf/amf/amfd/include/ntf.h > @@ -31,6 +31,7 @@ > #include <comp.h> > #include <susi.h> > > +#include "imm.h" > #define AMF_NTF_SENDER "safApp=safAmfService" > > /* All states like oper, readiness etc starts from 1, so defining not > applicable values */ > diff --git a/osaf/services/saf/amf/amfd/main.cc > b/osaf/services/saf/amf/amfd/main.cc > --- a/osaf/services/saf/amf/amfd/main.cc > +++ b/osaf/services/saf/amf/amfd/main.cc > @@ -663,7 +663,7 @@ static void main_loop(void) > > if (pollretval == 0) { > // poll time out, submit some jobs (if any) > - polltmo = > retval_to_polltmo(Fifo::execute(cb->immOiHandle)); > + polltmo = retval_to_polltmo(Fifo::execute(cb)); > continue; > } > > @@ -686,7 +686,7 @@ static void main_loop(void) > > if (evt->rcv_evt == AVD_IMM_REINITIALIZED) { > TRACE("Received IMM reinit msg"); > - polltmo = > retval_to_polltmo(Fifo::execute(cb->immOiHandle)); > + polltmo = retval_to_polltmo(Fifo::execute(cb)); > continue; > } > > @@ -748,7 +748,7 @@ static void main_loop(void) > } > > // submit some jobs (if any) > - polltmo = retval_to_polltmo(Fifo::execute(cb->immOiHandle)); > + polltmo = retval_to_polltmo(Fifo::execute(cb)); > } > > syslog(LOG_CRIT, "AVD Thread Failed"); > diff --git a/osaf/services/saf/amf/amfd/ntf.cc > b/osaf/services/saf/amf/amfd/ntf.cc > --- a/osaf/services/saf/amf/amfd/ntf.cc > +++ b/osaf/services/saf/amf/amfd/ntf.cc > @@ -26,6 +26,7 @@ > #include <util.h> > #include <ntf.h> > #include "osaf_time.h" > +#include <queue> > > /***************************************************************************** > Name : avd_send_comp_inst_failed_alarm > @@ -529,6 +530,55 @@ SaAisErrorT fill_ntf_header_part_avd(SaN > > } > > +/* > + * @brief Tries to send notification if not sent already. If not > successful then > + * does not try to Free() it. Same function will be called from job > queue > + * to complete remaining task. > + * @param[in] ptr to NtfSend > + * @return SaAisErrorT. > + * TODO: Make it a member function of NtfSend() when all notification > related handling > + * is moved in separate thread. > + */ > +SaAisErrorT avd_try_send_notification(NtfSend *job) { > + TRACE_ENTER2("Ntf Type:%x, sent status:%u", job->myntf.notificationType, > + job->already_sent); > + > + SaNtfNotificationsT *myntf = &job->myntf; > + SaAisErrorT rc = SA_AIS_OK; > + SaNtfNotificationHeaderT *header = nullptr; > + SaNtfNotificationHandleT notificationHandle = 0; > + > + if (myntf->notificationType == SA_NTF_TYPE_STATE_CHANGE) { > + header = &myntf->notification.stateChangeNotification.notificationHeader; > + notificationHandle = > myntf->notification.stateChangeNotification.notificationHandle; > + } else if (myntf->notificationType == SA_NTF_TYPE_ALARM) { > + header = &myntf->notification.alarmNotification.notificationHeader; > + notificationHandle = > myntf->notification.alarmNotification.notificationHandle; > + } > + > + //Try to send the notification if not sent. > + if (job->already_sent == false) { > + rc = saNtfNotificationSend(notificationHandle); > + if ((rc == SA_AIS_ERR_TRY_AGAIN) || (rc == SA_AIS_ERR_TIMEOUT)) { > + TRACE("Notification Send unsuccesful TRY_AGAIN or TIMEOUT rc:%u",rc); > + goto done; > + } else { > + //To remember only Free is pending as NotificationFree() may hit with > TRY AGAIN. > + job->already_sent = true; > + } > + osaf_extended_name_free(header->notificationObject); > + osaf_extended_name_free(header->notifyingObject); > + } > + > + rc = saNtfNotificationFree(notificationHandle); > + if ((rc == SA_AIS_ERR_TRY_AGAIN) || (rc == SA_AIS_ERR_TIMEOUT)) { > + TRACE("Notification Free unsuccesful TRY_AGAIN or TIMEOUT rc:%u", rc); > + } > + > +done: > + TRACE_LEAVE(); > + return rc; > +} > uint32_t sendAlarmNotificationAvd(AVD_CL_CB *avd_cb, > const std::string& ntf_object, > SaUint8T *add_text, > @@ -540,7 +590,6 @@ uint32_t sendAlarmNotificationAvd(AVD_CL > int type) > { > uint32_t status = NCSCC_RC_FAILURE; > - SaNtfAlarmNotificationT myAlarmNotification; > SaUint16T add_info_items = 0; > SaUint64T allocation_size = 0; > > @@ -549,19 +598,19 @@ uint32_t sendAlarmNotificationAvd(AVD_CL > LOG_ER("Alarm lost for %s", ntf_object.c_str()); > return status; > } > - > if (avd_cb->ntfHandle == 0) { > LOG_ER("NTF handle has not been initialized, alarm notification > " > "for (%s) will be lost", ntf_object.c_str()); > return status; > } > + NtfSend *job = new NtfSend{}; > > if (type != 0) { > add_info_items = 1; > allocation_size = SA_NTF_ALLOC_SYSTEM_LIMIT; > } > > - status = saNtfAlarmNotificationAllocate(avd_cb->ntfHandle, > &myAlarmNotification, > + status = saNtfAlarmNotificationAllocate(avd_cb->ntfHandle, > &job->myntf.notification.alarmNotification, > /* numCorrelatedNotifications */ > 0, > /* lengthAdditionalText */ > @@ -582,7 +631,7 @@ uint32_t sendAlarmNotificationAvd(AVD_CL > return NCSCC_RC_FAILURE; > } > > - status = > fill_ntf_header_part_avd(&myAlarmNotification.notificationHeader, > + status = > fill_ntf_header_part_avd(&job->myntf.notification.alarmNotification.notificationHeader, > SA_NTF_ALARM_PROCESSING, > ntf_object, > add_text, > @@ -591,34 +640,21 @@ uint32_t sendAlarmNotificationAvd(AVD_CL > const_cast<SaInt8T*>(AMF_NTF_SENDER), > add_info, > type, > - myAlarmNotification.notificationHandle); > + > job->myntf.notification.alarmNotification.notificationHandle); > > if (status != SA_AIS_OK) { > LOG_ER("%s: fill_ntf_header_part_avd Failed (%u)", > __FUNCTION__, status); > - saNtfNotificationFree(myAlarmNotification.notificationHandle); > + > saNtfNotificationFree(job->myntf.notification.alarmNotification.notificationHandle); > return NCSCC_RC_FAILURE; > } > > - *(myAlarmNotification.probableCause) = > static_cast<SaNtfProbableCauseT>(probableCause); > - *(myAlarmNotification.perceivedSeverity) = > static_cast<SaNtfSeverityT>(perceivedSeverity); > + *(job->myntf.notification.alarmNotification.probableCause) = > static_cast<SaNtfProbableCauseT>(probableCause); > + *(job->myntf.notification.alarmNotification.perceivedSeverity) = > static_cast<SaNtfSeverityT>(perceivedSeverity); > > - status = saNtfNotificationSend(myAlarmNotification.notificationHandle); > + job->myntf.notificationType = SA_NTF_TYPE_ALARM; > > - > osaf_extended_name_free(myAlarmNotification.notificationHeader.notificationObject); > - > osaf_extended_name_free(myAlarmNotification.notificationHeader.notifyingObject); > - > - if (status != SA_AIS_OK) { > - saNtfNotificationFree(myAlarmNotification.notificationHandle); > - LOG_ER("%s: saNtfNotificationSend Failed (%u)", __FUNCTION__, > status); > - return NCSCC_RC_FAILURE; > - } > - > - status = saNtfNotificationFree(myAlarmNotification.notificationHandle); > - > - if (status != SA_AIS_OK) { > - LOG_ER("%s: saNtfNotificationFree Failed (%u)", __FUNCTION__, > status); > - return NCSCC_RC_FAILURE; > - } > + //Give atleast one try for of Send() and Free() API. > + Fifo::queue(job); > > return status; > > @@ -637,7 +673,6 @@ uint32_t sendStateChangeNotificationAvd( > int additional_info_is_present) > { > uint32_t status = NCSCC_RC_FAILURE; > - SaNtfStateChangeNotificationT myStateNotification; > SaUint16T add_info_items = 0; > SaUint64T allocation_size = 0; > SaUint16T num_of_changedStates = 1; > @@ -665,9 +700,9 @@ uint32_t sendStateChangeNotificationAvd( > if (stateId == STATE_ID_NA) { > num_of_changedStates = 0; > } > - > + NtfSend *job = new NtfSend{}; > status = saNtfStateChangeNotificationAllocate(avd_cb->ntfHandle,/* > handle to Notification Service instance */ > - &myStateNotification, > + > &job->myntf.notification.stateChangeNotification, > /* number of correlated > notifications */ > 0, > /* length of additional > text */ > @@ -684,7 +719,7 @@ uint32_t sendStateChangeNotificationAvd( > return NCSCC_RC_FAILURE; > } > > - status = > fill_ntf_header_part_avd(&myStateNotification.notificationHeader, > + status = > fill_ntf_header_part_avd(&job->myntf.notification.stateChangeNotification.notificationHeader, > SA_NTF_OBJECT_STATE_CHANGE, > ntf_object, > add_text, > @@ -693,40 +728,26 @@ uint32_t sendStateChangeNotificationAvd( > const_cast<SaInt8T*>(AMF_NTF_SENDER), > add_info, > additional_info_is_present, > - myStateNotification.notificationHandle); > + > job->myntf.notification.stateChangeNotification.notificationHandle); > > if (status != SA_AIS_OK) { > LOG_ER("%s: fill_ntf_header_part_avd Failed (%u)", > __FUNCTION__, status); > - saNtfNotificationFree(myStateNotification.notificationHandle); > + > saNtfNotificationFree(job->myntf.notification.stateChangeNotification.notificationHandle); > return NCSCC_RC_FAILURE; > } > > - *(myStateNotification.sourceIndicator) = > static_cast<SaNtfSourceIndicatorT>(sourceIndicator); > + *(job->myntf.notification.stateChangeNotification.sourceIndicator) = > static_cast<SaNtfSourceIndicatorT>(sourceIndicator); > > if (num_of_changedStates == 1) { > - myStateNotification.changedStates->stateId = stateId; > - myStateNotification.changedStates->oldStatePresent = SA_TRUE; > - myStateNotification.changedStates->oldState = oldstate; > - myStateNotification.changedStates->newState = newState; > + > job->myntf.notification.stateChangeNotification.changedStates->stateId = > stateId; > + > job->myntf.notification.stateChangeNotification.changedStates->oldStatePresent > = SA_TRUE; > + > job->myntf.notification.stateChangeNotification.changedStates->oldState = > oldstate; > + > job->myntf.notification.stateChangeNotification.changedStates->newState = > newState; > } > + job->myntf.notificationType = SA_NTF_TYPE_STATE_CHANGE; > > - status = saNtfNotificationSend(myStateNotification.notificationHandle); > - > - > osaf_extended_name_free(myStateNotification.notificationHeader.notificationObject); > - > osaf_extended_name_free(myStateNotification.notificationHeader.notifyingObject); > - > - if (status != SA_AIS_OK) { > - saNtfNotificationFree(myStateNotification.notificationHandle); > - LOG_ER("%s: saNtfNotificationSend Failed (%u)", __FUNCTION__, > status); > - return NCSCC_RC_FAILURE; > - } > - > - status = saNtfNotificationFree(myStateNotification.notificationHandle); > - > - if (status != SA_AIS_OK) { > - LOG_ER("%s: saNtfNotificationFree Failed (%u)", __FUNCTION__, > status); > - return NCSCC_RC_FAILURE; > - } > + //Give atleast one try for of Send() and Free() API. > + Fifo::queue(job); > > return status; > > @@ -838,3 +859,32 @@ SaAisErrorT avd_start_ntf_init_bg(void) > > return SA_AIS_OK; > } > + > +AvdJobDequeueResultT NtfSend::exec(const AVD_CL_CB *cb) { > + AvdJobDequeueResultT res; > + SaAisErrorT rc = SA_AIS_OK; > + TRACE_ENTER2("Ntf Type:%x, sent status:%u", myntf.notificationType, > + already_sent); > + > + rc = avd_try_send_notification(this); > + 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_TIMEOUT) { > + TRACE("TIMEOUT"); > + res = JOB_ETRYAGAIN; > + } else { > + delete Fifo::dequeue(); > + LOG_ER("%s: Notification Send FAILED %u", __FUNCTION__, rc); > + res = JOB_ERR; > + } > + > + TRACE_LEAVE(); > + return res; > +} > + > +NtfSend::~NtfSend() { > +} > diff --git a/osaf/services/saf/amf/amfd/sgproc.cc > b/osaf/services/saf/amf/amfd/sgproc.cc > --- a/osaf/services/saf/amf/amfd/sgproc.cc > +++ b/osaf/services/saf/amf/amfd/sgproc.cc > @@ -801,7 +801,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb > */ > AvdJobDequeueResultT job_res = > JOB_EXECUTED; > while (job_res == JOB_EXECUTED) > - job_res = > Fifo::execute(cb->immOiHandle); > + job_res = > Fifo::execute(cb); > > goto done; > } else { ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
