gluster-block: add logrotate support

README.md

@@ -78,7 +78,7 @@ commands:
         Glfs objects cache capacity [max: 512] [default: 5]
   --log-level <LOGLEVEL>
         Logging severity. Valid options are,
-        TRACE, DEBUG, INFO, WARNING, ERROR and NONE [default: INFO]
+        TRACE, DEBUG, INFO, WARNING, ERROR, CRIT and NONE [default: INFO]
   --no-remote-rpc
         Ignore remote rpc communication, capabilities check and
         other node sanity checks

cli/gluster-block.c

@@ -65,19 +65,19 @@ glusterBlockCLILoadConfig(void)
 
   if (GB_ALLOC(cfg) < 0) {
     LOG("cli", GB_LOG_ERROR,
-        "Alloc GB config failed for configPath: %s!\n", GB_DEF_CONFIGPATH);
+        "Alloc GB config failed for configPath: %s!", GB_DEF_CONFIGPATH);
     return NULL;
   }
 
   if (GB_STRDUP(cfg->configPath, GB_DEF_CONFIGPATH) < 0) {
     LOG("cli", GB_LOG_ERROR,
-        "failed to copy configPath: %s\n", GB_DEF_CONFIGPATH);
+        "failed to copy configPath: %s", GB_DEF_CONFIGPATH);
     goto freeConfig;
   }
 
   if (glusterBlockLoadConfig(cfg, false)) {
     LOG("cli", GB_LOG_ERROR,
-        "Loading GB config failed for configPath: %s!\n", GB_DEF_CONFIGPATH);
+        "Loading GB config failed for configPath: %s!", GB_DEF_CONFIGPATH);
     goto freeConfigPath;
   }
 
@@ -154,7 +154,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
   if (!conf) {
       LOG("cli", GB_LOG_ERROR,
           "glusterBlockCLILoadConfig() failed, for block %s create on volume %s"
-          " with hosts %s\n", create_obj->block_name, create_obj->volume,
+          " with hosts %s", create_obj->block_name, create_obj->volume,
           create_obj->block_hosts);
       goto out;
   }
