On Thu, Nov 28, 2019 at 4:45 PM Slavomir Kaslev <slavomir.kaslev@xxxxxxxxx> 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