Re: [devel] [PATCH 1 of 1] amfd: execute imm jobs based on latest status of attributes and objects[#2009] V2

2016-11-10 Thread Gary Lee 
Hi Praveen

Ack with one comment below (review only)

Thanks
Gary

> On 10 Nov. 2016, at 6:13 pm, praveen.malv...@oracle.com wrote:
> 
> osaf/services/saf/amf/amfd/ckpt_updt.cc   |4 +
> osaf/services/saf/amf/amfd/imm.cc |  386 
> +-
> osaf/services/saf/amf/amfd/include/imm.h  |   22 +-
> osaf/services/saf/amf/amfd/include/util.h |2 +
> osaf/services/saf/amf/amfd/siass.cc   |7 +-
> osaf/services/saf/amf/amfd/util.cc|   28 ++
> 6 files changed, 438 insertions(+), 11 deletions(-)
> 
> 
> V2:Incorporated comments given by Gary.
> 
> App Si is moving to UNASSIGNED state after middleware failover.
> 
> Standby controller maintains a job queue for SU, SISU and COMPCSI.
> In job queue both old and new state of a object or attribute will be present.
> The issue is not observed normally because standby will update correct
> state eventually from old to new. In the cases mentioned in the ticket:
> -in one case, SI was deleted. Because of this after failover, IMM returns 
> failure for creating the SISU because
>  this new active AMFD is executing old job.
> -in other case, Su was deleted and was added with new state. Here also after 
> failover new active AMFD updates
>  IMM with old state as it is executing old job.
> 
> Patch fixes the problem by:
> -remebering if job was created in active or stadby state of AMFD.
> -After failover if new active finds some job that was created in standby 
> state, it checks
>  the status of objects or attributes and based on that job will be executed 
> with latest values or simple deleted.
> 
> diff --git a/osaf/services/saf/amf/amfd/ckpt_updt.cc 
> b/osaf/services/saf/amf/amfd/ckpt_updt.cc
> --- a/osaf/services/saf/amf/amfd/ckpt_updt.cc
> +++ b/osaf/services/saf/amf/amfd/ckpt_updt.cc
> @@ -463,6 +463,10 @@ uint32_t avd_ckpt_siass(AVD_CL_CB *cb, A
>   } else {
>   su_si_rel_ptr->comp_name = "";
>   su_si_rel_ptr->csi_name = "";
> + //This is update of SUSI because of 
> modification of assignment. 
> + if (action == NCS_MBCSV_ACT_UPDATE) {
> + avd_susi_update(su_si_rel_ptr, 
> su_si_rel_ptr->state);
> + }
>   }
>   } else {
>   LOG_ER("%s:%u", __FUNCTION__, __LINE__);
> 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
> @@ -142,6 +142,12 @@ AvdJobDequeueResultT ImmObjCreate::exec(
>   TRACE_ENTER2("Create %s", parentName_.c_str());
> 
>   const SaNameTWrapper parent_name(parentName_);
> + //Check in AMF Db before creation in IMM.
> + if (immobj_update_required() == false){
> + delete Fifo::dequeue();
> + res = JOB_EXECUTED;
> + goto done;  
> + }
>   rc = saImmOiRtObjectCreate_2(immOiHandle, className_,
>parent_name, attrValues_);
> 
> @@ -163,7 +169,7 @@ AvdJobDequeueResultT ImmObjCreate::exec(
>   LOG_ER("%s: create FAILED %u", __FUNCTION__, rc);
>   res = JOB_ERR;
>   }
> - 
> +done:
>   TRACE_LEAVE();
>   
>   return res;
> @@ -211,6 +217,12 @@ AvdJobDequeueResultT ImmObjUpdate::exec(
> 
>   TRACE_ENTER2("Update '%s' %s", dn.c_str(), attributeName_);
> 
> + //update latest values.
> + if (immobj_update_required() == false){
> + delete Fifo::dequeue();
> + res = JOB_EXECUTED;
> + goto done;  
> + }
>   attrMod.modType = SA_IMM_ATTR_VALUES_REPLACE;
>   attrMod.modAttr.attrName = attributeName_;
>   attrMod.modAttr.attrValuesNumber = 1;
> @@ -237,7 +249,7 @@ AvdJobDequeueResultT ImmObjUpdate::exec(
>   LOG_ER("%s: update FAILED %u", __FUNCTION__, rc);
>   res = JOB_ERR;
>   }
> - 
> +done:
>   TRACE_LEAVE();
>   return res;
> }
> @@ -260,6 +272,12 @@ AvdJobDequeueResultT ImmObjDelete::exec(
> 
>   TRACE_ENTER2("Delete %s", dn.c_str());
> 
> + //Check AMF db before deletion.
> + if (immobj_update_required() == false){
> + delete Fifo::dequeue();
> + res = JOB_EXECUTED;
> + goto done;  
> + }
>   rc = saImmOiRtObjectDelete_o3(immOiHandle, dn.c_str());
> 
>   if ((rc == SA_AIS_OK) || (rc == SA_AIS_ERR_NOT_EXIST)) {
> @@ -280,7 +298,7 @@ AvdJobDequeueResultT ImmObjDelete::exec(
>   LOG_ER("%s: delete FAILED %u", __FUNCTION__, rc);
>   res = JOB_ERR;
>   }
> - 
> +done:
>   TRACE_LEAVE();
>   return res;
> }
> @@ -478,8 +496,8 @@ static const char *avd_class_names[] = {
>   "SaAmfSIDependency",
>   "SaAmfSIRankedSU",
> 
> - "SaAmfCSIAssignment",
> - "SaAmfSIAssignment"
> +

[devel] [PATCH 1 of 1] amfd: execute imm jobs based on latest status of attributes and objects[#2009] V2

2016-11-09 Thread praveen . malviya 
 osaf/services/saf/amf/amfd/ckpt_updt.cc   |4 +
 osaf/services/saf/amf/amfd/imm.cc |  386 +-
 osaf/services/saf/amf/amfd/include/imm.h  |   22 +-
 osaf/services/saf/amf/amfd/include/util.h |2 +
 osaf/services/saf/amf/amfd/siass.cc   |7 +-
 osaf/services/saf/amf/amfd/util.cc|   28 ++
 6 files changed, 438 insertions(+), 11 deletions(-)


V2:Incorporated comments given by Gary.

App Si is moving to UNASSIGNED state after middleware failover.

Standby controller maintains a job queue for SU, SISU and COMPCSI.
In job queue both old and new state of a object or attribute will be present.
The issue is not observed normally because standby will update correct
state eventually from old to new. In the cases mentioned in the ticket:
 -in one case, SI was deleted. Because of this after failover, IMM returns 
failure for creating the SISU because
  this new active AMFD is executing old job.
 -in other case, Su was deleted and was added with new state. Here also after 
failover new active AMFD updates
  IMM with old state as it is executing old job.

Patch fixes the problem by:
 -remebering if job was created in active or stadby state of AMFD.
 -After failover if new active finds some job that was created in standby 
state, it checks
  the status of objects or attributes and based on that job will be executed 
with latest values or simple deleted.

diff --git a/osaf/services/saf/amf/amfd/ckpt_updt.cc 
b/osaf/services/saf/amf/amfd/ckpt_updt.cc
--- a/osaf/services/saf/amf/amfd/ckpt_updt.cc
+++ b/osaf/services/saf/amf/amfd/ckpt_updt.cc
@@ -463,6 +463,10 @@ uint32_t avd_ckpt_siass(AVD_CL_CB *cb, A
} else {
su_si_rel_ptr->comp_name = "";
su_si_rel_ptr->csi_name = "";
+   //This is update of SUSI because of 
modification of assignment. 
+   if (action == NCS_MBCSV_ACT_UPDATE) {
+   avd_susi_update(su_si_rel_ptr, 
su_si_rel_ptr->state);
+   }
}
} else {
LOG_ER("%s:%u", __FUNCTION__, __LINE__);
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
@@ -142,6 +142,12 @@ AvdJobDequeueResultT ImmObjCreate::exec(
TRACE_ENTER2("Create %s", parentName_.c_str());
 
const SaNameTWrapper parent_name(parentName_);
+   //Check in AMF Db before creation in IMM.
+   if (immobj_update_required() == false){
+   delete Fifo::dequeue();
+   res = JOB_EXECUTED;
+   goto done;  
+   }
rc = saImmOiRtObjectCreate_2(immOiHandle, className_,
 parent_name, attrValues_);
 
@@ -163,7 +169,7 @@ AvdJobDequeueResultT ImmObjCreate::exec(
LOG_ER("%s: create FAILED %u", __FUNCTION__, rc);
res = JOB_ERR;
}
-   
+done:
TRACE_LEAVE();

return res;
@@ -211,6 +217,12 @@ AvdJobDequeueResultT ImmObjUpdate::exec(
 
TRACE_ENTER2("Update '%s' %s", dn.c_str(), attributeName_);
 
+   //update latest values.
+   if (immobj_update_required() == false){
+   delete Fifo::dequeue();
+   res = JOB_EXECUTED;
+   goto done;  
+   }
attrMod.modType = SA_IMM_ATTR_VALUES_REPLACE;
attrMod.modAttr.attrName = attributeName_;
attrMod.modAttr.attrValuesNumber = 1;
@@ -237,7 +249,7 @@ AvdJobDequeueResultT ImmObjUpdate::exec(
LOG_ER("%s: update FAILED %u", __FUNCTION__, rc);
res = JOB_ERR;
}
-   
+done:
TRACE_LEAVE();
return res;
 }
@@ -260,6 +272,12 @@ AvdJobDequeueResultT ImmObjDelete::exec(
 
TRACE_ENTER2("Delete %s", dn.c_str());
 
+   //Check AMF db before deletion.
+   if (immobj_update_required() == false){
+   delete Fifo::dequeue();
+   res = JOB_EXECUTED;
+   goto done;  
+   }
rc = saImmOiRtObjectDelete_o3(immOiHandle, dn.c_str());
 
if ((rc == SA_AIS_OK) || (rc == SA_AIS_ERR_NOT_EXIST)) {
@@ -280,7 +298,7 @@ AvdJobDequeueResultT ImmObjDelete::exec(
LOG_ER("%s: delete FAILED %u", __FUNCTION__, rc);
res = JOB_ERR;
}
-   
+done:  
TRACE_LEAVE();
return res;
 }
@@ -478,8 +496,8 @@ static const char *avd_class_names[] = {
"SaAmfSIDependency",
"SaAmfSIRankedSU",
 
-   "SaAmfCSIAssignment",
-   "SaAmfSIAssignment"
+   "SaAmfSIAssignment",
+   "SaAmfCSIAssignment"
 };
 
 static AvdImmOiCcbApplyCallbackT ccb_apply_callback[AVSV_SA_AMF_CLASS_MAX];
@@ -1683,6 +1701,8 @@ void avd_saImmOiRtObjectUpdate(const std
return;
 
ImmObjUpdate *ajob = new