Hi Anders,

Ack, Code review only

Regards,
Ravi

-----Original Message-----
From: Anders Widell [mailto:[email protected]] 
Sent: Friday, December 01, 2017 3:26 PM
To: Ravi Sekhar Reddy Konda <[email protected]>
Cc: [email protected]; Anders Widell 
<[email protected]>
Subject: [PATCH 1/1] dtm: Support pretty-printing OpenSAF logs using the 
osaflog command [#2709]

Add support to the osaflog command for parsing and pretty-printing the OpenSAF 
log messages. Initially, we only support simple pretty-printing by removing 
information which is not frequently needed when reading the logs. In future 
ticket(s), we can also add support for filtering log messages.

The following example command will print the MDS log:

osaflog mds.log
---
 src/dtm/common/osaflog_protocol.h |  20 ++++
 src/dtm/tools/osaflog.cc          | 220 ++++++++++++++++++++++++++++++--------
 src/dtm/transport/log_server.cc   |  17 +--
 src/dtm/transport/log_server.h    |   2 -
 4 files changed, 196 insertions(+), 63 deletions(-)

diff --git a/src/dtm/common/osaflog_protocol.h 
b/src/dtm/common/osaflog_protocol.h
index d580b61bc..61e9f6f39 100644
--- a/src/dtm/common/osaflog_protocol.h
+++ b/src/dtm/common/osaflog_protocol.h
@@ -17,7 +17,9 @@
 #define DTM_COMMON_OSAFLOG_PROTOCOL_H_
 
 #include <sys/socket.h>
+#include <cstddef>
 #include <cstdint>
+#include <cstring>
 #include "osaf/configmake.h"
 
 namespace Osaflog {
@@ -45,6 +47,24 @@ struct __attribute__((__packed__)) ClientAddress {
   uint32_t pid;
 };
 
+// Returns a pointer the space-separated field with index @a field_no 
+within // @buf of size @a size, or nullptr if @a buf does not contain 
+enough fields. @a // field_size is an output parameter where the length of the 
field is returned.
+static inline const char* GetField(const char* buf, size_t size, int field_no,
+                                   size_t* field_size) {
+  while (field_no != 0) {
+    const char* pos = static_cast<const char*>(memchr(buf, ' ', size));
+    if (pos == nullptr) return nullptr;
+    ++pos;
+    size -= pos - buf;
+    buf = pos;
+    --field_no;
+  }
+  const char* pos = static_cast<const char*>(memchr(buf, ' ', size));
+  *field_size = (pos != nullptr) ? (pos - buf) : size;
+  return buf;
+}
+
 }  // namespace Osaflog
 
 #endif  // DTM_COMMON_OSAFLOG_PROTOCOL_H_ diff --git 
a/src/dtm/tools/osaflog.cc b/src/dtm/tools/osaflog.cc index 
c81df8bcd..385447b09 100644
--- a/src/dtm/tools/osaflog.cc
+++ b/src/dtm/tools/osaflog.cc
@@ -13,9 +13,11 @@
  *
  */
 
+#include <fcntl.h>
 #include <poll.h>
 #include <sched.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/un.h>
 #include <unistd.h>
 #include <cerrno>
@@ -24,62 +26,72 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <list>
 #include <memory>
 #include <random>
+#include <string>
 #include "base/time.h"
 #include "base/unix_server_socket.h"
 #include "dtm/common/osaflog_protocol.h"
+#include "osaf/configmake.h"
 
 namespace {
 
-uint64_t Random64Bits(uint64_t seed) {
-  std::mt19937_64 generator{base::TimespecToNanos(base::ReadRealtimeClock()) *
-                            seed};
-  return generator();
-}
+void PrintUsage(const char* program_name); bool Flush();
+base::UnixServerSocket* CreateSocket(); uint64_t Random64Bits(uint64_t 
+seed); bool PrettyPrint(const std::string& log_stream); std::list<int> 
+OpenLogFiles(const std::string& log_stream); std::string PathName(const 
+std::string& log_stream, int suffix); uint64_t GetInode(int fd); bool 
+PrettyPrint(FILE* stream); bool PrettyPrint(const char* line, size_t 
+size);
 
-void PrintUsage(const char* program_name) {
-  printf(
-      "Usage: %s <OPTION>\n"
-      "Send a command to the node-local internal OpenSAF log server\n"
-      "\n"
-      "Opions:\n"
-      "  --flush    Flush all buffered log messages from memory to disk\n",
-      program_name);
-}
-
-base::UnixServerSocket* CreateSocket() {
-  base::UnixServerSocket* sock = nullptr;
-  Osaflog::ClientAddress addr{};
-  addr.pid = getpid();
-  for (uint64_t i = 1; i <= 1000; ++i) {
-    addr.random = Random64Bits(i);
-    sock = new base::UnixServerSocket(addr.sockaddr(), addr.sockaddr_length(),
-                                      base::UnixSocket::kNonblocking);
-    if (sock->fd() >= 0 || errno != EADDRINUSE) break;
-    delete sock;
-    sock = nullptr;
-    sched_yield();
-  }
-  if (sock != nullptr && sock->fd() < 0) {
-    delete sock;
-    sock = nullptr;
-  }
-  return sock;
-}
+char buf[65 * 1024];
 
 }  // namespace
 
 int main(int argc, char** argv) {
-  if (argc != 2 || strcmp(argv[1], "--flush") != 0) {
+  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)) {
     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); }
