summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKaleb S. KEITHLEY <kkeithle@redhat.com>2014-06-20 08:40:47 -0400
committerKrishnan Parthasarathi <kparthas@redhat.com>2014-06-24 22:58:15 -0700
commitf20d0ef8ad7d2f65a9234fc11101830873a9f6ab (patch)
treed2334be592acc17c37a3e2321c1a1495805dad42
parent10c04c617276dbd9af7ae04ae287fe6f75e2f2ae (diff)
xlators/mgmt: don't allow glusterd fork bomb (cache the brick inode size)
Was don't leave zombies if required programs aren't installed Also, the existing if (strcmp (foo, bar) == 0) antipattern leaves me underwhelmed -- table driven is better; I like fully qualified paths to system tools too. File systems aren't going to change their inode size. Rather than fork-and-exec a tool repeatedly, hang on to the answer for subsequent use. Even if there are hundreds of volumes the size of a dict to keep this in memory is small. Change-Id: I704a8b1215446488b6e9e051a3e031af21b37adb BUG: 1081013 Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com> Reviewed-on: http://review.gluster.org/8134 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com> Tested-by: Krishnan Parthasarathi <kparthas@redhat.com>
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c92
1 files changed, 62 insertions, 30 deletions
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index d89eba6a4ab..3f36cef42c5 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -7071,6 +7071,21 @@ out:
return needle;
}
+static struct fs_info {
+ char *fs_type_name;
+ char *fs_tool_name;
+ char *fs_tool_arg;
+ char *fs_tool_pattern;
+ char *fs_tool_pkg;
+} glusterd_fs[] = {
+ /* some linux have these in /usr/sbin/and others in /sbin/? */
+ { "xfs", "xfs_info", NULL, "isize=", "xfsprogs" },
+ { "ext3", "tune2fs", "-l", "Inode size:", "e2fsprogs" },
+ { "ext4", "tune2fs", "-l", "Inode size:", "e2fsprogs" },
+ { "btrfs", NULL, NULL, NULL, NULL },
+ { NULL, NULL, NULL, NULL, NULL}
+};
+
static int
glusterd_add_inode_size_to_dict (dict_t *dict, int count)
{
@@ -7081,9 +7096,11 @@ glusterd_add_inode_size_to_dict (dict_t *dict, int count)
char *device = NULL;
char *fs_name = NULL;
char *cur_word = NULL;
- char *pattern = NULL;
char *trail = NULL;
runner_t runner = {0, };
+ struct fs_info *fs = NULL;
+ char fs_tool_name[256] = {0, };
+ static dict_t *cached_fs = NULL;
memset (key, 0, sizeof (key));
snprintf (key, sizeof (key), "brick%d.device", count);
@@ -7091,6 +7108,14 @@ glusterd_add_inode_size_to_dict (dict_t *dict, int count)
if (ret)
goto out;
+ if (cached_fs) {
+ if (dict_get_str (cached_fs, device, &cur_word) == 0) {
+ goto cached;
+ }
+ } else {
+ cached_fs = dict_new ();
+ }
+
memset (key, 0, sizeof (key));
snprintf (key, sizeof (key), "brick%d.fs_name", count);
ret = dict_get_str (dict, key, &fs_name);
@@ -7099,23 +7124,25 @@ glusterd_add_inode_size_to_dict (dict_t *dict, int count)
runinit (&runner);
runner_redir (&runner, STDOUT_FILENO, RUN_PIPE);
- /* get inode size for xfs or ext2/3/4 */
- if (!strcmp (fs_name, "xfs")) {
-
- runner_add_args (&runner, "xfs_info", device, NULL);
- pattern = "isize=";
- } else if (IS_EXT_FS(fs_name)) {
-
- runner_add_args (&runner, "tune2fs", "-l", device, NULL);
- pattern = "Inode size:";
-
- } else {
- ret = 0;
- gf_log (THIS->name, GF_LOG_INFO, "Skipped fetching "
- "inode size for %s: FS type not recommended",
- fs_name);
- goto out;
+ for (fs = glusterd_fs ; glusterd_fs->fs_type_name; fs++) {
+ if (strcmp (fs_name, fs->fs_type_name) == 0) {
+ snprintf (fs_tool_name, sizeof fs_tool_name,
+ "/usr/sbin/%s", fs->fs_tool_name);
+ if (access (fs_tool_name, R_OK|X_OK) == 0)
+ runner_add_arg (&runner, fs_tool_name);
+ else {
+ snprintf (fs_tool_name, sizeof fs_tool_name,
+ "/sbin/%s", fs->fs_tool_name);
+ if (access (fs_tool_name, R_OK|X_OK) == 0)
+ runner_add_arg (&runner, fs_tool_name);
+ }
+ if (runner.argv[0]) {
+ if (fs->fs_tool_arg)
+ runner_add_arg (&runner, fs->fs_tool_arg);
+ }
+ break;
+ }
}
ret = runner_start (&runner);
@@ -7131,10 +7158,7 @@ glusterd_add_inode_size_to_dict (dict_t *dict, int count)
* child and free resources. Fortunately, that seems to
* be harmless for other kinds of failures.
*/
- if (runner_end(&runner)) {
- gf_log (THIS->name, GF_LOG_ERROR,
- "double failure calling runner_end");
- }
+ (void) runner_end(&runner);
goto out;
}
@@ -7146,32 +7170,40 @@ glusterd_add_inode_size_to_dict (dict_t *dict, int count)
if (trail)
*trail = '\0';
- cur_word = glusterd_parse_inode_size (buffer, pattern);
+ cur_word =
+ glusterd_parse_inode_size (buffer, fs->fs_tool_pattern);
+
if (cur_word)
break;
}
ret = runner_end (&runner);
if (ret) {
- gf_log (THIS->name, GF_LOG_ERROR, "%s exited with non-zero "
- "exit status", ((!strcmp (fs_name, "xfs")) ?
- "xfs_info" : "tune2fs"));
+ gf_log (THIS->name, GF_LOG_ERROR,
+ "%s exited with non-zero exit status",
+ fs->fs_tool_name);
+
goto out;
}
if (!cur_word) {
ret = -1;
- gf_log (THIS->name, GF_LOG_ERROR, "Unable to retrieve inode "
- "size using %s",
- (!strcmp (fs_name, "xfs")? "xfs_info": "tune2fs"));
+ gf_log (THIS->name, GF_LOG_ERROR,
+ "Unable to retrieve inode size using %s",
+ fs->fs_tool_name);
goto out;
}
- inode_size = gf_strdup (cur_word);
+ if (dict_set_dynstr_with_alloc (cached_fs, device, cur_word)) {
+ /* not fatal if not entered into the cache */
+ gf_log (THIS->name, GF_LOG_DEBUG,
+ "failed to cache fs inode size for %s", device);
+ }
+cached:
memset (key, 0, sizeof (key));
snprintf (key, sizeof (key), "brick%d.inode_size", count);
- ret = dict_set_dynstr (dict, key, inode_size);
+ ret = dict_set_dynstr_with_alloc (dict, key, cur_word);
out:
if (ret)