Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread Anders Widell 
Hi!

Please find my comments inline below, marked AndersW>.

regards,

Anders Widell


On 09/29/2016 08:25 AM, Hoang Vo wrote:
>   osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>   osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>   osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>   osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>   osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>   osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 -
>   6 files changed, 6 insertions(+), 39 deletions(-)
>
>
> Problem:
> Statistically the check point create time for SC and PL (sync and async) has
> degradation more than 30% after bring in patch 8004
>
> Solution:
> Remove unnecessary checking that cost time. imm will take the role of checking
>
> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
> deleted file mode 100644
> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
> b/osaf/libs/common/cpsv/include/cpnd_init.h
> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>   uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO *tmr_info);
>   void cpnd_proc_free_read_data(CPSV_EVT *evt);
>   SaUint32T cpnd_get_scAbsenceAllowed_attr();
> -SaUint32T cpnd_get_longDnsAllowed_attr();
>   /* End cpnd_proc.c */
>   
>   /* File : ---  cpnd_amf.c */
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
> b/osaf/services/saf/cpsv/cpd/cpd_db.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>   err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
>   if (err != SA_AIS_OK) {
>   LOG_ER("create runtime ckpt object failed with error: 
> %u",err);
> + if (err == SA_AIS_ERR_INVALID_PARAM) {
> + return NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
AndersW> Style issue: please insert spaces around binary operators (the 
vertical bar above).
AndersW> As far as I know this would become the first place in OpenSAF 
where we return a combination of several NCSCC_RC_* error codes at the 
same time. Can you change the code above so that you only return 
NCSS_RC_INVALID_INPUT?
> + }
>   return NCSCC_RC_FAILURE;
>   }
>   }
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>   rc = SA_AIS_ERR_NO_MEMORY;
>   goto send_rsp;
>   } else if (proc_rc != NCSCC_RC_SUCCESS) {
> -
>   TRACE_4("cpd ckpt create failure ckpt name,dest :  %s, 
> %"PRIu64, ckpt_name, sinfo->dest);
>   rc = SA_AIS_ERR_LIBRARY;
> + if (proc_rc_RC_INVALID_INPUT) {
Anders> Essentially the same comments as above. Change the line above to 
"if (proc_rc == NCSCC_RC_INVALID_INPUT) {"
> + rc = SA_AIS_ERR_INVALID_PARAM;
> + }
>   goto send_rsp;
>   }
>   
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> @@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
>   TRACE_ENTER();
>   memset(_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(>info.openReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> @@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
>   TRACE_ENTER();
>   memset(_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(>info.ulinkReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.ulinkRsp.error = 
> SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.ulinkRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> @@ -2735,31 +2735,6 @@ SaUint32T

Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread A V Mahesh 
ACK not tested

-AVM


On 9/29/2016 12:27 PM, Vo Minh Hoang wrote:
> Dear Mahesh,
>
> Thank you very much for your comment.
>
> The function osaf_is_an_extended_name() just check input SaNameT is longDN
> or not.
> So it do not have any use here.
> It is mostly used for handling encode/decode part.
>
> Best regards,
> Hoang
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Thursday, September 29, 2016 1:44 PM
> To: Hoang Vo <hoang.m...@dektech.com.au>; anders.wid...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking
> each checkpoint creating time [#2068] V2
>
> Hi Hoang,
>
> It looks  osaf_is_an_extended_name() checks longDnsAllowed other services
> are checking that way, so jut keep the code as I suggested in V1 patch.
>
> -AVM
>
> On 9/29/2016 12:08 PM, A V Mahesh wrote:
>> ACK not tested
>>
>> -AVM
>>
>> On 9/29/2016 11:55 AM, Hoang Vo wrote:
>>> osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>>> osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>>> osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>>> osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>>> osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>>> osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25
> -
>>> 6 files changed, 6 insertions(+), 39 deletions(-)
>>>
>>>
>>> Problem:
>>> Statistically the check point create time for SC and PL (sync and
>>> async) has degradation more than 30% after bring in patch 8004
>>>
>>> Solution:
>>> Remove unnecessary checking that cost time. imm will take the role of
>>> checking
>>>
>>> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old
>>> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
>>> deleted file mode 100644
>>> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h
>>> b/osaf/libs/common/cpsv/include/cpnd_init.h
>>> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
>>> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
>>> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>>> uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO
> *tmr_info);
>>> void cpnd_proc_free_read_data(CPSV_EVT *evt);
>>> SaUint32T cpnd_get_scAbsenceAllowed_attr(); -SaUint32T
>>> cpnd_get_longDnsAllowed_attr();
>>> /* End cpnd_proc.c */
>>> 
>>> /* File : ---  cpnd_amf.c */
>>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> b/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>>> err = create_runtime_ckpt_object(ckpt_node, 
>>> immOiHandle);
>>> if (err != SA_AIS_OK) {
>>> LOG_ER("create runtime ckpt object failed with
> error: %u",err);
>>> +   if (err == SA_AIS_ERR_INVALID_PARAM) {
>>> +   return
> NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
>>> +   }
>>> return NCSCC_RC_FAILURE;
>>> }
>>> }
>>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>>> rc = SA_AIS_ERR_NO_MEMORY;
>>> goto send_rsp;
>>> } else if (proc_rc != NCSCC_RC_SUCCESS) {
>>> -
>>> TRACE_4("cpd ckpt create failure ckpt name,dest :  %s,
> %"PRIu64, ckpt_name, sinfo->dest);
>>> rc = SA_AIS_ERR_LIBRARY;
>>> +   if (proc_rc_RC_INVALID_INPUT) {
>>> +   rc = SA_AIS_ERR_INVALID_PARAM;
>>> +   }
>>> goto send_rsp;
>>> }
>>> 
>>> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> @@ -605,12 +605,6 @@ static u

Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread Vo Minh Hoang 
Dear Mahesh,

Thank you very much for your comment.

The function osaf_is_an_extended_name() just check input SaNameT is longDN
or not.
So it do not have any use here.
It is mostly used for handling encode/decode part.

Best regards,
Hoang

-Original Message-
From: A V Mahesh [mailto:mahesh.va...@oracle.com] 
Sent: Thursday, September 29, 2016 1:44 PM
To: Hoang Vo <hoang.m...@dektech.com.au>; anders.wid...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking
each checkpoint creating time [#2068] V2

Hi Hoang,

It looks  osaf_is_an_extended_name() checks longDnsAllowed other services
are checking that way, so jut keep the code as I suggested in V1 patch.

-AVM

On 9/29/2016 12:08 PM, A V Mahesh wrote:
> ACK not tested
>
> -AVM
>
> On 9/29/2016 11:55 AM, Hoang Vo wrote:
>>osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>>osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>>osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>>osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>>osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>>osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25
-
>>6 files changed, 6 insertions(+), 39 deletions(-)
>>
>>
>> Problem:
>> Statistically the check point create time for SC and PL (sync and 
>> async) has degradation more than 30% after bring in patch 8004
>>
>> Solution:
>> Remove unnecessary checking that cost time. imm will take the role of 
>> checking
>>
>> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
>> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
>> deleted file mode 100644
>> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
>> b/osaf/libs/common/cpsv/include/cpnd_init.h
>> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
>> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
>> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>>uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO
*tmr_info);
>>void cpnd_proc_free_read_data(CPSV_EVT *evt);
>>SaUint32T cpnd_get_scAbsenceAllowed_attr(); -SaUint32T 
>> cpnd_get_longDnsAllowed_attr();
>>/* End cpnd_proc.c */
>>
>>/* File : ---  cpnd_amf.c */
>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
>> b/osaf/services/saf/cpsv/cpd/cpd_db.c
>> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
>> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
>> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>>  err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
>>  if (err != SA_AIS_OK) {
>>  LOG_ER("create runtime ckpt object failed with
error: %u",err);
>> +if (err == SA_AIS_ERR_INVALID_PARAM) {
>> +return
NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
>> +}
>>  return NCSCC_RC_FAILURE;
>>  }
>>  }
>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
>> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
>> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>>  rc = SA_AIS_ERR_NO_MEMORY;
>>  goto send_rsp;
>>  } else if (proc_rc != NCSCC_RC_SUCCESS) {
>> -
>>  TRACE_4("cpd ckpt create failure ckpt name,dest :  %s,
%"PRIu64, ckpt_name, sinfo->dest);
>>  rc = SA_AIS_ERR_LIBRARY;
>> +if (proc_rc_RC_INVALID_INPUT) {
>> +rc = SA_AIS_ERR_INVALID_PARAM;
>> +}
>>  goto send_rsp;
>>  }
>>
>> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
>> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>> @@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
>>  TRACE_ENTER();
>>  memset(_evt, '\0', sizeof(CPSV_EVT));
>>
>> -if ((cpnd_get_longDnsAllowed_attr() == 0) &&
osaf_is_an_extended_name(>info.openReq.ckpt_name)) {
>> -LOG_ER("cpnd - longDnsAllowed == false - NOT supporting
extended name");
>> -send_evt.info.cpa.info.openRsp.error =
SA_AIS_ERR_INVALID_PARAM;
>> -goto agent_rsp;
>> -}
>> -
>>  if (!cpnd_is_cpd_up(cb)) {
>>  send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TR

Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread A V Mahesh 
Hi Hoang,

It looks  osaf_is_an_extended_name() checks longDnsAllowed other 
services are checking that way,
so jut keep the code as I suggested in V1 patch.

-AVM

On 9/29/2016 12:08 PM, A V Mahesh wrote:
> ACK not tested
>
> -AVM
>
> On 9/29/2016 11:55 AM, Hoang Vo wrote:
>>osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>>osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>>osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>>osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>>osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>>osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 
>> -
>>6 files changed, 6 insertions(+), 39 deletions(-)
>>
>>
>> Problem:
>> Statistically the check point create time for SC and PL (sync and async) has
>> degradation more than 30% after bring in patch 8004
>>
>> Solution:
>> Remove unnecessary checking that cost time. imm will take the role of 
>> checking
>>
>> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
>> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
>> deleted file mode 100644
>> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
>> b/osaf/libs/common/cpsv/include/cpnd_init.h
>> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
>> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
>> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>>uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO 
>> *tmr_info);
>>void cpnd_proc_free_read_data(CPSV_EVT *evt);
>>SaUint32T cpnd_get_scAbsenceAllowed_attr();
>> -SaUint32T cpnd_get_longDnsAllowed_attr();
>>/* End cpnd_proc.c */
>>
>>/* File : ---  cpnd_amf.c */
>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
>> b/osaf/services/saf/cpsv/cpd/cpd_db.c
>> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
>> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
>> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>>  err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
>>  if (err != SA_AIS_OK) {
>>  LOG_ER("create runtime ckpt object failed with error: 
>> %u",err);
>> +if (err == SA_AIS_ERR_INVALID_PARAM) {
>> +return NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
>> +}
>>  return NCSCC_RC_FAILURE;
>>  }
>>  }
>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
>> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
>> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>>  rc = SA_AIS_ERR_NO_MEMORY;
>>  goto send_rsp;
>>  } else if (proc_rc != NCSCC_RC_SUCCESS) {
>> -
>>  TRACE_4("cpd ckpt create failure ckpt name,dest :  %s, 
>> %"PRIu64, ckpt_name, sinfo->dest);
>>  rc = SA_AIS_ERR_LIBRARY;
>> +if (proc_rc_RC_INVALID_INPUT) {
>> +rc = SA_AIS_ERR_INVALID_PARAM;
>> +}
>>  goto send_rsp;
>>  }
>>
>> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
>> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>> @@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
>>  TRACE_ENTER();
>>  memset(_evt, '\0', sizeof(CPSV_EVT));
>>
>> -if ((cpnd_get_longDnsAllowed_attr() == 0) && 
>> osaf_is_an_extended_name(>info.openReq.ckpt_name)) {
>> -LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
>> extended name");
>> -send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_INVALID_PARAM;
>> -goto agent_rsp;
>> -}
>> -
>>  if (!cpnd_is_cpd_up(cb)) {
>>  send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TRY_AGAIN;
>>  goto agent_rsp;
>> @@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
>>  TRACE_ENTER();
>>  memset(_evt, '\0', sizeof(CPSV_EVT));
>>
>> -if ((cpnd_get_longDnsAllowed_attr() == 0) && 
>> osaf_is_an_extended_name(>info.ulinkReq.ckpt_name)) {
>> -LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
>> extended name");
>> -send_evt.info.cpa.info.ulinkRsp.error = 
>> SA_AIS_ERR_INVALID_PARAM;
>> -goto agent_rsp;
>> -}
>> -
>>  if (!cpnd_is_cpd_up(cb)) {
>>  send_evt.info.cpa.info.ulinkRsp.error = SA_AIS_ERR_TRY_AGAIN;
>>  goto agent_rsp;
>> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c 
>> b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
>> --- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
>> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
>> @@ -2735,31 +2735,6 @@ SaUint32T cpnd_get_scAbsenceAllowed_attr
>>}
>>
>>
>> /
>> - * Name  : cpnd_get_longDnsAllowed_attr()
>> - *
>> - *

Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread A V Mahesh 
ACK not tested

-AVM

On 9/29/2016 11:55 AM, Hoang Vo wrote:
>   osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>   osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>   osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>   osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>   osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>   osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 -
>   6 files changed, 6 insertions(+), 39 deletions(-)
>
>
> Problem:
> Statistically the check point create time for SC and PL (sync and async) has
> degradation more than 30% after bring in patch 8004
>
> Solution:
> Remove unnecessary checking that cost time. imm will take the role of checking
>
> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
> deleted file mode 100644
> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
> b/osaf/libs/common/cpsv/include/cpnd_init.h
> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>   uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO *tmr_info);
>   void cpnd_proc_free_read_data(CPSV_EVT *evt);
>   SaUint32T cpnd_get_scAbsenceAllowed_attr();
> -SaUint32T cpnd_get_longDnsAllowed_attr();
>   /* End cpnd_proc.c */
>   
>   /* File : ---  cpnd_amf.c */
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
> b/osaf/services/saf/cpsv/cpd/cpd_db.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>   err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
>   if (err != SA_AIS_OK) {
>   LOG_ER("create runtime ckpt object failed with error: 
> %u",err);
> + if (err == SA_AIS_ERR_INVALID_PARAM) {
> + return NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
> + }
>   return NCSCC_RC_FAILURE;
>   }
>   }
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>   rc = SA_AIS_ERR_NO_MEMORY;
>   goto send_rsp;
>   } else if (proc_rc != NCSCC_RC_SUCCESS) {
> -
>   TRACE_4("cpd ckpt create failure ckpt name,dest :  %s, 
> %"PRIu64, ckpt_name, sinfo->dest);
>   rc = SA_AIS_ERR_LIBRARY;
> + if (proc_rc_RC_INVALID_INPUT) {
> + rc = SA_AIS_ERR_INVALID_PARAM;
> + }
>   goto send_rsp;
>   }
>   
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> @@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
>   TRACE_ENTER();
>   memset(_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(>info.openReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> @@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
>   TRACE_ENTER();
>   memset(_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(>info.ulinkReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.ulinkRsp.error = 
> SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.ulinkRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> @@ -2735,31 +2735,6 @@ SaUint32T cpnd_get_scAbsenceAllowed_attr
>   }
>   
>   
> /
> - * Name  : cpnd_get_longDnsAllowed_attr()
> - *
> - * Description   : This function gets scAbsenceAllowed attribute
> - *
> - * Arguments : -
> - *
> - * Return Values : scAbsenceAllowed attribute (0 = not allowed)
> - 
> */
> -SaUint32T cpnd_get_longDnsAllowed_attr()
> -{
> -

[devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread Hoang Vo 
 osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0 
 osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
 osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
 osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
 osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
 osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 -
 6 files changed, 6 insertions(+), 39 deletions(-)


Problem:
Statistically the check point create time for SC and PL (sync and async) has
degradation more than 30% after bring in patch 8004

Solution:
Remove unnecessary checking that cost time. imm will take the role of checking

diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
deleted file mode 100644
diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
b/osaf/libs/common/cpsv/include/cpnd_init.h
--- a/osaf/libs/common/cpsv/include/cpnd_init.h
+++ b/osaf/libs/common/cpsv/include/cpnd_init.h
@@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
 uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO *tmr_info);
 void cpnd_proc_free_read_data(CPSV_EVT *evt);
 SaUint32T cpnd_get_scAbsenceAllowed_attr();
-SaUint32T cpnd_get_longDnsAllowed_attr();
 /* End cpnd_proc.c */
 
 /* File : ---  cpnd_amf.c */
diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
b/osaf/services/saf/cpsv/cpd/cpd_db.c
--- a/osaf/services/saf/cpsv/cpd/cpd_db.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
@@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
if (err != SA_AIS_OK) {
LOG_ER("create runtime ckpt object failed with error: 
%u",err);
+   if (err == SA_AIS_ERR_INVALID_PARAM) {
+   return NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
+   }
return NCSCC_RC_FAILURE;
}
}
diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
b/osaf/services/saf/cpsv/cpd/cpd_evt.c
--- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
@@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
rc = SA_AIS_ERR_NO_MEMORY;
goto send_rsp;
} else if (proc_rc != NCSCC_RC_SUCCESS) {
-
TRACE_4("cpd ckpt create failure ckpt name,dest :  %s, 
%"PRIu64, ckpt_name, sinfo->dest);
rc = SA_AIS_ERR_LIBRARY;
+   if (proc_rc_RC_INVALID_INPUT) {
+   rc = SA_AIS_ERR_INVALID_PARAM;
+   }
goto send_rsp;
}
 
diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
--- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
+++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
@@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
TRACE_ENTER();
memset(_evt, '\0', sizeof(CPSV_EVT));
 
-   if ((cpnd_get_longDnsAllowed_attr() == 0) && 
osaf_is_an_extended_name(>info.openReq.ckpt_name)) {
-   LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
extended name");
-   send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_INVALID_PARAM;
-   goto agent_rsp;
-   }
-
if (!cpnd_is_cpd_up(cb)) {
send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TRY_AGAIN;
goto agent_rsp;
@@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
TRACE_ENTER();
memset(_evt, '\0', sizeof(CPSV_EVT));
 
-   if ((cpnd_get_longDnsAllowed_attr() == 0) && 
osaf_is_an_extended_name(>info.ulinkReq.ckpt_name)) {
-   LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
extended name");
-   send_evt.info.cpa.info.ulinkRsp.error = 
SA_AIS_ERR_INVALID_PARAM;
-   goto agent_rsp;
-   }
-
if (!cpnd_is_cpd_up(cb)) {
send_evt.info.cpa.info.ulinkRsp.error = SA_AIS_ERR_TRY_AGAIN;
goto agent_rsp;
diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c 
b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
--- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
+++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
@@ -2735,31 +2735,6 @@ SaUint32T cpnd_get_scAbsenceAllowed_attr
 }
 
 
/
- * Name  : cpnd_get_longDnsAllowed_attr()
- *
- * Description   : This function gets scAbsenceAllowed attribute
- *
- * Arguments : -
- * 
- * Return Values : scAbsenceAllowed attribute (0 = not allowed)
- 
*/
-SaUint32T cpnd_get_longDnsAllowed_attr()
-{
-   SaUint32T rc_attr_val = 0;
-   char *attribute_names[] = {
-   "longDnsAllowed",
-   NULL
-   };
-
-   TRACE_ENTER();
-
-   rc_attr_val = cpnd_get_imm_attr(attribute_names);
-
-   TRACE_LEAVE();