+
+namespace {
+
+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"
+          "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",
+          program_name);
+}
+
+bool Flush() {
   auto sock = std::unique_ptr<base::UnixServerSocket>(CreateSocket());
 
   if (!sock) {
     fprintf(stderr, "Failed to create UNIX domain socket\n");
-    exit(EXIT_FAILURE);
+    return false;
   }
 
   struct sockaddr_un osaftransportd_addr; @@ -90,12 +102,11 @@ int main(int 
argc, char** argv) {
                                 &osaftransportd_addr, addrlen);
   if (result < 0) {
     perror("Failed to send message to osaftransportd");
-    exit(EXIT_FAILURE);
+    return false;
   } else if (static_cast<size_t>(result) != (sizeof(flush_command) - 1)) {
     fprintf(stderr, "Failed to send message to osaftransportd\n");
-    exit(EXIT_FAILURE);
+    return false;
   }
-  char buf[256];
   static const char expected_reply[] = "!flush";
   struct timespec end_time = base::ReadMonotonicClock() + base::kTenSeconds;
   for (;;) {
@@ -106,16 +117,16 @@ int main(int argc, char** argv) {
     result = 0;
     if (current_time >= end_time) {
       fprintf(stderr, "Timeout\n");
-      exit(EXIT_FAILURE);
+      return false;
     }
     struct timespec timeout = end_time - current_time;
     result = ppoll(&fds, 1, &timeout, NULL);
     if (result < 0) {
       perror("Failed to wait for reply from osaftransportd");
-      exit(EXIT_FAILURE);
+      return false;
     } else if (result == 0) {
       fprintf(stderr, "Timeout\n");
-      exit(EXIT_FAILURE);
+      return false;
     }
     struct sockaddr_un sender_addr;
     socklen_t sender_addrlen = sizeof(sender_addr); @@ -127,11 +138,130 @@ int 
main(int argc, char** argv) {
   }
   if (result < 0) {
     perror("Failed to receive reply from osaftransportd");
-    exit(EXIT_FAILURE);
+    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");
-    exit(EXIT_FAILURE);
+    return false;
+  }
+  return true;
+}
+
+base::UnixServerSocket* CreateSocket() {
+  base::UnixServerSocket* sock = nullptr;
+  Osaflog::ClientAddress addr{};
+  addr.pid = getpid();
+  for (uint64_t i = 1; i <= 1000; ++i) {
+    addr.random = Random64Bits(i);
+    sock = new base::UnixServerSocket(addr.sockaddr(), addr.sockaddr_length(),
+                                      base::UnixSocket::kNonblocking);
+    if (sock->fd() >= 0 || errno != EADDRINUSE) break;
+    delete sock;
+    sock = nullptr;
+    sched_yield();
+  }
+  if (sock != nullptr && sock->fd() < 0) {
+    delete sock;
+    sock = nullptr;
+  }
+  return sock;
+}
+
+uint64_t Random64Bits(uint64_t seed) {
+  std::mt19937_64 generator{base::TimespecToNanos(base::ReadRealtimeClock()) *
+                            seed};
+  return generator();
+}
+
+bool PrettyPrint(const std::string& log_stream) {
+  std::list<int> fds = OpenLogFiles(log_stream);
+  uint64_t last_inode = ~UINT64_C(0);
+  bool result = true;
+  for (const auto& fd : fds) {
+    uint64_t inode = GetInode(fd);
+    if (inode == last_inode) {
+      close(fd);
+      continue;
+    }
+    last_inode = inode;
+    FILE* stream = fdopen(fd, "r");
+    if (stream != nullptr) {
+      if (!PrettyPrint(stream)) result = false;
+      fclose(stream);
+    } else {
+      result = false;
+      close(fd);
+    }
+  }
+  return result;
+}
+
+std::list<int> OpenLogFiles(const std::string& log_stream) {
+  std::list<int> result{};
+  bool last_open_failed = false;
+  for (int suffix = 0; suffix < 100; ++suffix) {
+    std::string path_name = PathName(log_stream, suffix);
+    std::string next_path_name = PathName(log_stream, suffix + 1);
+    int fd;
+    do {
+      fd = open(path_name.c_str(), O_RDONLY);
+    } while (fd < 0 && errno == EINTR);
+    if (fd >= 0) {
+      result.push_front(fd);
+      last_open_failed = false;
+    } else {
+      if (errno != ENOENT) perror("Could not open log file");
+      if (last_open_failed || errno != ENOENT) break;
+      last_open_failed = true;
+    }
+  }
+  return result;
+}
+
+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);
+  return path_name;
+}
+
+uint64_t GetInode(int fd) {
+  struct stat buf;
+  int result = fstat(fd, &buf);
+  return result == 0 ? buf.st_ino : ~UINT64_C(0); }
+
+bool PrettyPrint(FILE* stream) {
+  bool result = true;
+  for (;;) {
+    char* s = fgets(buf, sizeof(buf), stream);
+    if (s == nullptr) break;
+    if (!PrettyPrint(s, strlen(buf))) result = false;
   }
-  exit(EXIT_SUCCESS);
+  if (!feof(stream)) {
+    fprintf(stderr, "Error reading from file\n");
+    result = false;
+  }
+  return result;
+}
+
+bool PrettyPrint(const char* line, size_t size) {
+  size_t date_size;
+  const char* date_field = Osaflog::GetField(line, size, 1, 
+&date_size);
+  size_t msg_size;
+  const char* msg_field = Osaflog::GetField(line, size, 8, &msg_size);
+  char pretty_date[33];
+  if (date_field == nullptr || date_size < 19 ||
+      date_size >= sizeof(pretty_date) || date_field[10] != 'T' ||
+      msg_field == nullptr || msg_size == 0)
+    return false;
+  memcpy(pretty_date, date_field, date_size);
+  memset(pretty_date + date_size, '0', sizeof(pretty_date) - 1 - 
+date_size);
+  pretty_date[10] = ' ';
+  pretty_date[19] = '.';
+  pretty_date[23] = '\0';
+  printf("%s %s", pretty_date, msg_field);
+  return true;
 }
