Re: [devel] [PATCH 1/1] dtm: configure trace file size and no of backups in transportd.conf [#2731]
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] dtm: configure trace file size and no of backups in transportd.conf [#2731]
---
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) {
+ syslog(LOG_ERR, "Recived the SIGUSR2 Signal");
+ ReadConfig(TRANSPORTD_CONFIG_FILE);
+ signal(SIGUSR2, usr2_sig_handler);
+}
+
LogServer::LogStream* LogServer::GetStream(const char* msg_id,
size_t msg_id_size) {
if (msg_id_size == current_stream_->log_name_size() &&
@@ -99,7 +118,8 @@ LogServer::LogStream* LogServer::GetStream(const char*
msg_id,
if (iter != log_streams_.end()) return iter->second;
if (no_of_log_streams_ >= kMaxNoOfStreams) return nullptr;
if (!ValidateLogName(msg_id, msg_id_size)) return nullptr;
- LogStream* stream = new LogStream{log_name, 9};
+
+ LogStream* stream = new LogStream{log_name, LogServer::no_of_backups,
LogServer::kmax_file_size};
auto result = log_streams_.insert(
std::map::value_type{log_name, stream});
if (!result.second) osaf_abort(msg_id_size);
@@ -107,6 +127,71 @@ LogServer::LogStream* LogServer::GetStream(const char*
msg_id,
return stream;
}
+bool LogServer::ReadConfig(const char *transport_config_file) {
+ FILE *transport_conf_file;
+ char line[256];
+ size_t maxFileSize=0;
+ size_t noOfBackupFiles=0;
+ int i, n, comment_line, tag_len = 0;
+
+ /* Open transportd.conf config file. */
+ transport_conf_file = fopen(transport_config_file, "r");
+ if (transport_conf_file == nullptr) {
+
+syslog(LOG_ERR,"Not able to read transportd.conf: %s", strerror(errno));
+return