@@ -175,7 +175,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
     create_obj = cobj;
     if (block_create_cli_1(create_obj, &reply, clnt) != RPC_SUCCESS) {
       LOG("cli", GB_LOG_ERROR,
-          "%s block %s create on volume %s with hosts %s failed\n",
+          "%s block %s create on volume %s with hosts %s failed",
           clnt_sperror(clnt, "block_create_cli_1"), create_obj->block_name,
           create_obj->volume, create_obj->block_hosts);
       goto out;

configure.ac

@@ -129,6 +129,9 @@ fi
 GLUSTER_BLOCKD_LIBEXECDIR="$(eval echo ${libexecdir}/gluster-block)"
 AC_SUBST(GLUSTER_BLOCKD_LIBEXECDIR)
 
+GLUSTER_BLOCKD_LOGROTATEDIR="$(eval echo /etc/logrotate.d)"
+AC_SUBST(GLUSTER_BLOCKD_LOGROTATEDIR)
+
 # Checks for typedefs, structures, and compiler characteristics.
 AC_CHECK_HEADER_STDBOOL
 AC_TYPE_SIZE_T

daemon/gluster-blockd.c

@@ -54,7 +54,7 @@ glusterBlockDHelp(void)
       "        Glfs objects cache capacity [max: 512] [default: 5]\n"
       "  --log-level <LOGLEVEL>\n"
       "        Logging severity. Valid options are,\n"
-      "        TRACE, DEBUG, INFO, WARNING, ERROR and NONE [default: INFO]\n"
+      "        TRACE, DEBUG, INFO, WARNING, ERROR, CRIT and NONE [default: INFO]\n"
       "  --no-remote-rpc\n"
       "        Ignore remote rpc communication, capabilities check and\n"
       "        other node sanity checks\n"

docs/gluster-blockd.8

@@ -26,7 +26,7 @@ gluster-blockd daemon is responsible for block management, hence the daemon must
 glfs objects cache capacity [max: 512] [default: 5]
 .TP
 \fB\-\-log\-level\fR <LOGLEVEL>
-Logging severity. Valid options are TRACE, DEBUG, INFO, WARNING, ERROR and NONE [default: INFO].
+Logging severity. Valid options are TRACE, DEBUG, INFO, WARNING, ERROR, CRIT and NONE [default: INFO].
 .TP
 \fB\-\-no\-remote\-rpc
 Ignore remote rpc communication, capabilities check and other node sanity checks

extras/Makefile.am

@@ -7,11 +7,15 @@ CLEANFILES = *~
 install-data-local:
 	$(MKDIR_P) $(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR);                           \
 	$(INSTALL_DATA) -m 755 $(top_srcdir)/extras/wait-for-bricks.sh               \
-		$(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR)/wait-for-bricks.sh;                 \
+		$(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR)/wait-for-bricks.sh;               \
 	$(INSTALL_DATA) -m 755 $(top_srcdir)/extras/upgrade_activities.sh            \
-		$(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR)/upgrade_activities.sh
+		$(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR)/upgrade_activities.sh;            \
+	$(MKDIR_P) $(DESTDIR)$(GLUSTER_BLOCKD_LOGROTATEDIR);                         \
+	$(INSTALL_DATA) -m 644 gluster-block.logrotate                               \
+		$(DESTDIR)$(GLUSTER_BLOCKD_LOGROTATEDIR)/gluster-block;
 
 uninstall-local:
 	rm -f $(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR)/wait-for-bricks.sh              \
-		$(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR)/upgrade_activities.sh               \
-		$(DESTDIR)$(GLUSTER_BLOCKD_WORKDIR)/gb_upgrade.status;
+		$(DESTDIR)$(GLUSTER_BLOCKD_LIBEXECDIR)/upgrade_activities.sh             \
+		$(DESTDIR)$(GLUSTER_BLOCKD_WORKDIR)/gb_upgrade.status                    \
+		$(DESTDIR)$(GLUSTER_BLOCKD_LOGROTATEDIR)/gluster-block;

extras/gluster-block.logrotate

@@ -0,0 +1,18 @@
+# Rotate all the gluster-block logs
+/var/log/gluster-block/*.log {
+  sharedscripts
+  weekly
+  maxsize 10M
+  minsize 100k
+
+# 6 months of logs are good enough
+  rotate 26
+
+  missingok
+  compress
+  delaycompress
+  notifempty
+  postrotate
+  killall -q -s 1 gluster-blockd > /dev/null 2>&1 || true
+  endscript
+}

gluster-block.spec.in

@@ -82,13 +82,17 @@ rm -rf ${RPM_BUILD_ROOT}
 %attr(0755,-,-) %{_initddir}/gluster-blockd
 %endif
 %config(noreplace) %{_sysconfdir}/sysconfig/gluster-blockd
+%config(noreplace) %{_sysconfdir}/logrotate.d/gluster-block
 %dir %attr(0755,-,-) %{_libexecdir}/gluster-block
      %attr(0755,-,-) %{_libexecdir}/gluster-block/wait-for-bricks.sh
      %attr(0755,-,-) %{_libexecdir}/gluster-block/upgrade_activities.sh
 %dir %attr(0755,-,-) %{_sharedstatedir}/gluster-block
      %attr(0644,-,-) %{_sharedstatedir}/gluster-block/gluster-block-caps.info
 
 %changelog
+* Sun Apr 3 2019 Xiubo Li <[email protected]>
+- Add logrotate support
+
 * Sun Oct 14 2018 Prasanna Kumar Kalever <[email protected]>
 - add install details for upgrade_activities.sh
 

systemd/gluster-blockd.sysconfig

@@ -10,7 +10,7 @@
 #GB_GLFS_LRU_COUNT=5
 
 
-# Supported loglevels [ NONE, ERROR, WARNING, INFO, DEBUG, TRACE ]
+# Supported loglevels [ NONE, CRIT, ERROR, WARNING, INFO, DEBUG, TRACE ]
 # And the default logging level is INFO, if you want to change the
 # default level, uncomment it and set your level:
 #GB_LOG_LEVEL=INFO

utils/dyn-config.c

@@ -254,7 +254,7 @@ glusterBlockReadConfig(gbConfig *cfg, ssize_t *len)
   }
   if (fp == NULL) {
     LOG("mgmt", GB_LOG_ERROR,
-        "Failed to open file '%s'\n", cfg->configPath);
+        "Failed to open file '%s'", cfg->configPath);
     GB_FREE(buf);
     return NULL;
   }
@@ -429,7 +429,7 @@ glusterBlockParseOption(char **cur, const char *end)
     break;
   default:
     LOG("mgmt", GB_LOG_ERROR,
-        "option type %d not supported!\n", option->type);
+        "option type %d not supported!", option->type);
     break;
   }
 }
@@ -459,7 +459,7 @@ glusterBlockLoadConfig(gbConfig *cfg, bool reloading)
   buf = glusterBlockReadConfig(cfg, &len);
   if (buf == NULL) {
     LOG("mgmt", GB_LOG_ERROR,
-        "Failed to read file '%s'\n", cfg->configPath);
+        "Failed to read file '%s'", cfg->configPath);
     return -1;
   }
 
@@ -476,62 +476,72 @@ glusterBlockDynConfigStart(void *arg)
   int monitor, wd, len;
   char buf[GB_BUF_LEN];
   struct inotify_event *event;
+  struct timespec mtim = {0, }; /* Time of last modification.  */
+  struct stat statbuf;
   char *p;
 
 
   monitor = inotify_init();
   if (monitor == -1) {
     LOG("mgmt", GB_LOG_ERROR,
-        "Failed to init inotify %d\n", monitor);
+        "Failed to init inotify %d", monitor);
     return NULL;
   }
 
