diff mbox series

[v3,3/6] trace-cmd: Add tracecmd_create_recorder_virt function

Message ID 20190114152800.31060-4-kaslevs@vmware.com (mailing list archive)
State Superseded
Headers show
Series Add VM kernel tracing over vsock sockets | expand

Commit Message

Slavomir Kaslev Jan. 14, 2019, 3:27 p.m. UTC
Add tracecmd_create_recorder_virt function that will be used for
tracing VM guests.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 include/trace-cmd/trace-cmd.h  |  1 +
 lib/trace-cmd/trace-recorder.c | 53 ++++++++++++++++++++++++----------
 2 files changed, 39 insertions(+), 15 deletions(-)

Comments

Steven Rostedt Jan. 14, 2019, 10:10 p.m. UTC | #1
On Mon, 14 Jan 2019 17:27:57 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:


> @@ -194,8 +196,9 @@ tracecmd_create_buffer_recorder_fd(int fd, int cpu, unsigned flags, const char *
>  	return tracecmd_create_buffer_recorder_fd2(fd, -1, cpu, flags, buffer, 0);
>  }
>  
> -struct tracecmd_recorder *
> -tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags, const char *buffer)
> +static struct tracecmd_recorder *
> +__tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
> +				  const char *buffer)
>  {
>  	struct tracecmd_recorder *recorder;
>  	int fd;
> @@ -258,6 +261,26 @@ tracecmd_create_buffer_recorder_maxkb(const char *file, int cpu, unsigned flags,
>  	goto out;
>  }
>  
> +struct tracecmd_recorder *
> +tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
> +				const char *buffer)
> +{
> +	return __tracecmd_create_buffer_recorder(file, cpu, flags, buffer);
> +}
> +
> +struct tracecmd_recorder *
> +tracecmd_create_recorder_virt(const char *file, int cpu, int trace_fd)

As this looks to be something that may be a library call someday, I
would keep flags as a parameter.

