Ideals for tep plugin options

Ideals for tep plugin options

[Steven Rostedt]

Hi Tzvetomir,

I was looking at the plugin options, and I believe it needs to be
updated.

1) the tep_plugin_option structure:

  - rename "file" to "plugin"
  - remove "plugin_alias"

2) Currently we have the following plugin option functions exported in
trace-cmd:

 char **tep_plugin_list_options(void);
 void tep_plugin_free_options_list(char **list);
 int tep_plugin_add_options(const char *name,
			   struct tep_plugin_option *options);
 int tep_plugin_add_option(const char *name, const char *val);
 void tep_plugin_remove_options(struct tep_plugin_option *options);
 void tep_plugin_print_options(struct trace_seq *s);
 void tep_print_plugins(struct trace_seq *s,
			const char *prefix, const char *suffix,
			const struct tep_plugin_list *list);

What we should only have is:

 int tep_plugin_add_options(const char *plugin,
			  struct tep_plugin_option *options);
 int tep_plugin_set_option(const char *plugin, const char *name,
			  const char *val);
 int tep_plugin_remove_options(struct tep_plugin_option *options);
 struct tep_plugin_option const * const tep_plugin_options_list(void);


Where:

  tep_plugin_add_options() is basically the same as we have now. But we
    should change it so that if "plugin" is already registered, it
    should fail with -EBUSY.

 tep_plugin_set_option() lets us update the option. If "plugin" is
   NULL, then it will ignore that value, and will set all options with
   "name" to "val", regardless of what plugin it belongs to.

 tep_plugin_remove_options() is basically the same as what we have now,
   and removes the options from the list.

 tep_plugin_options_list() returns an immutable list of all the
   registered plugin options. This is the list that the application can
   use to see what options are registered, as well as their default
   (and current) values.

Does this make sense?

-- Steve

Re: Ideals for tep plugin options

[Tzvetomir (Ceco) Stoyanov]

Hi Steven,

On Mon, Jan 6, 2020 at 6:30 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Hi Tzvetomir,
>
> I was looking at the plugin options, and I believe it needs to be
> updated.
>
> 1) the tep_plugin_option structure:
>
>   - rename "file" to "plugin"
>   - remove "plugin_alias"
>
> 2) Currently we have the following plugin option functions exported in
> trace-cmd:
>
>  char **tep_plugin_list_options(void);
>  void tep_plugin_free_options_list(char **list);
>  int tep_plugin_add_options(const char *name,
>                            struct tep_plugin_option *options);
>  int tep_plugin_add_option(const char *name, const char *val);
>  void tep_plugin_remove_options(struct tep_plugin_option *options);
>  void tep_plugin_print_options(struct trace_seq *s);
>  void tep_print_plugins(struct trace_seq *s,
>                         const char *prefix, const char *suffix,
>                         const struct tep_plugin_list *list);
>
> What we should only have is:
>
>  int tep_plugin_add_options(const char *plugin,
>                           struct tep_plugin_option *options);
>  int tep_plugin_set_option(const char *plugin, const char *name,
>                           const char *val);
>  int tep_plugin_remove_options(struct tep_plugin_option *options);
>  struct tep_plugin_option const * const tep_plugin_options_list(void);
>
>
> Where:
>
>   tep_plugin_add_options() is basically the same as we have now. But we
>     should change it so that if "plugin" is already registered, it
>     should fail with -EBUSY.
>
>  tep_plugin_set_option() lets us update the option. If "plugin" is
>    NULL, then it will ignore that value, and will set all options with
>    "name" to "val", regardless of what plugin it belongs to.
>
>  tep_plugin_remove_options() is basically the same as what we have now,
>    and removes the options from the list.
>
>  tep_plugin_options_list() returns an immutable list of all the
>    registered plugin options. This is the list that the application can
>    use to see what options are registered, as well as their default
>    (and current) values.
>
> Does this make sense?

Yes, it makes sense. I'll submit a patch set with these changes, in
the kernel tree.
I'm going to add also a check at the plugin loading time, if it is
already loaded -
the old one will be unloaded before the new one is registered.

>
> -- Steve

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