-  wd = inotify_add_watch(monitor, cfg->configPath, IN_ALL_EVENTS);
+  /* Editors (vim, nano ..) follow different approaches to save conf file.
+   * The two commonly followed techniques are to overwrite the existing
+   * file, or to write to a new file (.swp, .tmp ..) and move it to actual
+   * file name later. In the later case, the inotify fails, because the
+   * file it's been intended to watch no longer exists, as the new file
+   * is a different file with just a same name.
+   * To handle both the file save approaches mentioned above, it is better
+   * we watch the directory and filter for MODIFY events.
+   */
+  wd = inotify_add_watch(monitor, GB_DEF_CONFIGDIR, IN_MODIFY);
   if (wd == -1) {
     LOG("mgmt", GB_LOG_ERROR,
-        "Failed to add \"%s\" to inotify (%d)\n", cfg->configPath, monitor);
+        "Failed to add \"%s\" to inotify (%d)", GB_DEF_CONFIGDIR, monitor);
     return NULL;
   }
 
   LOG("mgmt", GB_LOG_INFO,
-      "Inotify is watching \"%s\", wd: %d, mask: IN_ALL_EVENTS\n",
-      cfg->configPath, wd);
+      "Inotify is watching \"%s\", wd: %d, mask: IN_MODIFY", GB_DEF_CONFIGDIR, wd);
 
   while (1) {
     len = read(monitor, buf, GB_BUF_LEN);
     if (len == -1) {
-      LOG("mgmt", GB_LOG_WARNING, "Failed to read inotify: %d\n", len);
+      LOG("mgmt", GB_LOG_WARNING, "Failed to read inotify: %d", len);
       continue;
     }
 
-    for (p = buf; p < buf + len;) {
+    for (p = buf; p < buf + len; p += sizeof(*event) + event->len) {
       event = (struct inotify_event *)p;
 
-      LOG("mgmt", GB_LOG_INFO, "event->mask: 0x%x\n", event->mask);
+      LOG("mgmt", GB_LOG_INFO, "event->mask: 0x%x", event->mask);
 
       if (event->wd != wd) {
         continue;
       }
 
-      /*
-       * If force to write to the unwritable or crashed
-       * config file, the vi/vim will try to move and
-       * delete the config file and then recreate it again
-       * via the *.swp
-       */
-      if ((event->mask & IN_IGNORED) && !access(cfg->configPath, F_OK)) {
-        wd = inotify_add_watch(monitor, cfg->configPath, IN_ALL_EVENTS);
+      /* If stat fails we will skip the modify time check */
+      if (!stat(cfg->configPath, &statbuf)) {
+          if (statbuf.st_mtim.tv_sec == mtim.tv_sec &&
+              statbuf.st_mtim.tv_nsec == mtim.tv_nsec) {
+              continue;
+          }
       }
 
+      mtim.tv_sec = statbuf.st_mtim.tv_sec;
+      mtim.tv_nsec = statbuf.st_mtim.tv_nsec;
+
       /* Try to reload the config file */
-      if (event->mask & IN_MODIFY || event->mask & IN_IGNORED) {
+      if (event->mask & IN_MODIFY) {
         glusterBlockLoadConfig(cfg, true);
       }
-
-      p += sizeof(struct inotify_event) + event->len;
     }
   }
 
+  inotify_rm_watch(monitor, wd);
   return NULL;
 }
 
@@ -547,17 +557,17 @@ glusterBlockSetupConfig(const char *configPath)
   }
 
   if (GB_ALLOC(cfg) < 0) {
-    LOG("mgmt", GB_LOG_ERROR, "Alloc GB config failed for configPath: %s!\n", configPath);
+    LOG("mgmt", GB_LOG_ERROR, "Alloc GB config failed for configPath: %s!", configPath);
     return NULL;
   }
 
   if (GB_STRDUP(cfg->configPath, configPath) < 0) {
-    LOG("mgmt", GB_LOG_ERROR, "failed to copy configPath: %s\n", configPath);
+    LOG("mgmt", GB_LOG_ERROR, "failed to copy configPath: %s", configPath);
     goto freeConfig;
   }
 
   if (glusterBlockLoadConfig(cfg, false)) {
-    LOG("mgmt", GB_LOG_ERROR, "Loading GB config failed for configPath: %s!\n", configPath);
+    LOG("mgmt", GB_LOG_ERROR, "Loading GB config failed for configPath: %s!", configPath);
     goto freeConfigPath;
   }
 
@@ -568,7 +578,7 @@ glusterBlockSetupConfig(const char *configPath)
   ret = pthread_create(&cfg->threadId, NULL, glusterBlockDynConfigStart, cfg);
   if (ret) {
     LOG("mgmt", GB_LOG_WARNING,
-        "Dynamic config started failed, fallling back to static %d!\n", ret);
+        "Dynamic config started failed, fallling back to static %d!", ret);
   } else {
     cfg->isDynamic = true;
   }
@@ -592,18 +602,18 @@ glusterBlockCancelConfigThread(gbConfig *cfg)
 
   ret = pthread_cancel(threadId);
   if (ret) {
-    LOG("mgmt", GB_LOG_ERROR, "pthread_cancel failed with value %d\n", ret);
+    LOG("mgmt", GB_LOG_ERROR, "pthread_cancel failed with value %d", ret);
     return;
   }
 
   ret = pthread_join(threadId, &join_retval);
   if (ret) {
-    LOG("mgmt", GB_LOG_ERROR, "pthread_join failed with value %d\n", ret);
+    LOG("mgmt", GB_LOG_ERROR, "pthread_join failed with value %d", ret);
     return;
   }
 
   if (join_retval != PTHREAD_CANCELED) {
-    LOG("mgmt", GB_LOG_ERROR, "unexpected join retval: %p\n", join_retval);
+    LOG("mgmt", GB_LOG_ERROR, "unexpected join retval: %p", join_retval);
   }
 }
 

utils/lru.c

@@ -29,7 +29,7 @@ glusterBlockSetLruCount(const size_t lruCount)
     MSG(stderr, "glfsLruCount should be [0 < COUNT < %d]\n",
         LRU_COUNT_MAX);
     LOG("mgmt", GB_LOG_ERROR,
-        "glfsLruCount should be [0 < COUNT < %d]\n",
+        "glfsLruCount should be [0 < COUNT < %d]",
         LRU_COUNT_MAX);
     return -1;
   }
@@ -38,8 +38,8 @@ glusterBlockSetLruCount(const size_t lruCount)
   gbConf.glfsLruCount = lruCount;
   UNLOCK(gbConf.lock);
 
-  LOG("mgmt", GB_LOG_INFO,
-      "glfsLruCount now is %lu\n", lruCount);
+  LOG("mgmt", GB_LOG_CRIT,
+      "glfsLruCount now is %lu", lruCount);
   return 0;
 }
 

utils/utils.c

@@ -44,8 +44,8 @@ glusterBlockSetLogLevel(unsigned int logLevel)
   LOCK(gbConf.lock);
   gbConf.logLevel = logLevel;
   UNLOCK(gbConf.lock);
-  LOG("mgmt", GB_LOG_INFO,
-      "logLevel now is %s\n", LogLevelLookup[logLevel]);
+  LOG("mgmt", GB_LOG_CRIT,
+      "logLevel now is %s", LogLevelLookup[logLevel]);
 
   return 0;
 }

utils/utils.h

@@ -57,7 +57,8 @@
 
 # define  GB_METASTORE_RESERVE   10485760   /* 10 MiB reserve for block-meta */
 
-# define  GB_DEF_CONFIGPATH      "/etc/sysconfig/gluster-blockd" /* the default config file */
+# define  GB_DEF_CONFIGDIR       "/etc/sysconfig" /* the default config file directory */
+# define  GB_DEF_CONFIGPATH      GB_DEF_CONFIGDIR"/gluster-blockd" /* the default config file */
 
 # define  GB_TIME_STRING_BUFLEN  \
           (4 + 1 + 2 + 1 + 2 + 1 + 2 + 1 + 2 + 1 + 2 + 1 + 6 + 1 +   5)
@@ -456,17 +457,19 @@ static const char *const gbDaemonCmdlineOptLookup[] = {
 
 typedef enum LogLevel {
   GB_LOG_NONE       = 0,
-  GB_LOG_ERROR      = 1,
-  GB_LOG_WARNING    = 2,
-  GB_LOG_INFO       = 3,
-  GB_LOG_DEBUG      = 4,
-  GB_LOG_TRACE      = 5,
+  GB_LOG_CRIT       = 1,
+  GB_LOG_ERROR      = 2,
+  GB_LOG_WARNING    = 3,
+  GB_LOG_INFO       = 4,
+  GB_LOG_DEBUG      = 5,
+  GB_LOG_TRACE      = 6,
 
   GB_LOG_MAX
 } LogLevel;
 
 static const char *const LogLevelLookup[] = {
   [GB_LOG_NONE]       = "NONE",
+  [GB_LOG_CRIT]       = "CRIT",
   [GB_LOG_ERROR]      = "ERROR",
   [GB_LOG_WARNING]    = "WARNING",
   [GB_LOG_INFO]       = "INFO",