Hi Anders Thanks, please see [GL].
On 08/02/18 02:06, Anders Widell wrote:
Ack with minor comments inline, marked AndersW>. regards, Anders Widell On 02/06/2018 03:43 AM, Gary Lee wrote:---src/fm/fmd/fm_cb.h | 89 ++++++++++++++++++++++++++-------------------------src/fm/fmd/fm_main.cc | 32 +++++++++--------- src/fm/fmd/fm_mds.cc | 27 ++++++++-------- src/fm/fmd/fm_mds.h | 2 ++ 4 files changed, 79 insertions(+), 71 deletions(-) diff --git a/src/fm/fmd/fm_cb.h b/src/fm/fmd/fm_cb.h index ffb3d8478..cfa50d883 100644 --- a/src/fm/fmd/fm_cb.h +++ b/src/fm/fmd/fm_cb.h @@ -18,19 +18,20 @@ #ifndef FM_FMD_FM_CB_H_ #define FM_FMD_FM_CB_H_ -#include <saAmf.h> -#include <saClm.h> #include <stdbool.h> #include <stdint.h> +#include <time.h> +#include <atomic> +#include <string> +#include <saAmf.h> +#include <saClm.h> +#include "base/mutex.h" #include "base/ncssysf_ipc.h" #include "base/ncssysf_tmr.h" -#include "fm_amf.h" +#include "fm/fmd/fm_amf.h" #include "mds/mds_papi.h" #include "rde/agent/rda_papi.h" -#include <stdbool.h> -#include <stdint.h> -#include <time.h> extern uint32_t gl_fm_hdl; @@ -55,58 +56,60 @@ typedef struct fm_tmr { } FM_TMR; /* FM control block */ -typedef struct fm_cb { - uint32_t cb_hdl; - SYSF_MBX mbx; +struct FM_CB { + uint32_t cb_hdl{}; + SYSF_MBX mbx{}; /* FM AMF CB */ - FM_AMF_CB fm_amf_cb; - NODE_ID node_id; - SaNameT node_name; + FM_AMF_CB fm_amf_cb{}; + std::atomic<NODE_ID> node_id{}; + SaNameT node_name{}; - SaNameT peer_node_name; - NODE_ID peer_node_id; - MDS_DEST peer_adest; /* will be 0 if peer is not up */ + std::string peer_node_name{}; + std::atomic<NODE_ID> peer_node_id{}; + std::atomic<MDS_DEST> peer_adest{}; /* will be 0 if peer is not up */ /* Holds own role. */ - PCS_RDA_ROLE role; + PCS_RDA_ROLE role{}; /* AMF HA state for FM */ - SaAmfHAStateT amf_state; + SaAmfHAStateT amf_state{}; /* MDS handles. */ - MDS_DEST adest; - MDS_HDL adest_hdl; - MDS_HDL adest_pwe1_hdl; + MDS_DEST adest{}; + MDS_HDL adest_hdl{}; + MDS_HDL adest_pwe1_hdl{}; /* Timers */ - FM_TMR promote_active_tmr; - FM_TMR activation_supervision_tmr; + FM_TMR promote_active_tmr{}; + FM_TMR activation_supervision_tmr{}; /* Time in terms of one hundredth of seconds (500 for 5 secs.) */ - uint32_t active_promote_tmr_val; - uint32_t activation_supervision_tmr_val; - bool fully_initialized; - bool csi_assigned; + uint32_t active_promote_tmr_val{}; + uint32_t activation_supervision_tmr_val{}; + bool fully_initialized{false}; + bool csi_assigned{false}; /* Variable to indicate OpenSAF control of TIPC transport */ - bool control_tipc; + bool control_tipc{true}; /* Booleans to mark service down events of critical Osaf Services */ - bool immd_down; - bool immnd_down; - bool amfnd_down; - bool amfd_down; - bool fm_down; - - bool peer_sc_up; - bool well_connected; - uint64_t cluster_size; - struct timespec last_well_connected; - struct timespec node_isolation_timeout; - SaClmHandleT clm_hdl; - bool use_remote_fencing; - SaNameT peer_clm_node_name; - bool peer_node_terminated; -} FM_CB; + bool immd_down{true}; + bool immnd_down{true}; + std::atomic<bool> amfnd_down{true}; + bool amfd_down{true}; + bool fm_down{false}; + + std::atomic<bool> peer_sc_up{false}; + bool well_connected{false}; + uint64_t cluster_size{}; + struct timespec last_well_connected{}; + struct timespec node_isolation_timeout{}; + SaClmHandleT clm_hdl{}; + bool use_remote_fencing{false}; + SaNameT peer_clm_node_name{}; + std::atomic<bool> peer_node_terminated{false}; + + base::Mutex mutex_{}; +}; extern const char *role_string[]; extern FM_CB *fm_cb; diff --git a/src/fm/fmd/fm_main.cc b/src/fm/fmd/fm_main.cc index 693bf9438..d8adcb583 100644 --- a/src/fm/fmd/fm_main.cc +++ b/src/fm/fmd/fm_main.cc @@ -30,7 +30,7 @@ This file contains the main() routine for FM. #include "base/osaf_extended_name.h" #include "base/osaf_poll.h" #include "base/osaf_time.h" -#include "fm.h" +#include "fm/fmd/fm.h" #include "nid/agent/nid_api.h" #include "osaf/configmake.h" #include "osaf/consensus/consensus.h"@@ -107,7 +107,7 @@ void rda_cb(uint32_t cb_hdl, PCS_RDA_CB_INFO *cb_info, rc = ncs_ipc_send(&fm_cb->mbx, (NCS_IPC_MSG *)evt, NCS_IPC_PRIORITY_HIGH);if (rc != NCSCC_RC_SUCCESS) { - syslog(LOG_ERR, "IPC send failed %d", rc); + syslog(LOG_ERR, "IPC send failed %u", rc); free(evt); } @@ -150,11 +150,8 @@ int main(int argc, char *argv[]) { goto fm_init_failed; } - memset(fm_cb, 0, sizeof(FM_CB)); fm_cb->fm_amf_cb.nid_started = nid_started; fm_cb->fm_amf_cb.amf_fd = -1; - fm_cb->fully_initialized = false; - fm_cb->csi_assigned = false;/* Variable to control whether FM will trigger failover immediately* upon recieving down event of critical services or will wait @@ -169,11 +166,6 @@ int main(int argc, char *argv[]) { */ fm_cb->control_tipc = true; /* Default behaviour */AndersW> control_tipc is already initialized, so no need to do it here again?
[GL] I kept it there basically as a placeholder for the comment block above the line.
- fm_cb->immd_down = true; - fm_cb->immnd_down = true; - fm_cb->amfnd_down = true; - fm_cb->amfd_down = true; - /* Create CB handle */gl_fm_hdl = ncshm_create_hdl(NCS_HM_POOL_ID_COMMON, NCS_SERVICE_ID_GFM,(NCSCONTEXT)fm_cb); @@ -395,8 +387,9 @@ static uint32_t fm_get_args(FM_CB *fm_cb) { fm_cb->node_name.length = strlen(value); memcpy(fm_cb->node_name.value, value, fm_cb->node_name.length); LOG_NO("EE_ID : %s", fm_cb->node_name.value); - } else + } else { fm_cb->node_name.length = 0; + } /* Update fm_cb configuration fields */ fm_cb->node_id = m_NCS_GET_NODE_ID;@@ -587,8 +580,12 @@ static void fm_mbx_msg_handler(FM_CB *fm_cb, FM_EVT *fm_mbx_evt) {fm_cb->peer_clm_node_name.value); } } else { + std::string peer_node_name; + fm_cb->mutex_.Lock(); + peer_node_name = fm_cb->peer_node_name; + fm_cb->mutex_.Unlock(); opensaf_reboot(fm_cb->peer_node_id, - (char *)fm_cb->peer_node_name.value, + peer_node_name.c_str(), "Received Node Down for peer controller"); } if (!((fm_cb->role == PCS_RDA_ACTIVE) &&@@ -610,7 +607,7 @@ static void fm_mbx_msg_handler(FM_CB *fm_cb, FM_EVT *fm_mbx_evt) {LOG_WA("Two active controllers observed in a cluster, newActive: %x and ""old-Active: %x", - fm_cb->node_id, fm_cb->peer_node_id); + unsigned(fm_cb->node_id), unsigned(fm_cb->peer_node_id)); opensaf_reboot(0, NULL,"Received svc up from peer node (old-active is not ""fully DOWN), hence rebooting the new Active");@@ -644,7 +641,8 @@ static void fm_mbx_msg_handler(FM_CB *fm_cb, FM_EVT *fm_mbx_evt) {/* Now. Try resetting other blade */ fm_cb->role = PCS_RDA_ACTIVE;- LOG_NO("Reseting peer controller node id: %x", fm_cb->peer_node_id);+ LOG_NO("Reseting peer controller node id: %x", + unsigned(fm_cb->peer_node_id)); if (fm_cb->use_remote_fencing) { if (fm_cb->peer_node_terminated == false) { opensaf_reboot(fm_cb->peer_node_id,@@ -655,8 +653,12 @@ static void fm_mbx_msg_handler(FM_CB *fm_cb, FM_EVT *fm_mbx_evt) {fm_cb->peer_clm_node_name.value); } } else { + std::string peer_node_name; + fm_cb->mutex_.Lock(); + peer_node_name = fm_cb->peer_node_name; + fm_cb->mutex_.Unlock(); opensaf_reboot(fm_cb->peer_node_id, - (char *)fm_cb->peer_node_name.value, + peer_node_name.c_str(), "Received Node Down for Active peer"); } fm_rda_set_role(fm_cb, PCS_RDA_ACTIVE); diff --git a/src/fm/fmd/fm_mds.cc b/src/fm/fmd/fm_mds.cc index fce6e6789..d97d715f4 100644 --- a/src/fm/fmd/fm_mds.cc +++ b/src/fm/fmd/fm_mds.cc @@ -16,9 +16,11 @@ * */ +#include "fm/fmd/fm_mds.h" +#include <string> #include "base/ncssysf_def.h" +#include "base/osaf_extended_name.h" #include "base/osaf_time.h" -#include "fm.h"const MDS_CLIENT_MSG_FORMAT_VER fm_fm_msg_fmt_map_table[FM_SUBPART_VER_MAX] = {FM_FM_MSG_FMT_VER_1};@@ -290,23 +292,23 @@ void fm_proc_svc_down(FM_CB *cb, uint32_t node_id, NCSMDS_SVC_ID svc_id) {switch (svc_id) { case NCSMDS_SVC_ID_IMMND: cb->immnd_down = true; - LOG_NO("IMMND down on: %x", cb->peer_node_id); + LOG_NO("IMMND down on: %x", unsigned(cb->peer_node_id)); break; case NCSMDS_SVC_ID_AVND: cb->amfnd_down = true; - LOG_NO("AMFND down on: %x", cb->peer_node_id); + LOG_NO("AMFND down on: %x", unsigned(cb->peer_node_id));; break; case NCSMDS_SVC_ID_IMMD: cb->immd_down = true; - LOG_NO("IMMD down on: %x", cb->peer_node_id); + LOG_NO("IMMD down on: %x", unsigned(cb->peer_node_id)); break; case NCSMDS_SVC_ID_AVD: cb->amfd_down = true; - LOG_NO("AVD down on: %x", cb->peer_node_id); + LOG_NO("AVD down on: %x", unsigned(cb->peer_node_id)); break; case NCSMDS_SVC_ID_GFM: cb->fm_down = true; - LOG_NO("FM down on: %x", cb->peer_node_id); + LOG_NO("FM down on: %x", unsigned(cb->peer_node_id)); break; default: break;@@ -631,13 +633,12 @@ static uint32_t fm_mds_rcv_evt(FM_CB *cb, MDS_CALLBACK_RECEIVE_INFO *rcv_info) {case GFM_GFM_EVT_NODE_INFO_EXCHANGE: cb->peer_node_id = gfm_rcv_msg->info.node_info.node_id; - cb->peer_node_name.length = - gfm_rcv_msg->info.node_info.node_name.length; - memcpy(cb->peer_node_name.value, - gfm_rcv_msg->info.node_info.node_name.value, - cb->peer_node_name.length); - LOG_IN("Peer Node_id %u : EE_ID %s", cb->peer_node_id, - cb->peer_node_name.value); + cb->mutex_.Lock(); + cb->peer_node_name = std::string( + osaf_extended_name_borrow(&gfm_rcv_msg->info.node_info.node_name)); + cb->mutex_.Unlock();+ LOG_IN("Peer Node_id %u : EE_ID %s", unsigned(cb->peer_node_id),+ cb->peer_node_name.c_str());AndersW> Don't access cb->peer_node_name outside of the critical section. Better to use osaf_extended_name_borrow(&gfm_rcv_msg->info.node_info.node_name)) here (you can save the pointer and use it twice).
[GL] OK, will change. But I guess it was fine anyway because only the MDS thread writes to cb->peer_node_name.
break; case GFM_GFM_EVT_PEER_IS_TERMINATING: fm_cb->peer_node_terminated = true; diff --git a/src/fm/fmd/fm_mds.h b/src/fm/fmd/fm_mds.h index ccdf30060..d984c4f59 100644 --- a/src/fm/fmd/fm_mds.h +++ b/src/fm/fmd/fm_mds.h @@ -15,6 +15,8 @@ * */ +#include "fm/fmd/fm.h" + #ifndef FM_FMD_FM_MDS_H_ #define FM_FMD_FM_MDS_H_
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
