Hi Mahesh, In osaf/services/saf/amf/amfnd/main.cc, I think there is a fault case. The fds[FD_CLM].fd is updated once before start the while loop. Thus, the fds[FD_CLM].fd is not updated in case cb->clm_sel_obj is updated. As result, if the cb->clm_sel_obj is different with the previous one. The amfnd doesn't poll() correct fds[FD_CLM].fd.
Regarding "- Not tested/hit the new thread code of saClmDispatch() fail case.", The saClmDispatch() fail case happens when a SC is up after headless state. If you agree to keep the V2, could you please help to push the patch? Thanks. Best regards, Nhat Pham -----Original Message----- From: A V Mahesh [mailto:mahesh.va...@oracle.com] Sent: Friday, May 13, 2016 11:20 AM To: Nhat Pham <nhat.p...@dektech.com.au>; anders.wid...@ericsson.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1 of 1] cpnd: To improve handling fault code returned by saClmInitialize [#1821] V2 Hi Nhat Pham, ACK with following : We can still optimize the code with single FD_CLM ( smiler was achieved in osaf/services/saf/amf/amfnd/main.cc ) - Done normal node bring-up - Not tested/hit the new thread code of saClmDispatch() fail case. - Simulate the problem before pushing. -AVM On 5/12/2016 8:49 AM, wrote: > osaf/libs/common/cpsv/include/cpnd_cb.h | 2 + > osaf/services/saf/cpsv/cpnd/cpnd_init.c | 213 ++++++++++++++++++++++++------- > 2 files changed, 163 insertions(+), 52 deletions(-) > > > There are 2 places where the saClmInitialize() is called. > 1. At start up time, in cpnd_lib_init() (cpnd_init.c) 2. In > cpnd_main_process() (cpnd_init.c), when saClmDispatch() returns SA_AIS_ERR_BAD_HANDLE. > > The solution is: > For case 1: initialize clm is retried on the current executing thread. > (i.e not creating new thread) For case 2: a new thread is created to initialize clm. > > diff --git a/osaf/libs/common/cpsv/include/cpnd_cb.h > b/osaf/libs/common/cpsv/include/cpnd_cb.h > --- a/osaf/libs/common/cpsv/include/cpnd_cb.h > +++ b/osaf/libs/common/cpsv/include/cpnd_cb.h > @@ -321,6 +321,8 @@ typedef struct cpnd_cb_tag { > bool scAbsenceAllowed; > bool shm_alloc_guaranteed; > > + NCS_SEL_OBJ clm_updated_sel_obj; /* The CLM select object updated > +event */ > + > } CPND_CB; > > /* CB prototypes */ > diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_init.c > b/osaf/services/saf/cpsv/cpnd/cpnd_init.c > --- a/osaf/services/saf/cpsv/cpnd/cpnd_init.c > +++ b/osaf/services/saf/cpsv/cpnd/cpnd_init.c > @@ -30,12 +30,14 @@ > #include <daemon.h> > #include "cpnd.h" > #include "osaf_poll.h" > +#include "osaf_time.h" > > enum { > FD_TERM, > FD_MBX, > FD_AMF, > FD_CLM, > + FD_CLM_UPDATED, > NUMBER_OF_FDS > }; > > @@ -57,6 +59,10 @@ static uint32_t cpnd_cb_db_destroy(CPND_ > > static bool cpnd_clear_mbx(NCSCONTEXT arg, NCSCONTEXT msg); > > +static SaAisErrorT cpnd_clm_init(void); > + > +static SaAisErrorT cpnd_start_clm_init_bg(); > + > void cpnd_main_process(CPND_CB *cb); > > > /********************************************************************* > ******* @@ -173,16 +179,10 @@ static uint32_t cpnd_lib_init(CPND_CREAT > void *shm_ptr; > GBL_SHM_PTR gbl_shm_addr; > memset(&cpnd_open_req, '\0', sizeof(cpnd_open_req)); > - SaVersionT clm_version; > - SaClmClusterNodeT cluster_node; > - SaClmHandleT clmHandle; > > - m_CPSV_GET_AMF_VER(clm_version); > - SaClmCallbacksT gen_cbk; > char *ptr; > > TRACE_ENTER(); > - memset(&cluster_node, 0, sizeof(SaClmClusterNodeT)); > > /* allocate a CB */ > cb = m_MMGR_ALLOC_CPND_CB; > @@ -237,38 +237,14 @@ static uint32_t cpnd_lib_init(CPND_CREAT > LOG_ER("cpnd ipc attach failed"); > goto cpnd_ipc_att_fail; > } > - gen_cbk.saClmClusterNodeGetCallback = NULL; > - gen_cbk.saClmClusterTrackCallback = cpnd_clm_cluster_track_cb; > - rc = saClmInitialize(&clmHandle, &gen_cbk, &clm_version); > - while (rc == SA_AIS_ERR_TRY_AGAIN) { > - usleep(1000000); > - rc = saClmInitialize(&clmHandle, &gen_cbk, &clm_version); > - } > > + /* Initalize the CLM service */ > + rc = cpnd_clm_init(); > if (rc != SA_AIS_OK) { > LOG_ER("cpnd clm init failed with return value:%d",rc); > goto cpnd_clm_init_fail; > } > - cb->clm_hdl = clmHandle; > > - if (SA_AIS_OK != (rc = saClmSelectionObjectGet(cb->clm_hdl, &cb->clm_sel_obj))) { > - LOG_ER("cpnd clm selection object Get failed with return value:%d",rc); > - TRACE_LEAVE(); > - return rc; > - } > - > - rc = saClmClusterNodeGet(cb->clm_hdl, SA_CLM_LOCAL_NODE_ID, CPND_CLM_API_TIMEOUT, &cluster_node); > - if (rc != SA_AIS_OK) { > - LOG_ER("cpnd clm node get failed with return value:%d",rc); > - goto cpnd_clm_fail; > - } > - cb->nodeid = cluster_node.nodeId; > - > - rc = saClmClusterTrack(cb->clm_hdl, (SA_TRACK_CURRENT | SA_TRACK_CHANGES), NULL); > - if (rc != SA_AIS_OK) { > - LOG_ER("cpnd clm clusterTrack failed with return value:%d",rc); > - goto cpnd_clm_fail; > - } > /* Initialise with the AMF service */ > if (cpnd_amf_init(cb) != NCSCC_RC_SUCCESS) { > LOG_ER("cpnd amf init failed"); > @@ -283,7 +259,7 @@ static uint32_t cpnd_lib_init(CPND_CREAT > > /* CODE FOR THE NO REDUNDANCY */ > memset(&gbl_shm_addr, 0, sizeof(GBL_SHM_PTR)); > - shm_ptr = cpnd_restart_shm_create(&cpnd_open_req, cb, cluster_node.nodeId); > + shm_ptr = cpnd_restart_shm_create(&cpnd_open_req, cb, cb->nodeid); > if (shm_ptr) { > gbl_shm_addr.base_addr = shm_ptr; /* Store base address of shared memory, but not used for any operations as of now */ > gbl_shm_addr.cli_addr = shm_ptr + sizeof(CPND_SHM_VERSION); @@ > -340,9 +316,6 @@ static uint32_t cpnd_lib_init(CPND_CREAT > cpnd_mds_fail: > m_NCS_TASK_STOP(cb->task_hdl); > > - cpnd_clm_fail: > - saClmFinalize(cb->clm_hdl); > - > cpnd_clm_init_fail: > m_NCS_IPC_DETACH(&cb->cpnd_mbx, cpnd_clear_mbx, cb); > > @@ -533,6 +506,10 @@ void cpnd_main_process(CPND_CB *cb) > struct pollfd fds[NUMBER_OF_FDS]; > int term_fd; > > + ncs_sel_obj_create(&cb->clm_updated_sel_obj); > + fds[FD_CLM_UPDATED].fd = m_GET_FD_FROM_SEL_OBJ(cb->clm_updated_sel_obj); > + fds[FD_CLM_UPDATED].events = POLLIN; > + > mbx_fd = ncs_ipc_get_sel_obj(&cb->cpnd_mbx); > fds[FD_MBX].fd = m_GET_FD_FROM_SEL_OBJ(mbx_fd); > fds[FD_MBX].events = POLLIN; > @@ -578,31 +555,22 @@ void cpnd_main_process(CPND_CB *cb) > } > } > > + /* process all the CLM messages */ > if (fds[FD_CLM].revents & POLLIN) { > clm_error = saClmDispatch(cb->clm_hdl, SA_DISPATCH_ALL); > if (clm_error == SA_AIS_ERR_BAD_HANDLE) { > - SaVersionT clm_version; > - SaClmHandleT clmHandle; > - SaClmCallbacksT gen_cbk; > + LOG_NO("Bad CLM handle. Reinitializing."); > + osaf_nanosleep(&kHundredMilliseconds); > > - LOG_NO("Bad CLM handle. Reinitializing."); > - usleep(100000); > + cpnd_start_clm_init_bg(); > > - m_CPSV_GET_AMF_VER(clm_version); > - gen_cbk.saClmClusterNodeGetCallback = NULL; > - gen_cbk.saClmClusterTrackCallback = cpnd_clm_cluster_track_cb; > - > - clm_error = saClmInitialize(&clmHandle, &gen_cbk, &clm_version); > - if (clm_error != SA_AIS_OK) { > - LOG_ER("cpnd clm init failed with return value:%d", clm_error); > - TRACE_LEAVE(); > - return; > - } > - cb->clm_hdl = clmHandle; > + /* Ignore the FD_CLM */ > + fds[FD_CLM].fd = -1; > } else if (clm_error != SA_AIS_OK) { > LOG_ER("cpnd clm dispatch failure %u", clm_error); > } > } > + > /* process the CPND Mail box */ > if (fds[FD_MBX].revents & POLLIN) { > > @@ -611,7 +579,148 @@ void cpnd_main_process(CPND_CB *cb) > cpnd_process_evt(evt); > } > } > + > + /* process the CLM object updated event */ > + if (fds[FD_CLM_UPDATED].revents & POLLIN) { > + fds[FD_CLM].fd = cb->clm_sel_obj; > + LOG_NO("CLM selection object was updated. (%lld)", > +cb->clm_sel_obj); > + > + ncs_sel_obj_rmv_ind(&cb->clm_updated_sel_obj, true, true); > + } > } > TRACE_LEAVE(); > return; > } > + > +/************************************************************************** ** > + * Name : cpnd_clm_init > + * > + * Description : This function initialize CLM, get Node Id, and enalbe > + * tracking callback. > + * > + * Arguments : - > + * > + * Return Values : - > + * > + * Notes : None. > + > +********************************************************************* > +********/ static SaAisErrorT cpnd_clm_init(void) > +{ > + CPND_CB *cb = NULL; > + SaAisErrorT rc = SA_AIS_OK; > + SaVersionT clm_version; > + SaClmClusterNodeT cluster_node; > + SaClmHandleT clmHandle; > + m_CPSV_GET_AMF_VER(clm_version); > + SaClmCallbacksT gen_cbk; > + > + cb = m_CPND_TAKE_CPND_CB; > + > + TRACE_ENTER(); > + > + memset(&cluster_node, 0, sizeof(SaClmClusterNodeT)); > + > + gen_cbk.saClmClusterNodeGetCallback = NULL; > + gen_cbk.saClmClusterTrackCallback = cpnd_clm_cluster_track_cb; > + rc = saClmInitialize(&clmHandle, &gen_cbk, &clm_version); > + while (rc == SA_AIS_ERR_TRY_AGAIN || rc == SA_AIS_ERR_TIMEOUT || > + rc == SA_AIS_ERR_NO_RESOURCES || rc == SA_AIS_ERR_UNAVAILABLE) { > + if (rc != SA_AIS_ERR_TRY_AGAIN) { > + LOG_WA("cpnd_lib_init: saClmInitialize returned %u", rc); > + } > + osaf_nanosleep(&kHundredMilliseconds); > + rc = saClmInitialize(&clmHandle, &gen_cbk, &clm_version); > + } > + > + if (rc != SA_AIS_OK) { > + LOG_ER("cpnd clm init failed with return value:%d",rc); > + goto cpnd_clm_initialize_fail; > + } > + > + cb->clm_hdl = clmHandle; > + > + if (SA_AIS_OK != (rc = saClmSelectionObjectGet(cb->clm_hdl, &cb->clm_sel_obj))) { > + LOG_ER("cpnd clm selection object Get failed with return value:%d",rc); > + goto cpnd_clm_fail; > + } > + > + TRACE("cpnd clm selection object = %lld", cb->clm_sel_obj); > + > + rc = saClmClusterNodeGet(cb->clm_hdl, SA_CLM_LOCAL_NODE_ID, CPND_CLM_API_TIMEOUT, &cluster_node); > + if (rc != SA_AIS_OK) { > + LOG_ER("cpnd clm node get failed with return value:%d",rc); > + goto cpnd_clm_fail; > + } > + cb->nodeid = cluster_node.nodeId; > + > + rc = saClmClusterTrack(cb->clm_hdl, (SA_TRACK_CURRENT | SA_TRACK_CHANGES), NULL); > + if (rc != SA_AIS_OK) { > + LOG_ER("cpnd clm clusterTrack failed with return value:%d",rc); > + goto cpnd_clm_fail; > + } > + > + /* Notify main process to update clm select object */ > + ncs_sel_obj_ind(&cb->clm_updated_sel_obj); > + > + TRACE_LEAVE(); > + return rc; > + > + cpnd_clm_fail: > + saClmFinalize(cb->clm_hdl); > + > + cpnd_clm_initialize_fail: > + TRACE_LEAVE(); > + return rc; > +} > + > +/************************************************************************** ** > + * Name : cpnd_clm_init_thread > + * > + * Description : This function is thread function to initialize clm > + * > + * Arguments : - > + * > + * Return Values : - > + * > + * Notes : None. > + > +********************************************************************* > +********/ static void* cpnd_clm_init_thread(void* arg) { > + TRACE_ENTER(); > + > + SaAisErrorT rc = cpnd_clm_init(); > + if (rc != SA_AIS_OK) { > + exit(EXIT_FAILURE); > + } > + > + TRACE_LEAVE(); > + return NULL; > +} > + > +/************************************************************************** ** > + * Name : cpnd_start_clm_init_bg > + * > + * Description : This function is to start initialize clm thread > + * > + * Arguments : - > + * > + * Return Values : - > + * > + * Notes : None. > + > +********************************************************************* > +********/ static SaAisErrorT cpnd_start_clm_init_bg() { > + pthread_t thread; > + pthread_attr_t attr; > + pthread_attr_init(&attr); > + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); > + > + if (pthread_create(&thread, &attr, cpnd_clm_init_thread, NULL) != 0) { > + LOG_ER("pthread_create FAILED: %s", strerror(errno)); > + exit(EXIT_FAILURE); > + } > + > + pthread_attr_destroy(&attr); > + > + return SA_AIS_OK; > +} ------------------------------------------------------------------------------ Mobile security can be enabling, not merely restricting. Employees who bring their own devices (BYOD) to work are irked by the imposition of MDM restrictions. Mobile Device Manager Plus allows you to control only the apps on BYO-devices by containerizing them, leaving personal data untouched! https://ad.doubleclick.net/ddm/clk/304595813;131938128;j _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
