Hi Danh,

Ack with minor comments.

Regards, Vu

> -----Original Message-----
> From: Danh Vo <danh.c...@dektech.com.au>
> Sent: Wednesday, August 1, 2018 3:29 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/Makefile.am          |   4 +-
>  src/imm/common/immsv_evt.c   |   2 +-
>  src/imm/common/immsv_evt.h   |   1 +
>  src/imm/immnd/immnd_evt.c    |   7 ++
>  src/imm/immnd/immnd_utils.cc | 201
> +++++++++++++++++++++++++++++++++++++++++++
>  src/imm/immnd/immnd_utils.h  |  36 ++++++++
>  6 files changed, 249 insertions(+), 2 deletions(-)
>  create mode 100644 src/imm/immnd/immnd_utils.cc
>  create mode 100644 src/imm/immnd/immnd_utils.h
> 
> diff --git a/src/imm/Makefile.am b/src/imm/Makefile.am
> index 2240d30..e83ba3a 100644
> --- a/src/imm/Makefile.am
> +++ b/src/imm/Makefile.am
> @@ -166,7 +166,8 @@ noinst_HEADERS += \
>       src/imm/immnd/immnd_cb.h \
>       src/imm/immnd/immnd_init.h \
>       src/imm/immpbed/immpbe.h \
> -     src/imm/tools/imm_dumper.h
> +     src/imm/tools/imm_dumper.h \
> +     src/imm/immnd/immnd_utils.h
> 
>  bin_PROGRAMS += bin/immadm bin/immcfg bin/immdump bin/immfind
> bin/immlist
>  osaf_execbin_PROGRAMS += bin/osafimmd bin/osafimmloadd
> bin/osafimmnd bin/osafimmpbed
> @@ -344,6 +345,7 @@ bin_osafimmnd_SOURCES = \
>       src/imm/immnd/immnd_mds.c \
>       src/imm/immnd/immnd_proc.c \
>       src/imm/immnd/immnd_clm.c \
> +     src/imm/immnd/immnd_utils.cc \
>       src/imm/immnd/ImmAttrValue.cc \
>       src/imm/immnd/ImmSearchOp.cc \
>       src/imm/immnd/ImmModel.cc
> 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..5241607 100644
> --- a/src/imm/immnd/immnd_evt.c
> +++ b/src/imm/immnd/immnd_evt.c
> @@ -30,6 +30,7 @@
>  #include <pwd.h>
>  #include "base/osaf_secutil.h"
>  #include "immnd.h"
> +#include "immnd_utils.h"
>  #include "imm/common/immsv_api.h"
>  #include "base/ncssysf_mem.h"
>  #include "mds/mds_papi.h"
> @@ -9665,6 +9666,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);
> +     CollectEvtData(&frwrd_evt.info.immnd, msgNo);
> 
>       switch (frwrd_evt.info.immnd.type) {
>       case IMMND_EVT_A2ND_OBJ_CREATE:
> @@ -10735,6 +10737,7 @@ 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;
> +
[Vu] Remove this change.

>       TRACE_ENTER();
> 
>       if (cb->highestProcessed >= msgNo) {
> @@ -10784,6 +10787,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);
> +                     SyslogRecentFevs();
>                       immnd_ackToNid(NCSCC_RC_FAILURE);
>                       exit(1);
>               } else if (cb
> @@ -10792,6 +10796,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);
> +                     SyslogRecentFevs();
>                       immnd_ackToNid(NCSCC_RC_FAILURE);
>                       exit(1);
>               } else if (cb->mState < IMM_SERVER_LOADING_PENDING) {
> @@ -10912,6 +10917,8 @@ static void
> immnd_evt_proc_discard_node(IMMND_CB *cb, IMMND_EVT *evt,
>                      cb->node_id);
>               exit(1);
>       }
> +
> +     SyslogRecentFevs();
>       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
> diff --git a/src/imm/immnd/immnd_utils.cc
> b/src/imm/immnd/immnd_utils.cc
> new file mode 100644
> index 0000000..579fdfa
> --- /dev/null
> +++ b/src/imm/immnd/immnd_utils.cc
> @@ -0,0 +1,201 @@
> +/*      -*- OpenSAF  -*-
> + *
> + * Copyright Ericsson AB 2018 - All Rights Reserved.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> licensed
> + * under the GNU Lesser General Public License Version 2.1, February
1999.
> + * The complete license can be accessed from the following location:
> + * http://opensource.org/licenses/lgpl-license.php
> + * See the Copying file included with the OpenSAF distribution for full
> + * licensing terms.
> + *
> + */
> +
> +#include "imm/immnd/immnd_utils.h"
> +#include "base/saf_error.h"
> +#include "base/osaf_extended_name.h"
> +
> +
[Vu] Use <cstdio>
> +#include <string>
> +#include <vector>

