Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* Ideals for tep plugin options
@ 2020-01-06 16:30 Steven Rostedt
  2020-01-09 13:21 ` Tzvetomir (Ceco) Stoyanov
  0 siblings, 1 reply; 2+ messages in thread
From: Steven Rostedt @ 2020-01-06 16:30 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: Linux Trace Devel

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Ideals for tep plugin options
  2020-01-06 16:30 Ideals for tep plugin options Steven Rostedt
@ 2020-01-09 13:21 ` Tzvetomir (Ceco) Stoyanov
  0 siblings, 0 replies; 2+ messages in thread
From: Tzvetomir (Ceco) Stoyanov @ 2020-01-09 13:21 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Trace Devel

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06 16:30 Ideals for tep plugin options Steven Rostedt
2020-01-09 13:21 ` Tzvetomir (Ceco) Stoyanov

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git