Hi, Am 03.02.2013 22:55, schrieb Moritz Mühlenhoff:
On Fri, Jan 25, 2013 at 01:39:52PM +0100, Julien Cristau wrote:On Fri, Jan 25, 2013 at 09:56:25 +0100, Patrick Matthäi wrote:+diff -Naur glusterfs-3.2.7.orig/libglusterfs/src/statedump.c glusterfs-3.2.7/libglusterfs/src/statedump.c +--- glusterfs-3.2.7.orig/libglusterfs/src/statedump.c 2012-06-10 19:44:15.000000000 +0200 ++++ glusterfs-3.2.7/libglusterfs/src/statedump.c 2013-01-25 08:57:35.601175617 +0100 +@@ -408,37 +404,45 @@ + void + gf_proc_dump_info (int signum) + { +- int ret = -1; +- glusterfs_ctx_t *ctx = NULL; +- ++ int ret = -1; ++ glusterfs_ctx_t *ctx = NULL; ++ char brick_name[PATH_MAX] = {0,}; ++ char tmp_dump_name[] = "/tmp/dumpXXXXXX"; ++ char path[PATH_MAX] = {0,}; + + gf_proc_dump_lock (); +- ret = gf_proc_dump_open (); +- if (ret < 0) +- goto out; + + ret = gf_proc_dump_options_init (); + + if (ret < 0) + goto out; + +- if (GF_PROC_DUMP_IS_OPTION_ENABLED (mem)) +- gf_proc_dump_mem_info (); +- + ctx = glusterfs_ctx_get (); + +- if (ctx) { +- if (GF_PROC_DUMP_IS_OPTION_ENABLED (iobuf)) +- iobuf_stats_dump (ctx->iobuf_pool); +- if (GF_PROC_DUMP_IS_OPTION_ENABLED (callpool)) +- gf_proc_dump_pending_frames (ctx->pool); +- if (ctx->active) +- gf_proc_dump_xlator_info (ctx->active->top); ++ if (!ctx) ++ goto out; + +- } ++ if (ctx->cmd_args.brick_name) { ++ GF_REMOVE_SLASH_FROM_PATH (ctx->cmd_args.brick_name, brick_name); ++ } else ++ strncpy (brick_name, "glusterdump", sizeof (brick_name)); ++ ++ snprintf (path, sizeof path, "%s/%s.%d.dump.%"PRIu64, "/tmp", ++ brick_name, getpid(), (uint64_t) time (NULL)); ++Srsly?
For C newbiebs like me I would welcome some more details ;)
++ ret = gf_proc_dump_open (tmp_dump_name); ++ if (GF_PROC_DUMP_IS_OPTION_ENABLED (mem)) ++ gf_proc_dump_mem_info (); ++ if (GF_PROC_DUMP_IS_OPTION_ENABLED (iobuf)) ++ iobuf_stats_dump (ctx->iobuf_pool); ++ if (GF_PROC_DUMP_IS_OPTION_ENABLED (callpool)) ++ gf_proc_dump_pending_frames (ctx->pool); ++ if (ctx->active) ++ gf_proc_dump_xlator_info (ctx->active->top); + + gf_proc_dump_close (); + out: ++ rename (tmp_dump_name, path);That seems unnecessary. You could just change the template you pass to mkstemp to have the right name... (Plus, this isn't even in the right place, goto out happens before the temp file is created.)
Upstream just said these days, that the HEAD fix may also be unsecure/incomplete/broken/whatever, if this patch is not QA acceptable.
Also some gluterfs developers stated, that they have not got the time for working on this issue and that "people seeing problems" may fix it better.
+ gf_proc_dump_unlock (); + + return; +diff -Naur glusterfs-3.2.7.orig/xlators/mgmt/glusterd/src/glusterd-utils.c glusterfs-3.2.7/xlators/mgmt/glusterd/src/glusterd-utils.c +--- glusterfs-3.2.7.orig/xlators/mgmt/glusterd/src/glusterd-utils.c 2012-06-10 19:44:17.000000000 +0200 ++++ glusterfs-3.2.7/xlators/mgmt/glusterd/src/glusterd-utils.c 2013-01-25 08:57:35.601175617 +0100 +@@ -63,7 +63,7 @@ + #define MOUNTV3_VERSION 3 + #define MOUNTV1_VERSION 1 + +-char *glusterd_sock_dir = "/tmp"; ++char *glusterd_sock_dir = "/var/run"; + static glusterd_lock_t lock; + + static int32_tWhat's that bit about?
I think we could drop this change, or not?
--
/*
Mit freundlichem Gruß / With kind regards,
Patrick Matthäi
GNU/Linux Debian Developer
E-Mail: pmatthaei@debian.org
patrick@linux-dev.org
*/