[Vu] The order of includes should be:
#include "imm/immnd/immnd_utils.h"

#include <stdio.h>
#include <string>
#include <vector>

> +#include "base/saf_error.h"
> +#include "base/osaf_extended_name.h"

Refer to
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Include
s


> +
> +static std::vector<std::string> latest_fevs;
[Vu] static std::vector<std::string> latest_fevs(MAX_NUMBER_FEVS_MSG);
> +
> +void SyslogRecentFevs()
> +{
> +  LOG_NO("Recent fevs");
> +  std::vector<std::string>::iterator it;
> +  for (it = latest_fevs.begin(); it != latest_fevs.end(); ++it) {
> +    LOG_NO("%s", (*it).c_str());
[Vu] Change above lines to:
for (const auto& i : latest_fevs) {
LOG_NO("%s", i.c_str());
}
> +  }
> +}
> +
> +void CollectEvtData(const IMMND_EVT *evt, const SaUint64T msg_no)
> +{
> +  char evt_data[MAX_LENGTH_FEVS_MSG];
> +  char evt_info[MAX_LENGTH_FEVS_MSG] = "(none)";
> +  const char* evt_name = immsv_get_immnd_evt_name(evt->type);
> +
> +  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), "%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:%x",
> +          evt->info.ctrl.nodeId);
> +      break;
> +
> +    default:
> +      break;
> +  }
> +
> +  snprintf(evt_data, MAX_LENGTH_FEVS_MSG,
> +      "<%llu>[%s -> %s]", msg_no, evt_name, (const char*) evt_info);
> +
> +  if(latest_fevs.size() >= MAX_NUMBER_FEVS_MSG) {
> +    latest_fevs.erase(latest_fevs.begin());
> +  }
> +
> +  latest_fevs.push_back(std::string((const char *)evt_data));
[Vu] using push_back/erase() may be not a good choice for the fixed vector.
Consider using:
static int index = 0;
latest_fevs[index] = std::string{evt_info};

if (++index > MAX_NUMBER_FEVS_MSG - 1) index = 0;

> +}
> \ No newline at end of file
> diff --git a/src/imm/immnd/immnd_utils.h b/src/imm/immnd/immnd_utils.h
> new file mode 100644
> index 0000000..99f892f
> --- /dev/null
> +++ b/src/imm/immnd/immnd_utils.h
> @@ -0,0 +1,36 @@
> +/*      -*- OpenSAF  -*-
> + *
> + * Copyright Ericsson AB 2018 - All Rights Reserved.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> licensed
> + * under the GNU Lesser General Public License Version 2.1, February
1999.
> + * The complete license can be accessed from the following location:
> + * http://opensource.org/licenses/lgpl-license.php
> + * See the Copying file included with the OpenSAF distribution for full
> + * licensing terms.
> + *
> + */
> +
> +#ifndef IMMND_UTILS_H_
> +#define IMMND_UTILS_H_
[Vu] The macro should be: IMM_IMMND_IMMND_UTILS_H_
> +
> +#include "imm/immnd/immnd.h"
> +#include <saAis.h>
> +
> +#define MAX_NUMBER_FEVS_MSG 5   // Number of printed last FEVS when
> IMMND down
> +#define MAX_LENGTH_FEVS_MSG 256 // Max length of printed FEVS info
[Vu] If these macros are only used in immnd_utils.cc, move them to the
source file immnd_utils.cc

> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +void SyslogRecentFevs();
> +void CollectEvtData(const IMMND_EVT *evt, const SaUint64T msg_no);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* IMMND_UTILS_H_ */
[Vu] the right format should be:
#endif  // IMM_IMMND_IMMND_UTILS_H_

> \ No newline at end of file
> --
> 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

Reply via email to