From c1ab63ed17189cacf1247751e8633f589c977a00 Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Thu, 27 Mar 2014 21:59:38 +0000 Subject: [PATCH] iowatcher: Fix up directory trace processing Similar to the fix for spaces in file names in commit 5d845e3, this patch fixes processing of directories with spaces in their names by using posix_spawnp() to run the blkparse command instead of system(). In doing so, combine_blktrace_devs() and match_trace() have been reworked to use a list structure instead of doing a lot of strdup()ing and string appending. Also make sure that trailing slashes are removed from the directory name before attempting to use it as the base of the .dump filename. Update the -t entry in the manpage to mention directory behaviour, too. Signed-off-by: Andrew Price --- iowatcher/blkparse.c | 141 ++++++++++++++++++++++++++++-------------- iowatcher/iowatcher.1 | 3 +- iowatcher/tracers.c | 34 ++++++---- iowatcher/tracers.h | 1 + 4 files changed, 119 insertions(+), 60 deletions(-) diff --git a/iowatcher/blkparse.c b/iowatcher/blkparse.c index 43eb269..bf58b45 100644 --- a/iowatcher/blkparse.c +++ b/iowatcher/blkparse.c @@ -707,81 +707,123 @@ int filter_outliers(struct trace *trace, u64 min_offset, u64 max_offset, } static char footer[] = ".blktrace.0"; -static int footer_len = sizeof(footer); +static int footer_len = sizeof(footer) - 1; -static void match_trace(char *name, char **traces) +static int match_trace(char *name, int *len) { int match_len; - char *match; int footer_start; match_len = strlen(name); if (match_len <= footer_len) - return; + return 0; footer_start = match_len - footer_len; - if (strcmp(name + footer_start + 1, footer) != 0) - return; - - match = strdup(name); - if (!match) - goto enomem; - - match[footer_start + 1] = '\0'; - snprintf(line, line_len, "%s -i '%s'", *traces ? *traces : "", match); - free(match); - - match = strdup(line); - if (!match) - goto enomem; - - free(*traces); - *traces = match; - return; + if (strcmp(name + footer_start, footer) != 0) + return 0; -enomem: - perror("memory allocation failed"); - exit(1); - return; + if (len) + *len = match_len; + return 1; } -static char *combine_blktrace_devs(char *dir_name) -{ - DIR *dir; - char *traces = NULL; - struct dirent *d; - int len; - int ret; +struct tracelist { + struct tracelist *next; + char *name; +}; - dir = opendir(dir_name); +static struct tracelist *traces_list(char *dir_name, int *len) +{ + int count = 0; + struct tracelist *traces = NULL; + DIR *dir = opendir(dir_name); if (!dir) return NULL; while (1) { - d = readdir(dir); + int len; + struct tracelist *tl; + struct dirent *d = readdir(dir); if (!d) break; - len = strlen(d->d_name); - if (len > footer_len) - match_trace(d->d_name, &traces); + if (!match_trace(d->d_name, &len)) + continue; + + /* Allocate space for tracelist + filename */ + tl = calloc(1, sizeof(struct tracelist) + (sizeof(char) * (len + 1))); + if (!tl) + return NULL; + tl->next = traces; + tl->name = (char *)(tl + 1); + strncpy(tl->name, d->d_name, len); + traces = tl; + count++; } closedir(dir); + if (len) + *len = count; + + return traces; +} + +static void traces_free(struct tracelist *traces) +{ + while (traces) { + struct tracelist *tl = traces; + traces = traces->next; + free(tl); + } +} + +static char *combine_blktrace_devs(char *dir_name) +{ + struct tracelist *traces = NULL; + struct tracelist *tl; + char *ret = NULL; + char **argv = NULL; + char *dumpfile; + int argc = 0; + int i; + int err; + + if (!asprintf(&dumpfile, "%s.dump", dir_name)) + goto out; + + traces = traces_list(dir_name, &argc); if (!traces) - return NULL; + goto out; - snprintf(line, line_len, "blkparse -O %s -D %s -d '%s.%s'", - traces, dir_name, dir_name, "dump"); + argc *= 2; /* {"-i", trace } */ + argc += 6; /* See below */ + argv = calloc(argc + 1, sizeof(char *)); + if (!argv) + goto out; + + i = 0; + argv[i++] = "blkparse"; + argv[i++] = "-O"; + argv[i++] = "-D"; + argv[i++] = dir_name; + argv[i++] = "-d"; + argv[i++] = dumpfile; + for (tl = traces; tl != NULL; tl = tl->next) { + argv[i++] = "-i"; + argv[i++] = tl->name; + } - ret = system(line); - if (ret) { - fprintf(stderr, "blkparse failure %s\n", line); + err = run_program2(argc, argv); + free(argv); + if (err) { + fprintf(stderr, "blkparse failed with exit code %d\n", err); exit(1); } - snprintf(line, line_len, "%s.%s", dir_name, "dump"); - return strdup(line); + ret = dumpfile; +out: + traces_free(traces); + return ret; } static char *find_trace_file(char *filename) @@ -806,6 +848,13 @@ static char *find_trace_file(char *filename) found_dir = 1; } + if (found_dir) { + int i; + /* Eat up trailing '/'s */ + for (i = strlen(filename) - 1; filename[i] == '/'; i--) + filename[i] = '\0'; + } + /* * try tacking .dump onto the end and see if that already * has been generated diff --git a/iowatcher/iowatcher.1 b/iowatcher/iowatcher.1 index 5951c7b..8a06126 100644 --- a/iowatcher/iowatcher.1 +++ b/iowatcher/iowatcher.1 @@ -27,9 +27,10 @@ Program to run while blktrace is run. \fB-K, --keep-movie-svgs\fP Keep the SVG files generated for movie mode. .TP -\fB-t, --trace\fP +\fB-t, --trace\fP Controls the name of the blktrace file. iowatcher uses a dump from blkparse, so -t tries to guess the name of the corresponding per CPU blktrace data files if the dump file doesn't already exist. If you want more than one trace in a given graph, you can specify -t more than once. +If a directory is specified, iowatcher will use the name of the directory as the base name of the dump file and all trace files found inside the directory will be processed. .TP \fB-l, --label\fP