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

[Slavomir Kaslev <kaslevs@vmware.com>]

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;
>