Message ID | 20210622171338.6447f199@gandalf.local.home (mailing list archive) |
---|---|
State | Accepted |
Commit | 8618f7aa8c2df7fbc9e1f404a97191055a6167a2 |
Headers | show |
Series | trace-cmd split: Copy trace_clock from input handler to output handler | expand |
On Tue, 22 Jun 2021, Steven Rostedt wrote: > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> > > When splitting a trace file, the clock is needed to create a output file > from the input file. But the output descriptor clock is never set, and the > clock returned from retrieving the descriptor is NULL (unless you are > root, in which case the code will read the machine trace clock instead, Thanks for the fix. I should have mentioned another difference between the machine where it worked and the machine where it didn't, which is that I was root on the former and not root on the latter. > and continue as if nothing was wrong). This caused a failure, and once > again, trace-cmd split, errored out without failure, but just did not > append all the data. > > Although this fixes the commit listed below, the problem was there before, > as the code before that commit tried to read the clock from the file > system, but just wouldn't error out if it couldn't read it. > > Add a new function to retrieve the trace_clock from the input handle, to > be used to pass it to the output handle before appending the CPU data. Just to confirm, is it still possible to run trace-cmd split without being root? thanks, julia > Reported-by: Julia Lawall <julia.lawall@inria.fr> > Fixes: 72670886 ("trace-cmd: Save only the selected clock in the trace.dat file") > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > lib/trace-cmd/include/private/trace-cmd-private.h | 3 ++- > lib/trace-cmd/trace-input.c | 13 +++++++++++++ > lib/trace-cmd/trace-output.c | 2 +- > tracecmd/trace-split.c | 1 + > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h > index 7194cb30..6440084d 100644 > --- a/lib/trace-cmd/include/private/trace-cmd-private.h > +++ b/lib/trace-cmd/include/private/trace-cmd-private.h > @@ -87,7 +87,8 @@ tracecmd_plugin_context_output(struct trace_plugin_context *trace_context); > > void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet); > bool tracecmd_get_quiet(struct tracecmd_output *handle); > -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock); > +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock); > +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle); > > static inline int tracecmd_host_bigendian(void) > { > diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c > index 655bd654..ac57bc4f 100644 > --- a/lib/trace-cmd/trace-input.c > +++ b/lib/trace-cmd/trace-input.c > @@ -4075,6 +4075,19 @@ size_t tracecmd_get_options_offset(struct tracecmd_input *handle) > return handle->options_start; > } > > +/** > + * tracecmd_get_trace_clock - return the saved trace clock > + * @handle: input handle for the trace.dat file > + * > + * Returns a string of the clock that was saved in the trace.dat file. > + * The string should not be freed, as it points to the internal > + * structure data. > + */ > +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle) > +{ > + return handle->trace_clock; > +} > + > /** > * tracecmd_get_show_data_func - return the show data func > * @handle: input handle for the trace.dat file > diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c > index 78a25350..a8de107c 100644 > --- a/lib/trace-cmd/trace-output.c > +++ b/lib/trace-cmd/trace-output.c > @@ -122,7 +122,7 @@ void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet) > handle->quiet = set_quiet; > } > > -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock) > +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock) > { > if (handle && clock) > handle->trace_clock = strdup(clock); > diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c > index 8366d128..10d0943d 100644 > --- a/tracecmd/trace-split.c > +++ b/tracecmd/trace-split.c > @@ -384,6 +384,7 @@ static double parse_file(struct tracecmd_input *handle, > for (cpu = 0; cpu < cpus; cpu ++) > cpu_list[cpu] = cpu_data[cpu].file; > > + tracecmd_set_out_clock(ohandle, tracecmd_get_trace_clock(handle)); > tracecmd_append_cpu_data(ohandle, cpus, cpu_list); > > current = end; > -- > 2.29.2 > >
On Tue, 22 Jun 2021 23:25:23 +0200 (CEST) Julia Lawall <julia.lawall@inria.fr> wrote: > > Add a new function to retrieve the trace_clock from the input handle, to > > be used to pass it to the output handle before appending the CPU data. > > Just to confirm, is it still possible to run trace-cmd split without being > root? Yes, it was a bug that you couldn't read it as non-root. And it shouldn't be reading the local trace_clock anyway, even if you are root. 1) because it could be splitting a trace.dat file from another machine (which is what I was doing) 2) the trace_clock of the machine could have changed since the trace was run. -- Steve
diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h index 7194cb30..6440084d 100644 --- a/lib/trace-cmd/include/private/trace-cmd-private.h +++ b/lib/trace-cmd/include/private/trace-cmd-private.h @@ -87,7 +87,8 @@ tracecmd_plugin_context_output(struct trace_plugin_context *trace_context); void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet); bool tracecmd_get_quiet(struct tracecmd_output *handle); -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock); +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock); +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle); static inline int tracecmd_host_bigendian(void) { diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c index 655bd654..ac57bc4f 100644 --- a/lib/trace-cmd/trace-input.c +++ b/lib/trace-cmd/trace-input.c @@ -4075,6 +4075,19 @@ size_t tracecmd_get_options_offset(struct tracecmd_input *handle) return handle->options_start; } +/** + * tracecmd_get_trace_clock - return the saved trace clock + * @handle: input handle for the trace.dat file + * + * Returns a string of the clock that was saved in the trace.dat file. + * The string should not be freed, as it points to the internal + * structure data. + */ +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle) +{ + return handle->trace_clock; +} + /** * tracecmd_get_show_data_func - return the show data func * @handle: input handle for the trace.dat file diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c index 78a25350..a8de107c 100644 --- a/lib/trace-cmd/trace-output.c +++ b/lib/trace-cmd/trace-output.c @@ -122,7 +122,7 @@ void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet) handle->quiet = set_quiet; } -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock) +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock) { if (handle && clock) handle->trace_clock = strdup(clock); diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c index 8366d128..10d0943d 100644 --- a/tracecmd/trace-split.c +++ b/tracecmd/trace-split.c @@ -384,6 +384,7 @@ static double parse_file(struct tracecmd_input *handle, for (cpu = 0; cpu < cpus; cpu ++) cpu_list[cpu] = cpu_data[cpu].file; + tracecmd_set_out_clock(ohandle, tracecmd_get_trace_clock(handle)); tracecmd_append_cpu_data(ohandle, cpus, cpu_list); current = end;