On Fri, 29 May 2020 10:02:19 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
>
> +static void set_cmd_check(struct common_record_context *ctx, char *param)
> +{
> + if (IS_CMDSET(ctx)) {
> + die("%s has no effect with set command\n"
> + "Did you mean 'record'?", param);
> + }
> +}
> +
> static void parse_record_options(int argc,
> char **argv,
> enum trace_cmd curr_cmd,
> @@ -5803,6 +5814,9 @@ static void parse_record_options(int argc,
>
> init_common_record_context(ctx, curr_cmd);
>
> + if (IS_CMDSET(ctx))
> + keep = 1;
> +
> for (;;) {
> int option_index = 0;
> int ret;
> @@ -5854,6 +5868,7 @@ static void parse_record_options(int argc,
> usage(argv);
> break;
> case 'a':
> + set_cmd_check(ctx, "-a");
> if (IS_EXTRACT(ctx)) {
> add_all_instances();
> } else {
> @@ -5927,10 +5942,12 @@ static void parse_record_options(int argc,
> break;
> }
> case 'F':
> + set_cmd_check(ctx, "-F");
> test_set_event_pid(ctx->instance);
> filter_task = 1;
> break;
I was thinking that perhaps we should have -F stay, and that way
anything we add can be filtered with the command. I think the same
should be allowed for start too. There's been several times I wanted
the code to just trace a command (not record it though), but couldn't
do it with trace-cmd.
> case 'G':
> + set_cmd_check(ctx, "-G");
Hmm, I just realized that we don't have -G documented.
> ctx->global = 1;
> break;
> case 'P':
> @@ -6006,6 +6023,7 @@ static void parse_record_options(int argc,
> ctx->disable = 1;
> break;
> case 'o':
> + set_cmd_check(ctx, "-o");
> if (IS_RECORD_AGENT(ctx))
> die("-o incompatible with agent recording");
> if (host)
> @@ -6043,10 +6061,12 @@ static void parse_record_options(int argc,
> save_option(ctx->instance, "stacktrace");
> break;
> case 'H':
> + set_cmd_check(ctx, "-H");
> add_hook(ctx->instance, optarg);
> ctx->events = 1;
> break;
> case 's':
> + set_cmd_check(ctx, "-s");
> if (IS_EXTRACT(ctx)) {
> if (optarg)
> usage(argv);
> @@ -6058,15 +6078,18 @@ static void parse_record_options(int argc,
> sleep_time = atoi(optarg);
> break;
> case 'S':
> + set_cmd_check(ctx, "-S");
> ctx->manual = 1;
> /* User sets events for profiling */
> if (!event)
> ctx->events = 0;
> break;
> case 'r':
> + set_cmd_check(ctx, "-r");
> rt_prio = atoi(optarg);
> break;
> case 'N':
> + set_cmd_check(ctx, "-N");
> if (!IS_RECORD(ctx))
> die("-N only available with record");
> if (IS_RECORD_AGENT(ctx))
> @@ -6086,6 +6109,7 @@ static void parse_record_options(int argc,
> ctx->instance->cpumask = alloc_mask_from_hex(ctx->instance, optarg);
> break;
> case 't':
> + set_cmd_check(ctx, "-t");
> if (IS_EXTRACT(ctx))
> ctx->topt = 1; /* Extract top instance also */
> else
> @@ -6103,20 +6127,24 @@ static void parse_record_options(int argc,
> ctx->instance->flags |= BUFFER_FL_PROFILE;
> break;
> case 'k':
> + set_cmd_check(ctx, "-k");
> keep = 1;
> break;
> case 'i':
> ignore_event_not_found = 1;
> break;
> case OPT_user:
> + set_cmd_check(ctx, "--user");
If we allow commands, then perhaps we can keep this too.
> ctx->user = strdup(optarg);
> if (!ctx->user)
> die("Failed to allocate user name");
> break;
> case OPT_procmap:
> + set_cmd_check(ctx, "--proc-map");
> ctx->instance->get_procmap = 1;
> break;
> case OPT_date:
> + set_cmd_check(ctx, "--date");
> ctx->date = 1;
> if (ctx->data_flags & DATA_FL_OFFSET)
> die("Can not use both --date and --ts-offset");
> @@ -6126,12 +6154,15 @@ static void parse_record_options(int argc,
> func_stack = 1;
> break;
> case OPT_nosplice:
> + set_cmd_check(ctx, "--nosplice");
> recorder_flags |= TRACECMD_RECORD_NOSPLICE;
> break;
> case OPT_nofifos:
> + set_cmd_check(ctx, "--nofifos");
> no_fifos = true;
> break;
> case OPT_profile:
> + set_cmd_check(ctx, "--profile");
> handle_init = trace_init_profile;
> ctx->instance->flags |= BUFFER_FL_PROFILE;
> ctx->events = 1;
> @@ -6145,9 +6176,11 @@ static void parse_record_options(int argc,
> dup2(2, 1);
> break;
> case OPT_bycomm:
> + set_cmd_check(ctx, "--by-comm");
> trace_profile_set_merge_like_comms();
> break;
> case OPT_tsoffset:
> + set_cmd_check(ctx, "--ts-offset");
> ctx->date2ts = strdup(optarg);
> if (ctx->data_flags & DATA_FL_DATE)
> die("Can not use both --date and --ts-offset");
> @@ -6160,9 +6193,11 @@ static void parse_record_options(int argc,
> die("Could not allocate option");
> break;
> case OPT_cmdlines_size:
> + set_cmd_check(ctx, "--cmdlines-size");
--cmdlines-size is something we definitely want to keep here. It's a
file in the tracefs directory, and something that we should allow
trace-cmd set to modify.
> ctx->saved_cmdlines_size = atoi(optarg);
> break;
> case OPT_no_filter:
> + set_cmd_check(ctx, "--no-filter");
> no_filter = true;
> break;
> case OPT_debug:
> @@ -6176,6 +6211,7 @@ static void parse_record_options(int argc,
> ctx->filtered = 0;
> break;
> case OPT_tsyncinterval:
> + set_cmd_check(ctx, "--tsync-interval");
> top_instance.tsync.loop_interval = atoi(optarg);
> guest_sync_set = true;
> break;
> @@ -6221,8 +6257,8 @@ static void parse_record_options(int argc,
> die(" -c can only be used with -F (or -P with event-fork support)");
>
> if ((argc - optind) >= 2) {
> - if (IS_START(ctx))
> - die("Command start does not take any commands\n"
> + if (IS_START(ctx) || IS_CMDSET(ctx))
> + die("Commands start and set do not take any commands\n"
Perhaps we should add a patch first that lets trace-cmd start run commands?
-- Steve
> "Did you mean 'record'?");
> if (IS_EXTRACT(ctx))
> die("Command extract does not take any commands\n"
> @@ -6265,7 +6301,8 @@ static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
> {CMD_extract, TRACE_TYPE_EXTRACT},
> {CMD_profile, TRACE_TYPE_STREAM},
> {CMD_start, TRACE_TYPE_START},
> - {CMD_record_agent, TRACE_TYPE_RECORD}
> + {CMD_record_agent, TRACE_TYPE_RECORD},
> + {CMD_set, TRACE_TYPE_SET}
> };
>
> for (int i = 0; i < ARRAY_SIZE(trace_type_per_command); i++) {
> @@ -6342,8 +6379,10 @@ static void record_trace(int argc, char **argv,
> ctx->topt = 1;
>
> update_first_instance(ctx->instance, ctx->topt);
> - check_doing_something();
> - check_function_plugin();
> + if (!IS_CMDSET(ctx)) {
> + check_doing_something();
> + check_function_plugin();
> + }
>
> if (!ctx->output)
> ctx->output = DEFAULT_INPUT_FILE;
> @@ -6371,7 +6410,8 @@ static void record_trace(int argc, char **argv,
>
> if (!is_guest(ctx->instance))
> fset = set_ftrace(!ctx->disable, ctx->total_disable);
> - tracecmd_disable_all_tracing(1);
> + if (!IS_CMDSET(ctx))
> + tracecmd_disable_all_tracing(1);
>
> for_all_instances(instance)
> set_clock(instance);
> @@ -6412,7 +6452,8 @@ static void record_trace(int argc, char **argv,
> start_threads(type, ctx);
> } else {
> update_task_filter();
> - tracecmd_enable_tracing();
> + if (!IS_CMDSET(ctx))
> + tracecmd_enable_tracing();
> exit(0);
> }
>
> @@ -6486,6 +6527,15 @@ void trace_start(int argc, char **argv)
> exit(0);
> }
>
|