Hi Hans
I was thinking typeid would be cheaper than using dynamic_cast. But it does
make the code more complicated than it needs to be.
Gary
On 31/7/18, 10:13 pm, "Hans Nordeback" <hans.nordeb...@ericsson.com> wrote:
Hi Gary,
a minor comment below.
/Regards HansN
On 07/06/2018 04:07 AM, Gary Lee wrote:
> If there is a queued update on a particular object's attribute,
> also queue further 'sync' updates so we don't end up with an
> inconsistent value.
> ---
> src/amf/amfd/imm.cc | 35 +++++++++++++++++++++++++++++------
> src/amf/amfd/imm.h | 9 ++++++---
> 2 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/src/amf/amfd/imm.cc b/src/amf/amfd/imm.cc
> index ac7a3cbae..82d2b1329 100644
> --- a/src/amf/amfd/imm.cc
> +++ b/src/amf/amfd/imm.cc
> @@ -26,6 +26,7 @@
> #include <errno.h>
> #include <cstring>
> #include <sys/stat.h>
> +#include <typeinfo>
> #include <unistd.h>
>
> #include <saImmOm.h>
> @@ -393,7 +394,7 @@ Job *Fifo::peek() {
> }
>
> //
> -void Fifo::queue(Job *job) { job_.push(job); }
> +void Fifo::queue(Job *job) { job_.push_back(job); }
>
> //
> Job *Fifo::dequeue() {
> @@ -403,7 +404,7 @@ Job *Fifo::dequeue() {
> tmp = 0;
> } else {
> tmp = job_.front();
> - job_.pop();
> + job_.pop_front();
> }
>
> return tmp;
> @@ -550,8 +551,27 @@ void Fifo::trim_to_size(const uint32_t size) {
> TRACE_LEAVE();
> }
>
> +bool Fifo::pendingImmUpdateOp(const std::string& dn,
> + const std::string& attribute) {
> + TRACE_ENTER();
> +
> + for (auto job : job_) {
> + if (job->getJobType() == JOB_TYPE_IMM &&
> + typeid(*job) == typeid(ImmObjUpdate)) {
[HansN] you can remove the typeinfo include and the typeid funciton and
check the result of the dynamic_cast instead (nullptr), e.g:
if (ImmObjUpdate *update_job = dynamic_cast<ImmObjUpdate*>(job)) { ...
> + ImmObjUpdate *update_job = dynamic_cast<ImmObjUpdate*>(job);
> + if (update_job->dn == dn &&
> + update_job->attributeName_ == attribute) {
> + TRACE("Found an existing update on '%s'", dn.c_str());
> + return true;
> + }
> + }
> + }
> +
> + return false;
> +}
> +
> //
> -std::queue<Job *> Fifo::job_;
> +std::deque<Job *> Fifo::job_;
> //
>
> extern struct ImmutilWrapperProfile immutilWrapperProfile;
> @@ -1756,7 +1776,7 @@ SaAisErrorT avd_saImmOiRtObjectUpdate_sync(
> const std::string &dn, SaImmAttrNameT attributeName,
> SaImmValueTypeT attrValueType, void *value,
> SaImmAttrModificationTypeT modifyType) {
> - SaAisErrorT rc = SA_AIS_OK;
> + SaAisErrorT rc = SA_AIS_ERR_TRY_AGAIN;
> SaImmAttrModificationT_2 attrMod;
> const SaImmAttrModificationT_2 *attrMods[] = {&attrMod, nullptr};
> SaImmAttrValueT attrValues[] = {value};
> @@ -1764,7 +1784,10 @@ SaAisErrorT avd_saImmOiRtObjectUpdate_sync(
> bool isImmReady = isImmServiceReady(avd_cb);
>
> TRACE_ENTER2("'%s' %s", dn.c_str(), attributeName);
> - if (isImmReady == true) {
> + // Only perform the update if there isn't a pending IMM update
involving
> + // the attribute. Else queue it so the attribute's value remains
consistent.
> + if (isImmReady == true &&
> + Fifo::pendingImmUpdateOp(dn, attribute_name) == false) {
> attrMod.modType = modifyType;
> attrMod.modAttr.attrName = attributeName;
> attrMod.modAttr.attrValuesNumber = 1;
> @@ -1777,7 +1800,7 @@ SaAisErrorT avd_saImmOiRtObjectUpdate_sync(
> attributeName, rc);
> }
>
> - if (rc != SA_AIS_OK || isImmReady == false) {
> + if (rc != SA_AIS_OK) {
> // Now it will be updated through job queue.
> avd_saImmOiRtObjectUpdate(dn, attribute_name, attrValueType, value);
> }
> diff --git a/src/amf/amfd/imm.h b/src/amf/amfd/imm.h
> index 1778d27ee..3cfc207cf 100644
> --- a/src/amf/amfd/imm.h
> +++ b/src/amf/amfd/imm.h
> @@ -26,10 +26,10 @@
> #ifndef AMF_AMFD_IMM_H_
> #define AMF_AMFD_IMM_H_
>
> +#include <deque>
> +#include <string>
> #include "amf/amfd/cb.h"
> #include "osaf/immutil/immutil.h"
> -#include <queue>
> -#include <string>
>
> typedef void (*AvdImmOiCcbApplyCallbackT)(CcbUtilOperationData_t
*opdata);
> typedef SaAisErrorT (*AvdImmOiCcbCompletedCallbackT)(
> @@ -177,8 +177,11 @@ class Fifo {
>
> static void trim_to_size(const uint32_t size);
>
> + static bool pendingImmUpdateOp(const std::string& dn,
> + const std::string& attribute);
> +
> private:
> - static std::queue<Job *> job_;
> + static std::deque<Job *> job_;
> };
> //
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