+
+}  // namespace
diff --git a/src/dtm/transport/log_server.cc b/src/dtm/transport/log_server.cc 
index 9afc643c2..2d6c96159 100644
--- a/src/dtm/transport/log_server.cc
+++ b/src/dtm/transport/log_server.cc
@@ -56,7 +56,7 @@ void LogServer::Run() {
           buffer[result - 1] = '\n';
         }
         size_t msg_id_size;
-        const char* msg_id = GetField(buffer, result, 5, &msg_id_size);
+        const char* msg_id = Osaflog::GetField(buffer, result, 5, 
+ &msg_id_size);
         if (msg_id == nullptr) continue;
         LogStream* stream = GetStream(msg_id, msg_id_size);
         if (stream == nullptr) continue; @@ -88,21 +88,6 @@ void 
LogServer::Run() {
   } while ((pfd[0].revents & POLLIN) == 0);  }
 
-const char* LogServer::GetField(const char* buf, size_t size, int field_no,
-                                size_t* field_size) {
-  while (field_no != 0) {
-    const char* pos = static_cast<const char*>(memchr(buf, ' ', size));
-    if (pos == nullptr) return nullptr;
-    ++pos;
-    size -= pos - buf;
-    buf = pos;
-    --field_no;
-  }
-  const char* pos = static_cast<const char*>(memchr(buf, ' ', size));
-  *field_size = (pos != nullptr) ? (pos - buf) : size;
-  return buf;
-}
-
 LogServer::LogStream* LogServer::GetStream(const char* msg_id,
                                            size_t msg_id_size) {
   if (msg_id_size == current_stream_->log_name_size() && diff --git 
a/src/dtm/transport/log_server.h b/src/dtm/transport/log_server.h index 
50204b499..c988b6193 100644
--- a/src/dtm/transport/log_server.h
+++ b/src/dtm/transport/log_server.h
@@ -70,8 +70,6 @@ class LogServer {
     struct timespec last_flush_;
     LogWriter log_writer_;
   };
-  static const char* GetField(const char* buf, size_t size, int field_no,
-                              size_t* field_size);
   LogStream* GetStream(const char* msg_id, size_t msg_id_size);
   // Validate the log stream name, for security reasons. This method will check
   // that the string, when used as a file name, does not traverse the directory
--
2.13.3


------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to