> +{
> +	struct tracecmd_recorder *recorder;
> +
> +	recorder = __tracecmd_create_buffer_recorder(
> +		file, cpu, TRACECMD_RECORD_NOSPLICE, NULL);

Then you could pass this as:

	recorder = __tracecmd_create_buffer_recorder(file, cpu,
			flags | TRACECMD_RECORD_NOSPLICE, NULL);

-- Steve

> +	if (recorder)
> +		recorder->trace_fd = trace_fd;
> +
> +	return recorder;
> +}
> +
>  struct tracecmd_recorder *tracecmd_create_recorder_fd(int fd, int cpu, unsigned flags)
>  {
>  	const char *tracing;
Slavomir Kaslev Jan. 15, 2019, 2:21 p.m. UTC | #2
On Tue, Jan 15, 2019 at 12:12 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 14 Jan 2019 17:27:57 +0200
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
>
>
> > @@ -194,8 +196,9 @@ tracecmd_create_buffer_recorder_fd(int fd, int cpu, unsigned flags, const char *
> >       return tracecmd_create_buffer_recorder_fd2(fd, -1, cpu, flags, buffer, 0);
> >  }
> >
> > -struct tracecmd_recorder *
> > -tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags, const char *buffer)
> > +static struct tracecmd_recorder *
> > +__tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
> > +                               const char *buffer)
> >  {
> >       struct tracecmd_recorder *recorder;
> >       int fd;
> > @@ -258,6 +261,26 @@ tracecmd_create_buffer_recorder_maxkb(const char *file, int cpu, unsigned flags,
> >       goto out;
> >  }
> >
> > +struct tracecmd_recorder *
> > +tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
> > +                             const char *buffer)
> > +{
> > +     return __tracecmd_create_buffer_recorder(file, cpu, flags, buffer);
> > +}
> > +
> > +struct tracecmd_recorder *
> > +tracecmd_create_recorder_virt(const char *file, int cpu, int trace_fd)
>
> As this looks to be something that may be a library call someday, I
> would keep flags as a parameter.
>
> > +{
> > +     struct tracecmd_recorder *recorder;
> > +
> > +     recorder = __tracecmd_create_buffer_recorder(
> > +             file, cpu, TRACECMD_RECORD_NOSPLICE, NULL);
>
> Then you could pass this as:
>
>         recorder = __tracecmd_create_buffer_recorder(file, cpu,
>                         flags | TRACECMD_RECORD_NOSPLICE, NULL);
>

Makes sense but (continues below)

> -- Steve
>
> > +     if (recorder)
> > +             recorder->trace_fd = trace_fd;

it's this part that cannot be done outside of trace-recorder.c since
`recorder->trace_fd` is not exposed.
`tracecmd_create_recorder_virt` just allows a user to provide his own
file descriptor from where tracing data should be read (in this case
from the agent's vsock sockets)

On an unrelated note, the TRACECMD_RECORD_NOSPLICE flag is only needed
on Linux version <4.20. Do we want to detect if splicing on vsock
sockets works on startup and use this flag only if necessary?

> > +
> > +     return recorder;
> > +}
> > +
> >  struct tracecmd_recorder *tracecmd_create_recorder_fd(int fd, int cpu, unsigned flags)
> >  {
> >       const char *tracing;
>
Steven Rostedt Jan. 15, 2019, 2:46 p.m. UTC | #3
On Tue, 15 Jan 2019 14:21:11 +0000
Slavomir Kaslev <kaslevs@vmware.com> wrote:

> On an unrelated note, the TRACECMD_RECORD_NOSPLICE flag is only needed
> on Linux version <4.20. Do we want to detect if splicing on vsock
> sockets works on startup and use this flag only if necessary?

Yes, sounds reasonable.

-- Steve
diff mbox series

Patch

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 99a455c..f0747c5 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -277,6 +277,7 @@  enum {
 void tracecmd_free_recorder(struct tracecmd_recorder *recorder);
 struct tracecmd_recorder *tracecmd_create_recorder(const char *file, int cpu, unsigned flags);
 struct tracecmd_recorder *tracecmd_create_recorder_fd(int fd, int cpu, unsigned flags);
+struct tracecmd_recorder *tracecmd_create_recorder_virt(const char *file, int cpu, int trace_fd);
 struct tracecmd_recorder *tracecmd_create_recorder_maxkb(const char *file, int cpu, unsigned flags, int maxkb);
 struct tracecmd_recorder *tracecmd_create_buffer_recorder_fd(int fd, int cpu, unsigned flags, const char *buffer);
 struct tracecmd_recorder *tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags, const char *buffer);
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index 5331925..497f752 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -148,16 +148,22 @@  tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
 	recorder->fd1 = fd;
 	recorder->fd2 = fd2;
 
-	if (flags & TRACECMD_RECORD_SNAPSHOT)
-		ret = asprintf(&path, "%s/per_cpu/cpu%d/snapshot_raw", buffer, cpu);
-	else
-		ret = asprintf(&path, "%s/per_cpu/cpu%d/trace_pipe_raw", buffer, cpu);
-	if (ret < 0)
-		goto out_free;
+	if (buffer) {
+		if (flags & TRACECMD_RECORD_SNAPSHOT)
+			ret = asprintf(&path, "%s/per_cpu/cpu%d/snapshot_raw",
+				       buffer, cpu);
+		else
+			ret = asprintf(&path, "%s/per_cpu/cpu%d/trace_pipe_raw",
+				       buffer, cpu);
+		if (ret < 0)
+			goto out_free;
+
+		recorder->trace_fd = open(path, O_RDONLY);
+		free(path);
 
-	recorder->trace_fd = open(path, O_RDONLY);
-	if (recorder->trace_fd < 0)
-		goto out_free;
+		if (recorder->trace_fd < 0)
+			goto out_free;
+	}
 
 	if ((recorder->flags & TRACECMD_RECORD_NOSPLICE) == 0) {
 		ret = pipe(recorder->brass);
@@ -177,13 +183,9 @@  tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
 		recorder->pipe_size = pipe_size;
 	}
 
-	free(path);
-
 	return recorder;
 
  out_free:
-	free(path);
-
 	tracecmd_free_recorder(recorder);
 	return NULL;
 }
@@ -194,8 +196,9 @@  tracecmd_create_buffer_recorder_fd(int fd, int cpu, unsigned flags, const char *
 	return tracecmd_create_buffer_recorder_fd2(fd, -1, cpu, flags, buffer, 0);
 }
 
-struct tracecmd_recorder *
-tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags, const char *buffer)
+static struct tracecmd_recorder *
+__tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
+				  const char *buffer)
 {
 	struct tracecmd_recorder *recorder;
 	int fd;
@@ -258,6 +261,26 @@  tracecmd_create_buffer_recorder_maxkb(const char *file, int cpu, unsigned flags,
 	goto out;
 }
 
+struct tracecmd_recorder *
+tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
+				const char *buffer)
+{
+	return __tracecmd_create_buffer_recorder(file, cpu, flags, buffer);
+}
+
+struct tracecmd_recorder *
+tracecmd_create_recorder_virt(const char *file, int cpu, int trace_fd)
+{
+	struct tracecmd_recorder *recorder;
+
+	recorder = __tracecmd_create_buffer_recorder(
+		file, cpu, TRACECMD_RECORD_NOSPLICE, NULL);
+	if (recorder)
+		recorder->trace_fd = trace_fd;
+
+	return recorder;
+}
+
 struct tracecmd_recorder *tracecmd_create_recorder_fd(int fd, int cpu, unsigned flags)
 {
 	const char *tracing;