From 9072b817b0803f999081c6244b18a9ae8fb0234c Mon Sep 17 00:00:00 2001 From: Csaba Henk Date: Thu, 26 May 2011 03:32:27 +0000 Subject: reimplement invocation of external programs with run API Signed-off-by: Csaba Henk Signed-off-by: Anand Avati BUG: 2562 (invoke external commands precisely with fork + exec) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=2562 --- cli/src/cli-cmd-volume.c | 37 +--- cli/src/cli-rpc-ops.c | 56 +++--- libglusterfs/src/common-utils.c | 59 ------- libglusterfs/src/common-utils.h | 1 - libglusterfs/src/compat.h | 5 + libglusterfs/src/graph.y | 2 + xlators/mgmt/glusterd/src/glusterd-op-sm.c | 235 ++++++++++--------------- xlators/mgmt/glusterd/src/glusterd-rebalance.c | 47 ++--- xlators/mgmt/glusterd/src/glusterd-utils.c | 67 ++++--- xlators/mgmt/glusterd/src/glusterd.c | 149 +++++++++------- 10 files changed, 282 insertions(+), 376 deletions(-) diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c index 7ca9ed4bb..f2edbd0b1 100644 --- a/cli/src/cli-cmd-volume.c +++ b/cli/src/cli-cmd-volume.c @@ -32,6 +32,7 @@ #include "cli-cmd.h" #include "cli-mem-types.h" #include "cli1-xdr.h" +#include "run.h" extern struct rpc_clnt *global_rpc; @@ -1053,30 +1054,10 @@ out: static int cli_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; - struct stat stat_buff; - - if (strlen (GSYNCD_PREFIX)+1 > PATH_MAX-strlen("/gsyncd")) { - ret = -1; - goto out; - } - - ret = snprintf (cmd, sizeof(cmd), GSYNCD_PREFIX "/gsyncd"); - if (ret < 0) { - ret = 0; - goto out; - } - ret = lstat (cmd, &stat_buff); - - if (ret || !(stat_buff.st_mode & S_IXUSR)) { - ret = -1; - gf_log ("", GF_LOG_INFO, "geo-replication not installed"); - goto out; - } ret = setenv ("_GLUSTERD_CALLED_", "1", 1); if (-1 == ret) { @@ -1085,16 +1066,16 @@ cli_check_gsync_present () goto out; } - memset (cmd, 0, sizeof (cmd)); - ret = snprintf (cmd, sizeof(cmd), GSYNCD_PREFIX"/gsyncd --version"); - - if (!(in = popen(cmd, "r"))) { - 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) { gf_log ("", GF_LOG_INFO, "geo-replication not installed"); 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; @@ -1105,7 +1086,7 @@ cli_check_gsync_present () goto out; } - ret = pclose (in); + ret = runner_end (&runner); if (ret) gf_log ("", GF_LOG_ERROR, "geo-replication not installed"); diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c index 24afb66f0..ee52ccc95 100644 --- a/cli/src/cli-rpc-ops.c +++ b/cli/src/cli-rpc-ops.c @@ -43,6 +43,8 @@ #include "glusterfs3.h" #include "portmap.h" +#include "run.h" + extern rpc_clnt_prog_t *cli_rpc_prog; extern int cli_op_ret; extern int connected; @@ -1318,7 +1320,8 @@ print_limit_list (char *volname, char *limit_list) char ret_str [1024] = {0, }; char value [1024] = {0, }; char mountdir [] = "/tmp/mntXXXXXX"; - char cmd_str [PATH_MAX + 1024] = {0, }; + char abspath [PATH_MAX] = {0, }; + runner_t runner = {0,}; GF_VALIDATE_OR_GOTO ("cli", volname, out); GF_VALIDATE_OR_GOTO ("cli", limit_list, out); @@ -1336,10 +1339,8 @@ print_limit_list (char *volname, char *limit_list) /* Mount a temporary client to fetch the disk usage * of the directory on which the limit is set. */ - snprintf (cmd_str, sizeof (cmd_str), GFS_PREFIX "/sbin/glusterfs -s localhost " - "--volfile-id %s %s", volname, mountdir); - - ret = system (cmd_str); + ret = runcmd (GFS_PREFIX"/sbin/glusterfs", "-s", + "localhost", "--volfile-id", volname, mountdir, NULL); if (ret) { gf_log ("cli", GF_LOG_WARNING, "failed to mount glusterfs client"); ret = -1; @@ -1373,10 +1374,9 @@ print_limit_list (char *volname, char *limit_list) } value [j] = '\0'; - memset (&cmd_str, 0, sizeof (cmd_str)); - snprintf (cmd_str, sizeof (cmd_str), "%s/%s", mountdir, path); + snprintf (abspath, sizeof (abspath), "%s/%s", mountdir, path); - ret = getxattr (cmd_str, "trusted.limit.list", (void *) ret_str, 4096); + ret = getxattr (abspath, "trusted.limit.list", (void *) ret_str, 4096); if (ret < 0) { cli_out ("%-20s %10s", path, value); } else { @@ -1389,15 +1389,18 @@ print_limit_list (char *volname, char *limit_list) } unmount: - memset (&cmd_str, 0, sizeof (cmd_str)); + + runinit (&runner); + runner_add_args (&runner, "umount", #if GF_LINUX_HOST_OS - snprintf (cmd_str, sizeof (cmd_str), "umount -l %s", mountdir); -#else - snprintf (cmd_str, sizeof (cmd_str), "umount %s", mountdir); + "-l", #endif - ret = system (cmd_str); + mountdir, NULL); + ret = runner_run_reuse (&runner); if (ret) - gf_log ("cli", GF_LOG_WARNING, "error executing: %s", cmd_str); + runner_log (&runner, "cli", GF_LOG_WARNING, "error executing"); + runner_end (&runner); + rm_dir: rmdir (mountdir); out: @@ -2654,7 +2657,7 @@ out: int gf_cli3_1_gsync_config_command (dict_t *dict) { - char cmd[PATH_MAX] = {0,}; + runner_t runner = {0,}; char *subop = NULL; char *gwd = NULL; char *slave = NULL; @@ -2674,16 +2677,21 @@ gf_cli3_1_gsync_config_command (dict_t *dict) return -1; if (dict_get_str (dict, "master", &master) != 0) - master = ""; + master = NULL; if (dict_get_str (dict, "op_name", &op_name) != 0) - op_name = ""; - - snprintf (cmd, PATH_MAX, - GSYNCD_PREFIX"/gsyncd -c %s/"GSYNC_CONF" %s%s %s --config-%s%s%s", - gwd, *master ? ":" : "", master, slave, - subop, *op_name ? " " : "", op_name); - - return system (cmd) ? -1 : 0; + op_name = NULL; + + runinit (&runner); + runner_add_args (&runner, GSYNCD_PREFIX"/gsyncd", "-c", NULL); + runner_argprintf (&runner, "%s/"GSYNC_CONF, gwd); + if (master) + runner_argprintf (&runner, ":%s", master); + runner_add_arg (&runner, slave); + runner_argprintf (&runner, "--config-%s", subop); + if (op_name) + runner_add_arg (&runner, op_name); + + return runner_run (&runner); } int diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c index e745ec0fa..7cb389ba3 100644 --- a/libglusterfs/src/common-utils.c +++ b/libglusterfs/src/common-utils.c @@ -1485,65 +1485,6 @@ get_checksum_for_file (int fd, uint32_t *checksum) } -/* One should pass the command here with command with full path, - otherwise, execv will fail */ -int -gf_system (const char *command) -{ - int ret = -1; - pid_t pid = 0; - int status = 0; - int idx = 0; - char *dupcmd = NULL; - char *arg = NULL; - char *tmp = NULL; - char *argv[100] = { NULL, }; - - dupcmd = gf_strdup (command); - if (!dupcmd) - goto out; - - pid = fork (); - if (pid < 0) { - /* failure */ - goto out; - } - if (pid == 0) { - /* Child process */ - /* Step 0: Prepare the argv */ - arg = strtok_r (dupcmd, " ", &tmp); - while (arg) { - argv[idx] = arg; - arg = strtok_r (NULL, " ", &tmp); - idx++; - } - /* Step 1: Close all 'fd' */ - for (idx = 3; idx < 65536; idx++) { - close (idx); - } - /* Step 2: execv (); */ - ret = execvp (argv[0], argv); - - /* Code will not come here at all */ - gf_log ("", GF_LOG_ERROR, "execv of (%s) failed", command); - - kill (getpid(), SIGKILL); - } - if (pid > 0) { - /* Current, ie, parent process */ - pid = waitpid (pid, &status, 0); - if (WIFEXITED(status) && WEXITSTATUS(status) == EXIT_SUCCESS) - ret = 0; - else - ret = -1; - } -out: - if (dupcmd) - GF_FREE (dupcmd); - - return ret; -} - int get_checksum_for_path (char *path, uint32_t *checksum) { diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h index a1df3de35..d3218b018 100644 --- a/libglusterfs/src/common-utils.h +++ b/libglusterfs/src/common-utils.h @@ -344,7 +344,6 @@ int gf_unlockfd (int fd); int get_checksum_for_file (int fd, uint32_t *checksum); int log_base2 (unsigned long x); -int gf_system (const char *command); int get_checksum_for_path (char *path, uint32_t *checksum); char *strtail (char *str, const char *pattern); diff --git a/libglusterfs/src/compat.h b/libglusterfs/src/compat.h index a6f1f01bd..4d336ea06 100644 --- a/libglusterfs/src/compat.h +++ b/libglusterfs/src/compat.h @@ -378,4 +378,9 @@ dirent_size (struct dirent *entry) #define IXDR_PUT_U_LONG(buf, v) IXDR_PUT_LONG(buf, (long)(v)) #endif +#if defined(__GNUC__) && !defined(RELAX_POISONING) +/* Use run API, see run.h */ +#pragma GCC poison system popen +#endif + #endif /* __COMPAT_H__ */ diff --git a/libglusterfs/src/graph.y b/libglusterfs/src/graph.y index 52e15940a..3e566aa1d 100644 --- a/libglusterfs/src/graph.y +++ b/libglusterfs/src/graph.y @@ -29,6 +29,8 @@ #include #include +#define RELAX_POISONING + #include "xlator.h" #include "graph-utils.h" #include "logging.h" 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 #include @@ -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; -- cgit