Re: [devel] [PATCH 1 of 3] log: add alternative destinations of log records [#2258]

2017-02-21 Thread Lennart Lund
Hi Vu

Ack with commants
See comments below [Lennart]

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 21 februari 2017 10:34
> To: Lennart Lund ; mahesh.va...@oracle.com;
> Canh Van Truong 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 3] log: add alternative destinations of log records
> [#2258]
> 
>  src/log/Makefile.am  |8 +-
>  src/log/config/logsv_classes.xml |7 +-
>  src/log/logd/lgs_config.cc   |  169 -
>  src/log/logd/lgs_config.h|3 +-
>  src/log/logd/lgs_dest.cc |  707
> +++
>  src/log/logd/lgs_dest.h  |  576 +++
>  src/log/logd/lgs_evt.cc  |   33 +
>  src/log/logd/lgs_imm.cc  |  202 +-
>  src/log/logd/lgs_main.cc |8 +
>  src/log/logd/lgs_mbcsv.cc|  103 +-
>  src/log/logd/lgs_mbcsv.h |6 +-
>  src/log/logd/lgs_mbcsv_v5.cc |   10 +
>  src/log/logd/lgs_mbcsv_v7.cc |  177 +
>  src/log/logd/lgs_mbcsv_v7.h  |   67 +++
>  src/log/logd/lgs_stream.cc   |   60 +++-
>  src/log/logd/lgs_stream.h|   16 +
>  src/log/logd/lgs_util.cc |   63 +++
>  src/log/logd/lgs_util.h  |   11 +-
>  18 files changed, 2154 insertions(+), 72 deletions(-)
> 
> 
> Here are major info, detailed info will be added to PR doc soon.
> 1) Add attribute "saLogRecordDestination" to log stream.
> 2) Add Local socket destintion handler
> 3) Integrate into first increment made by Lennart
> 
> diff --git a/src/log/Makefile.am b/src/log/Makefile.am
> --- a/src/log/Makefile.am
> +++ b/src/log/Makefile.am
> @@ -72,9 +72,11 @@ noinst_HEADERS += \
>   src/log/logd/lgs_mbcsv_v2.h \
>   src/log/logd/lgs_mbcsv_v3.h \
>   src/log/logd/lgs_mbcsv_v5.h \
> + src/log/logd/lgs_mbcsv_v7.h \
>   src/log/logd/lgs_recov.h \
>   src/log/logd/lgs_stream.h \
> - src/log/logd/lgs_util.h
> + src/log/logd/lgs_util.h \
> + src/log/logd/lgs_dest.h
> 
>  bin_PROGRAMS += bin/saflogger
>  osaf_execbin_PROGRAMS += bin/osaflogd
> @@ -118,10 +120,12 @@ bin_osaflogd_SOURCES = \
>   src/log/logd/lgs_mbcsv_v2.cc \
>   src/log/logd/lgs_mbcsv_v3.cc \
>   src/log/logd/lgs_mbcsv_v5.cc \
> + src/log/logd/lgs_mbcsv_v7.cc \
>   src/log/logd/lgs_mds.cc \
>   src/log/logd/lgs_recov.cc \
>   src/log/logd/lgs_stream.cc \
> - src/log/logd/lgs_util.cc
> + src/log/logd/lgs_util.cc \
> + src/log/logd/lgs_dest.cc
> 
>  bin_osaflogd_LDADD = \
>   lib/libosaf_common.la \
> diff --git a/src/log/config/logsv_classes.xml
> b/src/log/config/logsv_classes.xml
> --- a/src/log/config/logsv_classes.xml
> +++ b/src/log/config/logsv_classes.xml
> @@ -147,12 +147,13 @@
>   SA_CONFIG
>   SA_WRITABLE
>   
> - 
> + 
>   saLogRecordDestination
> - SA_UINT32_T
> + SA_STRING_T
>   SA_CONFIG
>   SA_WRITABLE
> -SA_MULTI_VALUE
> + SA_MULTI_VALUE
> + SA_NO_DUPLICATES
>   
>   
>   saLogStreamCreationTimestamp
> diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
> --- a/src/log/logd/lgs_config.cc
> +++ b/src/log/logd/lgs_config.cc
> @@ -40,6 +40,7 @@
>  #include "osaf/immutil/immutil.h"
>  #include "log/logd/lgs_file.h"
>  #include "log/logd/lgs.h"
> +#include "log/logd/lgs_util.h"
[Lennart] This include should be removed, not needed
> 
>  static SaVersionT immVersion = { 'A', 2, 11 };
> 
> @@ -709,33 +710,162 @@ static int lgs_cfg_verify_log_filesys_co
>return rc;
>  }
> 
> +//>
> +// Utility functions to validate destination configuration format
> +//<
> +// Typedef for shorten declaration
> +// This type could be used to hold set of strings with formats
> +// 1) {"name;type;value", etc.}
> +// 2) {"name", "type", "value"}
> +// 3) etc.
> +using VectorString = std::vector;
> +//>
> +// Define the "token" possition in destination configuration.
> +// kName : use this index to get "name" token
> +// kType : use this index to get "type" token
> +// kValue: use this index to get "value" token
> +// kSize : this is the maximum size of the dest configuration vector.
> +//<
> +enum { kName = 0, kType, kValue, kSize };
> +// Tokens seperator
> +const char kSemicolon[] = ";";
> +
> +
> +// The format of destination must be one of followings:
> +// 1) "name;type;"
> +// 2) "name;type;value"
> +// So, in destination configuration, must have 02 semiconlons
> +// no more, no less.
> +//
> +bool is_right_destination_fmt(const VectorString& vdest) {
> +  int nl_cnt = 0;
> +  // Check each single destination
> +  for (const auto& it : vdest) {
> +nl_cnt = 

Re: [devel] [PATCH 1 of 3] log: add alternative destinations of log records [#2258]

2017-02-16 Thread Lennart Lund
Hi Vu

See comments inline tagged [Lennart]

General comments:
Update code and comments regarding changed delimiters in configuration string 
from '\n' to ';'

Fix incorrect changes and usage of the configuration handler

Regards
Lennart


> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 16 februari 2017 11:30
> To: Lennart Lund ; mahesh.va...@oracle.com;
> Canh Van Truong 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 3] log: add alternative destinations of log records
> [#2258]
> 
>  src/log/Makefile.am  |8 +-
>  src/log/config/logsv_classes.xml |7 +-
>  src/log/logd/lgs_config.cc   |  193 +++--
>  src/log/logd/lgs_config.h|3 +-
>  src/log/logd/lgs_dest.cc |  746
> +++
>  src/log/logd/lgs_dest.h  |  580 ++
>  src/log/logd/lgs_evt.cc  |   33 +
>  src/log/logd/lgs_imm.cc  |  140 ++-
>  src/log/logd/lgs_mbcsv.cc|   91 -
>  src/log/logd/lgs_mbcsv.h |6 +-
>  src/log/logd/lgs_mbcsv_v7.cc |  177 +
>  src/log/logd/lgs_mbcsv_v7.h  |   67 +++
>  src/log/logd/lgs_stream.cc   |   54 ++-
>  src/log/logd/lgs_stream.h|   16 +
>  14 files changed, 2041 insertions(+), 80 deletions(-)
> 
> 
> Here are major info, detailed info will be added to PR doc soon.
> 1) Add attribute "saLogRecordDestination" to log stream.
> 2) Add Local socket destintion handler
> 3) Integrate into first increment made by Lennart
> 
> diff --git a/src/log/Makefile.am b/src/log/Makefile.am
> --- a/src/log/Makefile.am
> +++ b/src/log/Makefile.am
> @@ -71,9 +71,11 @@ noinst_HEADERS += \
>   src/log/logd/lgs_mbcsv_v2.h \
>   src/log/logd/lgs_mbcsv_v3.h \
>   src/log/logd/lgs_mbcsv_v5.h \
> + src/log/logd/lgs_mbcsv_v7.h \
>   src/log/logd/lgs_recov.h \
>   src/log/logd/lgs_stream.h \
> - src/log/logd/lgs_util.h
> + src/log/logd/lgs_util.h \
> + src/log/logd/lgs_dest.h
> 
>  bin_PROGRAMS += bin/saflogger
>  osaf_execbin_PROGRAMS += bin/osaflogd
> @@ -117,10 +119,12 @@ bin_osaflogd_SOURCES = \
>   src/log/logd/lgs_mbcsv_v2.cc \
>   src/log/logd/lgs_mbcsv_v3.cc \
>   src/log/logd/lgs_mbcsv_v5.cc \
> + src/log/logd/lgs_mbcsv_v7.cc \
>   src/log/logd/lgs_mds.cc \
>   src/log/logd/lgs_recov.cc \
>   src/log/logd/lgs_stream.cc \
> - src/log/logd/lgs_util.cc
> + src/log/logd/lgs_util.cc \
> + src/log/logd/lgs_dest.cc
> 
>  bin_osaflogd_LDADD = \
>   lib/libosaf_common.la \
> diff --git a/src/log/config/logsv_classes.xml
> b/src/log/config/logsv_classes.xml
> --- a/src/log/config/logsv_classes.xml
> +++ b/src/log/config/logsv_classes.xml
> @@ -147,12 +147,13 @@
>   SA_CONFIG
>   SA_WRITABLE
>   
> - 
> + 
>   saLogRecordDestination
> - SA_UINT32_T
> + SA_STRING_T
>   SA_CONFIG
>   SA_WRITABLE
> -SA_MULTI_VALUE
> + SA_MULTI_VALUE
> + SA_NO_DUPLICATES
>   
>   
>   saLogStreamCreationTimestamp
> diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
> --- a/src/log/logd/lgs_config.cc
> +++ b/src/log/logd/lgs_config.cc
> @@ -40,6 +40,7 @@
>  #include "osaf/immutil/immutil.h"
>  #include "log/logd/lgs_file.h"
>  #include "log/logd/lgs.h"
> +#include "log/logd/lgs_dest.h"
> 
>  static SaVersionT immVersion = { 'A', 2, 11 };
> 
> @@ -117,10 +118,6 @@ typedef struct _lgs_conf_t {
>std::vector logRecordDestinationConfiguration; // Default
> empty
>/* --- end correspond to IMM Class --- */
> 
> -  /* --- Used with OpenSafLogCurrentConfig runtime object only --- */
> -  /* Note: Has no cnfflag */
> -  std::vector logRecordDestinationStatus; // Default empty
> -
>/* Used for checkpointing time when files are closed */
>time_t chkp_file_close_time;
> 
> @@ -254,6 +251,9 @@ void lgs_cfgupd_multival_add(const std::
>   const std::vector& value_list,
>   lgs_config_chg_t *config_data) {
>TRACE_ENTER();
> +
> +  CfgDestination(value_list, ModifyType::kAdd);
> +
>// Get the existing multi-values and add them to the config data list
>lgs_logconfGet_t param_id = param_name_to_id(attribute_name);
>const std::vector *exist_list =
> @@ -294,6 +294,9 @@ void lgs_cfgupd_multival_delete(std::str
>  std::vector value_list,
>  lgs_config_chg_t *config_data) {
>TRACE_ENTER();
> +
> +  CfgDestination(value_list, ModifyType::kDelete);
> +
>// Get the existing multi-values
>lgs_logconfGet_t param_id = param_name_to_id(attribute_name);
>const