reimplement invocation of external programs with run API

Signed-off-by: Csaba Henk <csaba@gluster.com>
Signed-off-by: Anand Avati <avati@gluster.com>

BUG: 2562 (invoke external commands precisely with fork + exec)
URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=2562

4 files changed, 234 insertions, 264 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index e7ed6d53e..d0ad5745e 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -49,6 +49,7 @@
 #include "syscall.h"
 #include "cli1.h"
 #include "common-utils.h"
+#include "run.h"
 
 #include <sys/types.h>
 #include <signal.h>
@@ -103,11 +104,6 @@ static char *gsync_reserved_opts[] = {
         NULL
 };
 
-typedef struct glusterd_quota_child_info {
-        pid_t pid;
-        char  mountdir [15];
-} glusterd_quota_child_info_t;
-
 static int
 glusterd_restart_brick_servers (glusterd_volinfo_t *);
 
@@ -1833,25 +1829,25 @@ out:
 
 
 int
-glusterd_query_extutil (char *resbuf, char *cmd)
+glusterd_query_extutil (char *resbuf, runner_t *runner)
 {
-        FILE               *in  = NULL;
         char               *ptr = NULL;
         int                 ret = 0;
 
-        if (!(in = popen(cmd, "r"))) {
-                gf_log ("", GF_LOG_ERROR, "popen failed");
+        runner_redir (runner, STDOUT_FILENO, RUN_PIPE);
+        if (runner_start (runner) != 0) {
+                gf_log ("", GF_LOG_ERROR, "spawning child failed");
+
                 return -1;
         }
 
-        ptr = fgets(resbuf, PATH_MAX, in);
+        ptr = fgets(resbuf, PATH_MAX, runner_chio (runner, STDOUT_FILENO));
         if (ptr)
                 resbuf[strlen(resbuf)-1] = '\0'; //strip off \n
 
-        ret |= pclose (in);
-
+        ret = runner_end (runner);
         if (ret)
-                gf_log ("", GF_LOG_ERROR, "popen failed");
+                gf_log ("", GF_LOG_ERROR, "reading data from child failed");
 
         return ret ? -1 : 0;
 }
@@ -1859,7 +1855,7 @@ glusterd_query_extutil (char *resbuf, char *cmd)
 static int
 glusterd_get_canon_url (char *cann, char *name, gf_boolean_t cann_esc)
 {
-        char                cmd[PATH_MAX] = {0, };
+        runner_t            runner = {0,};
         glusterd_conf_t    *priv  = NULL;
 
         GF_ASSERT (THIS);
@@ -1867,29 +1863,29 @@ glusterd_get_canon_url (char *cann, char *name, gf_boolean_t cann_esc)
 
         priv = THIS->private;
 
-        snprintf (cmd, PATH_MAX, GSYNCD_PREFIX"/gsyncd --canonicalize-%surl %s",
-                  cann_esc? "escape-": "",name);
+        runinit (&runner);
+        runner_add_arg (&runner, GSYNCD_PREFIX"/gsyncd");
+        runner_argprintf (&runner, "--canonicalize-%surl",
+                          cann_esc ? "escape-" : "");
+        runner_add_arg(&runner, name);
 
-        return glusterd_query_extutil (cann, cmd);
+        return glusterd_query_extutil (cann, &runner);
 }
 
 int
 glusterd_gsync_get_param_file (char *prmfile, const char *param, char *master,
                                char *slave, char *gl_workdir)
 {
-        char                cmd[PATH_MAX] = {0, };
-        glusterd_conf_t    *priv  = NULL;
+        runner_t            runner = {0,};
 
-        GF_ASSERT (THIS);
-        GF_ASSERT (THIS->private);
+        runinit (&runner);
+        runner_add_args  (&runner, GSYNCD_PREFIX"/gsyncd", "-c", NULL);
+        runner_argprintf (&runner, "%s/"GSYNC_CONF, gl_workdir);
+        runner_argprintf (&runner, ":%s", master);
+        runner_add_args  (&runner, slave, "--config-get", NULL);
+        runner_argprintf (&runner, "%s-file", param);
 
-        priv = THIS->private;
-
-        snprintf (cmd, PATH_MAX,
-                  GSYNCD_PREFIX"/gsyncd -c %s/"GSYNC_CONF" :%s %s --config-get %s-file",
-                  gl_workdir, master, slave, param);
-
-        return glusterd_query_extutil (prmfile, cmd);
+        return glusterd_query_extutil (prmfile, &runner);
 }
 
 static int
@@ -1980,7 +1976,6 @@ out:
 int
 gsync_verify_config_options (dict_t *dict, char **op_errstr)
 {
-        char    cmd[PATH_MAX] = {0,};
         char  **resopt    = NULL;
         int     i         = 0;
         char   *subop     = NULL;
@@ -2010,8 +2005,7 @@ gsync_verify_config_options (dict_t *dict, char **op_errstr)
                 return -1;
         }
 
-        snprintf (cmd, PATH_MAX, GSYNCD_PREFIX"/gsyncd --config-check %s", op_name);
-        if (system (cmd)) {
+        if (runcmd (GSYNCD_PREFIX"/gsyncd", "--config-check", op_name, NULL)) {
                 gf_log ("", GF_LOG_WARNING, "Invalid option %s", op_name);
                 *op_errstr = gf_strdup ("Invalid option");
 
@@ -2990,7 +2984,7 @@ rb_spawn_dst_brick (glusterd_volinfo_t *volinfo,
                     glusterd_brickinfo_t *brickinfo)
 {
         glusterd_conf_t    *priv           = NULL;
-        char                cmd_str[8192]  = {0,};
+        runner_t            runner         = {0,};
         int                 ret            = -1;
         int32_t             port           = 0;
 
@@ -3001,16 +2995,16 @@ rb_spawn_dst_brick (glusterd_volinfo_t *volinfo,
 
         GF_ASSERT (port);
 
-        snprintf (cmd_str, 8192,
-                  "%s/sbin/glusterfs -f %s/vols/%s/%s -p %s/vols/%s/%s "
-                  "--xlator-option src-server.listen-port=%d",
-                  GFS_PREFIX, priv->workdir, volinfo->volname,
-                  RB_DSTBRICKVOL_FILENAME,
-                  priv->workdir, volinfo->volname,
-                  RB_DSTBRICK_PIDFILE,
-                  port);
+        runinit (&runner);
+        runner_add_arg (&runner, GFS_PREFIX"/sbin/glusterfs");
+        runner_argprintf (&runner, "-f" "%s/vols/%s/"RB_DSTBRICKVOL_FILENAME,
+                          priv->workdir, volinfo->volname);
+        runner_argprintf (&runner, "-p" "%s/vols/%s/"RB_DSTBRICK_PIDFILE,
+                          priv->workdir, volinfo->volname);
+        runner_add_arg (&runner, "--xlator-option");
+        runner_argprintf (&runner, "src-server.listen-port=%d", port);
 
-        ret = gf_system (cmd_str);
+        ret = runner_run (&runner);
         if (ret) {
                 gf_log ("", GF_LOG_DEBUG,
                         "Could not start glusterfs");
@@ -3033,19 +3027,20 @@ rb_spawn_glusterfs_client (glusterd_volinfo_t *volinfo,
 {
         glusterd_conf_t    *priv           = NULL;
         char                cmd_str[8192]  = {0,};
+        runner_t            runner         = {0,};
         struct stat         buf;
         int                 ret            = -1;
 
         priv = THIS->private;
 
-        snprintf (cmd_str, 4096,
-                  "%s/sbin/glusterfs -f %s/vols/%s/%s %s/vols/%s/%s",
-                  GFS_PREFIX, priv->workdir, volinfo->volname,
-                  RB_CLIENTVOL_FILENAME,
-                  priv->workdir, volinfo->volname,
-                  RB_CLIENT_MOUNTPOINT);
+        runinit (&runner);
+        runner_add_arg (&runner, GFS_PREFIX"/sbin/glusterfs");
+        runner_argprintf (&runner, "-f" "%s/vols/%s/"RB_CLIENTVOL_FILENAME,
+                          priv->workdir, volinfo->volname);
+        runner_argprintf (&runner, "%s/vols/%s/"RB_CLIENT_MOUNTPOINT,
+                          priv->workdir, volinfo->volname);
 
-        ret = gf_system (cmd_str);
+        ret = runner_run (&runner);
         if (ret) {
                 gf_log ("", GF_LOG_DEBUG,
                         "Could not start glusterfs");
@@ -3239,7 +3234,7 @@ rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo,
                                 glusterd_brickinfo_t *src_brickinfo)
 {
         glusterd_conf_t  *priv                        = NULL;
-        char              cmd_str[8192]               = {0,};
+        runner_t          runner                      = {0,};
         char              filename[PATH_MAX]          = {0,};
         struct stat       buf;
         char              mount_point_path[PATH_MAX]  = {0,};
@@ -3259,11 +3254,12 @@ rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo,
                 goto out;
         }
 
-        snprintf (cmd_str, 8192, "/bin/umount -f %s/vols/%s/%s",
-                  priv->workdir, volinfo->volname,
-                  RB_CLIENT_MOUNTPOINT);
+        runinit (&runner);
+        runner_add_args (&runner, "/bin/umount", "-f", NULL);
+        runner_argprintf (&runner, "%s/vols/%s/"RB_CLIENT_MOUNTPOINT,
+                          priv->workdir, volinfo->volname);
 
-        ret = gf_system (cmd_str);
+        ret = runner_run (&runner);
         if (ret) {
                 gf_log ("", GF_LOG_DEBUG,
                         "umount failed on maintenance client");
@@ -4204,12 +4200,9 @@ glusterd_gsync_configure (glusterd_volinfo_t *volinfo, char *slave,
         int32_t         ret     = -1;
         char            *op_name = NULL;
         char            *op_value = NULL;
-        char            cmd[1024] = {0,};
+        runner_t        runner    = {0,};
         glusterd_conf_t *priv   = NULL;
         char            *subop  = NULL;
-        char            *q1 = NULL;
-        char            *q2 = NULL;
-        char            *cm = NULL;
         char            *master = NULL;
 
         GF_ASSERT (slave);
@@ -4233,12 +4226,6 @@ glusterd_gsync_configure (glusterd_volinfo_t *volinfo, char *slave,
                 ret = dict_get_str (dict, "op_value", &op_value);
                 if (ret != 0)
                         goto out;
-                q1 = " \"";
-                q2 = "\"";
-        } else {
-                q1 = "";
-                op_value = "";
-                q2 = "";
         }
 
         if (THIS)
@@ -4249,19 +4236,20 @@ glusterd_gsync_configure (glusterd_volinfo_t *volinfo, char *slave,
                 goto out;
         }
 
+        master = "";
+        runinit (&runner);
+        runner_add_args (&runner, GSYNCD_PREFIX"/gsyncd", "-c", NULL);
+        runner_argprintf (&runner, "%s/"GSYNC_CONF, priv->workdir);
         if (volinfo) {
-                cm = ":";
                 master = volinfo->volname;
-        } else {
-                cm = "";
-                master = "";
+                runner_argprintf (&runner, ":%s", master);
         }
-
-        ret = snprintf (cmd, 1024, GSYNCD_PREFIX"/gsyncd -c %s/"GSYNC_CONF" %s%s %s"
-                        " --config-%s %s" "%s%s%s", priv->workdir,
-                        cm, master, slave, subop, op_name,
-                        q1, op_value, q2);
-        ret = system (cmd);
+        runner_add_arg (&runner, slave);
+        runner_argprintf (&runner, "--config-%s", subop);
+        runner_add_arg (&runner, op_name);
+        if (op_value)
+                runner_add_arg (&runner, op_value);
+        ret = runner_run (&runner);
         if (ret) {
                 gf_log ("", GF_LOG_WARNING, "gsyncd failed to "
                         "%s %s option for %s %s peers",
@@ -4838,47 +4826,14 @@ out:
         return ret;
 }
 
-
-static void *
-glusterd_quota_child_waitpid (void *arg)
-{
-        char cmd [PATH_MAX] = {0,};
-        glusterd_quota_child_info_t *child_info = NULL;
-
-        GF_VALIDATE_OR_GOTO ("glusterd", arg, out);
-
-        child_info = (glusterd_quota_child_info_t *)arg;
-
-#ifdef GF_LINUX_HOST_OS
-        snprintf (cmd, sizeof (cmd), "umount -l %s",
-                  child_info->mountdir);
-        system (cmd);
-
-        waitpid (child_info->pid, NULL, 0);
-#else
-        waitpid (child_info->pid, NULL, 0);
-        snprintf (cmd, sizeof (cmd), "umount %s",
-                  child_info->mountdir);
-        system (cmd);
-#endif
-        rmdir (child_info->mountdir);
-
-        GF_FREE (child_info);
-
-out:
-        return NULL;
-}
-
 int32_t
 glusterd_quota_initiate_fs_crawl (glusterd_conf_t *priv, char *volname)
 {
         int32_t   ret = 0;
-        pthread_t th;
         pid_t     pid;
-        int32_t   idx = 0;
         char      mountdir [] = "/tmp/mntXXXXXX";
-        char      cmd_str [PATH_MAX + 1024]  = {0, };
-        glusterd_quota_child_info_t *child_info = NULL;
+        runner_t  runner = {0,};
+        int       status = 0;
 
         if (mkdtemp (mountdir) == NULL) {
                 gf_log ("glusterd", GF_LOG_DEBUG,
@@ -4887,19 +4842,21 @@ glusterd_quota_initiate_fs_crawl (glusterd_conf_t *priv, char *volname)
                 goto out;
         }
 
-        snprintf (cmd_str, sizeof (cmd_str), "%s/sbin/glusterfs -s localhost "
-                  "--volfile-id %s %s", GFS_PREFIX, volname, mountdir);
-
-        ret = gf_system (cmd_str);
+        runinit (&runner);
+        runner_add_args (&runner, GFS_PREFIX"/sbin/glusterfs", "-s",
+                         "localhost", "--volfile-id", volname, mountdir, NULL);
+        ret = runner_run_reuse (&runner);
         if (ret == -1) {
-                gf_log("glusterd", GF_LOG_DEBUG, "command: %s failed", cmd_str);
+                runner_log (&runner, "glusterd", GF_LOG_DEBUG, "command failed");
+                runner_end (&runner);
                 goto out;
         }
+        runner_end (&runner);
 
         if ((pid = fork ()) < 0) {
                 gf_log ("glusterd", GF_LOG_WARNING, "fork from parent failed");
                 ret = -1;
-                goto err;
+                goto out;
         } else if (pid == 0) {//first child
                 ret = chdir (mountdir);
                 if (ret == -1) {
@@ -4908,36 +4865,30 @@ glusterd_quota_initiate_fs_crawl (glusterd_conf_t *priv, char *volname)
                         exit (EXIT_FAILURE);
                 }
 
-                /* close all fd's */
-                for (idx = 3; idx < 65536; idx++) {
-                        close (idx);
-                }
-
-                execl ("/usr/bin/find", "find", ".", (char *) 0);
-                _exit (0);
-        } else {
-                ret = -1;
-
-                child_info = GF_MALLOC (sizeof (glusterd_quota_child_info_t),
-                                        gf_common_mt_char);
-                if (child_info == NULL)
-                        goto err;
-
-                strcpy (child_info->mountdir, mountdir);
-
-                child_info->pid = pid;
-                pthread_create (&th, NULL, glusterd_quota_child_waitpid, child_info);
-
-                ret = 0;
+#ifndef GF_LINUX_HOST_OS
+                /* fork one more to not hold back main process on
+                 * blocking call below
+                 */
+                pid = fork ();
+                if (pid)
+                        _exit (pid > 0 ? EXIT_SUCCESS : EXIT_FAILURE);
+#endif
 
-                goto out;
-        }
-err:
-        if (ret < 0) {
-                umount (mountdir);
-                if (child_info)
-                        GF_FREE (child_info);
+                runinit (&runner);
+                runner_add_args (&runner, "/usr/bin/find", "find", ".", NULL);
+                if (runner_start (&runner) == -1)
+                        _exit (EXIT_FAILURE);
+#ifndef GF_LINUX_HOST_OS
+                runner_end (&runner); /* blocks in waitpid */
+                runcmd ("umount", mountdir, NULL);
+#else
+                runcmd ("umount", "-l", mountdir, NULL);
+#endif
+                rmdir (mountdir);
+                _exit (EXIT_SUCCESS);
         }
+        ret = (waitpid (pid, &status, 0) == pid &&
+               WIFEXITED (status) && WEXITSTATUS (status) == EXIT_SUCCESS) ? 0 : -1;
 
 out:
         return ret;
diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
index fec0a1b2e..ee778d215 100644
--- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c
+++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
@@ -36,6 +36,7 @@
 #include "glusterd-op-sm.h"
 #include "glusterd-utils.h"
 #include "glusterd-store.h"
+#include "run.h"
 
 #include "syscall.h"
 #include "cli1.h"
@@ -264,7 +265,6 @@ glusterd_defrag_start (void *data)
 {
         glusterd_volinfo_t     *volinfo = data;
         glusterd_defrag_info_t *defrag  = NULL;
-        char                    cmd_str[1024] = {0,};
         int                     ret     = -1;
         struct stat             stbuf   = {0,};
         char                    value[128] = {0,};
@@ -339,8 +339,7 @@ out:
                 gf_log ("rebalance", GF_LOG_INFO, "rebalance on %s complete",
                         defrag->mount);
 
-                snprintf (cmd_str, 1024, "umount -l %s", defrag->mount);
-                ret = system (cmd_str);
+                ret = runcmd ("umount", "-l", defrag->mount, NULL);
                 LOCK_DESTROY (&defrag->lock);
                 GF_FREE (defrag);
         }
@@ -527,7 +526,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr,
 {
         int                    ret = -1;
         glusterd_defrag_info_t *defrag =  NULL;
-        char                   cmd_str[4096] = {0,};
+        runner_t               runner = {0,};
         glusterd_conf_t        *priv = NULL;
 
         priv    = THIS->private;
@@ -552,35 +551,41 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr,
         snprintf (defrag->mount, 1024, "%s/mount/%s",
                   priv->workdir, volinfo->volname);
         /* Create a directory, mount glusterfs over it, start glusterfs-defrag */
-        snprintf (cmd_str, sizeof (cmd_str), "mkdir -p %s", defrag->mount);
-        ret = system (cmd_str);
-
+        runinit (&runner);
+        runner_add_args (&runner, "mkdir", "-p", defrag->mount, NULL);
+        ret = runner_run_reuse (&runner);
         if (ret) {
-                gf_log("glusterd", GF_LOG_DEBUG, "command: %s failed", cmd_str);
+                runner_log (&runner, "glusterd", GF_LOG_DEBUG, "command failed");
+                runner_end (&runner);
                 goto out;
         }
-
-        snprintf (cmd_str, sizeof (cmd_str),
-                  "%s/sbin/glusterfs -s localhost --volfile-id %s "
-                  "--xlator-option *dht.use-readdirp=yes "
-                  "--xlator-option *dht.lookup-unhashed=yes %s",
-                  GFS_PREFIX, volinfo->volname,
-                  defrag->mount);
-        ret = gf_system (cmd_str);
+        runner_end (&runner);
+
+        runinit (&runner);
+        runner_add_args (&runner, GFS_PREFIX"/sbin/glusterfs",
+                         "-s", "localhost", "--volfile-id", volinfo->volname,
+                         "--xlator-option", "*dht.use-readdirp=yes",
+                         "--xlator-option", "*dht.lookup-unhashed=yes",
+                         volinfo->volname, defrag->mount, NULL);
+        ret = runner_run_reuse (&runner);
         if (ret) {
-                gf_log("glusterd", GF_LOG_DEBUG, "command: %s failed", cmd_str);
+                runner_log (&runner, "glusterd", GF_LOG_DEBUG, "command failed");
+                runner_end (&runner);
                 goto out;
         }
+        runner_end (&runner);
 
         volinfo->defrag_status = GF_DEFRAG_STATUS_LAYOUT_FIX_STARTED;
 
         ret = pthread_create (&defrag->th, NULL, glusterd_defrag_start,
                               volinfo);
         if (ret) {
-                snprintf (cmd_str, sizeof (cmd_str), "umount -l %s", defrag->mount);
-                if (system (cmd_str))
-                        gf_log("glusterd", GF_LOG_DEBUG, "command: %s "
-                               "failed", cmd_str);
+                runinit (&runner);
+                runner_add_args (&runner, "umount", "-l", defrag->mount, NULL);
+                ret = runner_run_reuse (&runner);
+                if (ret)
+                        runner_log (&runner, "glusterd", GF_LOG_DEBUG, "command failed");
+                runner_end (&runner);
         }
 out:
         gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index c1c9481e1..2c1d73681 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -33,6 +33,7 @@
 #include "defaults.h"
 #include "compat.h"
 #include "md5.h"
+#include "run.h"
 #include "compat-errno.h"
 #include "statedump.h"
 #include "syscall.h"
@@ -1022,7 +1023,7 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t  *volinfo,
         char                    pidfile[PATH_MAX] = {0,};
         char                    volfile[PATH_MAX] = {0,};
         char                    path[PATH_MAX] = {0,};
-        char                    cmd_str[8192] = {0,};
+        runner_t                runner = {0,};
         char                    rundir[PATH_MAX] = {0,};
         char                    exp_path[PATH_MAX] = {0,};
         char                    logfile[PATH_MAX] = {0,};
@@ -1110,15 +1111,19 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t  *volinfo,
         if (!port)
                 port = pmap_registry_alloc (THIS);
 
-        snprintf (cmd_str, 8192,
-                  "%s/sbin/glusterfsd --xlator-option %s-server.listen-port=%d "
-                  "-s localhost --volfile-id %s -p %s -S %s --brick-name %s "
-                  "--brick-port %d -l %s", GFS_PREFIX, volinfo->volname,
-                  port, volfile, pidfile, socketpath, brickinfo->path, port,
-                  brickinfo->logfile);
+        runinit (&runner);
+        runner_add_args (&runner, GFS_PREFIX"/sbin/glusterfsd",
+                         "-s", "localhost", "--volfile-id", volfile,
+                         "-p", pidfile, "-S", socketpath,
+                         "--brick-name", brickinfo->path,
+                         "-l", brickinfo->logfile, "--brick-port",  NULL);
+        runner_argprintf (&runner, "%d", port);
+        runner_add_arg (&runner, "--xlator-option");
+        runner_argprintf (&runner, "%s-server.listen-port=%d",
+                          volinfo->volname, port);
 
-	gf_log ("",GF_LOG_DEBUG,"Starting GlusterFS Command Executed: \n %s \n", cmd_str);
-        ret = gf_system (cmd_str);
+        runner_log (&runner, "", GF_LOG_DEBUG, "Starting GlusterFS");
+        ret = runner_run (&runner);
 
         if (ret == 0) {
                 //pmap_registry_bind (THIS, port, brickinfo->path);
@@ -1252,7 +1257,6 @@ glusterd_volume_compute_cksum (glusterd_volinfo_t  *volinfo)
         char                    buf[4096] = {0,};
         char                    sort_filepath[PATH_MAX] = {0};
         gf_boolean_t            unlink_sortfile = _gf_false;
-        char                    sort_cmd[2*PATH_MAX + 32];
         int                     sort_fd = 0;
 
         GF_ASSERT (volinfo);
@@ -1288,9 +1292,7 @@ glusterd_volume_compute_cksum (glusterd_volinfo_t  *volinfo)
                 unlink_sortfile = _gf_true;
         }
 
-        snprintf (sort_cmd, sizeof (sort_cmd), "sort %s -o %s",
-                  filepath, sort_filepath);
-        ret = system (sort_cmd);
+        ret = runcmd ("sort", filepath, "-o", sort_filepath, NULL);
         if (ret) {
                 gf_log ("", GF_LOG_ERROR, "failed to sort file %s to %s",
                         filepath, sort_filepath);
@@ -2157,7 +2159,6 @@ glusterd_nfs_server_start ()
         char                    logfile[PATH_MAX] = {0,};
         char                    volfile[PATH_MAX] = {0,};
         char                    path[PATH_MAX] = {0,};
-        char                    cmd_str[8192] = {0,};
         char                    rundir[PATH_MAX] = {0,};
 
         this = THIS;
@@ -2187,10 +2188,8 @@ glusterd_nfs_server_start ()
 
         snprintf (logfile, PATH_MAX, "%s/nfs.log", DEFAULT_LOG_FILE_DIRECTORY);
 
-        snprintf (cmd_str, 8192,
-                  "%s/sbin/glusterfs -f %s -p %s -l %s",
-                  GFS_PREFIX, volfile, pidfile, logfile);
-        ret = gf_system (cmd_str);
+        ret = runcmd (GFS_PREFIX"/sbin/glusterfs", "-f", volfile,
+                      "-p", pidfile, "-l", logfile, NULL);
 
 out:
         return ret;
@@ -3321,6 +3320,7 @@ glusterd_start_gsync (glusterd_volinfo_t *master_vol, char *slave,
         int32_t         status  = 0;
         char            buf[PATH_MAX]   = {0,};
         char            uuid_str [64] = {0};
+        runner_t        runner = {0,};
         xlator_t        *this = NULL;
         glusterd_conf_t *priv = NULL;
         int             errcode = 0;
@@ -3354,29 +3354,24 @@ glusterd_start_gsync (glusterd_volinfo_t *master_vol, char *slave,
         }
 
         uuid_utoa_r (master_vol->volume_id, uuid_str);
-        ret = snprintf (buf, PATH_MAX,
-                        GSYNCD_PREFIX"/gsyncd -c %s/"GSYNC_CONF" "
-                        ":%s %s --config-set session-owner %s",
-                        priv->workdir, master_vol->volname, slave, uuid_str);
-        if (ret <= 0 || ret >= PATH_MAX)
-                ret = -1;
-        if (ret != -1)
-                ret = gf_system (buf) ? -1 : 0;
+        runinit (&runner);
+        runner_add_args  (&runner, GSYNCD_PREFIX"/gsyncd", "-c", NULL);
+        runner_argprintf (&runner, "%s/"GSYNC_CONF, priv->workdir);
+        runner_argprintf (&runner, ":%s", master_vol->volname);
+        runner_add_args  (&runner, slave, "--config-set", "session-owner",
+                          uuid_str, NULL);
+        ret = runner_run (&runner);
         if (ret == -1) {
                 errcode = -1;
                 goto out;
         }
 
-        ret = snprintf (buf, PATH_MAX, GSYNCD_PREFIX "/gsyncd --monitor -c "
-                        "%s/"GSYNC_CONF" :%s %s", priv->workdir,
-                        master_vol->volname, slave);
-        if (ret <= 0) {
-                ret = -1;
-                errcode = -1;
-                goto out;
-        }
-
-        ret = gf_system (buf);
+        runinit (&runner);
+        runner_add_args  (&runner, GSYNCD_PREFIX"/gsyncd", "--monitor", "-c", NULL);
+        runner_argprintf (&runner, "%s/"GSYNC_CONF, priv->workdir);
+        runner_argprintf (&runner, ":%s", master_vol->volname);
+        runner_add_arg   (&runner, slave);
+        ret = runner_run (&runner);
         if (ret == -1) {
                 gf_asprintf (op_errstr, GEOREP" start failed for %s %s",
                              master_vol->volname, slave);
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index 28c0a597d..15894b32c 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -45,6 +45,7 @@
 #include "glusterd-store.h"
 #include "glusterd-utils.h"
 #include "common-utils.h"
+#include "run.h"
 
 static uuid_t glusterd_uuid;
 extern struct rpcsvc_program glusterd1_mop_prog;
@@ -245,27 +246,19 @@ extern int mkdir_if_missing (char *path);
 static int
 glusterd_check_gsync_present ()
 {
-        FILE               *in = NULL;
         char                buff[PATH_MAX] = {0, };
-        char                cmd[PATH_MAX + 256] = {0, };
+        runner_t            runner = {0,};
         char               *ptr = NULL;
         int                 ret = 0;
 
-        if (strlen (GSYNCD_PREFIX)+1 > PATH_MAX-strlen("/gsyncd")) {
-                ret = -1;
+        runinit (&runner);
+        runner_add_args (&runner, GSYNCD_PREFIX"/gsyncd", "--version", NULL);
+        runner_redir (&runner, STDOUT_FILENO, RUN_PIPE);
+        ret = runner_start (&runner);
+        if (ret == -1)
                 goto out;
-        }
-
-        snprintf (cmd, sizeof(cmd), GSYNCD_PREFIX"/gsyncd --version");
 
-        if (!(in = popen(cmd, "r"))) {
-                gf_log ("", GF_LOG_INFO, "geo-replication module not installed"
-                        " in the system");
-                ret = -1;
-                goto out;
-        }
-
-        ptr = fgets(buff, sizeof(buff), in);
+        ptr = fgets(buff, sizeof(buff), runner_chio (&runner, STDOUT_FILENO));
         if (ptr) {
                 if (!strstr (buff, "gsyncd")) {
                         ret = -1;
@@ -277,11 +270,12 @@ glusterd_check_gsync_present ()
         }
         ret = 0;
  out:
-        if ((in)&& (-1 == pclose (in))) {
-                        ret = -1;
+
+        if (ret == 0)
+                ret = runner_end (&runner);
+        if (ret == -1)
                         gf_log ("", GF_LOG_INFO, "geo-replication module not"
                                 " installed in the system");
-                }
 
         gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
@@ -348,14 +342,22 @@ glusterd_crt_georep_folders (char *georepdir, glusterd_conf_t *conf)
 }
 #endif
 
+static void
+runinit_gsyncd_setrx (runner_t *runner, glusterd_conf_t *conf)
+{
+        runinit (runner);
+        runner_add_args (runner, GSYNCD_PREFIX"/gsyncd", "-c", NULL);
+        runner_argprintf (runner, "%s/"GSYNC_CONF,conf->workdir);
+        runner_add_arg (runner, "--config-set-rx");
+}
+
 static int
 configure_syncdaemon (glusterd_conf_t *conf)
 {
         int ret = 0;
 #if SYNCDAEMON_COMPILE
+        runner_t runner = {0,};
         char georepdir[PATH_MAX] = {0,};
-        char cmd[2*PATH_MAX + 1024] = {0,};
-        int blen = 0;
 
         ret = setenv ("_GLUSTERD_CALLED_", "1", 1);
         if (ret < 0) {
@@ -369,80 +371,88 @@ configure_syncdaemon (glusterd_conf_t *conf)
                 goto out;
         }
 
-
         glusterd_crt_georep_folders (georepdir, conf);
         if (ret) {
                 ret = 0;
                 goto out;
         }
 
-        blen = snprintf (cmd, sizeof(cmd), GSYNCD_PREFIX"/gsyncd -c %s/"
-                         GSYNC_CONF " --config-set-rx ", conf->workdir);
-
-        /* Calling out to gsyncd to configure it:
-         * - use system(3) for options with multi-word values as system
-         *   groks quotes;
-         * - use gf_system() for options with template expanders so
-         *   that they are not messed up by shell
-         */
-
         /************
          * master pre-configuration
          ************/
 
         /* remote-gsyncd */
-        strcpy (cmd + blen, "remote-gsyncd " GSYNCD_PREFIX"/gsyncd . .");
-        ret = system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner, "remote-gsyncd", GSYNCD_PREFIX"/gsyncd", ".", ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
-        strcpy (cmd + blen,
-                "remote-gsyncd /usr/local/libexec/glusterfs/gsyncd . ^ssh:");
-        ret = system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner, "remote-gsyncd",
+                         "/usr/local/libexec/glusterfs/gsyncd", ".", "^ssh:", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* gluster-command */
         /* XXX $sbindir should be used (throughout the codebase) */
-        strcpy (cmd + blen,
-                "gluster-command '"GFS_PREFIX"/sbin/glusterfs "
-                "--xlator-option *-dht.assert-no-child-down=true' . .");
-        ret = system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner, "gluster-command",
+                         GFS_PREFIX"/sbin/glusterfs "
+                          "--xlator-option *-dht.assert-no-child-down=true",
+                         "." ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* ssh-command */
-        sprintf (cmd + blen,
-                 "ssh-command "
-                 "'ssh -oPasswordAuthentication=no -oStrictHostKeyChecking=no "
-                 "-i %s/secret.pem' . .", georepdir);
-        ret = system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_arg (&runner, "ssh-command");
+        runner_argprintf (&runner,
+                          "ssh -oPasswordAuthentication=no "
+                           "-oStrictHostKeyChecking=no "
+                           "-i %s/secret.pem", georepdir);
+        runner_add_args (&runner, ".", ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* pid-file */
-        sprintf (cmd + blen, "pid-file %s/${mastervol}/${eSlave}.pid . .", georepdir);
-        ret = gf_system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_arg (&runner, "pid-file");
+        runner_argprintf (&runner, "%s/${mastervol}/${eSlave}.pid", georepdir);
+        runner_add_args (&runner, ".", ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* state-file */
-        sprintf (cmd + blen, "state-file %s/${mastervol}/${eSlave}.status . .", georepdir);
-        ret = gf_system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_arg (&runner, "state-file");
+        runner_argprintf (&runner, "%s/${mastervol}/${eSlave}.status", georepdir);
+        runner_add_args (&runner, ".", ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* log-file */
-        strcpy (cmd + blen,
-                "log-file "DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"/${mastervol}/${eSlave}.log . .");
-        ret = gf_system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner,
+                         "log-file",
+                         DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"/${mastervol}/${eSlave}.log",
+                         ".", ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* gluster-log-file */
-        strcpy (cmd + blen, "gluster-log-file "
-                DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"/${mastervol}/${eSlave}.gluster.log . .");
-        ret = gf_system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner,
+                         "gluster-log-file",
+                         DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"/${mastervol}/${eSlave}.gluster.log",
+                         ".", ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
@@ -451,24 +461,32 @@ configure_syncdaemon (glusterd_conf_t *conf)
          ************/
 
         /* gluster-command */
-        strcpy (cmd + blen,
-                "gluster-command '"GFS_PREFIX"/sbin/glusterfs "
-                "--xlator-option *-dht.assert-no-child-down=true' .");
-        ret = system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner, "gluster-command",
+                         GFS_PREFIX"/sbin/glusterfs "
+                          "--xlator-option *-dht.assert-no-child-down=true",
+                         ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* log-file */
-        strcpy (cmd + blen,
-                "log-file "DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"-slaves/${session_owner}:${eSlave}.log .");
-        ret = gf_system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner,
+                         "log-file",
+                         DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"-slaves/${session_owner}:${eSlave}.log",
+                         ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
         /* gluster-log-file */
-        strcpy (cmd + blen, "gluster-log-file "
-                DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"-slaves/${session_owner}:${eSlave}.gluster.log .");
-        ret = gf_system (cmd);
+        runinit_gsyncd_setrx (&runner, conf);
+        runner_add_args (&runner,
+                         "gluster-log-file",
+                         DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"-slaves/${session_owner}:${eSlave}.gluster.log",
+                         ".", NULL);
+        ret = runner_run (&runner);
         if (ret)
                 goto out;
 
@@ -701,6 +719,7 @@ out:
                         GF_FREE (this->private);
                         this->private = NULL;
                 }
+
         }
 
         return ret;