Dear Mahesh, I would like to send updated patch following your comment. When this is minor comment, I send it as attached file.
Sincerely, Hoang -----Original Message----- From: A V Mahesh [mailto:mahesh.va...@oracle.com] Sent: Monday, August 22, 2016 12:45 PM To: Hoang Vo <hoang.m...@dektech.com.au> Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 3 of 8] cpa: Add support for extended SaNameT [#1574] v1 Hi Hoang, ACK for [PATCH 3 of 8] with following minor comment I think API should return SA_AIS_ERR_TOO_BIG = 26 instead of SA_AIS_ERR_INVALID_PARAM ( please sync-up with other service return values ). Note : tested default functionality , LONG DN functionality not tested in full fledged -AVM On 8/18/2016 12:48 PM, Hoang Vo wrote: > osaf/libs/agents/saf/cpa/Makefile.am | 1 + > osaf/libs/agents/saf/cpa/cpa_api.c | 36 ++++++++++++++++---------------- > osaf/libs/agents/saf/cpa/cpa_db.c | 2 + > osaf/libs/agents/saf/cpa/cpa_mds.c | 2 +- > osaf/libs/agents/saf/cpa/cpa_proc.c | 2 +- > osaf/libs/common/cpsv/include/cpa.h | 1 + > osaf/libs/common/cpsv/include/cpa_cb.h | 2 +- > osaf/libs/common/cpsv/include/cpa_proc.h | 2 +- > 8 files changed, 26 insertions(+), 22 deletions(-) > > > diff --git a/osaf/libs/agents/saf/cpa/Makefile.am > b/osaf/libs/agents/saf/cpa/Makefile.am > --- a/osaf/libs/agents/saf/cpa/Makefile.am > +++ b/osaf/libs/agents/saf/cpa/Makefile.am > @@ -22,6 +22,7 @@ noinst_LTLIBRARIES = libcpa.la > > libcpa_la_CPPFLAGS = \ > -DNCS_CPA=1 \ > + -DSA_EXTENDED_NAME_SOURCE \ > $(AM_CPPFLAGS) \ > -I$(top_srcdir)/osaf/libs/common/cpsv/include > > diff --git a/osaf/libs/agents/saf/cpa/cpa_api.c > b/osaf/libs/agents/saf/cpa/cpa_api.c > --- a/osaf/libs/agents/saf/cpa/cpa_api.c > +++ b/osaf/libs/agents/saf/cpa/cpa_api.c > @@ -870,19 +870,20 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > bool locked = false; > uint32_t time_out=0; > CPA_GLOBAL_CKPT_NODE *gc_node = NULL; > + SaConstStringT ckpt_name = NULL; > > TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); > - if ((checkpointName == NULL) || (checkpointHandle == NULL) || (checkpointName->length == 0)) { > + if ((checkpointName == NULL) || (checkpointHandle == NULL) || > +(osaf_extended_name_length(checkpointName) == 0)) { > TRACE_4("Cpa CkptOpen Api failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); > TRACE_LEAVE2("API return code = %u", rc); > return SA_AIS_ERR_INVALID_PARAM; > } > > - m_CPSV_SET_SANAMET(checkpointName); > + ckpt_name = osaf_extended_name_borrow(checkpointName); > > /* SA_AIS_ERR_INVALID_PARAM, bullet 4 in SAI-AIS-CKPT-B.02.02 > Section 3.6.1 saCkptCheckpointOpen() and saCkptCheckpointOpenAsync(), Return Values */ > - if (strncmp((const char *)checkpointName->value, "safCkpt=", 8) != 0) { > + if (strncmp(ckpt_name, "safCkpt=", 8) != 0) { > TRACE_4("Cpa CkptOpen:DN failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); > TRACE_LEAVE2("API return code = %u", rc); > return SA_AIS_ERR_INVALID_PARAM; [AVM] I think this should return SA_AIS_ERR_TOO_BIG = 26 ( please sync-up with other service return values ) > @@ -909,7 +910,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > > > /* Draft Validations */ > - rc = cpa_open_attr_validate(checkpointCreationAttributes, checkpointOpenFlags, checkpointName); > + rc = cpa_open_attr_validate(checkpointCreationAttributes, > +checkpointOpenFlags); > if (rc != SA_AIS_OK) { > /* No need to log, already logged inside the cpa_open_attr_validate */ > goto done; > @@ -965,7 +966,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > lc_node->cl_hdl = ckptHandle; > lc_node->open_flags = checkpointOpenFlags; > > - lc_node->ckpt_name = *checkpointName; > + lc_node->ckpt_name = strdup(ckpt_name); > > /* Add CPA_LOCAL_CKPT_NODE to lcl_ckpt_hdl_tree */ > proc_rc = cpa_lcl_ckpt_node_add(&cb->lcl_ckpt_tree, lc_node); @@ > -984,7 +985,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > evt.info.cpnd.info.openReq.client_hdl = ckptHandle; > evt.info.cpnd.info.openReq.lcl_ckpt_hdl = lc_node->lcl_ckpt_hdl; > > - evt.info.cpnd.info.openReq.ckpt_name = *checkpointName; > + osaf_extended_name_lend(ckpt_name, > +&evt.info.cpnd.info.openReq.ckpt_name); > > if (checkpointCreationAttributes) { > evt.info.cpnd.info.openReq.ckpt_attrib = > *checkpointCreationAttributes; @@ -1128,6 +1129,7 @@ gl_node_add_fail: > > lc_node_add_fail: > if (lc_node != NULL) { > + free((void *)lc_node->ckpt_name); > m_MMGR_FREE_CPA_LOCAL_CKPT_NODE(lc_node); > } > > @@ -1179,6 +1181,7 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa > CPA_LOCAL_CKPT_NODE *lc_node = NULL; > CPA_CLIENT_NODE *cl_node = NULL; > uint32_t proc_rc = NCSCC_RC_SUCCESS; > + SaConstStringT ckpt_name = NULL; > > TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); > > @@ -1188,19 +1191,17 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa > return SA_AIS_ERR_INVALID_PARAM; > } > > - /* Draft Validations */ > - > - m_CPSV_SET_SANAMET(checkpointName); > + ckpt_name = osaf_extended_name_borrow(checkpointName); > > /* SA_AIS_ERR_INVALID_PARAM, bullet 4 in SAI-AIS-CKPT-B.02.02 > Section 3.6.1 saCkptCheckpointOpen() and saCkptCheckpointOpenAsync(), Return Values */ > - if (strncmp((const char *)checkpointName->value, "safCkpt=", 8) != 0) { > + if (strncmp(ckpt_name, "safCkpt=", 8) != 0) { > TRACE_4("cpa CkptOpen:DN Api failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); > TRACE_LEAVE2("API return code = %u", rc); > return SA_AIS_ERR_INVALID_PARAM; > } > > - rc = cpa_open_attr_validate(checkpointCreationAttributes, checkpointOpenFlags, checkpointName); > + rc = cpa_open_attr_validate(checkpointCreationAttributes, > +checkpointOpenFlags); > if (rc != SA_AIS_OK) { > /* No need to log, it is already logged inside */ > goto done; > @@ -1261,7 +1262,7 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa > lc_node->lcl_ckpt_hdl = NCS_PTR_TO_UNS64_CAST(lc_node); > lc_node->cl_hdl = ckptHandle; > lc_node->open_flags = checkpointOpenFlags; > - lc_node->ckpt_name = *checkpointName; > + lc_node->ckpt_name = strdup(ckpt_name); > > /* Add CPA_LOCAL_CKPT_NODE to lcl_ckpt_hdl_tree */ > proc_rc = cpa_lcl_ckpt_node_add(&cb->lcl_ckpt_tree, lc_node); @@ > -1280,7 +1281,7 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa > evt.info.cpnd.info.openReq.client_hdl = ckptHandle; > evt.info.cpnd.info.openReq.lcl_ckpt_hdl = lc_node->lcl_ckpt_hdl; > > - evt.info.cpnd.info.openReq.ckpt_name = *checkpointName; > + osaf_extended_name_lend(ckpt_name, > +&evt.info.cpnd.info.openReq.ckpt_name); > > if (checkpointCreationAttributes) { > evt.info.cpnd.info.openReq.ckpt_attrib = > *checkpointCreationAttributes; @@ -1365,6 +1366,7 @@ SaAisErrorT > saCkptCheckpointOpenAsync(Sa > > lc_node_add_fail: > if (lc_node != NULL) { > + free((void *)lc_node->ckpt_name); > m_MMGR_FREE_CPA_LOCAL_CKPT_NODE(lc_node); > } > > @@ -1583,6 +1585,7 @@ SaAisErrorT saCkptCheckpointUnlink(SaCkp > uint32_t proc_rc = NCSCC_RC_SUCCESS; > CPA_CLIENT_NODE *cl_node = NULL; > CPA_CB *cb = NULL; > + SaConstStringT ckpt_name = NULL; > > TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); > /* For unlink CPA will not perform any operation other than passing > @@ -1593,7 +1596,7 @@ SaAisErrorT saCkptCheckpointUnlink(SaCkp > return rc; > } > > - m_CPSV_SET_SANAMET(checkpointName); > + ckpt_name = osaf_extended_name_borrow(checkpointName); > > /* retrieve CPA CB */ > m_CPA_RETRIEVE_CB(cb); > @@ -1634,10 +1637,7 @@ SaAisErrorT saCkptCheckpointUnlink(SaCkp > evt.type = CPSV_EVT_TYPE_CPND; > evt.info.cpnd.type = CPND_EVT_A2ND_CKPT_UNLINK; > > - /* evt.info.cpnd.info.ulinkReq.ckpt_name.length = checkpointName->length; > - memcpy(evt.info.cpnd.info.ulinkReq.ckpt_name.value,checkpointName->value,che ckpointName->length); */ > - > - evt.info.cpnd.info.ulinkReq.ckpt_name = *checkpointName; > + osaf_extended_name_lend(ckpt_name, > +&evt.info.cpnd.info.ulinkReq.ckpt_name); > > /* IF CPND IS DOWN */ > if (false == cb->is_cpnd_up) { > diff --git a/osaf/libs/agents/saf/cpa/cpa_db.c > b/osaf/libs/agents/saf/cpa/cpa_db.c > --- a/osaf/libs/agents/saf/cpa/cpa_db.c > +++ b/osaf/libs/agents/saf/cpa/cpa_db.c > @@ -267,6 +267,8 @@ uint32_t cpa_lcl_ckpt_node_delete(CPA_CB > ncshm_destroy_hdl(NCS_SERVICE_ID_CPA, lc_node->async_req_tmr.uarg); > } > > + free((void *)lc_node->ckpt_name); > + > m_MMGR_FREE_CPA_LOCAL_CKPT_NODE(lc_node); > } > > diff --git a/osaf/libs/agents/saf/cpa/cpa_mds.c > b/osaf/libs/agents/saf/cpa/cpa_mds.c > --- a/osaf/libs/agents/saf/cpa/cpa_mds.c > +++ b/osaf/libs/agents/saf/cpa/cpa_mds.c > @@ -517,7 +517,7 @@ static uint32_t cpa_mds_svc_evt(CPA_CB * > evt.type = CPSV_EVT_TYPE_CPND; > evt.info.cpnd.type = CPND_EVT_A2ND_CKPT_LIST_UPDATE; > evt.info.cpnd.info.ckptListUpdate.client_hdl = lc_node->cl_hdl; > - evt.info.cpnd.info.ckptListUpdate.ckpt_name = lc_node->ckpt_name ; > + osaf_extended_name_lend(lc_node->ckpt_name, > +&evt.info.cpnd.info.ckptListUpdate.ckpt_name); > > proc_rc = cpa_mds_msg_send(cb->cpa_mds_hdl, &cb->cpnd_mds_dest, > &evt, NCSMDS_SVC_ID_CPND); > > diff --git a/osaf/libs/agents/saf/cpa/cpa_proc.c > b/osaf/libs/agents/saf/cpa/cpa_proc.c > --- a/osaf/libs/agents/saf/cpa/cpa_proc.c > +++ b/osaf/libs/agents/saf/cpa/cpa_proc.c > @@ -87,7 +87,7 @@ uint32_t cpa_version_validate(SaVersionT > **************************************************************************** **/ > uint32_t cpa_open_attr_validate(const SaCkptCheckpointCreationAttributesT > *checkpointCreationAttributes, > - SaCkptCheckpointOpenFlagsT checkpointOpenFlags, const SaNameT *checkpointName) > + SaCkptCheckpointOpenFlagsT checkpointOpenFlags) > { > SaCkptCheckpointCreationFlagsT creationFlags = 0; > /* Check the Open Flags is set, it should */ diff --git > a/osaf/libs/common/cpsv/include/cpa.h > b/osaf/libs/common/cpsv/include/cpa.h > --- a/osaf/libs/common/cpsv/include/cpa.h > +++ b/osaf/libs/common/cpsv/include/cpa.h > @@ -33,6 +33,7 @@ > #ifndef CPA_H > #define CPA_H > > +#include "osaf_extended_name.h" > #include "cpsv.h" > #include "cpa_dl_api.h" > #include "cpsv_papi.h" > diff --git a/osaf/libs/common/cpsv/include/cpa_cb.h > b/osaf/libs/common/cpsv/include/cpa_cb.h > --- a/osaf/libs/common/cpsv/include/cpa_cb.h > +++ b/osaf/libs/common/cpsv/include/cpa_cb.h > @@ -41,7 +41,7 @@ typedef struct cpa_local_ckpt_node { > SaCkptHandleT cl_hdl; /* client handle */ > SaCkptCheckpointHandleT gbl_ckpt_hdl; /* globally aware handle */ > SaCkptCheckpointOpenFlagsT open_flags; > - SaNameT ckpt_name; > + SaConstStringT ckpt_name; > CPA_TMR async_req_tmr; /* Timer used for async requests */ > uint32_t sect_iter_cnt; > } CPA_LOCAL_CKPT_NODE; > diff --git a/osaf/libs/common/cpsv/include/cpa_proc.h > b/osaf/libs/common/cpsv/include/cpa_proc.h > --- a/osaf/libs/common/cpsv/include/cpa_proc.h > +++ b/osaf/libs/common/cpsv/include/cpa_proc.h > @@ -42,7 +42,7 @@ uint32_t cpa_process_evt(CPA_CB *cb, CPS > uint32_t cpa_version_validate(SaVersionT *version); > uint32_t cpa_open_attr_validate(const SaCkptCheckpointCreationAttributesT > *checkpointCreationAttributes, > - SaCkptCheckpointOpenFlagsT checkpointOpenFlags, const SaNameT *checkpointName); > + SaCkptCheckpointOpenFlagsT checkpointOpenFlags); > > uint32_t cpa_callback_ipc_init(CPA_CLIENT_NODE *client_info); > void cpa_callback_ipc_destroy(CPA_CLIENT_NODE *client_info);
1574_cpa_support_extended_SaNameT_v2.patch
Description: Binary data
------------------------------------------------------------------------------
_______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
