diff mbox series

[2/2] trace-cmd: Add new API tracecmd_open_head()

Message ID 20200409132825.79475-3-tz.stoyanov@gmail.com (mailing list archive)
State Accepted
Headers show
Series Split reading the trace.dat options from trace data | expand

Commit Message

Tzvetomir Stoyanov (VMware) April 9, 2020, 1:28 p.m. UTC
Add an API to create a tracecmd_handle from a file,
read and parse only the trace headers from the file.
This allows to implement opening a trace file on
stages - reading the trace headers and reading the trace data.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h |  1 +
 lib/trace-cmd/trace-input.c   | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

Comments

Steven Rostedt Jan. 13, 2021, 10:01 p.m. UTC | #1
On Thu,  9 Apr 2020 16:28:25 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Add an API to create a tracecmd_handle from a file,
> read and parse only the trace headers from the file.
> This allows to implement opening a trace file on
> stages - reading the trace headers and reading the trace data.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h |  1 +
>  lib/trace-cmd/trace-input.c   | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index 3f96bbde..e22aa251 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -143,6 +143,7 @@ typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
>  struct tracecmd_input *tracecmd_alloc(const char *file);
>  struct tracecmd_input *tracecmd_alloc_fd(int fd);
>  struct tracecmd_input *tracecmd_open(const char *file);
> +struct tracecmd_input *tracecmd_open_head(const char *file);
>  struct tracecmd_input *tracecmd_open_fd(int fd);
>  void tracecmd_ref(struct tracecmd_input *handle);
>  void tracecmd_close(struct tracecmd_input *handle);

Hi Tzvetomir,

I was thinking, before we release libtracecmd, we should add
tracecmd_open() and tracecmd_open_fd() to the library. Just because I find
that tracecmd_open_head() is a awkward name to have alone and only makes
sense if there's already a tracecmd_open(). It will also help with
differentiating the different functions.

I know we were going to only have functions that kernelshark uses, but that
may be a little too limiting.

Thanks!

-- Steve
Tzvetomir Stoyanov (VMware) Jan. 15, 2021, 4:54 a.m. UTC | #2
On Thu, Jan 14, 2021 at 12:01 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu,  9 Apr 2020 16:28:25 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Add an API to create a tracecmd_handle from a file,
> > read and parse only the trace headers from the file.
> > This allows to implement opening a trace file on
> > stages - reading the trace headers and reading the trace data.
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  include/trace-cmd/trace-cmd.h |  1 +
> >  lib/trace-cmd/trace-input.c   | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> > index 3f96bbde..e22aa251 100644
> > --- a/include/trace-cmd/trace-cmd.h
> > +++ b/include/trace-cmd/trace-cmd.h
> > @@ -143,6 +143,7 @@ typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
> >  struct tracecmd_input *tracecmd_alloc(const char *file);
> >  struct tracecmd_input *tracecmd_alloc_fd(int fd);
> >  struct tracecmd_input *tracecmd_open(const char *file);
> > +struct tracecmd_input *tracecmd_open_head(const char *file);
> >  struct tracecmd_input *tracecmd_open_fd(int fd);
> >  void tracecmd_ref(struct tracecmd_input *handle);
> >  void tracecmd_close(struct tracecmd_input *handle);
>
> Hi Tzvetomir,
>
> I was thinking, before we release libtracecmd, we should add
> tracecmd_open() and tracecmd_open_fd() to the library. Just because I find
> that tracecmd_open_head() is a awkward name to have alone and only makes
> sense if there's already a tracecmd_open(). It will also help with
> differentiating the different functions.

I'll add tracecmd_open() and tracecmd_open_fd() as APIs. The reason
why tracecmd_open_head()
is used by KernelShark is that tracecmd_open() reads headers + tracing
data and calculates the
timestamps. Thus makes it hard to open multiple tracing files from a
single trace session and adjust
timestamps to be in a same time space. The current KernelShark open flow is:
 - Open a trace file and parse only the headers with tracecmd_open_head()
 - Based on the metadata information from the headers, check if there
is already an opened file from
  the same tracing session. If such is found, pair both files with
tracecmd_pair_peer(), before reading
  the tracing data.
 - Read the tracing data from the file with tracecmd_init_data(). This
will adjust timestamps accordingly,
   if there is a paired peer to the file.

We can rename tracecmd_open_head() to something else, or redesign the
whole flow of opening trace files

>
> I know we were going to only have functions that kernelshark uses, but that
> may be a little too limiting.
>
> Thanks!
>
> -- Steve
diff mbox series

Patch

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 3f96bbde..e22aa251 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -143,6 +143,7 @@  typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
 struct tracecmd_input *tracecmd_alloc(const char *file);
 struct tracecmd_input *tracecmd_alloc_fd(int fd);
 struct tracecmd_input *tracecmd_open(const char *file);
+struct tracecmd_input *tracecmd_open_head(const char *file);
 struct tracecmd_input *tracecmd_open_fd(int fd);
 void tracecmd_ref(struct tracecmd_input *handle);
 void tracecmd_close(struct tracecmd_input *handle);
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index ee9bfb52..efc8d4bd 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3175,6 +3175,34 @@  struct tracecmd_input *tracecmd_open(const char *file)
 	return tracecmd_open_fd(fd);
 }
 
+/**
+ * tracecmd_open_head - create a tracecmd_handle from a given file, read
+ *			read and parse only the trace headers from the file
+ * @file: the file name of the file that is of tracecmd data type.
+ */
+struct tracecmd_input *tracecmd_open_head(const char *file)
+{
+	struct tracecmd_input *handle;
+	int fd;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	handle = tracecmd_alloc_fd(fd);
+	if (!handle)
+		return NULL;
+
+	if (tracecmd_read_headers(handle) < 0)
+		goto fail;
+
+	return handle;
+
+fail:
+	tracecmd_close(handle);
+	return NULL;
+}
+
 /**
  * tracecmd_ref - add a reference to the handle
  * @handle: input handle for the trace.dat file