Hi Danh,
Ack with minor comments.
My opinion is that, the newly added function `get_evt_data` should be used
only inside the immnd_evt.c (C code)
and should only be used when receiving fevs msg from IMMD regardless the
immnd_evt_proc_fevs_dispatch() returns OK or not.
With that, we should locate `get_evt_data` function internally inside
immnd_evt.c.
Regards, Vu
> -----Original Message-----
> From: Danh Vo <danh.c...@dektech.com.au>
> Sent: Monday, July 30, 2018 6:33 PM
> To: hans.nordeb...@ericsson.com; lennart.l...@ericsson.com;
> gary....@dektech.com.au; vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net; Danh Vo
> <danh.c...@dektech.com.au>
> Subject: [PATCH 1/1] imm: syslog recent fevs evts when immnd down [#2898]
>
> An enhancement for IMM to print out 5 last FEVS events into syslog when
> IMMND down for any reason.
> ---
> src/imm/common/immsv_evt.c | 2 +-
> src/imm/common/immsv_evt.h | 1 +
> src/imm/immnd/immnd_evt.c | 209
> ++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 208 insertions(+), 4 deletions(-)
>
> diff --git a/src/imm/common/immsv_evt.c b/src/imm/common/immsv_evt.c
> index b0e36d3..03a7f81 100644
> --- a/src/imm/common/immsv_evt.c
> +++ b/src/imm/common/immsv_evt.c
> @@ -189,7 +189,7 @@ static const char *immnd_evt_names[] = {
> "IMMND_EVT_D2ND_IMPLDELETE",
> "undefined (high)"};
>
> -static const char *immsv_get_immnd_evt_name(unsigned int id)
> +const char *immsv_get_immnd_evt_name(unsigned int id)
> {
> if (id < IMMND_EVT_MAX)
> return immnd_evt_names[id];
> diff --git a/src/imm/common/immsv_evt.h b/src/imm/common/immsv_evt.h
> index 3e96c98..156220d 100644
> --- a/src/imm/common/immsv_evt.h
> +++ b/src/imm/common/immsv_evt.h
> @@ -676,6 +676,7 @@ void
> immsv_evt_free_att_val(IMMSV_EDU_ATTR_VAL *v, SaImmValueTypeT t);
> void immsv_evt_free_att_val_raw(IMMSV_EDU_ATTR_VAL *v, long t);
> void immsv_free_attr_list_raw(IMMSV_EDU_ATTR_VAL_LIST *al, const long
> avt);
>
> +const char *immsv_get_immnd_evt_name(unsigned int id);
> void immsv_msg_trace_send(MDS_DEST to, IMMSV_EVT *evt);
> void immsv_msg_trace_rec(MDS_DEST from, IMMSV_EVT *evt);
>
> diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c
> index 730c490..e277fb5 100644
> --- a/src/imm/immnd/immnd_evt.c
> +++ b/src/imm/immnd/immnd_evt.c
> @@ -32,6 +32,7 @@
> #include "immnd.h"
> #include "imm/common/immsv_api.h"
> #include "base/ncssysf_mem.h"
> +#include "base/saf_error.h"
> #include "mds/mds_papi.h"
> #include "base/osaf_extended_name.h"
>
> @@ -39,6 +40,21 @@
> #define IMMND_SEARCH_BUNDLE_SIZE ((MDS_DIRECT_BUF_MAXSIZE /
> 100) * 90)
> #define IMMND_MAX_SEARCH_RESULT (IMMND_SEARCH_BUNDLE_SIZE /
> 300)
>
> +#define MAX_INDEX_FEVS_MSG 5
> +#define MAX_LENGTH_FEVS_MSG 256
> +
[Vu] Add a comment here to stress that this global array should be used only
to contain debugging data.
> +static char latest_fevs[MAX_INDEX_FEVS_MSG][MAX_LENGTH_FEVS_MSG]
> = {
> + "(none)", "(none)", "(none)", "(none)", "(none)"
> +};
> +
> +static void syslog_recent_fevs()
> +{
> + LOG_NO("Recent fevs");
> + for (int i = 0; i < MAX_INDEX_FEVS_MSG; i++) {
> + LOG_NO("%s", latest_fevs[i]);
> + }
> +}
> +
> static SaAisErrorT immnd_fevs_local_checks(IMMND_CB *cb, IMMSV_FEVS
> *fevsReq,
> const IMMSV_SEND_INFO *sinfo);
> static uint32_t immnd_evt_proc_cb_dump(IMMND_CB *cb);
> @@ -287,6 +303,8 @@ static uint32_t immnd_evt_proc_reset(IMMND_CB
> *cb, IMMND_EVT *evt,
>
> static uint32_t immnd_evt_impl_delete(IMMND_CB *cb, IMMND_EVT *evt);
>
> +static void get_evt_data(IMMND_CB *cb, IMMND_EVT *evt, char*
> evt_data);
> +
> #if 0 /* Only for debug */
> static void printImmValue(SaImmValueTypeT t, IMMSV_EDU_ATTR_VAL *v)
> {
> @@ -9601,7 +9619,8 @@ static uint32_t immnd_restricted_ok(IMMND_CB
> *cb, uint32_t id)
> static SaAisErrorT
> immnd_evt_proc_fevs_dispatch(IMMND_CB *cb, IMMSV_OCTET_STRING
> *msg,
> bool originatedAtThisNd, SaImmHandleT clnt_hdl,
> - MDS_DEST reply_dest, SaUint64T msgNo)
> + MDS_DEST reply_dest, SaUint64T msgNo,
> + char* evt_data)
> {
> SaAisErrorT error = SA_AIS_OK;
> IMMSV_EVT frwrd_evt;
> @@ -9665,7 +9684,7 @@ immnd_evt_proc_fevs_dispatch(IMMND_CB *cb,
> IMMSV_OCTET_STRING *msg,
>
> /*Dispatch the unpacked FEVS message */
> immsv_msg_trace_rec(frwrd_evt.sinfo.dest, &frwrd_evt);
> -
> + get_evt_data(immnd_cb, &frwrd_evt.info.immnd, evt_data);
> switch (frwrd_evt.info.immnd.type) {
> case IMMND_EVT_A2ND_OBJ_CREATE:
> case IMMND_EVT_A2ND_OBJ_CREATE_2:
> @@ -10735,7 +10754,11 @@ static uint32_t
> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> bool isObjSync = (evt->type ==
> IMMND_EVT_D2ND_GLOB_FEVS_REQ_2)
> ? evt->info.fevsReq.isObjSync
> : false;
> + static int index = 0;
> + char evt_data[MAX_LENGTH_FEVS_MSG];
> +
> TRACE_ENTER();
> + memset(evt_data, 0, sizeof(evt_data));
>
> if (cb->highestProcessed >= msgNo) {
> /*We have already received this message, discard it. */
> @@ -10784,6 +10807,7 @@ static uint32_t
> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> LOG_ER(
> "MESSAGE:%llu OUT OF ORDER my highest
> processed:%llu - exiting",
> msgNo, cb->highestProcessed);
> + syslog_recent_fevs();
> immnd_ackToNid(NCSCC_RC_FAILURE);
> exit(1);
> } else if (cb
> @@ -10792,6 +10816,7 @@ static uint32_t
> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> LOG_ER(
> "Sync MESSAGE:%llu OUT OF ORDER my highest
> processed:%llu - exiting",
> msgNo, cb->highestProcessed);
> + syslog_recent_fevs();
> immnd_ackToNid(NCSCC_RC_FAILURE);
> exit(1);
> } else if (cb->mState < IMM_SERVER_LOADING_PENDING) {
> @@ -10816,7 +10841,8 @@ static uint32_t
> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> TRACE("Coord discards object sync message");
> } else {
> err = immnd_evt_proc_fevs_dispatch(cb, msg,
> originatedAtThisNd,
> - clnt_hdl, reply_dest,
msgNo);
> + clnt_hdl, reply_dest,
> + msgNo, evt_data);
> }
>
> if (err != SA_AIS_OK) {
> @@ -10827,6 +10853,10 @@ static uint32_t
> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt,
> }
> }
>
> + snprintf(latest_fevs[index], MAX_LENGTH_FEVS_MSG,
> + "<%llu>[%s]", msgNo, evt_data);
> +
> + if (++index > MAX_INDEX_FEVS_MSG - 1) index = 0;
> done:
> cb->highestProcessed++;
> dequeue_outgoing(cb);
> @@ -10912,6 +10942,8 @@ static void
> immnd_evt_proc_discard_node(IMMND_CB *cb, IMMND_EVT *evt,
> cb->node_id);
> exit(1);
> }
> +
> + syslog_recent_fevs();
> LOG_NO("Global discard node received for nodeId:%x pid:%u",
> evt->info.ctrl.nodeId, evt->info.ctrl.ndExecPid);
> /* We should remember the nodeId/pid pair to avoid a redundant
> message
> @@ -12379,3 +12411,174 @@ static uint32_t
> immnd_evt_impl_delete(IMMND_CB *cb, IMMND_EVT *evt)
> failed:
> return rc;
> }
> +
> +
> +/*************************************************************
> *********
> + * An enhancement from ticket #2898 to get more info from events
[Vu] Should only describe what below method does.
> +
> **************************************************************
> ********/
> +static void get_evt_data(IMMND_CB *cb, IMMND_EVT *evt, char*
> evt_data)
[Vu] If the function says "get", the declaration should be: static const
char* get_evt_data(const IMMND_CB *cb, const IMMND_EVT *evt)
and using const for read-only parameters.
> +{
> + const char* evt_name = immsv_get_immnd_evt_name(evt->type);
> + char evt_info[MAX_LENGTH_FEVS_MSG];
> + int is_no_info = 0;
> +
> + switch (evt->type) {
> + case IMMND_EVT_A2ND_OBJ_CREATE:
> + case IMMND_EVT_A2ND_OBJ_CREATE_2:
> + case IMMND_EVT_A2ND_OI_OBJ_CREATE:
> + case IMMND_EVT_A2ND_OI_OBJ_CREATE_2:
> + snprintf(evt_info, sizeof(evt_info), "Create object of class %s",
> + (const char*)evt->info.objCreate.className.buf);
> + break;
> +
> + case IMMND_EVT_A2ND_OBJ_MODIFY:
> + case IMMND_EVT_A2ND_OI_OBJ_MODIFY:
> + snprintf(evt_info, sizeof(evt_info), "%s",
> + (const char*)evt->info.objModify.objectName.buf);
> + break;
> +
> + case IMMND_EVT_A2ND_OBJ_DELETE:
> + case IMMND_EVT_A2ND_OI_OBJ_DELETE:
> + snprintf(evt_info, sizeof(evt_info), "%s",
> + (const char*)evt->info.objDelete.objectName.buf);
> + break;
> +
> + case IMMND_EVT_A2ND_OBJ_SYNC:
> + case IMMND_EVT_A2ND_OBJ_SYNC_2:
> + snprintf(evt_info, sizeof(evt_info), "%s",
> + (const char*)evt->info.obj_sync.objectName.buf);
> + break;
> +
> + case IMMND_EVT_A2ND_IMM_ADMOP:
> + case IMMND_EVT_A2ND_IMM_ADMOP_ASYNC:
> + snprintf(evt_info, sizeof(evt_info), "%s(op_id:%llu)",
> + (const char*)evt->info.admOpReq.objectName.buf,
> + evt->info.admOpReq.operationId);
> + break;
> +
> + case IMMND_EVT_A2ND_CLASS_CREATE:
> + case IMMND_EVT_A2ND_CLASS_DELETE:
> + snprintf(evt_info, sizeof(evt_info), "%s",
> + (const char*)evt->info.classDescr.className.buf);
> + break;
> +
> + case IMMND_EVT_D2ND_DISCARD_IMPL:
> + case IMMND_EVT_A2ND_OI_IMPL_CLR:
> + case IMMND_EVT_D2ND_IMPLSET_RSP:
> + case IMMND_EVT_D2ND_IMPLSET_RSP_2:
> + snprintf(evt_info, sizeof(evt_info), "%s(%u)",
> + (const char*)evt->info.implSet.impl_name.buf,
> + evt->info.implSet.impl_id);
> + break;
> +
> + case IMMND_EVT_D2ND_ADMINIT:
> + snprintf(evt_info, sizeof(evt_info), "%s(%d)",
> + osaf_extended_name_borrow(
> + &(evt->info.adminitGlobal.i.adminOwnerName)),
> + evt->info.adminitGlobal.globalOwnerId);
> + break;
> +
> + case IMMND_EVT_D2ND_CCBINIT:
> + snprintf(evt_info, sizeof(evt_info), "ccb_id:%u",
> + evt->info.ccbinitGlobal.globalCcbId);
> + break;
> +
> + case IMMND_EVT_A2ND_AUG_ADMO:
> + snprintf(evt_info, sizeof(evt_info), "Add admo_id:%u to ccb_id:%u",
> + evt->info.objDelete.adminOwnerId, evt->info.objDelete.ccbId);
> + break;
> +
> + case IMMND_EVT_A2ND_OI_CL_IMPL_SET:
> + case IMMND_EVT_A2ND_OI_OBJ_IMPL_SET:
> + snprintf(evt_info, sizeof(evt_info), "Set impl_id:%u to %s",
> + evt->info.implSet.impl_id,
> + (const char*)evt->info.implSet.impl_name.buf);
> + break;
> +
> + case IMMND_EVT_A2ND_OI_CL_IMPL_REL:
> + case IMMND_EVT_A2ND_OI_OBJ_IMPL_REL:
> + snprintf(evt_info, sizeof(evt_info), "Release impl_id:%u out of
%s",
> + evt->info.implSet.impl_id,
> + (const char*)evt->info.implSet.impl_name.buf);
> + break;
> +
> + case IMMND_EVT_A2ND_ADMO_FINALIZE:
> + case IMMND_EVT_D2ND_ADMO_HARD_FINALIZE:
> + case IMMND_EVT_A2ND_ADMO_SET:
> + case IMMND_EVT_A2ND_ADMO_RELEASE:
> + case IMMND_EVT_A2ND_ADMO_CLEAR:
> + snprintf(evt_info, sizeof(evt_info), "admo_id:%u",
> + evt->info.admFinReq.adm_owner_id);
> + break;
> +
> + case IMMND_EVT_A2ND_OI_CCB_AUG_INIT:
> + snprintf(evt_info, sizeof(evt_info), "ccb_id:%u",
> + evt->info.ccbUpcallRsp.ccbId);
> + break;
> +
> + case IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP:
> + case IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP_2:
> + case IMMND_EVT_A2ND_CCB_OBJ_MODIFY_RSP:
> + case IMMND_EVT_A2ND_CCB_OBJ_MODIFY_RSP_2:
> + case IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP:
> + case IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2:
> + case IMMND_EVT_A2ND_CCB_COMPLETED_RSP:
> + case IMMND_EVT_A2ND_CCB_COMPLETED_RSP_2:
> + snprintf(evt_info, sizeof(evt_info), "ccb_id:%u, rc:%s",
> + evt->info.ccbUpcallRsp.ccbId,
> + saf_error(evt->info.ccbUpcallRsp.result));
> + break;
> +
> + case IMMND_EVT_A2ND_CCB_APPLY:
> + case IMMND_EVT_A2ND_CCB_VALIDATE:
> + case IMMND_EVT_A2ND_CCB_FINALIZE:
> + case IMMND_EVT_D2ND_ABORT_CCB:
> + snprintf(evt_info, sizeof(evt_info), "ccb_id:%u",
> + evt->info.ccbId);
> + break;
> +
> + case IMMND_EVT_A2ND_PBE_ADMOP_RSP:
> + snprintf(evt_info, sizeof(evt_info), "rc:%s",
> + saf_error(evt->info.admOpRsp.result));
> + break;
> +
> + case IMMND_EVT_A2ND_PBE_PRT_OBJ_CREATE_RSP:
> + case IMMND_EVT_A2ND_PBE_PRTO_DELETES_COMPLETED_RSP:
> + case IMMND_EVT_A2ND_PBE_PRT_ATTR_UPDATE_RSP:
> + snprintf(evt_info, sizeof(evt_info), "rc:%s",
> + saf_error(evt->info.ccbUpcallRsp.result));
> + break;
> +
> + case IMMND_EVT_D2ND_SYNC_FEVS_BASE:
> + snprintf(evt_info, sizeof(evt_info), "sync fevs base: %llu",
> + evt->info.syncFevsBase);
> + break;
> +
> + case IMMND_EVT_A2ND_OBJ_SAFE_READ:
> + snprintf(evt_info, sizeof(evt_info), "ccb_id:%u locks %s",
> + evt->info.searchInit.ccbId,
> + (const char*)evt->info.searchInit.rootName.buf);
> + break;
> +
> + case IMMND_EVT_D2ND_IMPLDELETE:
> + snprintf(evt_info, sizeof(evt_info), "Delete %u implementer(s)",
> + evt->info.impl_delete.size);
> + break;
> +
> + case IMMND_EVT_D2ND_DISCARD_NODE:
> + snprintf(evt_info, sizeof(evt_info), "node_id:%u",
> + evt->info.ctrl.nodeId);
> + break;
> +
> + default:
> + is_no_info = 1;
> + break;
> + }
> +
> + if (is_no_info){
[Vu] Add a space before the `{`.
> + snprintf(evt_data, MAX_LENGTH_FEVS_MSG, "%s", evt_name);
> + } else {
> + snprintf(evt_data, MAX_LENGTH_FEVS_MSG, "%s -> %s",
> + evt_name, (const char*) evt_info);
[Vu] Data casting here is needed?
> + }
> +}
> --
> 2.7.4
------------------------------------------------------------------------------
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
