Re: [PATCH v4 3/5] trace-cmd: Load libtraceevent plugins from build folder, if exists.

[Steven Rostedt <rostedt@goodmis.org>]

On Fri,  4 Oct 2019 16:36:45 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> When a development version of trace-cmd is built and run on the machine,
> by default it loads only installed plugins, from system directories.
> Thus, the development plugins will not be loaded. To simplify the development
> process, a new logic is added:
>   At plugins load time, check the location of trace-cmd application and look
>   for "plugins" directory around it. If found, load plugins from it. Those
>   plugins will be loaded last, so in case of duplication the "development"
>   plugins win.
> A two new APIs are introduced to libtraceevent, in order to accomplish this
> logic:
>  tep_load_plugins_dir() - loads tep plugins from a specific directory.
>  tep_plugins_append() - Append two plugin lists.

We should probably break this patch up into two. One that adds the new
interface to libtraceevent (making it easier to send to upstream
libtraceevent), and then one that uses it in trace-cmd.

And we should probably update the man pages when adding the tep_*()
patches. That said, I have some comments about this.

> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>


> diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
> index b08e377..5a84fc3 100644
> --- a/lib/trace-cmd/trace-util.c
> +++ b/lib/trace-cmd/trace-util.c
> @@ -884,6 +884,52 @@ void trace_util_free_plugin_files(char **files)
>  	free(files);
>  }
>  
> +static char *get_source_plugins_dir(void)
> +{
> +	char *p, path[PATH_MAX+1];
> +	int ret;
> +
> +	ret = readlink("/proc/self/exe", path, PATH_MAX);
> +	if (ret > PATH_MAX || ret < 0)
> +		return NULL;
> +
> +	dirname(path);
> +	p = strrchr(path, '/');
> +	if (!p)
> +		return NULL;
> +	/* Check if we are in the the source tree */
> +	if (strcmp(p, "/tracecmd") != 0)
> +		return NULL;
> +
> +	strcpy(p, "/lib/traceevent/plugins");
> +	return strdup(path);
> +}
> +
> +struct tep_plugin_list*
> +trace_load_tep_plugins(struct tep_handle *tep)
> +{
> +	struct tep_plugin_list *list_dev;
> +	struct tep_plugin_list *list;
> +	char *path;
> +
> +	if (tracecmd_disable_plugins)
> +		tep_set_flag(tep, TEP_DISABLE_PLUGINS);
> +	if (tracecmd_disable_sys_plugins)
> +		tep_set_flag(tep, TEP_DISABLE_SYS_PLUGINS);
> +
> +	list = tep_load_plugins(tep);
> +
> +	path = get_source_plugins_dir();
> +	if (path) {
> +		list_dev = tep_load_plugins_dir(tep, path);
> +		tep_plugins_append(list_dev, list);
> +		list = list_dev;
> +		free(path);

I think it would be better to be able to add paths to the tep, and then
call "tep_load_plugins()". That is:

	path = get_source_plugins_dir();
	if (path)
		tep_add_plugin_path(tep, path);

	list = tep_load_plugins(tep);

And have the tep_load_plugins() do all the work, and not have a
tep_plugins_append(). That append function is exposing too much of the
implementation of the tep code.

-- Steve

> +	}
> +
> +	return list;
> +}
> +
>  char *tracecmd_get_tracing_file(const char *name)
>  {
>  	static const char *tracing;
>