Re: [PATCH 5/7] trace-cmd: Added user configured clock to the record context

[Tzvetomir Stoyanov <tstoyanov@vmware.com>]

On Mon, Jan 21, 2019 at 11:19 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 16 Jan 2019 21:18:36 +0200
> Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:
>
> > Store the user configured clock (if any) in the record context,
> > struct common_record_context. The clock is going to be used when
> > synchronizing timestamps with remote tracing machine.
> > ---
> >  tracecmd/trace-record.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> > index f7349eb..900d0d4 100644
> > --- a/tracecmd/trace-record.c
> > +++ b/tracecmd/trace-record.c
> > @@ -227,6 +227,7 @@ struct common_record_context {
> >       int topt;
> >       int do_child;
> >       int run_command;
> > +     char *clock;
> >  };
> >
> >  static void add_reset_file(const char *file, const char *val, int prio)
> > @@ -5183,6 +5184,7 @@ static void parse_record_options(int argc,
> >                       break;
> >               case 'C':
> >                       ctx->instance->clock = optarg;
> > +                     ctx->clock = optarg;
>
> Hmm, this can cause an issue. The first thing I have to ask is, why
> can't we use ctx->instance->clock. I'm sure the answer is that we are
> adding more than one clock. But this begs the question, which clock
> should be use, if there are more than one clock to add?
>
> I think this patch is not needed. In the last patch, test if
> ctx->instance->clock exists, and if not, we search the other instances
> for a clock to use. Or something else that doesn't make which clock to
> use ambiguous.

Ok, I'll remove it and will search for the first ctx->instance->clock instead.
Current algorithm assumes that only one clock is used, and it is the
same in both
host and guest contexts. We should think how to handle the case with
multiple instances,
each one with different clock. I do not know, how instances are
written in the trace.dat file,
is there a different time offset option in the file, per instance ?

>
> -- Steve
>
> >                       break;
> >               case 'v':
> >                       neg_event = 1;
>


-- 

Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center