[RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[Alexander Aring]

This patch adds an argument for the fs-src plugin to declare the
directory to find the metadata file instead of placing it in every
subdir of traces.

If parameter assign every subdirectory which does not have a
subdirectory and at least some regular files will be assumed as a trace
directory. The regular files will be assumed as ctf trace files.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 cli/babeltrace-cfg-cli-args.c | 10 +++++
 plugins/ctf/fs-src/fs.c       | 88 +++++++++++++++++++++++++++++++++++--------
 plugins/ctf/fs-src/fs.h       |  3 +-
 plugins/ctf/fs-src/metadata.c |  8 +++-
 plugins/ctf/fs-src/metadata.h |  1 +
 plugins/ctf/fs-src/query.c    |  2 +-
 6 files changed, 94 insertions(+), 18 deletions(-)

diff --git a/cli/babeltrace-cfg-cli-args.c b/cli/babeltrace-cfg-cli-args.c
index 8f01e64b..ab76852e 100644
--- a/cli/babeltrace-cfg-cli-args.c
+++ b/cli/babeltrace-cfg-cli-args.c
@@ -1306,6 +1306,7 @@ enum {
 	OPT_CLOCK_GMT,
 	OPT_CLOCK_OFFSET,
 	OPT_CLOCK_OFFSET_NS,
+	OPT_METADATA_SRC,
 	OPT_CLOCK_SECONDS,
 	OPT_COLOR,
 	OPT_COMPONENT,
@@ -2789,6 +2790,7 @@ void print_convert_usage(FILE *fp)
 	fprintf(fp, "\n");
 	fprintf(fp, "      --clock-offset=SEC            Set clock offset to SEC seconds\n");
 	fprintf(fp, "      --clock-offset-ns=NS          Set clock offset to NS ns\n");
+	fprintf(fp, "      --metadata-src=PATH		 Set a path to find the metadata\n");
 	fprintf(fp, "\n");
 	fprintf(fp, "Implicit `sink.text.pretty` component options:\n");
 	fprintf(fp, "\n");
@@ -2886,6 +2888,7 @@ struct poptOption convert_long_options[] = {
 	{ "clock-gmt", '\0', POPT_ARG_NONE, NULL, OPT_CLOCK_GMT, NULL, NULL },
 	{ "clock-offset", '\0', POPT_ARG_STRING, NULL, OPT_CLOCK_OFFSET, NULL, NULL },
 	{ "clock-offset-ns", '\0', POPT_ARG_STRING, NULL, OPT_CLOCK_OFFSET_NS, NULL, NULL },
+	{ "metadata-src", '\0', POPT_ARG_STRING, NULL, OPT_METADATA_SRC, NULL, NULL },
 	{ "clock-seconds", '\0', POPT_ARG_NONE, NULL, OPT_CLOCK_SECONDS, NULL, NULL },
 	{ "color", '\0', POPT_ARG_STRING, NULL, OPT_COLOR, NULL, NULL },
 	{ "component", 'c', POPT_ARG_STRING, NULL, OPT_COMPONENT, NULL, NULL },
@@ -3942,6 +3945,7 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
 		case OPT_CLOCK_GMT:
 		case OPT_CLOCK_OFFSET:
 		case OPT_CLOCK_OFFSET_NS:
+		case OPT_METADATA_SRC:
 		case OPT_CLOCK_SECONDS:
 		case OPT_COLOR:
 		case OPT_DEBUG:
@@ -4104,6 +4108,12 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
 					&base_implicit_ctf_input_args,
 					"clock-class-offset-ns", arg);
 			break;
+		case OPT_METADATA_SRC:
+			base_implicit_ctf_input_args.exists = true;
+			append_implicit_component_param(
+					&base_implicit_ctf_input_args,
+					"metadata-src", arg);
+			break;
 		case OPT_CLOCK_SECONDS:
 			append_implicit_component_param(
 				&implicit_text_args, "clock-seconds", "yes");
diff --git a/plugins/ctf/fs-src/fs.c b/plugins/ctf/fs-src/fs.c
index 2dacf97d..7f98dda5 100644
--- a/plugins/ctf/fs-src/fs.c
+++ b/plugins/ctf/fs-src/fs.c
@@ -1039,26 +1039,70 @@ end:
 }
 
 BT_HIDDEN
-int ctf_fs_find_traces(GList **trace_paths, const char *start_path)
+int ctf_fs_find_traces(GList **trace_paths, const char *start_path,
+		       const char *metadata_src)
 {
 	int ret;
 	GError *error = NULL;
 	GDir *dir = NULL;
 	const char *basename = NULL;
+	bool subdirs = false;
+	bool regfile = false;
 
-	/* Check if the starting path is a CTF trace itself */
-	ret = path_is_ctf_trace(start_path);
-	if (ret < 0) {
-		goto end;
-	}
+	if (metadata_src) {
+		ret = path_is_ctf_trace(metadata_src);
+		if (ret < 0) {
+			goto end;
+		}
 
-	if (ret) {
-		/*
-		 * Stop recursion: a CTF trace cannot contain another
-		 * CTF trace.
-		 */
-		ret = add_trace_path(trace_paths, start_path);
-		goto end;
+		if (ret) {
+			dir = g_dir_open(start_path, 0, &error);
+			if (!dir) {
+				goto end;
+			}
+
+			while ((basename = g_dir_read_name(dir))) {
+				GString *sub_path = g_string_new(NULL);
+
+				if (!sub_path) {
+					ret = -1;
+					goto end;
+				}
+
+				g_string_printf(sub_path, "%s" G_DIR_SEPARATOR_S "%s", start_path, basename);
+				if (g_file_test(sub_path->str, G_FILE_TEST_IS_DIR)) {
+					subdirs = true;
+				}
+				if (g_file_test(sub_path->str, G_FILE_TEST_IS_REGULAR)) {
+					regfile = true;
+				}
+				g_string_free(sub_path, TRUE);
+			}
+
+			g_dir_close(dir);
+			dir = NULL;
+
+			/* Look if dir has no subdirs but regfile(s) */
+			if (!subdirs && regfile) {
+				ret = add_trace_path(trace_paths, start_path);
+				goto end;
+			}
+		}
+	} else {
+		/* Check if the starting path is a CTF trace itself */
+		ret = path_is_ctf_trace(start_path);
+		if (ret < 0) {
+			goto end;
+		}
+
+		if (ret) {
+			/*
+			 * Stop recursion: a CTF trace cannot contain another
+			 * CTF trace.
+			 */
+			ret = add_trace_path(trace_paths, start_path);
+			goto end;
+		}
 	}
 
 	/* Look for subdirectories */
@@ -1090,7 +1134,7 @@ int ctf_fs_find_traces(GList **trace_paths, const char *start_path)
 		}
 
 		g_string_printf(sub_path, "%s" G_DIR_SEPARATOR_S "%s", start_path, basename);
-		ret = ctf_fs_find_traces(trace_paths, sub_path->str);
+		ret = ctf_fs_find_traces(trace_paths, sub_path->str, metadata_src);
 		g_string_free(sub_path, TRUE);
 		if (ret) {
 			goto end;
@@ -1181,7 +1225,8 @@ int create_ctf_fs_traces(struct ctf_fs_component *ctf_fs,
 		goto error;
 	}
 
-	ret = ctf_fs_find_traces(&trace_paths, norm_path->str);
+	ret = ctf_fs_find_traces(&trace_paths, norm_path->str,
+				 ctf_fs->metadata_config.metadata_src);
 	if (ret) {
 		goto error;
 	}
@@ -1287,6 +1332,19 @@ struct ctf_fs_component *ctf_fs_create(struct bt_private_component *priv_comp,
 	value_ret = bt_value_string_get(value, &path_param);
 	assert(value_ret == BT_VALUE_STATUS_OK);
 	BT_PUT(value);
+
+	value = bt_value_map_get(params, "metadata-src");
+	if (value) {
+		if (!bt_value_is_string(value)) {
+			goto error;
+		}
+
+		value_ret = bt_value_string_get(value,
+			&ctf_fs->metadata_config.metadata_src);
+		assert(value_ret == BT_VALUE_STATUS_OK);
+		BT_PUT(value);
+	}
+
 	value = bt_value_map_get(params, "clock-class-offset-s");
 	if (value) {
 		if (!bt_value_is_integer(value)) {
diff --git a/plugins/ctf/fs-src/fs.h b/plugins/ctf/fs-src/fs.h
index bbac1bb4..f80aea74 100644
--- a/plugins/ctf/fs-src/fs.h
+++ b/plugins/ctf/fs-src/fs.h
@@ -154,7 +154,8 @@ BT_HIDDEN
 void ctf_fs_trace_destroy(struct ctf_fs_trace *trace);
 
 BT_HIDDEN
-int ctf_fs_find_traces(GList **trace_paths, const char *start_path);
+int ctf_fs_find_traces(GList **trace_paths, const char *start_path,
+		       const char *metadata_src);
 
 BT_HIDDEN
 GList *ctf_fs_create_trace_names(GList *trace_paths, const char *base_path);
diff --git a/plugins/ctf/fs-src/metadata.c b/plugins/ctf/fs-src/metadata.c
index 231d946c..02cbd0cd 100644
--- a/plugins/ctf/fs-src/metadata.c
+++ b/plugins/ctf/fs-src/metadata.c
@@ -96,8 +96,14 @@ int ctf_fs_metadata_set_trace(struct ctf_fs_trace *ctf_fs_trace,
 		.clock_class_offset_s = config ? config->clock_class_offset_s : 0,
 		.clock_class_offset_ns = config ? config->clock_class_offset_ns : 0,
 	};
+	const char *metadata_src;
 
-	file = get_file(ctf_fs_trace->path->str);
+	if (config->metadata_src)
+		metadata_src = config->metadata_src;
+	else
+		metadata_src = ctf_fs_trace->path->str;
+
+	file = get_file(metadata_src);
 	if (!file) {
 		BT_LOGE("Cannot create metadata file object");
 		ret = -1;
diff --git a/plugins/ctf/fs-src/metadata.h b/plugins/ctf/fs-src/metadata.h
index 496a5ca9..00d8de67 100644
--- a/plugins/ctf/fs-src/metadata.h
+++ b/plugins/ctf/fs-src/metadata.h
@@ -36,6 +36,7 @@ struct ctf_fs_metadata;
 struct ctf_fs_metadata_config {
 	int64_t clock_class_offset_s;
 	int64_t clock_class_offset_ns;
+	const char *metadata_src;
 };
 
 BT_HIDDEN
diff --git a/plugins/ctf/fs-src/query.c b/plugins/ctf/fs-src/query.c
index 04bf8c5b..6f2aa5fe 100644
--- a/plugins/ctf/fs-src/query.c
+++ b/plugins/ctf/fs-src/query.c
@@ -500,7 +500,7 @@ struct bt_component_class_query_method_return trace_info_query(
 	}
 	assert(path);
 
-	ret = ctf_fs_find_traces(&trace_paths, normalized_path->str);
+	ret = ctf_fs_find_traces(&trace_paths, normalized_path->str, NULL);
 	if (ret) {
 		goto error;
 	}
-- 
2.11.0

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[RFC babeltrace 2/2] doc: man: babeltrace-source.ctf.fs.7: add metadata-src

[Alexander Aring]

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 doc/man/babeltrace-source.ctf.fs.7.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/doc/man/babeltrace-source.ctf.fs.7.txt b/doc/man/babeltrace-source.ctf.fs.7.txt
index 74b08b67..e9da83cf 100644
--- a/doc/man/babeltrace-source.ctf.fs.7.txt
+++ b/doc/man/babeltrace-source.ctf.fs.7.txt
@@ -131,6 +131,13 @@ parameter.
 param:path='PATH' (string, mandatory)::
     Path to the directory to recurse for CTF traces.
 
+param:metadata-src (string)::
+    Path to search for metadata file.
++
+You can combine this parameter with the param:path parameter. Every file in
+a subdirectory which don't have subdirectories of param:path will be handled
+as ctf stream data.
+
 
 PORTS
 -----
-- 
2.11.0

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[Philippe Proulx]

On Mon, Apr 30, 2018 at 5:31 PM, Alexander Aring <aring@mojatatu.com> wrote:
>
> This patch adds an argument for the fs-src plugin to declare the
> directory to find the metadata file instead of placing it in every
> subdir of traces.
>
> If parameter assign every subdirectory which does not have a
> subdirectory and at least some regular files will be assumed as a trace
> directory. The regular files will be assumed as ctf trace files.

Although I really appreciate the contribution effort, before even
reading this patch, let me indicate that a CTF trace located on the file
system is a directory containing the data stream and metadata stream
files. Knowing this, this patch implements a hack to circumvent this,
but I'm personally not willing to have this upstream as, in my opinion,
it is the user's responsibility to have valid CTF traces to open.

Your custom scenario asks for a custom solution on your side: copying
the metadata file to each trace's directory seems appropriate here.

Also, do your CTF traces have UUIDs? If they do, they should all be
different, but having the same metadata file makes them all the same.
This is not valid either.

Phil

>
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>  cli/babeltrace-cfg-cli-args.c | 10 +++++
>  plugins/ctf/fs-src/fs.c       | 88 +++++++++++++++++++++++++++++++++++--------
>  plugins/ctf/fs-src/fs.h       |  3 +-
>  plugins/ctf/fs-src/metadata.c |  8 +++-
>  plugins/ctf/fs-src/metadata.h |  1 +
>  plugins/ctf/fs-src/query.c    |  2 +-
>  6 files changed, 94 insertions(+), 18 deletions(-)
>
> diff --git a/cli/babeltrace-cfg-cli-args.c b/cli/babeltrace-cfg-cli-args.c
> index 8f01e64b..ab76852e 100644
> --- a/cli/babeltrace-cfg-cli-args.c
> +++ b/cli/babeltrace-cfg-cli-args.c
> @@ -1306,6 +1306,7 @@ enum {
>         OPT_CLOCK_GMT,
>         OPT_CLOCK_OFFSET,
>         OPT_CLOCK_OFFSET_NS,
> +       OPT_METADATA_SRC,
>         OPT_CLOCK_SECONDS,
>         OPT_COLOR,
>         OPT_COMPONENT,
> @@ -2789,6 +2790,7 @@ void print_convert_usage(FILE *fp)
>         fprintf(fp, "\n");
>         fprintf(fp, "      --clock-offset=SEC            Set clock offset to SEC seconds\n");
>         fprintf(fp, "      --clock-offset-ns=NS          Set clock offset to NS ns\n");
> +       fprintf(fp, "      --metadata-src=PATH           Set a path to find the metadata\n");
>         fprintf(fp, "\n");
>         fprintf(fp, "Implicit `sink.text.pretty` component options:\n");
>         fprintf(fp, "\n");
> @@ -2886,6 +2888,7 @@ struct poptOption convert_long_options[] = {
>         { "clock-gmt", '\0', POPT_ARG_NONE, NULL, OPT_CLOCK_GMT, NULL, NULL },
>         { "clock-offset", '\0', POPT_ARG_STRING, NULL, OPT_CLOCK_OFFSET, NULL, NULL },
>         { "clock-offset-ns", '\0', POPT_ARG_STRING, NULL, OPT_CLOCK_OFFSET_NS, NULL, NULL },
> +       { "metadata-src", '\0', POPT_ARG_STRING, NULL, OPT_METADATA_SRC, NULL, NULL },
>         { "clock-seconds", '\0', POPT_ARG_NONE, NULL, OPT_CLOCK_SECONDS, NULL, NULL },
>         { "color", '\0', POPT_ARG_STRING, NULL, OPT_COLOR, NULL, NULL },
>         { "component", 'c', POPT_ARG_STRING, NULL, OPT_COMPONENT, NULL, NULL },
> @@ -3942,6 +3945,7 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
>                 case OPT_CLOCK_GMT:
>                 case OPT_CLOCK_OFFSET:
>                 case OPT_CLOCK_OFFSET_NS:
> +               case OPT_METADATA_SRC:
>                 case OPT_CLOCK_SECONDS:
>                 case OPT_COLOR:
>                 case OPT_DEBUG:
> @@ -4104,6 +4108,12 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
>                                         &base_implicit_ctf_input_args,
>                                         "clock-class-offset-ns", arg);
>                         break;
> +               case OPT_METADATA_SRC:
> +                       base_implicit_ctf_input_args.exists = true;
> +                       append_implicit_component_param(
> +                                       &base_implicit_ctf_input_args,
> +                                       "metadata-src", arg);
> +                       break;
>                 case OPT_CLOCK_SECONDS:
>                         append_implicit_component_param(
>                                 &implicit_text_args, "clock-seconds", "yes");
> diff --git a/plugins/ctf/fs-src/fs.c b/plugins/ctf/fs-src/fs.c
> index 2dacf97d..7f98dda5 100644
> --- a/plugins/ctf/fs-src/fs.c
> +++ b/plugins/ctf/fs-src/fs.c
> @@ -1039,26 +1039,70 @@ end:
>  }
>
>  BT_HIDDEN
> -int ctf_fs_find_traces(GList **trace_paths, const char *start_path)
> +int ctf_fs_find_traces(GList **trace_paths, const char *start_path,
> +                      const char *metadata_src)
>  {
>         int ret;
>         GError *error = NULL;
>         GDir *dir = NULL;
>         const char *basename = NULL;
> +       bool subdirs = false;
> +       bool regfile = false;
>
> -       /* Check if the starting path is a CTF trace itself */
> -       ret = path_is_ctf_trace(start_path);
> -       if (ret < 0) {
> -               goto end;
> -       }
> +       if (metadata_src) {
> +               ret = path_is_ctf_trace(metadata_src);
> +               if (ret < 0) {
> +                       goto end;
> +               }
>
> -       if (ret) {
> -               /*
> -                * Stop recursion: a CTF trace cannot contain another
> -                * CTF trace.
> -                */
> -               ret = add_trace_path(trace_paths, start_path);
> -               goto end;
> +               if (ret) {
> +                       dir = g_dir_open(start_path, 0, &error);
> +                       if (!dir) {
> +                               goto end;
> +                       }
> +
> +                       while ((basename = g_dir_read_name(dir))) {
> +                               GString *sub_path = g_string_new(NULL);
> +
> +                               if (!sub_path) {
> +                                       ret = -1;
> +                                       goto end;
> +                               }
> +
> +                               g_string_printf(sub_path, "%s" G_DIR_SEPARATOR_S "%s", start_path, basename);
> +                               if (g_file_test(sub_path->str, G_FILE_TEST_IS_DIR)) {
> +                                       subdirs = true;
> +                               }
> +                               if (g_file_test(sub_path->str, G_FILE_TEST_IS_REGULAR)) {
> +                                       regfile = true;
> +                               }
> +                               g_string_free(sub_path, TRUE);
> +                       }
> +
> +                       g_dir_close(dir);
> +                       dir = NULL;
> +
> +                       /* Look if dir has no subdirs but regfile(s) */
> +                       if (!subdirs && regfile) {
> +                               ret = add_trace_path(trace_paths, start_path);
> +                               goto end;
> +                       }
> +               }
> +       } else {
> +               /* Check if the starting path is a CTF trace itself */
> +               ret = path_is_ctf_trace(start_path);
> +               if (ret < 0) {
> +                       goto end;
> +               }
> +
> +               if (ret) {
> +                       /*
> +                        * Stop recursion: a CTF trace cannot contain another
> +                        * CTF trace.
> +                        */
> +                       ret = add_trace_path(trace_paths, start_path);
> +                       goto end;
> +               }
>         }
>
>         /* Look for subdirectories */
> @@ -1090,7 +1134,7 @@ int ctf_fs_find_traces(GList **trace_paths, const char *start_path)
>                 }
>
>                 g_string_printf(sub_path, "%s" G_DIR_SEPARATOR_S "%s", start_path, basename);
> -               ret = ctf_fs_find_traces(trace_paths, sub_path->str);
> +               ret = ctf_fs_find_traces(trace_paths, sub_path->str, metadata_src);
>                 g_string_free(sub_path, TRUE);
>                 if (ret) {
>                         goto end;
> @@ -1181,7 +1225,8 @@ int create_ctf_fs_traces(struct ctf_fs_component *ctf_fs,
>                 goto error;
>         }
>
> -       ret = ctf_fs_find_traces(&trace_paths, norm_path->str);
> +       ret = ctf_fs_find_traces(&trace_paths, norm_path->str,
> +                                ctf_fs->metadata_config.metadata_src);
>         if (ret) {
>                 goto error;
>         }
> @@ -1287,6 +1332,19 @@ struct ctf_fs_component *ctf_fs_create(struct bt_private_component *priv_comp,
>         value_ret = bt_value_string_get(value, &path_param);
>         assert(value_ret == BT_VALUE_STATUS_OK);
>         BT_PUT(value);
> +
> +       value = bt_value_map_get(params, "metadata-src");
> +       if (value) {
> +               if (!bt_value_is_string(value)) {
> +                       goto error;
> +               }
> +
> +               value_ret = bt_value_string_get(value,
> +                       &ctf_fs->metadata_config.metadata_src);
> +               assert(value_ret == BT_VALUE_STATUS_OK);
> +               BT_PUT(value);
> +       }
> +
>         value = bt_value_map_get(params, "clock-class-offset-s");
>         if (value) {
>                 if (!bt_value_is_integer(value)) {
> diff --git a/plugins/ctf/fs-src/fs.h b/plugins/ctf/fs-src/fs.h
> index bbac1bb4..f80aea74 100644
> --- a/plugins/ctf/fs-src/fs.h
> +++ b/plugins/ctf/fs-src/fs.h
> @@ -154,7 +154,8 @@ BT_HIDDEN
>  void ctf_fs_trace_destroy(struct ctf_fs_trace *trace);
>
>  BT_HIDDEN
> -int ctf_fs_find_traces(GList **trace_paths, const char *start_path);
> +int ctf_fs_find_traces(GList **trace_paths, const char *start_path,
> +                      const char *metadata_src);
>
>  BT_HIDDEN
>  GList *ctf_fs_create_trace_names(GList *trace_paths, const char *base_path);
> diff --git a/plugins/ctf/fs-src/metadata.c b/plugins/ctf/fs-src/metadata.c
> index 231d946c..02cbd0cd 100644
> --- a/plugins/ctf/fs-src/metadata.c
> +++ b/plugins/ctf/fs-src/metadata.c
> @@ -96,8 +96,14 @@ int ctf_fs_metadata_set_trace(struct ctf_fs_trace *ctf_fs_trace,
>                 .clock_class_offset_s = config ? config->clock_class_offset_s : 0,
>                 .clock_class_offset_ns = config ? config->clock_class_offset_ns : 0,
>         };
> +       const char *metadata_src;
>
> -       file = get_file(ctf_fs_trace->path->str);
> +       if (config->metadata_src)
> +               metadata_src = config->metadata_src;
> +       else
> +               metadata_src = ctf_fs_trace->path->str;
> +
> +       file = get_file(metadata_src);
>         if (!file) {
>                 BT_LOGE("Cannot create metadata file object");
>                 ret = -1;
> diff --git a/plugins/ctf/fs-src/metadata.h b/plugins/ctf/fs-src/metadata.h
> index 496a5ca9..00d8de67 100644
> --- a/plugins/ctf/fs-src/metadata.h
> +++ b/plugins/ctf/fs-src/metadata.h
> @@ -36,6 +36,7 @@ struct ctf_fs_metadata;
>  struct ctf_fs_metadata_config {
>         int64_t clock_class_offset_s;
>         int64_t clock_class_offset_ns;
> +       const char *metadata_src;
>  };
>
>  BT_HIDDEN
> diff --git a/plugins/ctf/fs-src/query.c b/plugins/ctf/fs-src/query.c
> index 04bf8c5b..6f2aa5fe 100644
> --- a/plugins/ctf/fs-src/query.c
> +++ b/plugins/ctf/fs-src/query.c
> @@ -500,7 +500,7 @@ struct bt_component_class_query_method_return trace_info_query(
>         }
>         assert(path);
>
> -       ret = ctf_fs_find_traces(&trace_paths, normalized_path->str);
> +       ret = ctf_fs_find_traces(&trace_paths, normalized_path->str, NULL);
>         if (ret) {
>                 goto error;
>         }
> --
> 2.11.0
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 0/2] fs-src: add metadata-src parameter

[Philippe Proulx]

On Mon, Apr 30, 2018 at 5:31 PM, Alexander Aring <aring@mojatatu.com> wrote:
> Hi,
>
> my use-case is just... we have several application with the same metadata
> file. Currently I build a wrapper around babeltrace to run something like:
>
> find $(TRACE_DIR}/* -maxdepth 1 -type d -exec cp ${PATH_TO_MY_APP_DATA}/ctf/metadata '{}' \;
>
> which is terrible... so I try this here somehow.
> As you see it has no support for fs-query and I didn't figured out yet
> for what fs-query is for.
>
> Also I was struggle some hours how to tell popt the right argument string,
> it need to be:
>
> --metadata-src=\"/usr/share/$APP_DIR/ctf\"
>
> with the ugly escape things... I saw there is a complex INI parser system
> behind (never saw such complex option parser in an open source project).

A note about this: there are two ways to add component initialization
parameters with `babeltrace run`: with

    --params 'key1="the value", key2=23, key3=simple_string'

and, for string parameters, with:

    --key key1 --value 'the value'

The --params option parses what you call a "complex INI" format. It's
not so complex in reality and well-explained in the man page.

The job of `babeltrace convert` (the default command) is only to create
a valid `babeltrace run` command line and run it. In your first patch,
you use append_implicit_component_param(): this one appends a key/value
pair to the --params option's argument of a given implicit component.
It's simply appending `,KEY=VAL` to the current INI-style parameter
string for that component, which is why you need to pass the double
quotes above, because the effective string to append is exactly:

    ,metadata-src="/usr/share/app-dir-expansion/ctf"

What you want to do is have this instead:

    --key metadata-src --value /usr/share/app-dir-expansion/ctf

(where `app-dir-expansion` is the expansion of $APP_DIR). You can use
append_implicit_component_extra_param() for this. This is what we use
for complex string values like for --begin, --end, --debug-info-dir,
etc. Then you could use:

    --metadata-src="/usr/share/$APP_DIR/ctf"

so that popt returns `/usr/share/app-dir-expansion/ctf`.

Hope it helps.

Phil

>
> It's not easy to add such use-case because the whole code things everything
> which has a metadata inside is ctf trace... and so far I know that's what
> the spec and man-page says [0].
>
> As there is no metadata file anymore needed with this parameter, it will
> assume every regular file is a ctf stream, if only there is no other subdir
> inside the directory. So only "leafs" regular files will be assume as a ctf
> stream. If there exists regular files not as leafs - babeltrace will fail.
>
> Also I think I still need to have a wrapper around babeltrace to specify the
> metadata dir as metadata-src then... but better than this cp in front of it.
>
> - Alex
>
> [0] http://man7.org/linux/man-pages/man7/babeltrace-source.ctf.fs.7.html
>     See "trace naming".
>
> Alexander Aring (2):
>   fs-src: add argument for metadata src dir
>   doc: man: babeltrace-source.ctf.fs.7: add metadata-src
>
>  cli/babeltrace-cfg-cli-args.c          | 10 ++++
>  doc/man/babeltrace-source.ctf.fs.7.txt |  7 +++
>  plugins/ctf/fs-src/fs.c                | 88 ++++++++++++++++++++++++++++------
>  plugins/ctf/fs-src/fs.h                |  3 +-
>  plugins/ctf/fs-src/metadata.c          |  8 +++-
>  plugins/ctf/fs-src/metadata.h          |  1 +
>  plugins/ctf/fs-src/query.c             |  2 +-
>  7 files changed, 101 insertions(+), 18 deletions(-)
>
> --
> 2.11.0
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[Philippe Proulx]

On Mon, Apr 30, 2018 at 6:36 PM, Philippe Proulx
<eeppeliteloop@gmail.com> wrote:
> On Mon, Apr 30, 2018 at 5:31 PM, Alexander Aring <aring@mojatatu.com> wrote:
>>
>> This patch adds an argument for the fs-src plugin to declare the
>> directory to find the metadata file instead of placing it in every
>> subdir of traces.
>>
>> If parameter assign every subdirectory which does not have a
>> subdirectory and at least some regular files will be assumed as a trace
>> directory. The regular files will be assumed as ctf trace files.
>
> Although I really appreciate the contribution effort, before even
> reading this patch, let me indicate that a CTF trace located on the file
> system is a directory containing the data stream and metadata stream
> files. Knowing this, this patch implements a hack to circumvent this,
> but I'm personally not willing to have this upstream as, in my opinion,
> it is the user's responsibility to have valid CTF traces to open.
>
> Your custom scenario asks for a custom solution on your side: copying
> the metadata file to each trace's directory seems appropriate here.

Just had a thought: you can also create symbolic links in each trace's
directory to your common metadata file.

Phil

>
> Also, do your CTF traces have UUIDs? If they do, they should all be
> different, but having the same metadata file makes them all the same.
> This is not valid either.
>
> Phil
>
>>
>> Signed-off-by: Alexander Aring <aring@mojatatu.com>
>> ---
>>  cli/babeltrace-cfg-cli-args.c | 10 +++++
>>  plugins/ctf/fs-src/fs.c       | 88 +++++++++++++++++++++++++++++++++++--------
>>  plugins/ctf/fs-src/fs.h       |  3 +-
>>  plugins/ctf/fs-src/metadata.c |  8 +++-
>>  plugins/ctf/fs-src/metadata.h |  1 +
>>  plugins/ctf/fs-src/query.c    |  2 +-
>>  6 files changed, 94 insertions(+), 18 deletions(-)
>>
>> diff --git a/cli/babeltrace-cfg-cli-args.c b/cli/babeltrace-cfg-cli-args.c
>> index 8f01e64b..ab76852e 100644
>> --- a/cli/babeltrace-cfg-cli-args.c
>> +++ b/cli/babeltrace-cfg-cli-args.c
>> @@ -1306,6 +1306,7 @@ enum {
>>         OPT_CLOCK_GMT,
>>         OPT_CLOCK_OFFSET,
>>         OPT_CLOCK_OFFSET_NS,
>> +       OPT_METADATA_SRC,
>>         OPT_CLOCK_SECONDS,
>>         OPT_COLOR,
>>         OPT_COMPONENT,
>> @@ -2789,6 +2790,7 @@ void print_convert_usage(FILE *fp)
>>         fprintf(fp, "\n");
>>         fprintf(fp, "      --clock-offset=SEC            Set clock offset to SEC seconds\n");
>>         fprintf(fp, "      --clock-offset-ns=NS          Set clock offset to NS ns\n");
>> +       fprintf(fp, "      --metadata-src=PATH           Set a path to find the metadata\n");
>>         fprintf(fp, "\n");
>>         fprintf(fp, "Implicit `sink.text.pretty` component options:\n");
>>         fprintf(fp, "\n");
>> @@ -2886,6 +2888,7 @@ struct poptOption convert_long_options[] = {
>>         { "clock-gmt", '\0', POPT_ARG_NONE, NULL, OPT_CLOCK_GMT, NULL, NULL },
>>         { "clock-offset", '\0', POPT_ARG_STRING, NULL, OPT_CLOCK_OFFSET, NULL, NULL },
>>         { "clock-offset-ns", '\0', POPT_ARG_STRING, NULL, OPT_CLOCK_OFFSET_NS, NULL, NULL },
>> +       { "metadata-src", '\0', POPT_ARG_STRING, NULL, OPT_METADATA_SRC, NULL, NULL },
>>         { "clock-seconds", '\0', POPT_ARG_NONE, NULL, OPT_CLOCK_SECONDS, NULL, NULL },
>>         { "color", '\0', POPT_ARG_STRING, NULL, OPT_COLOR, NULL, NULL },
>>         { "component", 'c', POPT_ARG_STRING, NULL, OPT_COMPONENT, NULL, NULL },
>> @@ -3942,6 +3945,7 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
>>                 case OPT_CLOCK_GMT:
>>                 case OPT_CLOCK_OFFSET:
>>                 case OPT_CLOCK_OFFSET_NS:
>> +               case OPT_METADATA_SRC:
>>                 case OPT_CLOCK_SECONDS:
>>                 case OPT_COLOR:
>>                 case OPT_DEBUG:
>> @@ -4104,6 +4108,12 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
>>                                         &base_implicit_ctf_input_args,
>>                                         "clock-class-offset-ns", arg);
>>                         break;
>> +               case OPT_METADATA_SRC:
>> +                       base_implicit_ctf_input_args.exists = true;
>> +                       append_implicit_component_param(
>> +                                       &base_implicit_ctf_input_args,
>> +                                       "metadata-src", arg);
>> +                       break;
>>                 case OPT_CLOCK_SECONDS:
>>                         append_implicit_component_param(
>>                                 &implicit_text_args, "clock-seconds", "yes");
>> diff --git a/plugins/ctf/fs-src/fs.c b/plugins/ctf/fs-src/fs.c
>> index 2dacf97d..7f98dda5 100644
>> --- a/plugins/ctf/fs-src/fs.c
>> +++ b/plugins/ctf/fs-src/fs.c
>> @@ -1039,26 +1039,70 @@ end:
>>  }
>>
>>  BT_HIDDEN
>> -int ctf_fs_find_traces(GList **trace_paths, const char *start_path)
>> +int ctf_fs_find_traces(GList **trace_paths, const char *start_path,
>> +                      const char *metadata_src)
>>  {
>>         int ret;
>>         GError *error = NULL;
>>         GDir *dir = NULL;
>>         const char *basename = NULL;
>> +       bool subdirs = false;
>> +       bool regfile = false;
>>
>> -       /* Check if the starting path is a CTF trace itself */
>> -       ret = path_is_ctf_trace(start_path);
>> -       if (ret < 0) {
>> -               goto end;
>> -       }
>> +       if (metadata_src) {
>> +               ret = path_is_ctf_trace(metadata_src);
>> +               if (ret < 0) {
>> +                       goto end;
>> +               }
>>
>> -       if (ret) {
>> -               /*
>> -                * Stop recursion: a CTF trace cannot contain another
>> -                * CTF trace.
>> -                */
>> -               ret = add_trace_path(trace_paths, start_path);
>> -               goto end;
>> +               if (ret) {
>> +                       dir = g_dir_open(start_path, 0, &error);
>> +                       if (!dir) {
>> +                               goto end;
>> +                       }
>> +
>> +                       while ((basename = g_dir_read_name(dir))) {
>> +                               GString *sub_path = g_string_new(NULL);
>> +
>> +                               if (!sub_path) {
>> +                                       ret = -1;
>> +                                       goto end;
>> +                               }
>> +
>> +                               g_string_printf(sub_path, "%s" G_DIR_SEPARATOR_S "%s", start_path, basename);
>> +                               if (g_file_test(sub_path->str, G_FILE_TEST_IS_DIR)) {
>> +                                       subdirs = true;
>> +                               }
>> +                               if (g_file_test(sub_path->str, G_FILE_TEST_IS_REGULAR)) {
>> +                                       regfile = true;
>> +                               }
>> +                               g_string_free(sub_path, TRUE);
>> +                       }
>> +
>> +                       g_dir_close(dir);
>> +                       dir = NULL;
>> +
>> +                       /* Look if dir has no subdirs but regfile(s) */
>> +                       if (!subdirs && regfile) {
>> +                               ret = add_trace_path(trace_paths, start_path);
>> +                               goto end;
>> +                       }
>> +               }
>> +       } else {
>> +               /* Check if the starting path is a CTF trace itself */
>> +               ret = path_is_ctf_trace(start_path);
>> +               if (ret < 0) {
>> +                       goto end;
>> +               }
>> +
>> +               if (ret) {
>> +                       /*
>> +                        * Stop recursion: a CTF trace cannot contain another
>> +                        * CTF trace.
>> +                        */
>> +                       ret = add_trace_path(trace_paths, start_path);
>> +                       goto end;
>> +               }
>>         }
>>
>>         /* Look for subdirectories */
>> @@ -1090,7 +1134,7 @@ int ctf_fs_find_traces(GList **trace_paths, const char *start_path)
>>                 }
>>
>>                 g_string_printf(sub_path, "%s" G_DIR_SEPARATOR_S "%s", start_path, basename);
>> -               ret = ctf_fs_find_traces(trace_paths, sub_path->str);
>> +               ret = ctf_fs_find_traces(trace_paths, sub_path->str, metadata_src);
>>                 g_string_free(sub_path, TRUE);
>>                 if (ret) {
>>                         goto end;
>> @@ -1181,7 +1225,8 @@ int create_ctf_fs_traces(struct ctf_fs_component *ctf_fs,
>>                 goto error;
>>         }
>>
>> -       ret = ctf_fs_find_traces(&trace_paths, norm_path->str);
>> +       ret = ctf_fs_find_traces(&trace_paths, norm_path->str,
>> +                                ctf_fs->metadata_config.metadata_src);
>>         if (ret) {
>>                 goto error;
>>         }
>> @@ -1287,6 +1332,19 @@ struct ctf_fs_component *ctf_fs_create(struct bt_private_component *priv_comp,
>>         value_ret = bt_value_string_get(value, &path_param);
>>         assert(value_ret == BT_VALUE_STATUS_OK);
>>         BT_PUT(value);
>> +
>> +       value = bt_value_map_get(params, "metadata-src");
>> +       if (value) {
>> +               if (!bt_value_is_string(value)) {
>> +                       goto error;
>> +               }
>> +
>> +               value_ret = bt_value_string_get(value,
>> +                       &ctf_fs->metadata_config.metadata_src);
>> +               assert(value_ret == BT_VALUE_STATUS_OK);
>> +               BT_PUT(value);
>> +       }
>> +
>>         value = bt_value_map_get(params, "clock-class-offset-s");
>>         if (value) {
>>                 if (!bt_value_is_integer(value)) {
>> diff --git a/plugins/ctf/fs-src/fs.h b/plugins/ctf/fs-src/fs.h
>> index bbac1bb4..f80aea74 100644
>> --- a/plugins/ctf/fs-src/fs.h
>> +++ b/plugins/ctf/fs-src/fs.h
>> @@ -154,7 +154,8 @@ BT_HIDDEN
>>  void ctf_fs_trace_destroy(struct ctf_fs_trace *trace);
>>
>>  BT_HIDDEN
>> -int ctf_fs_find_traces(GList **trace_paths, const char *start_path);
>> +int ctf_fs_find_traces(GList **trace_paths, const char *start_path,
>> +                      const char *metadata_src);
>>
>>  BT_HIDDEN
>>  GList *ctf_fs_create_trace_names(GList *trace_paths, const char *base_path);
>> diff --git a/plugins/ctf/fs-src/metadata.c b/plugins/ctf/fs-src/metadata.c
>> index 231d946c..02cbd0cd 100644
>> --- a/plugins/ctf/fs-src/metadata.c
>> +++ b/plugins/ctf/fs-src/metadata.c
>> @@ -96,8 +96,14 @@ int ctf_fs_metadata_set_trace(struct ctf_fs_trace *ctf_fs_trace,
>>                 .clock_class_offset_s = config ? config->clock_class_offset_s : 0,
>>                 .clock_class_offset_ns = config ? config->clock_class_offset_ns : 0,
>>         };
>> +       const char *metadata_src;
>>
>> -       file = get_file(ctf_fs_trace->path->str);
>> +       if (config->metadata_src)
>> +               metadata_src = config->metadata_src;
>> +       else
>> +               metadata_src = ctf_fs_trace->path->str;
>> +
>> +       file = get_file(metadata_src);
>>         if (!file) {
>>                 BT_LOGE("Cannot create metadata file object");
>>                 ret = -1;
>> diff --git a/plugins/ctf/fs-src/metadata.h b/plugins/ctf/fs-src/metadata.h
>> index 496a5ca9..00d8de67 100644
>> --- a/plugins/ctf/fs-src/metadata.h
>> +++ b/plugins/ctf/fs-src/metadata.h
>> @@ -36,6 +36,7 @@ struct ctf_fs_metadata;
>>  struct ctf_fs_metadata_config {
>>         int64_t clock_class_offset_s;
>>         int64_t clock_class_offset_ns;
>> +       const char *metadata_src;
>>  };
>>
>>  BT_HIDDEN
>> diff --git a/plugins/ctf/fs-src/query.c b/plugins/ctf/fs-src/query.c
>> index 04bf8c5b..6f2aa5fe 100644
>> --- a/plugins/ctf/fs-src/query.c
>> +++ b/plugins/ctf/fs-src/query.c
>> @@ -500,7 +500,7 @@ struct bt_component_class_query_method_return trace_info_query(
>>         }
>>         assert(path);
>>
>> -       ret = ctf_fs_find_traces(&trace_paths, normalized_path->str);
>> +       ret = ctf_fs_find_traces(&trace_paths, normalized_path->str, NULL);
>>         if (ret) {
>>                 goto error;
>>         }
>> --
>> 2.11.0
>>
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev@lists.lttng.org
>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 0/2] fs-src: add metadata-src parameter

[Alexander Aring]

Hi,

On Mon, Apr 30, 2018 at 07:11:30PM -0400, Philippe Proulx wrote:
> On Mon, Apr 30, 2018 at 5:31 PM, Alexander Aring <aring@mojatatu.com> wrote:
> > Hi,
> >
> > my use-case is just... we have several application with the same metadata
> > file. Currently I build a wrapper around babeltrace to run something like:
> >
> > find $(TRACE_DIR}/* -maxdepth 1 -type d -exec cp ${PATH_TO_MY_APP_DATA}/ctf/metadata '{}' \;
> >
> > which is terrible... so I try this here somehow.
> > As you see it has no support for fs-query and I didn't figured out yet
> > for what fs-query is for.
> >
> > Also I was struggle some hours how to tell popt the right argument string,
> > it need to be:
> >
> > --metadata-src=\"/usr/share/$APP_DIR/ctf\"
> >
> > with the ugly escape things... I saw there is a complex INI parser system
> > behind (never saw such complex option parser in an open source project).
> 
> A note about this: there are two ways to add component initialization
> parameters with `babeltrace run`: with
> 
>     --params 'key1="the value", key2=23, key3=simple_string'
> 
> and, for string parameters, with:
> 
>     --key key1 --value 'the value'
> 
> The --params option parses what you call a "complex INI" format. It's
> not so complex in reality and well-explained in the man page.

When I have a string I need to use --key key1 --value 'the value'
otherwise key/value pairs?

I remember I saw the key value args somewhere... it seeems not be part
of babeltrace convert. I tried --params, and as the manpage said [0]:

Important
	Like in the example above, make sure to single-quote the whole
	argument when you run this command from a shell.

This did libpopt to parse something, but still get some "cannot create
components" error. I need to debug this more to figure out the "why".
So put everything in single quotes are important... yes this will tell
the shell to handle it as a full arg, I agree. And also there is a big
IMPORTANT in --help. :-)

I saw that source.ctf.fs had it's own implicit argument "--clock-offset"
which seemed to work for me and I tried the same with --metadata-src.
Seems to be a shortcut for --key key1 --value 'the value'.

According to the key value arg options:

It's somewhat misleading that you have args (--FOO) which need to be in a
special order e.g. --key and then --value (where value is a must after key).
Does libpopt have such feature? Anyway I would use --params always as it
accept key/value pairs as optarg.

> 
> The job of `babeltrace convert` (the default command) is only to create
> a valid `babeltrace run` command line and run it. In your first patch,

I see that's why I see something when I just type my command. Then
source and sink will be connected by a magic way?

> you use append_implicit_component_param(): this one appends a key/value
> pair to the --params option's argument of a given implicit component.
> It's simply appending `,KEY=VAL` to the current INI-style parameter
> string for that component, which is why you need to pass the double
> quotes above, because the effective string to append is exactly:
> 
>     ,metadata-src="/usr/share/app-dir-expansion/ctf"
> 
> What you want to do is have this instead:
> 
>     --key metadata-src --value /usr/share/app-dir-expansion/ctf
> 

I am not sure how to pass these --key --value arguments. Manpage of
source.ctf.fs [0] doesn't say anything about these parameters.

> (where `app-dir-expansion` is the expansion of $APP_DIR). You can use
> append_implicit_component_extra_param() for this. This is what we use
> for complex string values like for --begin, --end, --debug-info-dir,
> etc. Then you could use:
> 
>     --metadata-src="/usr/share/$APP_DIR/ctf"
> 
> so that popt returns `/usr/share/app-dir-expansion/ctf`.
> 

When I implemented it and I try to run it. I get a confused parser error:

Command line error: Expecting value:

    metadata-src=/foobar/ctf

---

arg was:  --metadata-src="/foobar/ctf"

and maybe that's the reason why I don't like libpopt now. :-)

It's actually means babeltrace can not have "implicit shortcut args"
e.g. "--clock-offset" with a string value (as popt string type)?

Thanks for the answer and so fast...! :-)

- Alex

[0] http://man7.org/linux/man-pages/man1/babeltrace-convert.1.html
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 0/2] fs-src: add metadata-src parameter

[Philippe Proulx]

Try a few `babeltrace` commands and add the --run-args option. The output
is the arguments that would be pass to the `babeltrace run` command. You'll
find documentation for --key and --value in `babeltrace-run(1)`.

Phil

On Tue, May 1, 2018 at 10:51 AM, Alexander Aring <aring@mojatatu.com> wrote:
> Hi,
>
> On Mon, Apr 30, 2018 at 07:11:30PM -0400, Philippe Proulx wrote:
>> On Mon, Apr 30, 2018 at 5:31 PM, Alexander Aring <aring@mojatatu.com> wrote:
>> > Hi,
>> >
>> > my use-case is just... we have several application with the same metadata
>> > file. Currently I build a wrapper around babeltrace to run something like:
>> >
>> > find $(TRACE_DIR}/* -maxdepth 1 -type d -exec cp ${PATH_TO_MY_APP_DATA}/ctf/metadata '{}' \;
>> >
>> > which is terrible... so I try this here somehow.
>> > As you see it has no support for fs-query and I didn't figured out yet
>> > for what fs-query is for.
>> >
>> > Also I was struggle some hours how to tell popt the right argument string,
>> > it need to be:
>> >
>> > --metadata-src=\"/usr/share/$APP_DIR/ctf\"
>> >
>> > with the ugly escape things... I saw there is a complex INI parser system
>> > behind (never saw such complex option parser in an open source project).
>>
>> A note about this: there are two ways to add component initialization
>> parameters with `babeltrace run`: with
>>
>>     --params 'key1="the value", key2=23, key3=simple_string'
>>
>> and, for string parameters, with:
>>
>>     --key key1 --value 'the value'
>>
>> The --params option parses what you call a "complex INI" format. It's
>> not so complex in reality and well-explained in the man page.
>
> When I have a string I need to use --key key1 --value 'the value'
> otherwise key/value pairs?
>
> I remember I saw the key value args somewhere... it seeems not be part
> of babeltrace convert. I tried --params, and as the manpage said [0]:
>
> Important
>         Like in the example above, make sure to single-quote the whole
>         argument when you run this command from a shell.
>
> This did libpopt to parse something, but still get some "cannot create
> components" error. I need to debug this more to figure out the "why".
> So put everything in single quotes are important... yes this will tell
> the shell to handle it as a full arg, I agree. And also there is a big
> IMPORTANT in --help. :-)
>
> I saw that source.ctf.fs had it's own implicit argument "--clock-offset"
> which seemed to work for me and I tried the same with --metadata-src.
> Seems to be a shortcut for --key key1 --value 'the value'.
>
> According to the key value arg options:
>
> It's somewhat misleading that you have args (--FOO) which need to be in a
> special order e.g. --key and then --value (where value is a must after key).
> Does libpopt have such feature? Anyway I would use --params always as it
> accept key/value pairs as optarg.
>
>>
>> The job of `babeltrace convert` (the default command) is only to create
>> a valid `babeltrace run` command line and run it. In your first patch,
>
> I see that's why I see something when I just type my command. Then
> source and sink will be connected by a magic way?
>
>> you use append_implicit_component_param(): this one appends a key/value
>> pair to the --params option's argument of a given implicit component.
>> It's simply appending `,KEY=VAL` to the current INI-style parameter
>> string for that component, which is why you need to pass the double
>> quotes above, because the effective string to append is exactly:
>>
>>     ,metadata-src="/usr/share/app-dir-expansion/ctf"
>>
>> What you want to do is have this instead:
>>
>>     --key metadata-src --value /usr/share/app-dir-expansion/ctf
>>
>
> I am not sure how to pass these --key --value arguments. Manpage of
> source.ctf.fs [0] doesn't say anything about these parameters.
>
>> (where `app-dir-expansion` is the expansion of $APP_DIR). You can use
>> append_implicit_component_extra_param() for this. This is what we use
>> for complex string values like for --begin, --end, --debug-info-dir,
>> etc. Then you could use:
>>
>>     --metadata-src="/usr/share/$APP_DIR/ctf"
>>
>> so that popt returns `/usr/share/app-dir-expansion/ctf`.
>>
>
> When I implemented it and I try to run it. I get a confused parser error:
>
> Command line error: Expecting value:
>
>     metadata-src=/foobar/ctf
>
> ---
>
> arg was:  --metadata-src="/foobar/ctf"
>
> and maybe that's the reason why I don't like libpopt now. :-)
>
> It's actually means babeltrace can not have "implicit shortcut args"
> e.g. "--clock-offset" with a string value (as popt string type)?
>
> Thanks for the answer and so fast...! :-)
>
> - Alex
>
> [0] http://man7.org/linux/man-pages/man1/babeltrace-convert.1.html
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[Alexander Aring]

Hi,

On Mon, Apr 30, 2018 at 06:36:08PM -0400, Philippe Proulx wrote:
> On Mon, Apr 30, 2018 at 5:31 PM, Alexander Aring <aring@mojatatu.com> wrote:
> >
> > This patch adds an argument for the fs-src plugin to declare the
> > directory to find the metadata file instead of placing it in every
> > subdir of traces.
> >
> > If parameter assign every subdirectory which does not have a
> > subdirectory and at least some regular files will be assumed as a trace
> > directory. The regular files will be assumed as ctf trace files.
> 
> Although I really appreciate the contribution effort, before even
> reading this patch, let me indicate that a CTF trace located on the file
> system is a directory containing the data stream and metadata stream
> files. Knowing this, this patch implements a hack to circumvent this,
> but I'm personally not willing to have this upstream as, in my opinion,
> it is the user's responsibility to have valid CTF traces to open.
> 

Yea, I already thought that will happen... but I want to talk about my
use-case and how to handle it with babeltrace.

My use-case is the following:

 - I use barectf with a fs platform based platform. I want to contribute
   them back to barectf if this is welcome. I see I already have the
   barectf expert here. I use it for userspace application trace.

 - I have a distributed application which stores the stream files in a
   directory with the scheme:

   $TRACES/
           $HOSTNAME/
	             ....
	             $APPNAME_$PID_stream

Currently I mostly run my application on one host with one clock source,
which makes everything about clock handling very simple.

So far I know the multiple streams and merge has the use case to have a
trace per cpu, well we do it per application... I hope this is okay.

> Your custom scenario asks for a custom solution on your side: copying
> the metadata file to each trace's directory seems appropriate here.
> 

Really?

I thought about that I cannot use traces on a read-only filesystem,
because I am not allowed to copy there. One example where my solution
hits it's limitation...

I also thought about: using barectf and generate a object file linked
to my platform who has the metadata file inside and will be created to
my stream file when barectf init's (if it's not already exists).
So far I see it could be done because metadata file is compile time
related and binded to barectf generated code.

That will bloat my binary as one disadvantage, otherwise I can be sure
there is no mixed metadata handling everywhere.

> Also, do your CTF traces have UUIDs? If they do, they should all be
> different, but having the same metadata file makes them all the same.
> This is not valid either.
> 

I have UUID but my applications use all the same metadata file by
barectf. :-) I hope this is a valid point?

I admit, currently I have mostly functionality to make backwards
compatibility with an old trace handling. It's just put some ascii info
inside the stream. Later I want to move ascii into metadata description.
It works like tracelog [0], but just temporary.

- Alex

[0] http://man7.org/linux/man-pages/man3/tracelog.3.html
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[Philippe Proulx]

On Tue, May 1, 2018 at 11:12 AM, Alexander Aring <aring@mojatatu.com> wrote:
> Hi,
>
> On Mon, Apr 30, 2018 at 06:36:08PM -0400, Philippe Proulx wrote:
>> On Mon, Apr 30, 2018 at 5:31 PM, Alexander Aring <aring@mojatatu.com> wrote:
>> >
>> > This patch adds an argument for the fs-src plugin to declare the
>> > directory to find the metadata file instead of placing it in every
>> > subdir of traces.
>> >
>> > If parameter assign every subdirectory which does not have a
>> > subdirectory and at least some regular files will be assumed as a trace
>> > directory. The regular files will be assumed as ctf trace files.
>>
>> Although I really appreciate the contribution effort, before even
>> reading this patch, let me indicate that a CTF trace located on the file
>> system is a directory containing the data stream and metadata stream
>> files. Knowing this, this patch implements a hack to circumvent this,
>> but I'm personally not willing to have this upstream as, in my opinion,
>> it is the user's responsibility to have valid CTF traces to open.
>>
>
> Yea, I already thought that will happen... but I want to talk about my
> use-case and how to handle it with babeltrace.
>
> My use-case is the following:
>
>  - I use barectf with a fs platform based platform. I want to contribute
>    them back to barectf if this is welcome. I see I already have the
>    barectf expert here. I use it for userspace application trace.
>
>  - I have a distributed application which stores the stream files in a
>    directory with the scheme:
>
>    $TRACES/
>            $HOSTNAME/
>                      ....
>                      $APPNAME_$PID_stream
>
> Currently I mostly run my application on one host with one clock source,
> which makes everything about clock handling very simple.
>
> So far I know the multiple streams and merge has the use case to have a
> trace per cpu, well we do it per application... I hope this is okay.
>
>> Your custom scenario asks for a custom solution on your side: copying
>> the metadata file to each trace's directory seems appropriate here.
>>
>
> Really?
>
> I thought about that I cannot use traces on a read-only filesystem,
> because I am not allowed to copy there. One example where my solution
> hits it's limitation...
>
> I also thought about: using barectf and generate a object file linked
> to my platform who has the metadata file inside and will be created to
> my stream file when barectf init's (if it's not already exists).
> So far I see it could be done because metadata file is compile time
> related and binded to barectf generated code.

Yes this is a better idea. If you cannot postprocess the trace directories,
than it's better that you create valid traces in the beginning.

>
> That will bloat my binary as one disadvantage, otherwise I can be sure
> there is no mixed metadata handling everywhere.

Can you create symbolic links on this filesystem (when you have write
access)? This would avoid bloating.

>
>> Also, do your CTF traces have UUIDs? If they do, they should all be
>> different, but having the same metadata file makes them all the same.
>> This is not valid either.
>>
>
> I have UUID but my applications use all the same metadata file by
> barectf. :-) I hope this is a valid point?

Normally, a trace has a unique UUID. If you cannot ensure this, I
suggest that you remove the UUID field from the packet header.

Phil

>
> I admit, currently I have mostly functionality to make backwards
> compatibility with an old trace handling. It's just put some ascii info
> inside the stream. Later I want to move ascii into metadata description.
> It works like tracelog [0], but just temporary.
>
> - Alex
>
> [0] http://man7.org/linux/man-pages/man3/tracelog.3.html
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[Alexander Aring]

Hi,

On Tue, May 01, 2018 at 11:22:18AM -0400, Philippe Proulx wrote:
...
> >
> > I also thought about: using barectf and generate a object file linked
> > to my platform who has the metadata file inside and will be created to
> > my stream file when barectf init's (if it's not already exists).
> > So far I see it could be done because metadata file is compile time
> > related and binded to barectf generated code.
> 
> Yes this is a better idea. If you cannot postprocess the trace directories,
> than it's better that you create valid traces in the beginning.
> 

I see, I will have this idea in the background... I think I will switch
to it when I see some issues with metadata and this would avoid it.

> >
> > That will bloat my binary as one disadvantage, otherwise I can be sure
> > there is no mixed metadata handling everywhere.
> 
> Can you create symbolic links on this filesystem (when you have write
> access)? This would avoid bloating.
> 

I will switch to symbolic link and hopefully nobody will pack everything
in a .zip or something else that doesn't support symbolic links. :-)

> >
> >> Also, do your CTF traces have UUIDs? If they do, they should all be
> >> different, but having the same metadata file makes them all the same.
> >> This is not valid either.
> >>
> >
> > I have UUID but my applications use all the same metadata file by
> > barectf. :-) I hope this is a valid point?
> 
> Normally, a trace has a unique UUID. If you cannot ensure this, I
> suggest that you remove the UUID field from the packet header.
> 

I see, according [0]:

Trace UUID, used to ensure the event packet match the metadata used.
Note: we cannot use a metadata checksum in every cases instead of a UUID
because metadata can be appended to while tracing is active. This field
is optional.

---

Will this drop not a validation for me that the stream and metadata fits
together? So babeltrace will reject something when stream was made with
a different metadata as placed into my TRACES directory?

Somebody could use a different metadata file, by weird accident.

And "... metadata can be appended to while tracing is active." - woa, so
you can add more metadata during runtime and the streams can use them
(during runtime)?

I guess this is not possible with barectf where everything is made at
compile time... (currently).

- Alex

[0] http://diamon.org/ctf/#spec5
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC babeltrace 1/2] fs-src: add argument for metadata src dir

[Alexander Aring]

On Tue, May 01, 2018 at 11:43:32AM -0400, Alexander Aring wrote:
...
> > 
> > Normally, a trace has a unique UUID. If you cannot ensure this, I
> > suggest that you remove the UUID field from the packet header.
> > 
> 
> I see, according [0]:
> 
> Trace UUID, used to ensure the event packet match the metadata used.
> Note: we cannot use a metadata checksum in every cases instead of a UUID
> because metadata can be appended to while tracing is active. This field
> is optional.
> 

I removed some other fields which I don't need. Thanks for reminding me!
I removed:


 - Stream-ID: I have only one stream
 - events_discarded: I don't handle this in fs platform right now, could
   be maybe as "filesystem is full?"

May I drop uuid when I have only one metadata file, but validation if
metadata fits or not could be useful for handling the case if somebody
use a different one.

I see that barectf will still generate some uuid and place it in:

trace {
	uuid = ....
};

but I guess it will not be used anymore in the stream. So far I don't
see it anymore with a simple hexdump (Looks for me as the best way to
check what barectf is doing).

Thanks.

- Alex
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev