Re: [PATCH v15 14/18] trace-cmd: Add host trace clock as guest trace argument

[Tzvetomir Stoyanov <tz.stoyanov@gmail.com>]

On Thu, Nov 28, 2019 at 4:45 PM Slavomir Kaslev
<slavomir.kaslev@gmail.com> wrote:
>
 ...
> > +       struct buffer_instance *instance;
> > +       bool guest_config = false;
>
> guest_config is a bit vague of a name for what it's used here. Maybe
> has_clock_arg?
>
The same flag is used for similar host config, which is applied to
guests - in the following patches from the series.

> >
> >         init_common_record_context(ctx, curr_cmd);
> >
 ...
> > +       if (guest_config) {
> > +               /* If -C is specified, prepend clock to all guest VM flags */
> > +               for_all_instances(instance) {
> > +                       if (top_instance.ftrace->clock) {
> > +                               if (is_guest(instance)) {
> > +                                       add_argv(instance,
> > +                                                (char *)top_instance.ftrace->clock,
> > +                                                true);
> > +                                       add_argv(instance, "-C", true);
>
> Shouldn't those two add_arvgv() calls be swapped?
It is a little bit confusing, add_argv() actually prepends the
arguments - so the last call puts the first argument in the list.
The user specified arguments are always after those. When the args are
parsed in the guest, in case of duplication, the last one wins.

>
> > +                               }
> > +                       }
> > +               }
> > +       }
> > +
> >         if (!ctx->filtered && ctx->instance->filter_mod)
> >                 add_func(&ctx->instance->filter_funcs,
> >                          ctx->instance->filter_mod, "*");
> > --
> > 2.23.0
> >
>
> Cheers,
>
> -- Slavi

-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center