summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Price <anprice@redhat.com>2014-04-26 18:22:53 +0100
committerChris Mason <clm@fb.com>2014-09-24 12:02:09 -0700
commitce225d502b505e189c4c138224a3bc50d0e05be5 (patch)
tree8bcfbef9d28e041ea697265b1b0d3424f9f16823
parent33e3face4ae0c7e0f67e3a92edc8d1e818ef6515 (diff)
downloadblktrace-ce225d502b505e189c4c138224a3bc50d0e05be5.tar.gz
blktrace-ce225d502b505e189c4c138224a3bc50d0e05be5.tar.bz2
iowatcher: Rework --prog to make arg processing safer
Previously the --prog option required the program-to-be-run to be specified as a single string. This meant that shell escaping would be lost in translation and a sub-shell would be run. Rework --prog to not take an argument and accept the arguments left after option processing has ended as the argv for the program-to-be-run. As we have the program as an argv, run_program2() can now be used to run it, and now that run_program() is no longer used we can remove it and remove the '2' from run_program2. New usage example: # iowatcher -p -t foo -d /dev/sda3 sleep 10 running blktrace blktrace -b 8192 -a queue -a complete -a issue -a notify -D . -d /dev/sda3 -o foo running 'sleep' '10' sleep exited with 0 ... Docs have been updated accordingly. Signed-off-by: Andrew Price <anprice@redhat.com>
-rw-r--r--iowatcher/blkparse.c4
-rw-r--r--iowatcher/iowatcher.114
-rw-r--r--iowatcher/main.c45
-rw-r--r--iowatcher/tracers.c27
-rw-r--r--iowatcher/tracers.h5
5 files changed, 47 insertions, 48 deletions
diff --git a/iowatcher/blkparse.c b/iowatcher/blkparse.c
index 447d14c..e732af7 100644
--- a/iowatcher/blkparse.c
+++ b/iowatcher/blkparse.c
@@ -801,7 +801,9 @@ static int dump_traces(struct tracelist *traces, int count, char *dumpfile)
argv[i++] = tl->name;
}
- err = run_program2(argc, argv, 0, NULL);
+ err = run_program(argc, argv, 1, NULL);
+ if (err)
+ fprintf(stderr, "%s exited with %d, expected 0\n", argv[0], err);
free(argv);
return err;
}
diff --git a/iowatcher/iowatcher.1 b/iowatcher/iowatcher.1
index 8a06126..d9b77b8 100644
--- a/iowatcher/iowatcher.1
+++ b/iowatcher/iowatcher.1
@@ -5,7 +5,7 @@ iowatcher - Create visualizations from blktrace results
.SH SYNOPSIS
.B iowatcher
-\fIOPTIONS...\fR
+\fI[options]\fR [--] \fI[program arguments ...]\fR
.SH DESCRIPTION
iowatcher graphs the results of a blktrace run. It can graph the result of an existing blktrace, start a new blktrace, or start a new blktrace and a benchmark run. It can then create an image or movie of the IO from a given trace. iowatcher can produce either SVG files or movies in mp4 format (with ffmpeg) or ogg format (with png2theora).
@@ -21,8 +21,16 @@ Controls which device you are tracing. You can only trace one device at a time
\fB-D, --blktrace-destination\fP <destination>
Destination for blktrace.
.TP
-\fB-p, --prog\fP <program>
-Program to run while blktrace is run.
+\fB-p, --prog\fP
+Run a program while blktrace is run. The program and its arguments must be
+specified after all other options. Note that this option previously required
+the program to be given as a single argument but it now flags that iowatcher
+should expect extra arguments to be run during the trace.
+.TP
+\fB--\fP
+End option parsing. If \fB--prog\fP is specified, everything after \fB--\fP is
+the program to be run. This can be useful if the program name could otherwise
+be mistaken for an option.
.TP
\fB-K, --keep-movie-svgs\fP
Keep the SVG files generated for movie mode.
diff --git a/iowatcher/main.c b/iowatcher/main.c
index ebbcf3d..c9ddd9c 100644
--- a/iowatcher/main.c
+++ b/iowatcher/main.c
@@ -148,7 +148,8 @@ static char *blktrace_devices[MAX_DEVICES_PER_TRACE];
static int num_blktrace_devices = 0;
static char *blktrace_outfile = "trace";
static char *blktrace_dest_dir = ".";
-static char *program_to_run = NULL;
+static char **prog_argv = NULL;
+static int prog_argc = 0;
static char *ffmpeg_codec = "libx264";
static void alloc_mpstat_gld(struct trace_file *tf)
@@ -1307,7 +1308,7 @@ enum {
HELP_LONG_OPT = 1,
};
-char *option_string = "F:T:t:o:l:r:O:N:d:D:p:m::h:w:c:x:y:a:C:PK";
+char *option_string = "+F:T:t:o:l:r:O:N:d:D:pm::h:w:c:x:y:a:C:PK";
static struct option long_options[] = {
{"columns", required_argument, 0, 'c'},
{"fio-trace", required_argument, 0, 'F'},
@@ -1320,7 +1321,7 @@ static struct option long_options[] = {
{"only-graph", required_argument, 0, 'O'},
{"device", required_argument, 0, 'd'},
{"blktrace-destination", required_argument, 0, 'D'},
- {"prog", required_argument, 0, 'p'},
+ {"prog", no_argument, 0, 'p'},
{"movie", optional_argument, 0, 'm'},
{"codec", optional_argument, 0, 'C'},
{"keep-movie-svgs", no_argument, 0, 'K'},
@@ -1343,7 +1344,7 @@ static void print_usage(void)
"\t-F (--fio-trace): fio bandwidth trace (more than one allowed)\n"
"\t-l (--label): trace label in the graph\n"
"\t-o (--output): output file name (SVG only)\n"
- "\t-p (--prog): program to run while blktrace is run\n"
+ "\t-p (--prog): run a program while blktrace is run\n"
"\t-K (--keep-movie-svgs keep svgs generated for movie mode\n"
"\t-m (--movie [=spindle|rect]): create IO animations\n"
"\t-C (--codec): ffmpeg codec. Use ffmpeg -codecs to list\n"
@@ -1420,9 +1421,9 @@ static int parse_options(int ac, char **av)
{
int c;
int disabled = 0;
+ int p_flagged = 0;
while (1) {
- // int this_option_optind = optind ? optind : 1;
int option_index = 0;
c = getopt_long(ac, av, option_string,
@@ -1476,7 +1477,7 @@ static int parse_options(int ac, char **av)
}
break;
case 'p':
- program_to_run = strdup(optarg);
+ p_flagged = 1;
break;
case 'K':
keep_movie_svgs = 1;
@@ -1549,6 +1550,18 @@ action_err:
break;
}
}
+
+ if (optind < ac && p_flagged) {
+ prog_argv = &av[optind];
+ prog_argc = ac - optind;
+ } else if (p_flagged) {
+ fprintf(stderr, "--prog or -p given but no program specified\n");
+ exit(1);
+ } else if (optind < ac) {
+ fprintf(stderr, "Extra arguments '%s'... (and --prog not specified)\n", av[optind]);
+ exit(1);
+ }
+
return 0;
}
@@ -1629,20 +1642,14 @@ int main(int ac, char **av)
}
start_mpstat(path);
- if (program_to_run) {
- ret = run_program(program_to_run);
- if (ret) {
- fprintf(stderr, "failed to run %s\n",
- program_to_run);
- exit(1);
- }
- wait_for_tracers();
- } else {
- /* no program specified, just wait for
- * blktrace to exit
- */
- wait_for_tracers();
+
+ if (prog_argv && prog_argc) {
+ ret = run_program(prog_argc, prog_argv, 1, NULL);
+ if (ret != 127)
+ printf("%s exited with %d\n", prog_argv[0], ret);
}
+
+ wait_for_tracers();
free(path);
}
diff --git a/iowatcher/tracers.c b/iowatcher/tracers.c
index 12efb4e..4844deb 100644
--- a/iowatcher/tracers.c
+++ b/iowatcher/tracers.c
@@ -164,22 +164,7 @@ int start_blktrace(char **devices, int num_devices, char *trace_name, char *dest
return 0;
}
-int run_program(char *str)
-{
- int ret;
-
- fprintf(stderr, "running program %s\n", str);
- ret = system(str);
- if (ret == -1) {
- fprintf(stderr, "failed to run program %s error %s\n", str, strerror(errno));
- stop_all_tracers();
- return -errno;
- }
- stop_all_tracers();
- return 0;
-}
-
-int wait_program(pid_t pid, const char *pname, int expexit)
+int wait_program(pid_t pid, const char *pname)
{
int status;
int ret = 0;
@@ -189,9 +174,6 @@ int wait_program(pid_t pid, const char *pname, int expexit)
ret = WEXITSTATUS(status);
if (ret == 127) /* spawnp failed after forking */
fprintf(stderr, "Failed to run '%s'\n", pname);
- else if (ret != expexit)
- fprintf(stderr, "'%s' exit status %d, expected %d\n",
- pname, ret, expexit);
} else if (WIFSIGNALED(status)) {
fprintf(stderr, "'%s' killed by signal %d\n", pname, WTERMSIG(status));
ret = -1;
@@ -199,7 +181,7 @@ int wait_program(pid_t pid, const char *pname, int expexit)
return ret;
}
-int run_program2(int argc, char **argv, int expexit, pid_t *pid)
+int run_program(int argc, char **argv, int wait, pid_t *pid)
{
int i;
int err;
@@ -213,8 +195,8 @@ int run_program2(int argc, char **argv, int expexit, pid_t *pid)
err = posix_spawnp(&_pid, argv[0], NULL, NULL, argv, environ);
if (err != 0) {
fprintf(stderr, "Could not run '%s': %s\n", argv[0], strerror(err));
- } else if (expexit >= 0) {
- err = wait_program(_pid, argv[0], expexit);
+ } else if (wait) {
+ err = wait_program(_pid, argv[0]);
} else if (!pid) {
fprintf(stderr, "Warning: %s (%ld): Not saving pid and not waiting for it.\n",
argv[0], (long)_pid);
@@ -227,6 +209,7 @@ int run_program2(int argc, char **argv, int expexit, pid_t *pid)
int wait_for_tracers(void)
{
int status = 0;
+ stop_all_tracers();
if (blktrace_pid != 0) {
waitpid(blktrace_pid, &status, WUNTRACED);
blktrace_pid = 0;
diff --git a/iowatcher/tracers.h b/iowatcher/tracers.h
index d0b8b6b..ed57f5d 100644
--- a/iowatcher/tracers.h
+++ b/iowatcher/tracers.h
@@ -17,9 +17,8 @@
*/
#ifndef __IOWATCH_TRACERS
#define __IOWATCH_TRACERS
-int run_program(char *str);
-int run_program2(int argc, char **argv, int expexit, pid_t *pid);
-int wait_program(pid_t pid, const char *pname, int expexit);
+int run_program(int argc, char **argv, int wait, pid_t *pid);
+int wait_program(pid_t pid, const char *pname);
int stop_blktrace(void);
int start_blktrace(char **devices, int num_devices, char *trace_name, char *dest);
int start_mpstat(char *trace_name);