Hi!
Ack with comments:
1) The log file size is currently rounded up to the nearest 128KB. If
this is to be considered a feature it ought to be documented, but I
think you should instead fix it. The following patch will solve the problem:
diff --git a/src/dtm/transport/log_writer.cc
b/src/dtm/transport/log_writer.cc
index 60c0bf557..94c3b3a83 100644
--- a/src/dtm/transport/log_writer.cc
+++ b/src/dtm/transport/log_writer.cc
@@ -89,7 +89,8 @@ void LogWriter::RotateLog() {
void LogWriter::Write(size_t size) {
current_buffer_size_ += size;
- ess if (current_buffer_size_ > kBufferSize - kMaxMessageSize) Flush();
+ if (current_buffer_size_ > kBufferSize - kMaxMageSize ||
+ current_buffer_size_ >= kmax_file_size_) Flush();
}
2) You still haven't changed the protocol back to a text-based protocol.
I am acking the patch anyway and we can fix this in a separate ticket,
so that we get some progress on this issue.
3) Please read the style guide. A lot of my comments are about
whitespace and naming of structures and variables.
4) See more comments inline in the code below, marked AndersW>. Fix them
before pushing.
regards,
Anders Widell
On 03/14/2018 12:33 PM, Anders Widell wrote:
Hi!
The second patch in this patch series is just fixing review comments
in the first patch. Please squash the two patches into one single
patch using e.g. "git rebase -i". I have already done so and pasted
the resulting single patch below in this mail. I will give my review
comments to the squashed patch as a reply to this mail.
regards,
Anders Widell
---
opensaf.spec.in | 2 +
src/dtm/Makefile.am | 3 +
src/dtm/common/osaflog_protocol.h | 13 +++
src/dtm/tools/osaflog.cc | 168
++++++++++++++++++++++++-----
src/dtm/transport/log_server.cc | 111 ++++++++++++++++---
src/dtm/transport/log_server.h | 16 ++-
src/dtm/transport/log_writer.cc | 9 +-
src/dtm/transport/log_writer.h | 5 +-
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 +++
12 files changed, 298 insertions(+), 49 deletions(-)
create mode 100644 src/dtm/transport/transportd.conf
diff --git a/opensaf.spec.in b/opensaf.spec.in
index 187e20e7d..ac921747e 100644
--- a/opensaf.spec.in
+++ b/opensaf.spec.in
@@ -1400,6 +1400,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
@@ -1426,6 +1427,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 f3ba7200b..822249cbe 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/common/osaflog_protocol.h
b/src/dtm/common/osaflog_protocol.h
index 61e9f6f39..54551ee48 100644
--- a/src/dtm/common/osaflog_protocol.h
+++ b/src/dtm/common/osaflog_protocol.h
@@ -24,6 +24,19 @@
namespace Osaflog {
+enum cmd { FLUSH, MAXBACKUPS, MAXFILESIZE};
AndersW> According to the style guide, enum names shall start with a
capital letter. Abbreviations should be avoided. So rename Cmd to Command.
AndersW> According to the style guide, enum values shall be named
according to the same rules as constants. So rename them kFlush,
kMaxBackups and kMaxFileSize.
+enum cmdreply { RFLUSH = 101, RMAXBACKUPS, RMAXFILESIZE, FAILURE};
AndersW> This enum is unnecessary and should be removed. We can use the
Command enum also in replies.
+struct cmd_osaflog {
AndersW> Rename cmd_osaflog to Message.
+ char marker[4];
+ enum cmd m_cmd; // Command Enum
AndersW> The "enum" keyword is redundant and should be removed.
AndersW> The m_ prefix is not according to the naming conventions. Also,
avoid abbreviations. So rename m_cmd to command.
+ size_t m_value; // Value based on the command
AndersW> Rename m_value to value.
+};
+
+
+struct cmd_osaflog_resp {
+ enum cmdreply m_cmdreply; // Command Enum
+};
AndersW> The cmd_osaflog_resp structure is unnecessary. Remove it and
use the Message structure also for replies.
+
static constexpr const char* kServerSocketPath =
PKGLOCALSTATEDIR "/osaf_log.sock";
diff --git a/src/dtm/tools/osaflog.cc b/src/dtm/tools/osaflog.cc
index 3ce66f4eb..e0d135b8d 100644
--- a/src/dtm/tools/osaflog.cc
+++ b/src/dtm/tools/osaflog.cc
@@ -14,6 +14,7 @@
*/
#include <fcntl.h>
+#include <getopt.h>
#include <poll.h>
#include <sched.h>
#include <sys/socket.h>
@@ -39,6 +40,8 @@ namespace {
void PrintUsage(const char* program_name);
bool Flush();
+bool MaxTraceFileSize(size_t maxfilesize);
AndersW> Separate words with underscores in parameter names. So rename
maxfilesize to max_file_size.
+bool NoOfBackupFiles(size_t nooffiles);
AndersW> Rename nooffiles to number_of_backups
base::UnixServerSocket* CreateSocket();
uint64_t Random64Bits(uint64_t seed);
bool PrettyPrint(const std::string& log_stream);
@@ -53,20 +56,82 @@ char buf[65 * 1024];
} // namespace
int main(int argc, char** argv) {
- bool flush_option = false;
- if (argc >= 2 && strcmp(argv[1], "--flush") == 0) {
- flush_option = true;
- --argc;
- ++argv;
- }
- if ((argc != 2) && (argc != 1 || flush_option == false)) {
+ struct option long_options[] = {{"max-file-size",
required_argument, 0, 'm'},
+ {"max-backups", required_argument,
0, 'b'},
+ {"flush", no_argument, 0, 'f'},
+ {"print", required_argument, 0, 'p'},
+ {0, 0, 0, 0}};
AndersW> Use nullptr instead of 0 for fields containing a pointer.
+
+ size_t maxfilesize = 0;
AndersW> Rename to max_file_size.
+ size_t maxbackups = 0;
AndersW> Rename to max_backups.
+ char *pplog = NULL;
AndersW> Rename to pretty_print_argument.
+ int opt = 0;
AndersW> Rename to option.
+
+ int long_index = 0;
+ bool flush_result = true;
+ bool print_result = true;
+ bool maxfilesize_result = true;
AndersW> Rename to max_file_size_result
+ bool noof_backup_result = true;
AndersW> Rename to number_of_backups_result
+ bool flush_set = false;
+ bool prettyprint_set = false;
AndersW> Rename to pretty_print_set
+ bool maxfilesize_set = false;
AndersW> Rename to max_file_size_set.
+ bool maxbackups_set = false;
AndersW> Rename to max_backups_set
+
+ if (argc == 1) {
PrintUsage(argv[0]);
exit(EXIT_FAILURE);
}
- bool flush_result = Flush();
- bool print_result = true;
- if (argc == 2) print_result = PrettyPrint(argv[1]);
- exit((flush_result && print_result) ? EXIT_SUCCESS : EXIT_FAILURE);
+
+ while ((opt = getopt_long(argc, argv, "m:b:p:f",
+ long_options, &long_index)) != -1) {
+ switch (opt) {
+ case 'p':
+ pplog = optarg;
+ prettyprint_set = true;
+ flush_set = true;
+ break;
+ case 'f':
+ flush_set = true;
+ break;
+ case 'm':
+ maxfilesize_set = true;
+ maxfilesize = atoi(optarg);
+ break;
+ case 'b':
+ maxbackups_set = true;
+ maxbackups = atoi(optarg);
+ break;
+ default: PrintUsage(argv[0]);
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ if (argc - optind == 1) {
AndersW> Indentation of this whole block is wrong. Shift it two spaces
to the left.
+ flush_result = Flush();
+ flush_set = false;
+ print_result = PrettyPrint(argv[optind]);
+ prettyprint_set = false;
+ } else if (argc - optind > 1) {
+ PrintUsage(argv[0]);
+ exit(EXIT_FAILURE);
+ }
+
+ if (flush_set == true) {
+ flush_result = Flush();
+ }
+ if (prettyprint_set == true) {
+ print_result = PrettyPrint(pplog);
+ }
+ if (maxbackups_set == true) {
+ noof_backup_result = NoOfBackupFiles(maxbackups);
+ }
+ if (maxfilesize_set == true) {
+ maxfilesize_result = MaxTraceFileSize(maxfilesize);
+ }
+ if (flush_result && print_result && maxfilesize_result &&
+ noof_backup_result)
+ exit(EXIT_SUCCESS);
AndersW> Indentation of the line above is wrong. Shift it one space to
the right.
+ exit(EXIT_FAILURE);
}
namespace {
@@ -75,18 +140,27 @@ void PrintUsage(const char* program_name) {
fprintf(stderr,
"Usage: %s [OPTION] [LOGSTREAM]\n"
"\n"
- "Pretty-print the messages stored on disk for the specified\n"
+ "print the messages stored on disk for the specified\n"
"LOGSTREAM. When a LOGSTREAM argument is specified, the
option\n"
"--flush is implied.\n"
"\n"
"Opions:\n"
"\n"
- " --flush Flush all buffered messages in the log server to
disk\n"
- " even when no LOGSTREAM is specified\n",
+ "--flush Flush all buffered messages in the log
server to\n"
+ " disk even when no LOGSTREAM is specified\n"
+ "--print print the messages stored on disk for the
\n"
+ " specified LOGSTREAM.This option is
default\n"
+ " when no option is specified.\n"
+ "--max-file-size Set the maximum size (in bytes) of the
log file\n"
+ " before the log is rotated.\n"
+ "--max-backups Set the maximum number of backup files to
keep\n"
+ " when rotating the log.\n",
program_name);
}
-bool Flush() {
+
+bool cmdProcessor(struct Osaflog::cmd_osaflog cmd,
AndersW> Function names should start with a capital letter. Rename
cmdProcessor to SendCommand.
AndersW> Keyword "struct" is redundant and shall be removed.
AndersW> Rename cmd to command.
+ enum Osaflog::cmdreply m_cmdreply) {
AndersW> Keyword "enum" is redundant and shall be removed.
AndersW> Double spaces before the m_cmdreply - remove one of the spaces.
AndersW> Rename m_cmdreply to result.
auto sock = std::unique_ptr<base::UnixServerSocket>(CreateSocket());
if (!sock) {
@@ -97,17 +171,18 @@ bool Flush() {
struct sockaddr_un osaftransportd_addr;
socklen_t addrlen =
base::UnixSocket::SetAddress(Osaflog::kServerSocketPath,
&osaftransportd_addr);
- const char flush_command[] = "?flush";
- ssize_t result = sock->SendTo(flush_command, sizeof(flush_command)
- 1,
- &osaftransportd_addr, addrlen);
+
+ ssize_t result = sock->SendTo(&cmd, sizeof(cmd),
+ &osaftransportd_addr, addrlen);
if (result < 0) {
perror("Failed to send message to osaftransportd");
return false;
- } else if (static_cast<size_t>(result) != (sizeof(flush_command) -
1)) {
+ } else if (static_cast<size_t>(result) !=
+ (sizeof(struct
Osaflog::cmd_osaflog))) {
AndersW> Keyword "struct" is redundant and shall be removed.
fprintf(stderr, "Failed to send message to osaftransportd\n");
return false;
}
- static const char expected_reply[] = "!flush";
+
struct timespec end_time = base::ReadMonotonicClock() +
base::kTenSeconds;
for (;;) {
struct pollfd fds {
@@ -139,14 +214,57 @@ bool Flush() {
if (result < 0) {
perror("Failed to receive reply from osaftransportd");
return false;
- } else if (static_cast<size_t>(result) != (sizeof(expected_reply) -
1) ||
- memcmp(buf, expected_reply, result) != 0) {
- fprintf(stderr, "Received unexpected reply from osaftransportd\n");
- return false;
+ } else if (static_cast<size_t>(result) !=
+ (sizeof(struct
Osaflog::cmd_osaflog_resp) )) {
AndersW> Keyword "struct" is redundant and shall be removed.
AndersW> Extra space between the three closing parentheses at the end of
the line. Remove the space.
+ struct Osaflog::cmd_osaflog_resp cmdreply_buf;
AndersW> Keyword "struct" is redundant and shall be removed.
+ memcpy(&cmdreply_buf, buf, result);
+ if (cmdreply_buf.m_cmdreply != m_cmdreply) {
AndersW> Double spaces before != on the line above. Remove one of the
spaces.
+ fprintf(stderr, "Received unexpected reply from osaftransportd\n");
+ return false;
+ }
}
return true;
}
+bool MaxTraceFileSize(size_t maxfilesize) {
+ struct Osaflog::cmd_osaflog maxfilesize_cmd;
+
+ memset(&maxfilesize_cmd, 0, sizeof(maxfilesize_cmd));
+ maxfilesize_cmd.marker[0]='?';
AndersW> Missing spaces around '='.
+ maxfilesize_cmd.m_cmd = Osaflog::MAXFILESIZE;
+ maxfilesize_cmd.m_value = maxfilesize;
+
+ bool result;
+ result = cmdProcessor(maxfilesize_cmd, Osaflog::RMAXFILESIZE);
+ return result;
Anders> Why not simply replace the three lines above with one line:
return cmdProcessor(maxfilesize_cmd, Osaflog::RMAXFILESIZE)
+}
+
+bool NoOfBackupFiles(size_t nooffiles) {
AndersW> Rename nooffiles to number_of_files
+ struct Osaflog::cmd_osaflog maxbackups_cmd;
AndersW> Keyword "struct" is redundant and shall be removed.
+
+ memset(&maxbackups_cmd, 0, sizeof(maxbackups_cmd));
+ maxbackups_cmd.marker[0]='?';
AndersW> Missing spaces around '='.
+ maxbackups_cmd.m_cmd = Osaflog::MAXBACKUPS;
+ maxbackups_cmd.m_value = nooffiles;
+
+ bool result;
+ result = cmdProcessor(maxbackups_cmd, Osaflog::RMAXBACKUPS);
+ return result;
AndersW> return cmdProcessor(maxbackups_cmd, Osaflog::RMAXBACKUPS);
+}
+
+bool Flush() {
+ struct Osaflog::cmd_osaflog flush_cmd;
+
+ memset(&flush_cmd, 0, sizeof(flush_cmd));
+ flush_cmd.marker[0]='?';
AndersW> Missing spaces around '='.
+ flush_cmd.m_cmd = Osaflog::FLUSH;
+ flush_cmd.m_value = 0;
+
+ bool result;
+ result = cmdProcessor(flush_cmd, Osaflog::RFLUSH);
+ return result;
AndersW> return cmdProcessor(flush_cmd, Osaflog::RFLUSH)
+}
+
base::UnixServerSocket* CreateSocket() {
base::UnixServerSocket* sock = nullptr;
Osaflog::ClientAddress addr{};
@@ -221,7 +339,7 @@ std::list<int> OpenLogFiles(const std::string&
log_stream) {
std::string PathName(const std::string& log_stream, int suffix) {
std::string path_name{PKGLOGDIR "/"};
path_name += log_stream;
- if (suffix != 0) path_name += std::string{"."} +
std::to_string(suffix);
+ if (suffix != 0) path_name += std::string {"."} +
std::to_string(suffix);
AndersW> Don't add a space after std::string (i.e. the modification to
the line above shouldn't be done).
return path_name;
}
diff --git a/src/dtm/transport/log_server.cc
b/src/dtm/transport/log_server.cc
index 2d6c96159..e29b8d4d2 100644
--- a/src/dtm/transport/log_server.cc
+++ b/src/dtm/transport/log_server.cc
@@ -17,22 +17,27 @@
*/
#include "dtm/transport/log_server.h"
+#include <signal.h>
+#include <syslog.h>
#include <cstring>
#include "base/osaf_poll.h"
#include "base/time.h"
#include "dtm/common/osaflog_protocol.h"
#include "osaf/configmake.h"
+
const Osaflog::ClientAddressConstantPrefix LogServer::address_header_{};
LogServer::LogServer(int term_fd)
: term_fd_{term_fd},
+ no_of_backups_{9},
+ max_file_size_{5 * 1024 * 1024},
log_socket_{Osaflog::kServerSocketPath,
base::UnixSocket::kNonblocking},
log_streams_{},
- current_stream_{new LogStream{"mds.log", 1}},
+ current_stream_{new LogStream{"mds.log", 1, 5 * 1024 * 1024}},
no_of_log_streams_{1} {
log_streams_["mds.log"] = current_stream_;
-}
+ }
AndersW> Don't add spaces at the beginning of the line above (i.e. the
modification to the line above shouldn't be done).
LogServer::~LogServer() {
for (const auto& s : log_streams_) delete s.second;
@@ -40,6 +45,7 @@ LogServer::~LogServer() {
void LogServer::Run() {
struct pollfd pfd[2] = {{term_fd_, POLLIN, 0}, {log_socket_.fd(),
POLLIN, 0}};
+
do {
for (int i = 0; i < 256; ++i) {
char* buffer = current_stream_->current_buffer_position();
@@ -99,7 +105,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, no_of_backups_,
max_file_size_};
auto result = log_streams_.insert(
std::map<std::string, LogStream*>::value_type{log_name, stream});
if (!result.second) osaf_abort(msg_id_size);
@@ -107,6 +114,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 false;
+ }
+
+
+ /* Read file. */
+ while (fgets(line, 256, transport_conf_file)) {
+ /* If blank line, skip it - and set tag back to 0. */
+ if (strcmp(line, "\n") == 0) {
+ continue;
+ }
+
+ /* If a comment line, skip it. */
+ n = strlen(line);
+ comment_line = 0;
+ for (i = 0; i < n; i++) {
+ if ((line[i] == ' ') || (line[i] == '\t')) {
+ continue;
+ } else if (line[i] == '#') {
+ comment_line = 1;
+ break;
+ } else {
+ break;
+ }
+ }
+ if (comment_line) continue;
+ line[n - 1] = 0;
+
+ if (strncmp(line, "TRANSPORT_MAX_LOG_FILESIZE=",
+ strlen("TRANSPORT_MAX_LOG_FILESIZE=")) == 0) {
+ tag_len = strlen("TRANSPORT_MAX_LOG_FILESIZE=");
+ maxFileSize = atoi(&line[tag_len]);
+
+ if (maxFileSize > 1) {
+ max_file_size_ = maxFileSize;
+ }
+ }
+
+ if (strncmp(line, "TRANSPORT_NO_OF_BACKUP_LOG_FILES=",
+ strlen("TRANSPORT_NO_OF_BACKUP_LOG_FILES=")) == 0) {
+ tag_len = strlen("TRANSPORT_NO_OF_BACKUP_LOG_FILES=");
+ noOfBackupFiles = atoi(&line[tag_len]);
+
+ if (noOfBackupFiles > 1) {
+ no_of_backups_ = noOfBackupFiles;
+ }
+ }
+ }
+
+ /* Close file. */
+ fclose(transport_conf_file);
+
+ return true;
+}
+
bool LogServer::ValidateLogName(const char* msg_id, size_t
msg_id_size) {
if (msg_id_size < 1 || msg_id_size > LogStream::kMaxLogNameSize)
return false;
if (msg_id[0] == '.') return false;
@@ -125,8 +197,9 @@ void LogServer::ExecuteCommand(const char*
command, size_t size,
const struct sockaddr_un& addr,
socklen_t addrlen) {
if (ValidateAddress(addr, addrlen)) {
- std::string cmd_result = ExecuteCommand(std::string{command, size});
- log_socket_.SendTo(cmd_result.data(), cmd_result.size(), &addr,
addrlen);
+ struct Osaflog::cmd_osaflog_resp cmdreply_buf;
AndersW> Keyword "struct" is redundant and shall be removed.
AndersW> The cmdreply_buf structure is not initialized. Either use
memset to clear it, or add a constructor to the structure definition.
AndersW> Set marker[0] to '!'
AndersW> Rename cmdreply_buf to result.
+ cmdreply_buf.m_cmdreply = ExecuteCommand(command, size);
AndersW> Double space after '=' on the line above.
+ log_socket_.SendTo(&cmdreply_buf, sizeof(cmdreply_buf), &addr,
addrlen);
}
}
@@ -139,21 +212,27 @@ bool LogServer::ValidateAddress(const struct
sockaddr_un& addr,
}
}
-std::string LogServer::ExecuteCommand(const std::string& command) {
- if (command == "?flush") {
- for (const auto& s : log_streams_) {
- LogStream* stream = s.second;
- stream->Flush();
- }
- return std::string{"!flush"};
- } else {
- return std::string{"!not_supported"};
+enum Osaflog::cmdreply LogServer::ExecuteCommand(const char * command,
AndersW> Keyword "enum" is redundant and shall be removed.
+ size_t size) {
+ struct Osaflog::cmd_osaflog cmd_buf;
AndersW> Keyword "struct" is redundant and shall be removed.
AndersW> Rename cmd_buf to message.
+ memcpy(&cmd_buf, command, size);
AndersW> SECURITY FLAW: no check that the data fits into the destination
buffer!
+
+ if (cmd_buf.m_cmd == Osaflog::MAXFILESIZE) {
AndersW> Double space before '==' on the line above.
+ max_file_size_ = cmd_buf.m_value;
+ return Osaflog::RMAXFILESIZE;
+ } else if (cmd_buf.m_cmd == Osaflog::MAXBACKUPS) {
+ no_of_backups_ = cmd_buf.m_value;
+ return Osaflog::RMAXBACKUPS;
+ } else if (cmd_buf.m_cmd == Osaflog::FLUSH) {
+ return Osaflog::RFLUSH;
}
+ return Osaflog::FAILURE;
}
LogServer::LogStream::LogStream(const std::string& log_name,
- size_t no_of_backups)
- : log_name_{log_name}, last_flush_{}, log_writer_{log_name,
no_of_backups} {
+ size_t no_of_backups_, size_t
max_file_size_)
+ : log_name_{log_name}, last_flush_{}, log_writer_{log_name,
no_of_backups_,
+ max_file_size_} {
if (log_name.size() > kMaxLogNameSize) osaf_abort(log_name.size());
}
diff --git a/src/dtm/transport/log_server.h
b/src/dtm/transport/log_server.h
index c988b6193..eb04fc7a6 100644
--- a/src/dtm/transport/log_server.h
+++ b/src/dtm/transport/log_server.h
@@ -33,6 +33,8 @@
class LogServer {
public:
static constexpr size_t kMaxNoOfStreams = 32;
+ static constexpr const char* kTransportdConfigFile =
+ PKGSYSCONFDIR "/transportd.conf";
// @a term_fd is a file descriptor that will become readable when
the program
// should exit because it has received the SIGTERM signal.
explicit LogServer(int term_fd);
@@ -42,12 +44,15 @@ class LogServer {
// process has received the SIGTERM signal, which is indicated by
the caller
// by making the term_fd (provided in the constructor) readable.
void Run();
+ // To read Transportd.conf
+ bool ReadConfig(const char *transport_config_file);
private:
class LogStream {
public:
static constexpr size_t kMaxLogNameSize = 32;
- LogStream(const std::string& log_name, size_t no_of_backups);
+ LogStream(const std::string& log_name, size_t no_of_backups,
+ size_t kMaxFileSize);
AndersW> The parameter kMaxFileSize is not a constant. Rename it to
max_file_size.
size_t log_name_size() const { return log_name_.size(); }
const char* log_name_data() const { return log_name_.data(); }
@@ -79,10 +84,17 @@ class LogServer {
static bool ValidateLogName(const char* msg_id, size_t msg_id_size);
void ExecuteCommand(const char* command, size_t size,
const struct sockaddr_un& addr, socklen_t
addrlen);
+ // Signal handler for reading config file transportd.conf
+ static void usr2_sig_handler(int sig);
AndersW> The method usr2_sig_handler is not defined anywhere. Remove the
two lines above.
static bool ValidateAddress(const struct sockaddr_un& addr,
socklen_t addrlen);
- std::string ExecuteCommand(const std::string& command);
+// std::string ExecuteCommand(const std::string& command);
AndersW> Don't leave commented out lines in the code. Remove!
+ enum Osaflog::cmdreply ExecuteCommand(const char* command, size_t
size);
AndersW> They keyword "enum" is redundant and should be removed.
int term_fd_;
+ // Configuration for LogServer
+ size_t no_of_backups_;
+ size_t max_file_size_;
+
base::UnixServerSocket log_socket_;
std::map<std::string, LogStream*> log_streams_;
LogStream* current_stream_;
diff --git a/src/dtm/transport/log_writer.cc
b/src/dtm/transport/log_writer.cc
index 8f1d6a77f..60c0bf557 100644
--- a/src/dtm/transport/log_writer.cc
+++ b/src/dtm/transport/log_writer.cc
@@ -17,6 +17,7 @@
*/
#include "dtm/transport/log_writer.h"
+#include "dtm/transport/log_server.h"
AndersW> This include statement shall be moved down below the system
header files.
#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>
@@ -25,13 +26,15 @@
#include "base/getenv.h"
#include "osaf/configmake.h"
-LogWriter::LogWriter(const std::string& log_name, size_t no_of_backups)
+LogWriter::LogWriter(const std::string& log_name, size_t no_of_backups,
+ size_t kmax_file_size)
: log_file_{base::GetEnv<std::string>("pkglogdir", PKGLOGDIR) +
"/" +
log_name},
fd_{-1},
current_file_size_{0},
current_buffer_size_{0},
no_of_backups_{no_of_backups},
+ kmax_file_size_{kmax_file_size},
buffer_{new char[kBufferSize]} {}
LogWriter::~LogWriter() {
@@ -43,7 +46,7 @@ LogWriter::~LogWriter() {
std::string LogWriter::log_file(size_t backup) const {
std::string file_name = log_file_;
if (backup != 0) {
- file_name += std::string{"."} + std::to_string(backup);
+ file_name += std::string {"."} + std::to_string(backup);
AndersW> Don't add a space after std::string.
}
return file_name;
}
@@ -95,7 +98,7 @@ void LogWriter::Flush() {
if (size == 0) return;
if (fd_ < 0) Open();
if (fd_ < 0) return;
- if (current_file_size_ >= kMaxFileSize) {
+ if (current_file_size_ >= kmax_file_size_) {
RotateLog();
if (fd_ < 0) Open();
if (fd_ < 0) return;
diff --git a/src/dtm/transport/log_writer.h
b/src/dtm/transport/log_writer.h
index 89d7b37a2..b6cf6c1a7 100644
--- a/src/dtm/transport/log_writer.h
+++ b/src/dtm/transport/log_writer.h
@@ -29,7 +29,8 @@ class LogWriter {
public:
constexpr static const size_t kMaxMessageSize = 2 * size_t{1024};
- LogWriter(const std::string& log_name, size_t no_of_backups);
+ LogWriter(const std::string& log_name, size_t no_of_backups,
+ size_t kmax_file_size);
AndersW> Rename kmax_file_size to max_file_size.
virtual ~LogWriter();
char* current_buffer_position() { return buffer_ +
current_buffer_size_; }
@@ -43,7 +44,6 @@ class LogWriter {
private:
constexpr static const size_t kBufferSize = 128 * size_t{1024};
- constexpr static const size_t kMaxFileSize = 5000 * size_t{1024};
void Open();
void Close();
void RotateLog();
@@ -55,6 +55,7 @@ class LogWriter {
size_t current_file_size_;
size_t current_buffer_size_;
size_t no_of_backups_;
+ size_t kmax_file_size_;
AndersW> Rename kmax_file_size_ to max_file_size_
char* buffer_;
DELETE_COPY_AND_MOVE_OPERATORS(LogWriter);
diff --git a/src/dtm/transport/main.cc b/src/dtm/transport/main.cc
index cf091c85b..077601abb 100644
--- a/src/dtm/transport/main.cc
+++ b/src/dtm/transport/main.cc
@@ -26,6 +26,9 @@
#include "dtm/transport/log_server.h"
#include "dtm/transport/transport_monitor.h"
+
+static constexpr const char* kTransportdConfigFile =
+ PKGSYSCONFDIR "/transportd.conf";
constexpr static const int kDaemonStartWaitTimeInSeconds = 15;
enum Termination { kExit, kDaemonExit, kReboot };
@@ -39,6 +42,7 @@ struct Result {
static void* LogServerStartFunction(void* instance) {
LogServer* log_server = static_cast<LogServer*>(instance);
+ log_server->ReadConfig(kTransportdConfigFile);
log_server->Run();
return nullptr;
}
diff --git a/src/dtm/transport/osaf-transport.in
b/src/dtm/transport/osaf-transport.in
index 99a256396..a30ac6345 100644
--- a/src/dtm/transport/osaf-transport.in
+++ b/src/dtm/transport/osaf-transport.in
@@ -24,6 +24,7 @@ if [ ! -r $osafdirfile ]; then
exit 6
else
. $osafdirfile
+ . $pkgsysconfdir/transportd.conf
. $pkgsysconfdir/nid.conf
fi
diff --git a/src/dtm/transport/tests/log_writer_test.cc
b/src/dtm/transport/tests/log_writer_test.cc
index f57681cb1..e96831ef1 100644
--- a/src/dtm/transport/tests/log_writer_test.cc
+++ b/src/dtm/transport/tests/log_writer_test.cc
@@ -54,7 +54,7 @@ TEST_F(LogWriterTest, ExistingFileShouldBeAppended) {
std::ofstream ostr(tmpdir_ + std::string("/mds.log"));
ostr << first_line << std::endl;
ostr.close();
- LogWriter* log_writer = new LogWriter{"mds.log", 1};
+ LogWriter* log_writer = new LogWriter{"mds.log", 1, 5000 * 1024};
memcpy(log_writer->current_buffer_position(), second_line,
sizeof(second_line) - 1);
log_writer->current_buffer_position()[sizeof(second_line) - 1] = '\n';
diff --git a/src/dtm/transport/transportd.conf
b/src/dtm/transport/transportd.conf
new file mode 100644
index 000000000..2a410b8fb
--- /dev/null
+++ b/src/dtm/transport/transportd.conf
@@ -0,0 +1,13 @@
+# This file contains configuration for the Transportd service
+
+#
+# TRANSPORT_MAX_LOG_FILESIZE: The maximum size of the log file. The
size value should
+# be in Bytes i.e if you want to give 5 MB please mention it as
5242880. it is treated
+# as 5 MB. By default value will be 5242880 Bytes
+#TRANSPORT_MAX_LOG_FILESIZE=5242880
+
+#
+# TRANSPORT_NO_OF_BACKUP_LOG_FILES: Number of backup files to
maintain. Log rotation will
+# be done based on this value. Default value will be 9
+# i.e totally 10 log files will be maintain.
+#TRANSPORT_NO_OF_BACKUP_LOG_FILES=9
------------------------------------------------------------------------------
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
