[PATCH 04/11] trace-cmd: Extract parse_record_options() from trace_record()

From: Vladislav Valtchev (VMware)
Date: Thu Nov 23 2017 - 11:36:53 EST

In this patch a huge part of trace_record(), dealing with parsing the command
line options, has been extracted in a separate function. That allows the body
of trace_record() to be focused only on the proper actions involved in a given
type of tracing (record, stream, etc.) reducing, this way, the scope and the
complexity of trace_record().

Signed-off-by: Vladislav Valtchev (VMware) <vladislav.valtchev@xxxxxxxxx>
---
trace-record.c | 336 ++++++++++++++++++++++++++++++---------------------------
1 file changed, 180 insertions(+), 156 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 8f8d270..6dc828b 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4358,62 +4358,50 @@ void trace_reset(int argc, char **argv)
exit(0);
}

-void trace_record(int argc, char **argv)
+struct common_record_context {
+ struct buffer_instance *instance;
+ const char *output;
+ char *date2ts;
+ char *max_graph_depth;
+ int data_flags;
+
+ int record_all;
+ int total_disable;
+ int disable;
+ int events;
+ int global;
+ int filtered;
+ int date;
+ int manual;
+ int topt;
+ int do_child;
+ int run_command;
+
+ int extract;
+ int start;
+ int stream;
+ int profile;
+ int record;
+};
+
+
+static void parse_record_options(int argc,
+ char **argv,
+ struct common_record_context *ctx)
{
const char *plugin = NULL;
- const char *output = NULL;
const char *option;
struct event_list *event = NULL;
struct event_list *last_event = NULL;
- struct buffer_instance *instance = &top_instance;
- enum trace_type type = 0;
char *pids;
char *pid;
char *sav;
- char *date2ts = NULL;
- int record_all = 0;
- int total_disable = 0;
- int disable = 0;
- int events = 0;
- int record = 0;
- int extract = 0;
- int stream = 0;
- int profile = 0;
- int global = 0;
- int start = 0;
- int filtered = 0;
- int run_command = 0;
int neg_event = 0;
- int date = 0;
- int manual = 0;
- char *max_graph_depth = NULL;
- int topt = 0;
- int do_child = 0;
- int data_flags = 0;
-
- int c;
-
- init_instance(instance);
-
- cpu_count = count_cpus();
-
- if ((record = (strcmp(argv[1], "record") == 0)))
- ; /* do nothing */
- else if ((start = strcmp(argv[1], "start") == 0))
- ; /* do nothing */
- else if ((extract = strcmp(argv[1], "extract") == 0))
- ; /* do nothing */
- else if ((stream = strcmp(argv[1], "stream") == 0))
- ; /* do nothing */
- else if ((profile = strcmp(argv[1], "profile") == 0)) {
- handle_init = trace_init_profile;
- events = 1;
- } else
- usage(argv);

for (;;) {
int option_index = 0;
int ret;
+ int c;
const char *opts;
static struct option long_options[] = {
{"date", no_argument, NULL, OPT_date},
@@ -4431,7 +4419,7 @@ void trace_record(int argc, char **argv)
{NULL, 0, NULL, 0}
};

- if (extract)
+ if (ctx->extract)
opts = "+haf:Fp:co:O:sr:g:l:n:P:N:tb:B:ksiT";
else
opts = "+hae:f:Fp:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
@@ -4443,26 +4431,26 @@ void trace_record(int argc, char **argv)
usage(argv);
break;
case 'a':
- if (extract) {
+ if (ctx->extract) {
add_all_instances();
} else {
- record_all = 1;
+ ctx->record_all = 1;
record_all_events();
}
break;
case 'e':
- events = 1;
+ ctx->events = 1;
event = malloc(sizeof(*event));
if (!event)
die("Failed to allocate event %s", optarg);
memset(event, 0, sizeof(*event));
event->event = optarg;
- add_event(instance, event);
+ add_event(ctx->instance, event);
event->neg = neg_event;
event->filter = NULL;
last_event = event;

- if (!record_all)
+ if (!ctx->record_all)
list_event(optarg);
break;
case 'f':
@@ -4495,7 +4483,7 @@ void trace_record(int argc, char **argv)
filter_task = 1;
break;
case 'G':
- global = 1;
+ ctx->global = 1;
break;
case 'P':
test_set_event_pid();
@@ -4518,35 +4506,35 @@ void trace_record(int argc, char **argv)
do_ptrace = 1;
} else {
save_option("event-fork");
- do_child = 1;
+ ctx->do_child = 1;
}
break;
case 'C':
- instance->clock = optarg;
+ ctx->instance->clock = optarg;
break;
case 'v':
neg_event = 1;
break;
case 'l':
- add_func(&instance->filter_funcs,
- instance->filter_mod, optarg);
- filtered = 1;
+ add_func(&ctx->instance->filter_funcs,
+ ctx->instance->filter_mod, optarg);
+ ctx->filtered = 1;
break;
case 'n':
- add_func(&instance->notrace_funcs,
- instance->filter_mod, optarg);
- filtered = 1;
+ add_func(&ctx->instance->notrace_funcs,
+ ctx->instance->filter_mod, optarg);
+ ctx->filtered = 1;
break;
case 'g':
- add_func(&graph_funcs, instance->filter_mod, optarg);
- filtered = 1;
+ add_func(&graph_funcs, ctx->instance->filter_mod, optarg);
+ ctx->filtered = 1;
break;
case 'p':
- if (instance->plugin)
+ if (ctx->instance->plugin)
die("only one plugin allowed");
for (plugin = optarg; isspace(*plugin); plugin++)
;
- instance->plugin = plugin;
+ ctx->instance->plugin = plugin;
for (optarg += strlen(optarg) - 1;
optarg > plugin && isspace(*optarg); optarg--)
;
@@ -4554,25 +4542,25 @@ void trace_record(int argc, char **argv)
optarg[0] = '\0';
break;
case 'D':
- total_disable = 1;
+ ctx->total_disable = 1;
/* fall through */
case 'd':
- disable = 1;
+ ctx->disable = 1;
break;
case 'o':
if (host)
die("-o incompatible with -N");
- if (start)
+ if (ctx->start)
die("start does not take output\n"
"Did you mean 'record'?");
- if (stream)
+ if (ctx->stream)
die("stream does not take output\n"
"Did you mean 'record'?");
- if (output)
+ if (ctx->output)
die("only one output file allowed");
- output = optarg;
+ ctx->output = optarg;

- if (profile) {
+ if (ctx->profile) {
int fd;

/* pipe the output to this file instead of stdout */
@@ -4595,11 +4583,11 @@ void trace_record(int argc, char **argv)
save_option("stacktrace");
break;
case 'H':
- add_hook(instance, optarg);
- events = 1;
+ add_hook(ctx->instance, optarg);
+ ctx->events = 1;
break;
case 's':
- if (extract) {
+ if (ctx->extract) {
if (optarg)
usage(argv);
recorder_flags |= TRACECMD_RECORD_SNAPSHOT;
@@ -4610,47 +4598,47 @@ void trace_record(int argc, char **argv)
sleep_time = atoi(optarg);
break;
case 'S':
- manual = 1;
+ ctx->manual = 1;
/* User sets events for profiling */
if (!event)
- events = 0;
+ ctx->events = 0;
break;
case 'r':
rt_prio = atoi(optarg);
break;
case 'N':
- if (!record)
+ if (!ctx->record)
die("-N only available with record");
- if (output)
+ if (ctx->output)
die("-N incompatible with -o");
host = optarg;
break;
case 'm':
if (max_kb)
die("-m can only be specified once");
- if (!record)
+ if (!ctx->record)
die("only record take 'm' option");
max_kb = atoi(optarg);
break;
case 'M':
- instance->cpumask = alloc_mask_from_hex(optarg);
+ ctx->instance->cpumask = alloc_mask_from_hex(optarg);
break;
case 't':
- if (extract)
- topt = 1; /* Extract top instance also */
+ if (ctx->extract)
+ ctx->topt = 1; /* Extract top instance also */
else
use_tcp = 1;
break;
case 'b':
- instance->buffer_size = atoi(optarg);
+ ctx->instance->buffer_size = atoi(optarg);
break;
case 'B':
- instance = create_instance(optarg);
- if (!instance)
+ ctx->instance = create_instance(optarg);
+ if (!ctx->instance)
die("Failed to create instance");
- add_instance(instance);
- if (profile)
- instance->profile = 1;
+ add_instance(ctx->instance);
+ if (ctx->profile)
+ ctx->instance->profile = 1;
break;
case 'k':
keep = 1;
@@ -4659,10 +4647,10 @@ void trace_record(int argc, char **argv)
ignore_event_not_found = 1;
break;
case OPT_date:
- date = 1;
- if (data_flags & DATA_FL_OFFSET)
+ ctx->date = 1;
+ if (ctx->data_flags & DATA_FL_OFFSET)
die("Can not use both --date and --ts-offset");
- data_flags |= DATA_FL_DATE;
+ ctx->data_flags |= DATA_FL_DATE;
break;
case OPT_funcstack:
func_stack = 1;
@@ -4672,8 +4660,8 @@ void trace_record(int argc, char **argv)
break;
case OPT_profile:
handle_init = trace_init_profile;
- instance->profile = 1;
- events = 1;
+ ctx->instance->profile = 1;
+ ctx->events = 1;
break;
case OPT_stderr:
/* if -o was used (for profile), ignore this */
@@ -4687,26 +4675,26 @@ void trace_record(int argc, char **argv)
trace_profile_set_merge_like_comms();
break;
case OPT_tsoffset:
- date2ts = strdup(optarg);
- if (data_flags & DATA_FL_DATE)
+ ctx->date2ts = strdup(optarg);
+ if (ctx->data_flags & DATA_FL_DATE)
die("Can not use both --date and --ts-offset");
- data_flags |= DATA_FL_OFFSET;
+ ctx->data_flags |= DATA_FL_OFFSET;
break;
case OPT_max_graph_depth:
- free(max_graph_depth);
- max_graph_depth = strdup(optarg);
- if (!max_graph_depth)
+ free(ctx->max_graph_depth);
+ ctx->max_graph_depth = strdup(optarg);
+ if (!ctx->max_graph_depth)
die("Could not allocate option");
break;
case OPT_debug:
debug = 1;
break;
case OPT_module:
- if (instance->filter_mod)
- add_func(&instance->filter_funcs,
- instance->filter_mod, "*");
- instance->filter_mod = optarg;
- filtered = 0;
+ if (ctx->instance->filter_mod)
+ add_func(&ctx->instance->filter_funcs,
+ ctx->instance->filter_mod, "*");
+ ctx->instance->filter_mod = optarg;
+ ctx->filtered = 0;
break;
case OPT_quiet:
case 'q':
@@ -4717,106 +4705,141 @@ void trace_record(int argc, char **argv)
}
}

- if (!filtered && instance->filter_mod)
- add_func(&instance->filter_funcs,
- instance->filter_mod, "*");
+ if (!ctx->filtered && ctx->instance->filter_mod)
+ add_func(&ctx->instance->filter_funcs,
+ ctx->instance->filter_mod, "*");

if (do_ptrace && !filter_task && (filter_pid < 0))
die(" -c can only be used with -F (or -P with event-fork support)");
- if (do_child && !filter_task &&! filter_pid)
+ if (ctx->do_child && !filter_task &&! filter_pid)
die(" -c can only be used with -P or -F");

if ((argc - optind) >= 2) {
- if (start)
+ if (ctx->start)
die("Command start does not take any commands\n"
"Did you mean 'record'?");
- if (extract)
+ if (ctx->extract)
die("Command extract does not take any commands\n"
"Did you mean 'record'?");
- run_command = 1;
+ ctx->run_command = 1;
}

+}
+
+static void init_common_record_context(struct common_record_context *ctx)
+{
+ memset(ctx, 0, sizeof(*ctx));
+ ctx->instance = &top_instance;
+ cpu_count = count_cpus();
+}
+
+void trace_record(int argc, char **argv)
+{
+ struct common_record_context ctx;
+ enum trace_type type = 0;
+
+ init_common_record_context(&ctx);
+ init_instance(ctx.instance);
+
+ if ((ctx.record = (strcmp(argv[1], "record") == 0)))
+ ; /* do nothing */
+ else if ((ctx.start = strcmp(argv[1], "start") == 0))
+ ; /* do nothing */
+ else if ((ctx.extract = strcmp(argv[1], "extract") == 0))
+ ; /* do nothing */
+ else if ((ctx.stream = strcmp(argv[1], "stream") == 0))
+ ; /* do nothing */
+ else if ((ctx.profile = strcmp(argv[1], "profile") == 0)) {
+ handle_init = trace_init_profile;
+ ctx.events = 1;
+ } else
+ usage(argv);
+
+
+ parse_record_options(argc, argv, &ctx);
+
+
/*
* If this is a profile run, and no instances were set,
* then enable profiling on the top instance.
*/
- if (profile && !buffer_instances)
+ if (ctx.profile && !buffer_instances)
top_instance.profile = 1;

/*
* If top_instance doesn't have any plugins or events, then
* remove it from being processed.
*/
- if (!extract && !__check_doing_something(&top_instance)) {
+ if (!ctx.extract && !__check_doing_something(&top_instance)) {
if (!buffer_instances)
die("No instances reference??");
first_instance = buffer_instances;
} else
- topt = 1;
+ ctx.topt = 1;

- update_first_instance(instance, topt);
+ update_first_instance(ctx.instance, ctx.topt);

- if (!extract)
+ if (!ctx.extract)
check_doing_something();
check_function_plugin();

- if (output)
- output_file = output;
+ if (ctx.output)
+ output_file = ctx.output;

/* Save the state of tracing_on before starting */
- for_all_instances(instance) {
+ for_all_instances(ctx.instance) {

- if (!manual && instance->profile)
- enable_profile(instance);
+ if (!ctx.manual && ctx.instance->profile)
+ enable_profile(ctx.instance);

- instance->tracing_on_init_val = read_tracing_on(instance);
+ ctx.instance->tracing_on_init_val = read_tracing_on(ctx.instance);
/* Some instances may not be created yet */
- if (instance->tracing_on_init_val < 0)
- instance->tracing_on_init_val = 1;
+ if (ctx.instance->tracing_on_init_val < 0)
+ ctx.instance->tracing_on_init_val = 1;
}

/* Extracting data records all events in the system. */
- if (extract && !record_all)
+ if (ctx.extract && !ctx.record_all)
record_all_events();

- if (!extract)
+ if (!ctx.extract)
make_instances();

- if (events)
+ if (ctx.events)
expand_event_list();

page_size = getpagesize();

- if (!extract) {
- fset = set_ftrace(!disable, total_disable);
+ if (!ctx.extract) {
+ fset = set_ftrace(!ctx.disable, ctx.total_disable);
tracecmd_disable_all_tracing(1);

- for_all_instances(instance)
- set_clock(instance);
+ for_all_instances(ctx.instance)
+ set_clock(ctx.instance);

/* Record records the date first */
- if (record && date)
- date2ts = get_date_to_ts();
+ if (ctx.record && ctx.date)
+ ctx.date2ts = get_date_to_ts();

- for_all_instances(instance) {
- set_funcs(instance);
- set_mask(instance);
+ for_all_instances(ctx.instance) {
+ set_funcs(ctx.instance);
+ set_mask(ctx.instance);
}

- if (events) {
- for_all_instances(instance)
- enable_events(instance);
+ if (ctx.events) {
+ for_all_instances(ctx.instance)
+ enable_events(ctx.instance);
}
set_buffer_size();
}

- if (record)
+ if (ctx.record)
type = TRACE_TYPE_RECORD;
- else if (stream)
+ else if (ctx.stream)
type = TRACE_TYPE_STREAM;
- else if (extract)
+ else if (ctx.extract)
type = TRACE_TYPE_EXTRACT;
- else if (profile)
+ else if (ctx.profile)
type = TRACE_TYPE_STREAM;
else
type = TRACE_TYPE_START;
@@ -4825,10 +4848,10 @@ void trace_record(int argc, char **argv)

set_options();

- if (max_graph_depth) {
- for_all_instances(instance)
- set_max_graph_depth(instance, max_graph_depth);
- free(max_graph_depth);
+ if (ctx.max_graph_depth) {
+ for_all_instances(ctx.instance)
+ set_max_graph_depth(ctx.instance, ctx.max_graph_depth);
+ free(ctx.max_graph_depth);
}

allocate_seq();
@@ -4836,10 +4859,10 @@ void trace_record(int argc, char **argv)
if (type & (TRACE_TYPE_RECORD | TRACE_TYPE_STREAM)) {
signal(SIGINT, finish);
if (!latency)
- start_threads(type, global);
+ start_threads(type, ctx.global);
}

- if (extract) {
+ if (ctx.extract) {
flush_threads();

} else {
@@ -4849,7 +4872,7 @@ void trace_record(int argc, char **argv)
exit(0);
}

- if (run_command)
+ if (ctx.run_command)
run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
else {
update_task_filter();
@@ -4874,17 +4897,17 @@ void trace_record(int argc, char **argv)
tracecmd_disable_all_tracing(0);

/* extract records the date after extraction */
- if (extract && date) {
+ if (ctx.extract && ctx.date) {
/*
* We need to start tracing, don't let other traces
* screw with our trace_marker.
*/
tracecmd_disable_all_tracing(1);
- date2ts = get_date_to_ts();
+ ctx.date2ts = get_date_to_ts();
}

- if (record || extract) {
- record_data(date2ts, data_flags);
+ if (ctx.record || ctx.extract) {
+ record_data(ctx.date2ts, ctx.data_flags);
delete_thread_data();
} else
print_stats();
@@ -4904,15 +4927,16 @@ void trace_record(int argc, char **argv)
tracecmd_remove_instances();

/* If tracing_on was enabled before we started, set it on now */
- for_all_instances(instance) {
- if (instance->keep)
- write_tracing_on(instance, instance->tracing_on_init_val);
+ for_all_instances(ctx.instance) {
+ if (ctx.instance->keep)
+ write_tracing_on(ctx.instance,
+ ctx.instance->tracing_on_init_val);
}

if (host)
tracecmd_output_close(network_handle);

- if (profile)
+ if (ctx.profile)
trace_profile();

exit(0);
--
2.14.1