Re: [devel] [PATCH 1/1] dtm: configure trace file size and no of backups in transportd.conf [#2731]

2017-12-28 Thread Anders Widell 

Hi!

Here are my initial review comments:

* Avoid using preprocessor macros: TRANSPORTD_CONFIG_FILE should be a 
string constant. Also, according to naming conventions (Google C++ style 
guide) it should be named kTransportdConfigFile.


* LogServer::no_of_backups and LogServer:kmax_file_size should be 
instance variables (i.e. not declared static). Also, according to naming 
convention the names should end with an underscore character. Remove the 
initial "k" from kmax_file_size (since it is not a constant).


* Instead of using SIGUSR2 to trigger re-loading of the config file, you 
should add a new option to the osaflog command for this purpose. The 
osaflog command currently has a --flush option which sends a message to 
the osaftransportd service, and you can add a new similar option. 
Instead of re-reading the config file, the option could take the new log 
file size and number of backup files as arguments and send them in the 
message to the osaftransportd service. The options could be named 
--max-backups and --max-file-size. It should be possible to give either 
both of them in the same command, or just one of them (in which case the 
other setting is left unchanged).


* Please comment out (insert hash characters before) the two options in 
transportd.conf, since they have the default values.


regards,
Anders Widell

On 12/27/2017 12:25 PM, syam-talluri wrote:

---
  opensaf.spec.in|  2 +
  src/dtm/Makefile.am|  3 +
  src/dtm/transport/log_server.cc| 95 --
  src/dtm/transport/log_server.h | 10 +++-
  src/dtm/transport/log_writer.cc|  6 +-
  src/dtm/transport/log_writer.h |  4 +-
  src/dtm/transport/main.cc  |  4 ++
  src/dtm/transport/osaf-transport.in|  1 +
  src/dtm/transport/tests/log_writer_test.cc |  2 +-
  src/dtm/transport/transportd.conf  | 13 
  10 files changed, 129 insertions(+), 11 deletions(-)
  create mode 100644 src/dtm/transport/transportd.conf

diff --git a/opensaf.spec.in b/opensaf.spec.in
index db4b5be..452d1c8 100644
--- a/opensaf.spec.in
+++ b/opensaf.spec.in
@@ -1397,6 +1397,7 @@ fi
  %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.controller
  %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.payload
  %config(noreplace) %{_pkgsysconfdir}/dtmd.conf
+%config(noreplace) %{_pkgsysconfdir}/transportd.conf
  %{_pkglibdir}/osafrded
  %{_pkgclcclidir}/osaf-rded
  %{_pkglibdir}/osaffmd
@@ -1423,6 +1424,7 @@ fi
  %dir %{_pkgsysconfdir}
  %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.payload
  %config(noreplace) %{_pkgsysconfdir}/dtmd.conf
+%config(noreplace) %{_pkgsysconfdir}/transportd.conf
  %{_pkglibdir}/osafdtmd
  %{_pkglibdir}/osaftransportd
  %{_pkgclcclidir}/osaf-dtm
diff --git a/src/dtm/Makefile.am b/src/dtm/Makefile.am
index f3ba720..822249c 100644
--- a/src/dtm/Makefile.am
+++ b/src/dtm/Makefile.am
@@ -57,6 +57,9 @@ nodist_pkgclccli_SCRIPTS += \
  dist_pkgsysconf_DATA += \
src/dtm/dtmnd/dtmd.conf
  
+dist_pkgsysconf_DATA += \

+   src/dtm/transport/transportd.conf
+
  bin_osaftransportd_CXXFLAGS = $(AM_CXXFLAGS)
  
  bin_osaftransportd_CPPFLAGS = \

diff --git a/src/dtm/transport/log_server.cc b/src/dtm/transport/log_server.cc
index 2d6c961..780feb1 100644
--- a/src/dtm/transport/log_server.cc
+++ b/src/dtm/transport/log_server.cc
@@ -18,21 +18,28 @@
  
  #include "dtm/transport/log_server.h"

  #include 
+#include 
+#include 
  #include "base/osaf_poll.h"
  #include "base/time.h"
  #include "dtm/common/osaflog_protocol.h"
  #include "osaf/configmake.h"
  
+#define TRANSPORTD_CONFIG_FILE PKGSYSCONFDIR "/transportd.conf"

+
+size_t LogServer::no_of_backups = 9;
+size_t LogServer::kmax_file_size = 5000 * 1024;
+
  const Osaflog::ClientAddressConstantPrefix LogServer::address_header_{};
  
  LogServer::LogServer(int term_fd)

  : term_fd_{term_fd},
log_socket_{Osaflog::kServerSocketPath, base::UnixSocket::kNonblocking},
log_streams_{},
-  current_stream_{new LogStream{"mds.log", 1}},
+  current_stream_{new LogStream{"mds.log", 1, LogServer::kmax_file_size}},
no_of_log_streams_{1} {
log_streams_["mds.log"] = current_stream_;
-}
+  }
  
  LogServer::~LogServer() {

for (const auto& s : log_streams_) delete s.second;
@@ -40,6 +47,12 @@ LogServer::~LogServer() {
  
  void LogServer::Run() {

struct pollfd pfd[2] = {{term_fd_, POLLIN, 0}, {log_socket_.fd(), POLLIN, 
0}};
+
+  /* Initialize a signal handler for loading new configuration from 
transportd.conf */
+  if ((signal(SIGUSR2, usr2_sig_handler)) == SIG_ERR) {
+  syslog(LOG_ERR,"signal USR2 registration failed: %s", strerror(errno));
+  }
+
do {
  for (int i = 0; i < 256; ++i) {
char* buffer = current_stream_->current_buffer_position();
@@ -88,6 +101,12 @@ void LogServer::Run() {
} while ((pfd[0].revents & POLLIN) == 0);
  }
  
+void LogServer::usr2_sig_handler(int sig)

[devel] [PATCH 1/1] fmd: convert to C++ [#2750]

2017-12-28 Thread Gary Lee 
Source files renamed to .cc
Apply changes required to compile succesfully with g++
---
 src/fm/Makefile.am   |  8 
 src/fm/fmd/fm.h  |  2 +-
 src/fm/fmd/{fm_amf.c => fm_amf.cc}   |  6 +++---
 src/fm/fmd/fm_cb.h   |  4 ++--
 src/fm/fmd/{fm_main.c => fm_main.cc} | 21 ++---
 src/fm/fmd/{fm_mds.c => fm_mds.cc}   |  6 +++---
 src/fm/fmd/{fm_rda.c => fm_rda.cc}   |  0
 7 files changed, 23 insertions(+), 24 deletions(-)
 rename src/fm/fmd/{fm_amf.c => fm_amf.cc} (98%)
 rename src/fm/fmd/{fm_main.c => fm_main.cc} (97%)
 rename src/fm/fmd/{fm_mds.c => fm_mds.cc} (99%)
 rename src/fm/fmd/{fm_rda.c => fm_rda.cc} (100%)

diff --git a/src/fm/Makefile.am b/src/fm/Makefile.am
index ad666b905..d48a9146c 100644
--- a/src/fm/Makefile.am
+++ b/src/fm/Makefile.am
@@ -41,10 +41,10 @@ bin_osaffmd_CPPFLAGS = \
$(AM_CPPFLAGS)
 
 bin_osaffmd_SOURCES = \
-   src/fm/fmd/fm_amf.c \
-   src/fm/fmd/fm_main.c \
-   src/fm/fmd/fm_mds.c \
-   src/fm/fmd/fm_rda.c
+   src/fm/fmd/fm_amf.cc \
+   src/fm/fmd/fm_main.cc \
+   src/fm/fmd/fm_mds.cc \
+   src/fm/fmd/fm_rda.cc
 
 bin_osaffmd_LDADD = \
lib/libSaAmf.la \
diff --git a/src/fm/fmd/fm.h b/src/fm/fmd/fm.h
index 79734f241..ce71c4105 100644
--- a/src/fm/fmd/fm.h
+++ b/src/fm/fmd/fm.h
@@ -73,7 +73,7 @@
 #include "fm_evt.h"
 
 extern void amfnd_down_callback(void);
-extern void ava_install_amf_down_cb(void (*cb)(void));
+extern "C" void ava_install_amf_down_cb(void (*cb)(void));
 extern uint32_t initialize_for_assignment(FM_CB *cb, SaAmfHAStateT ha_state);
 extern void fm_tmr_stop(FM_TMR *tmr);
 #endif  // FM_FMD_FM_H_
diff --git a/src/fm/fmd/fm_amf.c b/src/fm/fmd/fm_amf.cc
similarity index 98%
rename from src/fm/fmd/fm_amf.c
rename to src/fm/fmd/fm_amf.cc
index 9e6eaf7de..5be2bf201 100644
--- a/src/fm/fmd/fm_amf.c
+++ b/src/fm/fmd/fm_amf.cc
@@ -34,14 +34,14 @@
 **/
 
 #include "fm.h"
-uint32_t gl_fm_hdl;
+extern uint32_t gl_fm_hdl;
 
 uint32_t fm_amf_init(FM_AMF_CB *fm_amf_cb);
 static uint32_t fm_amf_register(FM_AMF_CB *fm_amf_cb);
 static uint32_t fm_amf_healthcheck_start(FM_AMF_CB *fm_amf_cb);
 static FM_AMF_CB *fm_amf_take_hdl(void);
 static void fm_amf_give_hdl(void);
-static char *ha_role_string[] = {"ACTIVE", "STANDBY", "QUIESCED", "QUIESCING"};
+static const char *ha_role_string[] = {"ACTIVE", "STANDBY", "QUIESCED", 
"QUIESCING"};
 
 void amfnd_down_callback(void)
 {
@@ -79,7 +79,7 @@ FM_AMF_CB *fm_amf_take_hdl(void)
FM_CB *fm_cb = NULL;
 
/* Take handle */
-   fm_cb = ncshm_take_hdl(NCS_SERVICE_ID_GFM, gl_fm_hdl);
+   fm_cb = static_cast(ncshm_take_hdl(NCS_SERVICE_ID_GFM, 
gl_fm_hdl));
 
return _cb->fm_amf_cb;
 }
diff --git a/src/fm/fmd/fm_cb.h b/src/fm/fmd/fm_cb.h
index 248d70bd5..f8559b9c5 100644
--- a/src/fm/fmd/fm_cb.h
+++ b/src/fm/fmd/fm_cb.h
@@ -32,7 +32,7 @@
 #include 
 #include 
 
-uint32_t gl_fm_hdl;
+extern uint32_t gl_fm_hdl;
 
 typedef enum {
   FM_TMR_TYPE_MIN,
@@ -108,7 +108,7 @@ typedef struct fm_cb {
   bool peer_node_terminated;
 } FM_CB;
 
-extern char *role_string[];
+extern const char *role_string[];
 extern FM_CB *fm_cb;
 
 /*
diff --git a/src/fm/fmd/fm_main.c b/src/fm/fmd/fm_main.cc
similarity index 97%
rename from src/fm/fmd/fm_main.c
rename to src/fm/fmd/fm_main.cc
index 5b47efe96..db8395ee7 100644
--- a/src/fm/fmd/fm_main.c
+++ b/src/fm/fmd/fm_main.cc
@@ -42,8 +42,8 @@ static const SaClmCallbacksT_4 clm_callbacks = {0, 0};
 enum { FD_TERM = 0, FD_AMF = 1, FD_MBX };
 
 FM_CB *fm_cb = NULL;
-char *role_string[] = {"UNDEFINED", "ACTIVE", "STANDBY", "QUIESCED",
-  "QUIESCING"};
+const char *role_string[] = {"UNDEFINED", "ACTIVE", "STANDBY", "QUIESCED",
+ "QUIESCING"};
 
 /*
  *   *
@@ -97,7 +97,7 @@ void rda_cb(uint32_t cb_hdl, PCS_RDA_CB_INFO *cb_info,
 
TRACE_ENTER();
 
-   evt = calloc(1, sizeof(FM_EVT));
+   evt = static_cast(calloc(1, sizeof(FM_EVT)));
if (NULL == evt) {
LOG_ER("calloc failed");
goto done;
@@ -107,7 +107,7 @@ void rda_cb(uint32_t cb_hdl, PCS_RDA_CB_INFO *cb_info,
evt->info.rda_info.role = cb_info->info.io_role;
 
rc = ncs_ipc_send(_cb->mbx, (NCS_IPC_MSG *)evt,
- MDS_SEND_PRIORITY_HIGH);
+ NCS_IPC_PRIORITY_HIGH);
if (rc != NCSCC_RC_SUCCESS) {
syslog(LOG_ERR, "IPC send failed %d", rc);
free(evt);
@@ -378,7 +378,6 @@ static uint32_t fm_agents_startup(void)
}
 
return rc;
-   TRACE_LEAVE();
 }
 
 /
@@ -760,15 +759,15

[devel] [PATCH 0/1] Review Request for fmd: convert to C++ [#2750]

2017-12-28 Thread Gary Lee 
Summary: fmd: convert to C++ [#2750]
Review request for Ticket(s): 2750
Peer Reviewer(s): Anders, Ravi 
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2750
Base revision: 093ed05799aa58048dd3b8ef11f999823984d28e
Personal repository: git://git.code.sf.net/u/userid-2226215/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy 
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-

revision 04a68cb5b0cee402d0fac2bc052ec48cee6c99b2
Author: Gary Lee 
Date:   Thu, 28 Dec 2017 19:30:57 +1100

fmd: convert to C++ [#2750]

Source files renamed to .cc
Apply changes required to compile succesfully with g++



Complete diffstat:
--
 src/fm/Makefile.am   |  8 
 src/fm/fmd/fm.h  |  2 +-
 src/fm/fmd/{fm_amf.c => fm_amf.cc}   |  6 +++---
 src/fm/fmd/fm_cb.h   |  4 ++--
 src/fm/fmd/{fm_main.c => fm_main.cc} | 21 ++---
 src/fm/fmd/{fm_mds.c => fm_mds.cc}   |  6 +++---
 src/fm/fmd/{fm_rda.c => fm_rda.cc}   |  0
 7 files changed, 23 insertions(+), 24 deletions(-)


Testing Commands:
-
Compiles and passes legacy tests

Testing, Expected Results:
--
*** PASTE COMMAND OUTPUTS / TEST RESULTS ***


Conditions of Submission:
-
*** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC ***


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 y  n
x86_64  y  y 
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
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