Re: [devel] [PATCH 1 of 1] amfd: validate NG admin state at the time of creation [#1560]

2015-10-29 Thread Gary Lee 
ack (review only)

> On 29 Oct 2015, at 4:24 PM, praveen.malv...@oracle.com wrote:
> 
> osaf/services/saf/amf/amfd/nodegroup.cc |  10 --
> osaf/services/saf/amf/amfd/util.cc  |   2 +-
> 2 files changed, 9 insertions(+), 3 deletions(-)
> 
> 
> AMF allows creation of NG with invalid admin state.
> 
> Check is missing in CCB completed callback.
> 
> Patch adds the required check.
> 
> 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
> @@ -105,6 +105,13 @@ static int is_config_valid(const SaNameT
>   delete tmp_ng;
>   return 0;
>   }
> + //Check if admin state is valid or not.
> + if (!avd_admin_state_is_valid(tmp_ng->saAmfNGAdminState)) {
> + LOG_ER("Incorrect saAmfNGAdminState:'%u' for 
> '%s'",tmp_ng->saAmfNGAdminState,
> + tmp_ng->name.value);
> + delete tmp_ng;
> + return 0;
> + }
>   delete tmp_ng;
>   return 1;
> }
> @@ -159,9 +166,8 @@ static AVD_AMF_NG *ng_create(SaNameT *dn
>   if (immutil_getAttr(const_cast("saAmfNGAdminState"),
>   attributes, 0, &ng->saAmfNGAdminState) != 
> SA_AIS_OK) {
> ng->saAmfNGAdminState = SA_AMF_ADMIN_UNLOCKED;
> - LOG_NO("Setting saAmfNGAdminState to 
> :'%u'",ng->saAmfNGAdminState);
> + TRACE("Setting saAmfNGAdminState to 
> :'%u'",ng->saAmfNGAdminState);
>   }
> - //TODO_NG: Add protection against shutting down state and lock-in state.
>   rc = 0;
> done:
>   if (rc != 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
> @@ -1285,7 +1285,7 @@ uint32_t avd_snd_comp_validation_resp(AV
> 
> int avd_admin_state_is_valid(SaAmfAdminStateT state)
> {
> - return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state <= 
> SA_AMF_ADMIN_SHUTTING_DOWN));
> + return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state < 
> SA_AMF_ADMIN_SHUTTING_DOWN));
> }
> 
> /*
> 
> --
> ___
> Opensaf-devel mailing list
> Opensaf-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] amfd: validate NG admin state at the time of creation [#1560]

2015-10-29 Thread Hans Nordebäck 
ack, code review only/Thanks Hans

On 10/29/2015 06:24 AM, praveen.malv...@oracle.com wrote:
>   osaf/services/saf/amf/amfd/nodegroup.cc |  10 --
>   osaf/services/saf/amf/amfd/util.cc  |   2 +-
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
>
> AMF allows creation of NG with invalid admin state.
>
> Check is missing in CCB completed callback.
>
> Patch adds the required check.
>
> 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
> @@ -105,6 +105,13 @@ static int is_config_valid(const SaNameT
>   delete tmp_ng;
>   return 0;
>   }
> + //Check if admin state is valid or not.
> + if (!avd_admin_state_is_valid(tmp_ng->saAmfNGAdminState)) {
> + LOG_ER("Incorrect saAmfNGAdminState:'%u' for 
> '%s'",tmp_ng->saAmfNGAdminState,
> + tmp_ng->name.value);
> + delete tmp_ng;
> + return 0;
> + }
>   delete tmp_ng;
>   return 1;
>   }
> @@ -159,9 +166,8 @@ static AVD_AMF_NG *ng_create(SaNameT *dn
>   if (immutil_getAttr(const_cast("saAmfNGAdminState"),
>   attributes, 0, &ng->saAmfNGAdminState) != 
> SA_AIS_OK) {
>   ng->saAmfNGAdminState = SA_AMF_ADMIN_UNLOCKED;
> - LOG_NO("Setting saAmfNGAdminState to 
> :'%u'",ng->saAmfNGAdminState);
> + TRACE("Setting saAmfNGAdminState to 
> :'%u'",ng->saAmfNGAdminState);
>   }
> - //TODO_NG: Add protection against shutting down state and lock-in state.
>   rc = 0;
>   done:
>   if (rc != 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
> @@ -1285,7 +1285,7 @@ uint32_t avd_snd_comp_validation_resp(AV
>   
>   int avd_admin_state_is_valid(SaAmfAdminStateT state)
>   {
> - return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state <= 
> SA_AMF_ADMIN_SHUTTING_DOWN));
> + return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state < 
> SA_AMF_ADMIN_SHUTTING_DOWN));
>   }
>   
>   
> /*


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel