build: extend --enable-valgrind to support Memcheck and DRD

Extend '-enable-valgrind' to '--enable=valgrind[=memcheck,drd]'
to enable Memcheck or DRD Valgrind tool, respectively.

Change-Id: I80d13d72ba9756e0cbcdbeb6766b5c98e3e8c002
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Updates: #1002

12 files changed, 140 insertions, 41 deletions

diff --git a/configure.ac b/configure.ac
index cf22267aada..a13a3dbba82 100644
--- a/configure.ac
+++ b/configure.ac
@@ -409,12 +409,27 @@ esac
 
 # --enable-valgrind prevents calling dlclose(), this leaks memory
 AC_ARG_ENABLE([valgrind],
-              AC_HELP_STRING([--enable-valgrind],
-                             [Enable valgrind for resource leak debugging.]))
-if test "x$enable_valgrind" = "xyes"; then
-        AC_DEFINE(RUN_WITH_VALGRIND, 1, [define if all processes should run under valgrind])
-fi
-
+              AC_HELP_STRING([--enable-valgrind@<:@=memcheck,drd@:>@],
+                             [Enable valgrind for resource leak (memcheck, which is
+                              the default) or thread synchronization (drd) debugging.]))
+case x$enable_valgrind in
+  xmemcheck|xyes)
+    AC_DEFINE(RUN_WITH_MEMCHECK, 1,
+              [Define if all processes should run under 'valgrind --tool=memcheck'.])
+    VALGRIND_TOOL=memcheck
+    ;;
+  xdrd)
+    AC_DEFINE(RUN_WITH_DRD, 1,
+              [Define if all processes should run under 'valgrind --tool=drd'.])
+    VALGRIND_TOOL=drd
+    ;;
+  x|xno)
+    VALGRIND_TOOL=no
+    ;;
+  *)
+    AC_MSG_ERROR([Please specify --enable-valgrind@<:@=memcheck,drd@:>@])
+    ;;
+esac
 
 AC_ARG_WITH([previous-options],
         [AS_HELP_STRING([--with-previous-options],
@@ -1670,6 +1685,7 @@ echo "readline             : $BUILD_READLINE"
 echo "georeplication       : $BUILD_SYNCDAEMON"
 echo "Linux-AIO            : $BUILD_LIBAIO"
 echo "Enable Debug         : $BUILD_DEBUG"
+echo "Run with Valgrind    : $VALGRIND_TOOL"
 echo "Sanitizer enabled    : $SANITIZER"
 echo "Use syslog           : $USE_SYSLOG"
 echo "XML output           : $BUILD_XML_OUTPUT"
diff --git a/doc/developer-guide/identifying-resource-leaks.md b/doc/developer-guide/identifying-resource-leaks.md
index 851fc4424bc..950cae79b0a 100644
--- a/doc/developer-guide/identifying-resource-leaks.md
+++ b/doc/developer-guide/identifying-resource-leaks.md
@@ -174,3 +174,27 @@ In this case, the resource leak can be addressed by adding a single line to the
 
 Running the same Valgrind command and comparing the output will show that the
 memory leak in `xlators/meta/src/meta.c:init` is not reported anymore.
+
+### Running DRD, the Valgrind thread error detector
+
+When configuring GlusterFS with:
+
+```shell
+./configure --enable-valgrind
+```
+
+the default Valgrind tool (Memcheck) is enabled. But it's also possble to select
+one of Memcheck or DRD by using:
+
+```shell
+./configure --enable-valgrind=memcheck
+```
+
+or:
+
+```shell
+./configure --enable-valgrind=drd
+```
+
+respectively. When using DRD, it's recommended to consult
+https://valgrind.org/docs/manual/drd-manual.html before running.
diff --git a/extras/distributed-testing/distributed-test-runner.py b/extras/distributed-testing/distributed-test-runner.py
index 7bfb6c9652a..5a07e2feab1 100755
--- a/extras/distributed-testing/distributed-test-runner.py
+++ b/extras/distributed-testing/distributed-test-runner.py
@@ -383,14 +383,17 @@ class Handlers:
         return self.shell.call("make install") == 0
 
     @synchronized
-    def prove(self, id, test, timeout, valgrind=False, asan_noleaks=True):
+    def prove(self, id, test, timeout, valgrind="no", asan_noleaks=True):
         assert id == self.client_id
         self.shell.cd(self.gluster_root)
         env = "DEBUG=1 "
-        if valgrind:
+        if valgrind == "memcheck" or valgrind == "yes":
             cmd = "valgrind"
             cmd += " --tool=memcheck --leak-check=full --track-origins=yes"
             cmd += " --show-leak-kinds=all -v prove -v"
+        elif valgrind == "drd":
+            cmd = "valgrind"
+            cmd += " --tool=drd -v prove -v"
         elif asan_noleaks:
             cmd = "prove -v"
             env += "ASAN_OPTIONS=detect_leaks=0 "
@@ -827,8 +830,9 @@ parser.add_argument("--port", help="server port to listen",
                     type=int, default=DEFAULT_PORT)
 # test role
 parser.add_argument("--tester", help="start tester", action="store_true")
-parser.add_argument("--valgrind", help="run tests under valgrind",
-                    action="store_true")
+parser.add_argument("--valgrind[=memcheck,drd]",
+                    help="run tests with valgrind tool 'memcheck' or 'drd'",
+                    default="no")
 parser.add_argument("--asan", help="test with asan enabled",
                     action="store_true")
 parser.add_argument("--asan-noleaks", help="test with asan but no mem leaks",
diff --git a/libglusterfs/src/ctx.c b/libglusterfs/src/ctx.c
index 4a001c29209..3d890b04ec9 100644
--- a/libglusterfs/src/ctx.c
+++ b/libglusterfs/src/ctx.c
@@ -37,8 +37,12 @@ glusterfs_ctx_new()
 
     ctx->log.loglevel = DEFAULT_LOG_LEVEL;
 
-#ifdef RUN_WITH_VALGRIND
-    ctx->cmd_args.valgrind = _gf_true;
+#if defined(RUN_WITH_MEMCHECK)
+    ctx->cmd_args.vgtool = _gf_memcheck;
+#elif defined(RUN_WITH_DRD)
+    ctx->cmd_args.vgtool = _gf_drd;
+#else
+    ctx->cmd_args.vgtool = _gf_none;
 #endif
 
     /* lock is never destroyed! */
diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h
index fc69ad0b749..e6425618b7f 100644
--- a/libglusterfs/src/glusterfs/glusterfs.h
+++ b/libglusterfs/src/glusterfs/glusterfs.h
@@ -466,6 +466,8 @@ typedef struct _server_cmdline server_cmdline_t;
 #define GF_OPTION_DISABLE _gf_false
 #define GF_OPTION_DEFERRED 2
 
+typedef enum { _gf_none, _gf_memcheck, _gf_drd } gf_valgrind_tool;
+
 struct _cmd_args {
     /* basic options */
     char *volfile_server;
@@ -558,7 +560,8 @@ struct _cmd_args {
     /* Run this process with valgrind? Might want to prevent calling
      * functions that prevent valgrind from working correctly, like
      * dlclose(). */
-    int valgrind;
+    gf_valgrind_tool vgtool;
+
     int localtime_logging;
 
     /* For the subdir mount */
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c
index 539da7f4716..a54aadd76c7 100644
--- a/libglusterfs/src/xlator.c
+++ b/libglusterfs/src/xlator.c
@@ -821,7 +821,7 @@ xlator_members_free(xlator_t *xl)
 
     GF_FREE(xl->name);
     GF_FREE(xl->type);
-    if (!(xl->ctx && xl->ctx->cmd_args.valgrind) && xl->dlhandle)
+    if (!(xl->ctx && xl->ctx->cmd_args.vgtool != _gf_none) && xl->dlhandle)
         dlclose(xl->dlhandle);
     if (xl->options)
         dict_unref(xl->options);
diff --git a/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c b/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c
index b01fd4da24b..a0bfea41f0f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c
+++ b/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c
@@ -310,7 +310,7 @@ glusterd_gfproxydsvc_start(glusterd_svc_t *svc, int flags)
     }
     runinit(&runner);
 
-    if (this->ctx->cmd_args.valgrind) {
+    if (this->ctx->cmd_args.vgtool != _gf_none) {
         len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s",
                        svc->proc.logdir, svc->proc.logfile);
         if ((len < 0) || (len >= PATH_MAX)) {
@@ -318,8 +318,13 @@ glusterd_gfproxydsvc_start(glusterd_svc_t *svc, int flags)
             goto out;
         }
 
-        runner_add_args(&runner, "valgrind", "--leak-check=full",
-                        "--trace-children=yes", "--track-origins=yes", NULL);
+        if (this->ctx->cmd_args.vgtool == _gf_memcheck)
+            runner_add_args(&runner, "valgrind", "--leak-check=full",
+                            "--trace-children=yes", "--track-origins=yes",
+                            NULL);
+        else
+            runner_add_args(&runner, "valgrind", "--tool=drd", NULL);
+
         runner_argprintf(&runner, "--log-file=%s", valgrind_logfile);
     }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
index 2afd0fe1b74..458bf168ede 100644
--- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c
+++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
@@ -219,6 +219,9 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr,
     char valgrind_logfile[PATH_MAX] = {
         0,
     };
+    char msg[1024] = {
+        0,
+    };
     char *volfileserver = NULL;
     char *localtime_logging = NULL;
 
@@ -270,12 +273,17 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr,
              "rebalance");
     runinit(&runner);
 
-    if (this->ctx->cmd_args.valgrind) {
+    if (this->ctx->cmd_args.vgtool != _gf_none) {
         snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s-rebalance.log",
                  priv->logdir, volinfo->volname);
 
-        runner_add_args(&runner, "valgrind", "--leak-check=full",
-                        "--trace-children=yes", "--track-origins=yes", NULL);
+        if (this->ctx->cmd_args.vgtool == _gf_memcheck)
+            runner_add_args(&runner, "valgrind", "--leak-check=full",
+                            "--trace-children=yes", "--track-origins=yes",
+                            NULL);
+        else
+            runner_add_args(&runner, "valgrind", "--tool=drd", NULL);
+
         runner_argprintf(&runner, "--log-file=%s", valgrind_logfile);
     }
 
@@ -316,6 +324,10 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr,
             runner_add_arg(&runner, "--localtime-logging");
     }
 
+    snprintf(msg, sizeof(msg), "Starting the rebalance service for volume %s",
+             volinfo->volname);
+    runner_log(&runner, this->name, GF_LOG_DEBUG, msg);
+
     ret = runner_run_nowait(&runner);
     if (ret) {
         gf_msg_debug("glusterd", 0, "rebalance command failed");
diff --git a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c
index 1f3f4909cbb..d75f249b29e 100644
--- a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c
+++ b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c
@@ -304,7 +304,7 @@ glusterd_snapdsvc_start(glusterd_svc_t *svc, int flags)
     }
     runinit(&runner);
 
-    if (this->ctx->cmd_args.valgrind) {
+    if (this->ctx->cmd_args.vgtool != _gf_none) {
         len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-snapd.log",
                        svc->proc.logdir);
         if ((len < 0) || (len >= PATH_MAX)) {
@@ -313,8 +313,13 @@ glusterd_snapdsvc_start(glusterd_svc_t *svc, int flags)
             goto out;
         }
 
-        runner_add_args(&runner, "valgrind", "--leak-check=full",
-                        "--trace-children=yes", "--track-origins=yes", NULL);
+        if (this->ctx->cmd_args.vgtool == _gf_memcheck)
+            runner_add_args(&runner, "valgrind", "--leak-check=full",
+                            "--trace-children=yes", "--track-origins=yes",
+                            NULL);
+        else
+            runner_add_args(&runner, "valgrind", "--tool=drd", NULL);
+
         runner_argprintf(&runner, "--log-file=%s", valgrind_logfile);
     }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
index 99119d69e45..18b3fb13630 100644
--- a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
+++ b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
@@ -162,6 +162,9 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline)
     char *localtime_logging = NULL;
     char *log_level = NULL;
     char daemon_log_level[30] = {0};
+    char msg[1024] = {
+        0,
+    };
     int32_t len = 0;
 
     this = THIS;
@@ -187,7 +190,7 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline)
 
         runinit(&runner);
 
-        if (this->ctx->cmd_args.valgrind) {
+        if (this->ctx->cmd_args.vgtool != _gf_none) {
             len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s.log",
                            svc->proc.logdir, svc->name);
             if ((len < 0) || (len >= PATH_MAX)) {
@@ -195,9 +198,13 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline)
                 goto unlock;
             }
 
-            runner_add_args(&runner, "valgrind", "--leak-check=full",
-                            "--trace-children=yes", "--track-origins=yes",
-                            NULL);
+            if (this->ctx->cmd_args.vgtool == _gf_memcheck)
+                runner_add_args(&runner, "valgrind", "--leak-check=full",
+                                "--trace-children=yes", "--track-origins=yes",
+                                NULL);
+            else
+                runner_add_args(&runner, "valgrind", "--tool=drd", NULL);
+
             runner_argprintf(&runner, "--log-file=%s", valgrind_logfile);
         }
 
@@ -226,8 +233,8 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline)
         if (cmdline)
             dict_foreach(cmdline, svc_add_args, (void *)&runner);
 
-        gf_msg(this->name, GF_LOG_INFO, 0, GD_MSG_SVC_START_SUCCESS,
-               "Starting %s service", svc->name);
+        snprintf(msg, sizeof(msg), "Starting %s service", svc->name);
+        runner_log(&runner, this->name, GF_LOG_DEBUG, msg);
 
         if (flags == PROC_START_NO_WAIT) {
             ret = runner_run_nowait(&runner);
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 04790bbc75c..d7446a099a2 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -2076,8 +2076,8 @@ glusterd_volume_start_glusterfs(glusterd_volinfo_t *volinfo,
 retry:
     runinit(&runner);
 
-    if (this->ctx->cmd_args.valgrind) {
-        /* Run bricks with valgrind */
+    if (this->ctx->cmd_args.vgtool != _gf_none) {
+        /* Run bricks with valgrind. */
         if (volinfo->logdir) {
             len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s-%s.log",
                            volinfo->logdir, volinfo->volname, exp_path);
@@ -2091,8 +2091,13 @@ retry:
             goto out;
         }
 
-        runner_add_args(&runner, "valgrind", "--leak-check=full",
-                        "--trace-children=yes", "--track-origins=yes", NULL);
+        if (this->ctx->cmd_args.vgtool == _gf_memcheck)
+            runner_add_args(&runner, "valgrind", "--leak-check=full",
+                            "--trace-children=yes", "--track-origins=yes",
+                            NULL);
+        else
+            runner_add_args(&runner, "valgrind", "--tool=drd", NULL);
+
         runner_argprintf(&runner, "--log-file=%s", valgrind_logfile);
     }
 
@@ -2205,7 +2210,7 @@ retry:
     if (is_brick_mx_enabled())
         runner_add_arg(&runner, "--brick-mux");
 
-    runner_log(&runner, "", 0, "Starting GlusterFS");
+    runner_log(&runner, "", GF_LOG_DEBUG, "Starting GlusterFS");
 
     brickinfo->port = port;
     brickinfo->rdma_port = rdma_port;
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index 91c5f9ec5e3..7a86c2997b1 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -1423,7 +1423,7 @@ init(xlator_t *this)
     char *mountbroker_root = NULL;
     int i = 0;
     int total_transport = 0;
-    gf_boolean_t valgrind = _gf_false;
+    gf_valgrind_tool vgtool;
     char *valgrind_str = NULL;
     char *transport_type = NULL;
     char var_run_dir[PATH_MAX] = {
@@ -1436,6 +1436,14 @@ init(xlator_t *this)
     int32_t len = 0;
     int op_version = 0;
 
+#if defined(RUN_WITH_MEMCHECK)
+    vgtool = _gf_memcheck;
+#elif defined(RUN_WITH_DRD)
+    vgtool = _gf_drd;
+#else
+    vgtool = _gf_none;
+#endif
+
 #ifndef GF_DARWIN_HOST_OS
     {
         struct rlimit lim;
@@ -1925,18 +1933,24 @@ init(xlator_t *this)
     }
 
     /* Set option to run bricks on valgrind if enabled in glusterd.vol */
-    this->ctx->cmd_args.valgrind = valgrind;
+    this->ctx->cmd_args.vgtool = vgtool;
     ret = dict_get_str(this->options, "run-with-valgrind", &valgrind_str);
     if (ret < 0) {
         gf_msg_debug(this->name, 0, "cannot get run-with-valgrind value");
     }
     if (valgrind_str) {
-        if (gf_string2boolean(valgrind_str, &valgrind)) {
+        gf_boolean_t vg = _gf_false;
+
+        if (!strcmp(valgrind_str, "memcheck"))
+            this->ctx->cmd_args.vgtool = _gf_memcheck;
+        else if (!strcmp(valgrind_str, "drd"))
+            this->ctx->cmd_args.vgtool = _gf_drd;
+        else if (!gf_string2boolean(valgrind_str, &vg))
+            this->ctx->cmd_args.vgtool = (vg ? _gf_memcheck : _gf_none);
+        else
             gf_msg(this->name, GF_LOG_WARNING, EINVAL, GD_MSG_INVALID_ENTRY,
-                   "run-with-valgrind value not a boolean string");
-        } else {
-            this->ctx->cmd_args.valgrind = valgrind;
-        }
+                   "run-with-valgrind is neither boolean"
+                   " nor one of 'memcheck' or 'drd'");
     }
 
     /* Store ping-timeout in conf */