summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmar Tumballi <amarts@redhat.com>2018-07-24 15:42:28 +0530
committerjiffin tony Thottan <jthottan@redhat.com>2018-09-06 15:54:50 +0000
commitbcc1d689e8e131b6e23cbbe0b785dec8e23a108c (patch)
treed7145a78d4f52329aa6bb0267ff3f7719ef55e85
parent8356b9f169f73cfdfaf6e96fcd0af618efc3a649 (diff)
io-stats: dump io-stats info in /var/run/gluster
It wouldn't make sense to allow iostats file to be written in *any* directory. While the formating makes sure we try to append io-stats-name for the file, so overwriting existing file is slim, but in any case it makes sense to restrict dumping to one directory. Below are the sample commands, and files created for the corresponding values: $ setfattr -n trusted.io-stats-dump -v file-for-dump $M0 In this case, the file would be in /var/run/gluster/file-for-dump $ setfattr -n trusted.io-stats-dump -v /dir1/dir2/file-for-dump $M0 In this case, then the dump file is in /var/run/gluster/dir1-dir2-file-for-dump Note that the value passed for this virtual xattr would be treated as a file, and even if the value has '/' in it, it would be changed to '-' for sanity. BUG: 1625660 Change-Id: Id9ae6a40a190b8937c51662e6e1c2a0f6c86a0e0 Signed-off-by: Amar Tumballi <amarts@redhat.com>
-rwxr-xr-xtests/bugs/core/io-stats-1322825.t12
-rw-r--r--xlators/debug/io-stats/src/io-stats.c34
2 files changed, 31 insertions, 15 deletions
diff --git a/tests/bugs/core/io-stats-1322825.t b/tests/bugs/core/io-stats-1322825.t
index d232ecb..53f2d04 100755
--- a/tests/bugs/core/io-stats-1322825.t
+++ b/tests/bugs/core/io-stats-1322825.t
@@ -23,7 +23,7 @@ TEST $CLI volume profile $V0 start
TEST mkdir $M0/dir1
# Generate the stat dump across the io-stat instances
-TEST setfattr -n trusted.io-stats-dump -v /tmp/io-stats-1322825 $M0
+TEST setfattr -n trusted.io-stats-dump -v io-stats-1322825 $M0
# Check if $M0 is clean w.r.t xattr information
# TODO: if there are better ways to check we really get no attr error, please
@@ -42,12 +42,12 @@ ret=$(echo $?)
EXPECT 0 echo $ret
# Check if we have 5 io-stat files in /tmp
-EXPECT 5 ls -1 /tmp/io-stats-1322825*
+EXPECT 5 ls -1 /var/run/gluster/io-stats-1322825*
# Cleanup the 5 generated files
-rm -f /tmp/io-stats-1322825*
+rm -f /var/run/gluster/io-stats-1322825*
# Rinse and repeat above for a directory
-TEST setfattr -n trusted.io-stats-dump -v /tmp/io-stats-1322825 $M0/dir1
+TEST setfattr -n trusted.io-stats-dump -v io-stats-1322825 $M0/dir1
getfattr -n trusted.io-stats-dump $B0/${V0}1/dir1 2>&1 | grep -qi "no such attribute"
ret=$(echo $?)
EXPECT 0 echo $ret
@@ -61,7 +61,7 @@ getfattr -n trusted.io-stats-dump $B0/${V0}4/dir1 2>&1 | grep -qi "no such attri
ret=$(echo $?)
EXPECT 0 echo $ret
-EXPECT 5 ls -1 /tmp/io-stats-1322825*
-rm -f /tmp/io-stats-1322825*
+EXPECT 5 ls -1 /var/run/gluster/io-stats-1322825*
+rm -f /var/run/gluster/io-stats-1322825*
cleanup;
diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c
index f46474b..16a11df 100644
--- a/xlators/debug/io-stats/src/io-stats.c
+++ b/xlators/debug/io-stats/src/io-stats.c
@@ -45,6 +45,8 @@
#define DEFAULT_GRP_BUF_SZ 16384
#define IOS_BLOCK_COUNT_SIZE 32
+#define IOS_STATS_DUMP_DIR DEFAULT_VAR_RUN_DIRECTORY
+
typedef enum {
IOS_STATS_TYPE_NONE,
IOS_STATS_TYPE_OPEN,
@@ -3000,7 +3002,6 @@ io_stats_fsync (call_frame_t *frame, xlator_t *this,
return 0;
}
-
int
conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
{
@@ -3013,9 +3014,10 @@ conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
char *filename = NULL;
FILE *logfp = NULL;
struct ios_dump_args args = {0};
- int pid, namelen;
+ int pid, namelen, dirlen;
char dump_key[100];
char *slash_ptr = NULL;
+ char *path_in_value = NULL;
stub = data;
this = stub->this;
@@ -3024,16 +3026,30 @@ conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
name as well. This helps when there is more than a single io-stats
instance in the graph, or the client and server processes are running
on the same node */
- /* hmmm... no check for this */
- /* name format: <passed in path/filename>.<xlator name slashes to -> */
- namelen = value->len + strlen (this->name) + 2; /* '.' and '\0' */
+ /* For the sanity of where the file should be located, we should make
+ sure file is written only inside RUNDIR (ie, /var/run/gluster) */
+ /* TODO: provide an option to dump it to different directory of
+ choice, based on options */
+ /* name format: /var/run/gluster/<passed in path/filename>.<xlator name slashes to -> */
+
+ path_in_value = data_to_str (value);
+
+ if (strstr (path_in_value, "../")) {
+ gf_log (this->name, GF_LOG_ERROR,
+ "%s: no \"../\" allowed in path", path_in_value);
+ return -1;
+ }
+ dirlen = strlen (IOS_STATS_DUMP_DIR);
+ namelen = (dirlen + value->len + strlen (this->name) + 3);
+ /* +3 for '/', '.' and '\0' added in snprintf below*/
+
filename = alloca0 (namelen);
- memcpy (filename, data_to_str (value), value->len);
- memcpy (filename + value->len, ".", 1);
- memcpy (filename + value->len + 1, this->name, strlen(this->name));
+
+ snprintf (filename, namelen, "%s/%s.%s", IOS_STATS_DUMP_DIR,
+ path_in_value, this->name);
/* convert any slashes to '-' so that fopen works correctly */
- slash_ptr = strchr (filename + value->len + 1, '/');
+ slash_ptr = strchr (filename + dirlen + 1, '/');
while (slash_ptr) {
*slash_ptr = '-';
slash_ptr = strchr (slash_ptr, '/');