Re: [devel] [PATCH 1 of 5] amfd: cleanup SU [#713]
ack, code review only/BR HansN
On 04/22/14 12:58, Hans Feldt wrote:
> osaf/services/saf/amf/amfd/ckpt_dec.cc | 28 +
> osaf/services/saf/amf/amfd/ckpt_updt.cc | 1 -
> osaf/services/saf/amf/amfd/include/su.h | 38 +
> osaf/services/saf/amf/amfd/su.cc| 41
>
> osaf/services/saf/amf/amfd/util.cc | 1 -
> 5 files changed, 29 insertions(+), 80 deletions(-)
>
>
> su_act_state is not used thus can be made a dummy (not removed because of EDU
> usage). The corresponding enum AVD_SU_STATE removed. Checkpointing of
> su_act_state kept for backwards compatibility but reduced to a minimum.
>
> The use of "this->" is removed from some setters.
>
> Some non existing function signatures removed from the header file.
>
> diff --git a/osaf/services/saf/amf/amfd/ckpt_dec.cc
> b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> --- a/osaf/services/saf/amf/amfd/ckpt_dec.cc
> +++ b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> @@ -1846,34 +1846,10 @@ static uint32_t dec_su_switch(AVD_CL_CB
> \**/
> static uint32_t dec_su_act_state(AVD_CL_CB *cb, NCS_MBCSV_CB_DEC *dec)
> {
> - uint32_t status = NCSCC_RC_SUCCESS;
> - AVD_SU *su_ptr;
> - AVD_SU dec_su;
> - EDU_ERR ederror = static_cast(0);
> - AVD_SU *su_struct;
> -
> TRACE_ENTER();
> -
> - su_ptr = &dec_su;
> -
> - /*
> - * Action in this case is just to update.
> - */
> - status = ncs_edu_exec(&cb->edu_hdl, avsv_edp_ckpt_msg_su,
> - &dec->i_uba, EDP_OP_TYPE_DEC, (AVD_SU **)&su_ptr, &ederror, 2, 1,
> 13);
> -
> - osafassert(status == NCSCC_RC_SUCCESS);
> -
> - if (NULL == (su_struct = su_db->find(&su_ptr->name)))
> - osafassert(0);
> -
> - /* Update the fields received in this checkpoint message */
> - su_struct->su_act_state = su_ptr->su_act_state;
> -
> cb->async_updt_cnt.su_updt++;
> -
> - TRACE_LEAVE2("status '%u'", status);
> - return status;
> + TRACE_LEAVE();
> + return NCSCC_RC_SUCCESS;
> }
>
>
> /\
> 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
> @@ -200,7 +200,6 @@ uint32_t avd_ckpt_su(AVD_CL_CB *cb, AVD_
> memcpy(&su->saAmfSUHostedByNode, &ckpt_su->saAmfSUHostedByNode,
> sizeof(SaNameT));
> su->term_state = ckpt_su->term_state;
> su->su_switch = ckpt_su->su_switch;
> - su->su_act_state = ckpt_su->su_act_state;
> su->saAmfSURestartCount = ckpt_su->saAmfSURestartCount;
>
> done:
> diff --git a/osaf/services/saf/amf/amfd/include/su.h
> b/osaf/services/saf/amf/amfd/include/su.h
> --- a/osaf/services/saf/amf/amfd/include/su.h
> +++ b/osaf/services/saf/amf/amfd/include/su.h
> @@ -16,21 +16,12 @@
>*/
>
>
> /*
> -..
>
> -..
> -
> - DESCRIPTION:
> -
> - This module is the include file for handling Availability Directors
> - service unit structure.
> + DESCRIPTION: Service Unit class definition
>
>
> **
> */
>
> -/*
> - * Module Inclusion Control...
> - */
> #ifndef AVD_SU_H
> #define AVD_SU_H
>
> @@ -44,16 +35,8 @@
> #include
> #include "include/db_template.h"
>
> -/* The semantics the SU is undergoing. */
> -typedef enum {
> - AVD_SU_NO_STATE = 0,
> - AVD_SU_OPER,
> - AVD_SU_NODE_OPER
> -} AVD_SU_STATE;
> -
> -/* Avialability directors Service Unit structure(AVD_SU):
> - * This data structure lives in the AvD and reflects data points
> - * associated with the Service Unit (SU) on the AvD.
> +/**
> + * AMF director Service Unit representation.
>*/
> class AVD_SU {
>public:
> @@ -62,7 +45,8 @@ class AVD_SU {
> uint32_t saAmfSURank;
> SaNameT saAmfSUHostNodeOrNodeGroup;
> bool saAmfSUFailover;
> - bool saAmfSUFailover_configured; /* True when user configures
> saAmfSUFailover else false */
> + /* true when user has configured saAmfSUFailover */
> + bool saAmfSUFailover_configured;
> SaNameT saAmfSUMaintenanceCampaign;
>
> /* runtime attributes */
> @@ -71,11 +55,10 @@ class AVD_SU {
> SaAmfAdminStateT saAmfSUAdminState;
> SaAmfReadinessStateT saAmfSuReadinessState;
> SaAmfPresenceStateT saAmfSUPresenceState;
> - SaNameT **saAmfSUAssignedSIs;
> SaNameT saAmfSUHostedByNode;
> SaUint32T saAmfSUNumCurrActiveSIs;
> SaUint32T saAmfSUNumCurrStandbySIs;
> - SaUint32T saAmfSURestartCount; // T
[devel] [PATCH 1 of 5] amfd: cleanup SU [#713]
osaf/services/saf/amf/amfd/ckpt_dec.cc | 28 +
osaf/services/saf/amf/amfd/ckpt_updt.cc | 1 -
osaf/services/saf/amf/amfd/include/su.h | 38 +
osaf/services/saf/amf/amfd/su.cc| 41
osaf/services/saf/amf/amfd/util.cc | 1 -
5 files changed, 29 insertions(+), 80 deletions(-)
su_act_state is not used thus can be made a dummy (not removed because of EDU
usage). The corresponding enum AVD_SU_STATE removed. Checkpointing of
su_act_state kept for backwards compatibility but reduced to a minimum.
The use of "this->" is removed from some setters.
Some non existing function signatures removed from the header file.
diff --git a/osaf/services/saf/amf/amfd/ckpt_dec.cc
b/osaf/services/saf/amf/amfd/ckpt_dec.cc
--- a/osaf/services/saf/amf/amfd/ckpt_dec.cc
+++ b/osaf/services/saf/amf/amfd/ckpt_dec.cc
@@ -1846,34 +1846,10 @@ static uint32_t dec_su_switch(AVD_CL_CB
\**/
static uint32_t dec_su_act_state(AVD_CL_CB *cb, NCS_MBCSV_CB_DEC *dec)
{
- uint32_t status = NCSCC_RC_SUCCESS;
- AVD_SU *su_ptr;
- AVD_SU dec_su;
- EDU_ERR ederror = static_cast(0);
- AVD_SU *su_struct;
-
TRACE_ENTER();
-
- su_ptr = &dec_su;
-
- /*
-* Action in this case is just to update.
-*/
- status = ncs_edu_exec(&cb->edu_hdl, avsv_edp_ckpt_msg_su,
- &dec->i_uba, EDP_OP_TYPE_DEC, (AVD_SU **)&su_ptr, &ederror, 2, 1,
13);
-
- osafassert(status == NCSCC_RC_SUCCESS);
-
- if (NULL == (su_struct = su_db->find(&su_ptr->name)))
- osafassert(0);
-
- /* Update the fields received in this checkpoint message */
- su_struct->su_act_state = su_ptr->su_act_state;
-
cb->async_updt_cnt.su_updt++;
-
- TRACE_LEAVE2("status '%u'", status);
- return status;
+ TRACE_LEAVE();
+ return NCSCC_RC_SUCCESS;
}
/\
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
@@ -200,7 +200,6 @@ uint32_t avd_ckpt_su(AVD_CL_CB *cb, AVD_
memcpy(&su->saAmfSUHostedByNode, &ckpt_su->saAmfSUHostedByNode,
sizeof(SaNameT));
su->term_state = ckpt_su->term_state;
su->su_switch = ckpt_su->su_switch;
- su->su_act_state = ckpt_su->su_act_state;
su->saAmfSURestartCount = ckpt_su->saAmfSURestartCount;
done:
diff --git a/osaf/services/saf/amf/amfd/include/su.h
b/osaf/services/saf/amf/amfd/include/su.h
--- a/osaf/services/saf/amf/amfd/include/su.h
+++ b/osaf/services/saf/amf/amfd/include/su.h
@@ -16,21 +16,12 @@
*/
/*
-..
-..
-
- DESCRIPTION:
-
- This module is the include file for handling Availability Directors
- service unit structure.
+ DESCRIPTION: Service Unit class definition
**
*/
-/*
- * Module Inclusion Control...
- */
#ifndef AVD_SU_H
#define AVD_SU_H
@@ -44,16 +35,8 @@
#include
#include "include/db_template.h"
-/* The semantics the SU is undergoing. */
-typedef enum {
- AVD_SU_NO_STATE = 0,
- AVD_SU_OPER,
- AVD_SU_NODE_OPER
-} AVD_SU_STATE;
-
-/* Avialability directors Service Unit structure(AVD_SU):
- * This data structure lives in the AvD and reflects data points
- * associated with the Service Unit (SU) on the AvD.
+/**
+ * AMF director Service Unit representation.
*/
class AVD_SU {
public:
@@ -62,7 +45,8 @@ class AVD_SU {
uint32_t saAmfSURank;
SaNameT saAmfSUHostNodeOrNodeGroup;
bool saAmfSUFailover;
- bool saAmfSUFailover_configured; /* True when user configures
saAmfSUFailover else false */
+ /* true when user has configured saAmfSUFailover */
+ bool saAmfSUFailover_configured;
SaNameT saAmfSUMaintenanceCampaign;
/* runtime attributes */
@@ -71,11 +55,10 @@ class AVD_SU {
SaAmfAdminStateT saAmfSUAdminState;
SaAmfReadinessStateT saAmfSuReadinessState;
SaAmfPresenceStateT saAmfSUPresenceState;
- SaNameT **saAmfSUAssignedSIs;
SaNameT saAmfSUHostedByNode;
SaUint32T saAmfSUNumCurrActiveSIs;
SaUint32T saAmfSUNumCurrStandbySIs;
- SaUint32T saAmfSURestartCount; // TODO use this!
+ SaUint32T saAmfSURestartCount;
AVD_ADMIN_OPER_CBK pend_cbk;/* Stores zero invocation value of imm
adm cbk
* when no admin operation is going on.
@@ -94,10 +77,7 @@ class AVD_SU {
bool su_is_external
