Menu
▾
▴
Re: [devel] [PATCH 1 of 1] amfd: fix SHUTTING_DOWN state validation during standby AMFD initialization [#1560]
|
From: Hans N. <han...@er...> - 2015-11-17 11:16:25
|
ack, code review only. Minor comments below. /Thanks HansN
On 11/17/2015 09:56 AM, pra...@or... wrote:
> osaf/services/saf/amf/amfd/app.cc | 2 +-
> osaf/services/saf/amf/amfd/cluster.cc | 2 +-
> osaf/services/saf/amf/amfd/include/util.h | 3 ++-
> osaf/services/saf/amf/amfd/node.cc | 2 +-
> osaf/services/saf/amf/amfd/nodegroup.cc | 2 +-
> osaf/services/saf/amf/amfd/sg.cc | 2 +-
> osaf/services/saf/amf/amfd/si.cc | 2 +-
> osaf/services/saf/amf/amfd/su.cc | 2 +-
> osaf/services/saf/amf/amfd/util.cc | 7 +++++--
> 9 files changed, 14 insertions(+), 10 deletions(-)
>
>
> AMFD intialization fails for standby controller if any AMF entities is in SHUTTING_DOWN state.
>
> In the pushed patch for #1560, AMF does not allow creation of AMF entities in SHUTTING_DOWN
> state. Function avd_admin_state_is_valid() must handle both the cases
> 1)When controller joins the cluster.
> 2)When a new AMF having admin state entity is configured.
> Currently it is handling case 2) only.
>
> Patch fixes the problem for case 1).
>
> diff --git a/osaf/services/saf/amf/amfd/app.cc b/osaf/services/saf/amf/amfd/app.cc
> --- a/osaf/services/saf/amf/amfd/app.cc
> +++ b/osaf/services/saf/amf/amfd/app.cc
> @@ -190,7 +190,7 @@ static int is_config_valid(const SaNameT
> }
>
> if ((immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfApplicationAdminState"), attributes, 0, &admstate) == SA_AIS_OK) &&
> - !avd_admin_state_is_valid(admstate)) {
> + !avd_admin_state_is_valid(admstate, opdata)) {
> report_ccb_validation_error(opdata, "Invalid saAmfApplicationAdminState %u for '%s'", admstate, dn->value);
> return 0;
> }
> diff --git a/osaf/services/saf/amf/amfd/cluster.cc b/osaf/services/saf/amf/amfd/cluster.cc
> --- a/osaf/services/saf/amf/amfd/cluster.cc
> +++ b/osaf/services/saf/amf/amfd/cluster.cc
> @@ -243,7 +243,7 @@ SaAisErrorT avd_cluster_config_get(void)
> avd_cluster->saAmfClusterAdminState = SA_AMF_ADMIN_UNLOCKED;
> }
>
> - if (!avd_admin_state_is_valid(avd_cluster->saAmfClusterAdminState)) {
> + if (!avd_admin_state_is_valid(avd_cluster->saAmfClusterAdminState, nullptr)) {
> LOG_ER("Invalid saAmfClusterAdminState %u", avd_cluster->saAmfClusterAdminState);
> return static_cast<SaAisErrorT>(-1);
> }
> diff --git a/osaf/services/saf/amf/amfd/include/util.h b/osaf/services/saf/amf/amfd/include/util.h
> --- a/osaf/services/saf/amf/amfd/include/util.h
> +++ b/osaf/services/saf/amf/amfd/include/util.h
> @@ -39,6 +39,7 @@
> #include <amf_d2nmsg.h>
> #include <cb.h>
> #include <amf_util.h>
> +#include <immutil.h>
>
> class AVD_SU;
>
> @@ -133,7 +134,7 @@ uint32_t avd_d2d_msg_snd(struct cl_cb_ta
>
> std::string to_string(const SaNameT &s);
>
> -extern int avd_admin_state_is_valid(SaAmfAdminStateT state);
> +extern int avd_admin_state_is_valid(SaAmfAdminStateT state, const CcbUtilOperationData_t *opdata);
> extern SaAisErrorT avd_object_name_create(SaNameT *rdn_attr_value, SaNameT *parentName, SaNameT *object_name);
> int amfd_file_dump(const char* filename);
>
> diff --git a/osaf/services/saf/amf/amfd/node.cc b/osaf/services/saf/amf/amfd/node.cc
> --- a/osaf/services/saf/amf/amfd/node.cc
> +++ b/osaf/services/saf/amf/amfd/node.cc
> @@ -253,7 +253,7 @@ static int is_config_valid(const SaNameT
> }
>
> if ((immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfNodeAdminState"), attributes, 0, &admstate) == SA_AIS_OK) &&
> - !avd_admin_state_is_valid(admstate)) {
> + !avd_admin_state_is_valid(admstate, opdata)) {
> report_ccb_validation_error(opdata, "Invalid saAmfNodeAdminState %u for '%s'", admstate, dn->value);
> return 0;
> }
> diff --git a/osaf/services/saf/amf/amfd/nodegroup.cc b/osaf/services/saf/amf/amfd/nodegroup.cc
> --- a/osaf/services/saf/amf/amfd/nodegroup.cc
> +++ b/osaf/services/saf/amf/amfd/nodegroup.cc
> @@ -106,7 +106,7 @@ static int is_config_valid(const SaNameT
> return 0;
> }
> //Check if admin state is valid or not.
> - if (!avd_admin_state_is_valid(tmp_ng->saAmfNGAdminState)) {
> + if (!avd_admin_state_is_valid(tmp_ng->saAmfNGAdminState, opdata)) {
> LOG_ER("Incorrect saAmfNGAdminState:'%u' for '%s'",tmp_ng->saAmfNGAdminState,
> tmp_ng->name.value);
> delete tmp_ng;
> diff --git a/osaf/services/saf/amf/amfd/sg.cc b/osaf/services/saf/amf/amfd/sg.cc
> --- a/osaf/services/saf/amf/amfd/sg.cc
> +++ b/osaf/services/saf/amf/amfd/sg.cc
> @@ -242,7 +242,7 @@ static int is_config_valid(const SaNameT
> }
>
> if ((immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGAdminState"), attributes, 0, &admstate) == SA_AIS_OK) &&
> - !avd_admin_state_is_valid(admstate)) {
> + !avd_admin_state_is_valid(admstate, opdata)) {
> report_ccb_validation_error(opdata, "Invalid saAmfSGAdminState %u for '%s'", admstate, dn->value);
> return 0;
> }
> diff --git a/osaf/services/saf/amf/amfd/si.cc b/osaf/services/saf/amf/amfd/si.cc
> --- a/osaf/services/saf/amf/amfd/si.cc
> +++ b/osaf/services/saf/amf/amfd/si.cc
> @@ -528,7 +528,7 @@ static int is_config_valid(const SaNameT
> }
>
> if ((immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSIAdminState"), attributes, 0, &admstate) == SA_AIS_OK) &&
> - !avd_admin_state_is_valid(admstate)) {
[HansN] use space between parameters, (admstate, opdata)
> + !avd_admin_state_is_valid(admstate,opdata)) {
> report_ccb_validation_error(opdata, "Invalid saAmfSIAdminState %u for '%s'", admstate, dn->value);
> return 0;
> }
> diff --git a/osaf/services/saf/amf/amfd/su.cc b/osaf/services/saf/amf/amfd/su.cc
> --- a/osaf/services/saf/amf/amfd/su.cc
> +++ b/osaf/services/saf/amf/amfd/su.cc
> @@ -355,7 +355,7 @@ static int is_config_valid(const SaNameT
> }
>
> if ((immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSUAdminState"), attributes, 0, &admstate) == SA_AIS_OK) &&
> - !avd_admin_state_is_valid(admstate)) {
> + !avd_admin_state_is_valid(admstate, opdata)) {
> report_ccb_validation_error(opdata, "Invalid saAmfSUAdminState %u for '%s'", admstate, dn->value);
> return 0;
> }
> diff --git a/osaf/services/saf/amf/amfd/util.cc b/osaf/services/saf/amf/amfd/util.cc
> --- a/osaf/services/saf/amf/amfd/util.cc
> +++ b/osaf/services/saf/amf/amfd/util.cc
> @@ -1283,9 +1283,12 @@ uint32_t avd_snd_comp_validation_resp(AV
>
> }
>
> -int avd_admin_state_is_valid(SaAmfAdminStateT state)
> +int avd_admin_state_is_valid(SaAmfAdminStateT state, const CcbUtilOperationData_t *opdata)
> {
> - return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state < SA_AMF_ADMIN_SHUTTING_DOWN));
[HansN] Use curly brace in if stmt with else. (google style guide)
> + if (opdata == nullptr)
> + return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state <= SA_AMF_ADMIN_SHUTTING_DOWN));
> + else
> + return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state < SA_AMF_ADMIN_SHUTTING_DOWN));
> }
>
> /*****************************************************************************
|
