[PATCH v2 0/7] Bump trace file version

[PATCH v2 0/7] Bump trace file version

[Tzvetomir Stoyanov (VMware)]

Changed the trace file version to 7 and added support for choosing 
the desired file version.

v2 changes:
 - Include "Check if file version is supported" patch in the set, as other
   patches depend on it.
 - Reorganized the order of the patches, put the clean up patch "Remove
   unused private APIs for creating trace files" first.

Tzvetomir Stoyanov (VMware) (7):
  trace-cmd library: Remove unused private APIs for creating trace files
  trace-cmd: Check if file version is supported
  trace-cmd library: Bump the trace file version to 7
  trace-cmd library: Add new API to get file version of input handler
  trace-cmd library: Select the file version when writing trace file
  trace-cmd library: Extend the create file APIs to support different
    file version
  trace-cmd record: Add new parameter --file-version

 .../include/private/trace-cmd-private.h       | 29 +++---
 lib/trace-cmd/include/trace-cmd-local.h       |  2 +-
 lib/trace-cmd/trace-input.c                   | 19 ++++
 lib/trace-cmd/trace-output.c                  | 98 ++++++++++---------
 lib/trace-cmd/trace-util.c                    |  7 ++
 tracecmd/trace-dump.c                         |  7 ++
 tracecmd/trace-record.c                       | 25 +++--
 tracecmd/trace-restore.c                      |  4 +-
 tracecmd/trace-usage.c                        |  1 +
 9 files changed, 121 insertions(+), 71 deletions(-)

-- 
2.30.2

[PATCH v2 1/7] trace-cmd library: Remove unused private APIs for creating trace files

[Tzvetomir Stoyanov (VMware)]

Remove these unused private trace-cmd library APIs:
  tracecmd_create_file()
  tracecmd_create_file_glob()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 .../include/private/trace-cmd-private.h       |  6 ----
 lib/trace-cmd/trace-output.c                  | 29 -------------------
 2 files changed, 35 deletions(-)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 42e739fa..8ec33ea2 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -263,12 +263,6 @@ struct tracecmd_option;
 struct tracecmd_msg_handle;
 
 struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
-struct tracecmd_output *tracecmd_create_file(const char *output_file,
-					     int cpus, char * const *cpu_data_files);
-struct tracecmd_output *
-tracecmd_create_file_glob(const char *output_file,
-			  int cpus, char * const *cpu_data_files,
-			  struct tracecmd_event_list *event_globs);
 struct tracecmd_output *
 tracecmd_create_init_file_glob(const char *output_file,
 			       struct tracecmd_event_list *list);
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index a4a1eecc..9231c319 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1614,35 +1614,6 @@ struct tracecmd_output *tracecmd_get_output_handle_fd(int fd)
 	return NULL;
 }
 
-struct tracecmd_output *
-tracecmd_create_file_glob(const char *output_file,
-			  int cpus, char * const *cpu_data_files,
-			  struct tracecmd_event_list *list)
-{
-	struct tracecmd_output *handle;
-
-	handle = create_file(output_file, NULL, NULL, NULL, list);
-	if (!handle)
-		return NULL;
-
-	if (tracecmd_write_cmdlines(handle))
-		return NULL;
-
-	if (tracecmd_append_cpu_data(handle, cpus, cpu_data_files) < 0) {
-		tracecmd_output_close(handle);
-		return NULL;
-	}
-
-	return handle;
-}
-
-struct tracecmd_output *tracecmd_create_file(const char *output_file,
-					     int cpus, char * const *cpu_data_files)
-{
-	return tracecmd_create_file_glob(output_file, cpus,
-					 cpu_data_files, &all_event_list);
-}
-
 struct tracecmd_output *tracecmd_create_init_fd(int fd)
 {
 	return create_file_fd(fd, NULL, NULL, NULL, &all_event_list, NULL);
-- 
2.30.2

[PATCH v2 2/7] trace-cmd: Check if file version is supported

[Tzvetomir Stoyanov (VMware)]

When reading a trace file, version of the file is ignored. This could
case problems when bumping the version number because of changes in
in the structure of the file. The old code should detect unsupported
file version and should not try to read it.
A new trace-cmd library API is added to check if version is supported:
 tracecmd_is_version_supported()
Checks are added in the code to ensure not trying to read trace file
with unsupported version.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/include/private/trace-cmd-private.h |  2 ++
 lib/trace-cmd/trace-input.c                       | 10 ++++++++++
 lib/trace-cmd/trace-util.c                        |  7 +++++++
 tracecmd/trace-dump.c                             |  7 +++++++
 4 files changed, 26 insertions(+)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 8ec33ea2..cbd8dc1f 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -42,6 +42,8 @@ void tracecmd_record_ref(struct tep_record *record);
 void tracecmd_set_debug(bool set_debug);
 bool tracecmd_get_debug(void);
 
+bool tracecmd_is_version_supported(unsigned int version);
+
 struct tracecmd_output;
 struct tracecmd_recorder;
 struct hook_list;
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index b17b36e0..a00fa982 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -130,6 +130,7 @@ struct tracecmd_input {
 	bool			use_trace_clock;
 	bool			read_page;
 	bool			use_pipe;
+	int			file_version;
 	struct cpu_data 	*cpu_data;
 	long long		ts_offset;
 	struct tsc2nsec		tsc_calc;
@@ -3257,6 +3258,7 @@ struct tracecmd_input *tracecmd_alloc_fd(int fd, int flags)
 	unsigned int page_size;
 	char *version;
 	char buf[BUFSIZ];
+	unsigned long ver;
 
 	handle = malloc(sizeof(*handle));
 	if (!handle)
@@ -3281,6 +3283,14 @@ struct tracecmd_input *tracecmd_alloc_fd(int fd, int flags)
 	if (!version)
 		goto failed_read;
 	pr_stat("version = %s\n", version);
+	ver = strtol(version, NULL, 10);
+	if (!ver && errno)
+		goto failed_read;
+	if (!tracecmd_is_version_supported(ver)) {
+		tracecmd_warning("Unsupported file version %lu", ver);
+		goto failed_read;
+	}
+	handle->file_version = ver;
 	free(version);
 
 	if (do_read_check(handle, buf, 1))
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 2d3bc741..a6fc64c8 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -582,3 +582,10 @@ unsigned long long tracecmd_generate_traceid(void)
 	free(str);
 	return hash;
 }
+
+bool tracecmd_is_version_supported(unsigned int version)
+{
+	if (version <= FILE_VERSION)
+		return true;
+	return false;
+}
diff --git a/tracecmd/trace-dump.c b/tracecmd/trace-dump.c
index 3f56f65a..aa48eaad 100644
--- a/tracecmd/trace-dump.c
+++ b/tracecmd/trace-dump.c
@@ -10,6 +10,7 @@
 #include <getopt.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <errno.h>
 
 #include "trace-local.h"
 
@@ -145,6 +146,7 @@ static void dump_initial_format(int fd)
 	char magic[] = TRACECMD_MAGIC;
 	char buf[DUMP_SIZE];
 	int val4;
+	unsigned long ver;
 
 	do_print(SUMMARY, "\t[Initial format]\n");
 
@@ -166,6 +168,11 @@ static void dump_initial_format(int fd)
 		die("no version string");
 
 	do_print(SUMMARY, "\t\t%s\t[Version]\n", buf);
+	ver = strtol(buf, NULL, 10);
+	if (!ver && errno)
+		die("Invalid file version string %s", buf);
+	if (!tracecmd_is_version_supported(ver))
+		die("Unsupported file version %lu", ver);
 
 	/* get file endianness*/
 	if (read_file_bytes(fd, buf, 1))
-- 
2.30.2

[PATCH v2 3/7] trace-cmd library: Bump the trace file version to 7

[Tzvetomir Stoyanov (VMware)]

Adding a compression of the trace.dat file will change its structure.
These changes are not backward compatible, the old trace-cmd binaries
will not be able to read compressed trace files. Bumping the version to
7 will prevent old trace-cmd to read such files.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/include/trace-cmd-local.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index e8533b22..5578f00d 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -14,7 +14,7 @@ void tracecmd_warning(const char *fmt, ...);
 void tracecmd_fatal(const char *fmt, ...);
 
 /* trace.dat file format version */
-#define FILE_VERSION 6
+#define FILE_VERSION 7
 
 #define _STR(x)	#x
 #define STR(x)	_STR(x)
-- 
2.30.2

[PATCH v2 4/7] trace-cmd library: Add new API to get file version of input handler

[Tzvetomir Stoyanov (VMware)]

Added an API to get the version of the trace file, associated with given
input file handler.
  tracecmd_get_file_version()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/include/private/trace-cmd-private.h | 2 ++
 lib/trace-cmd/trace-input.c                       | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index cbd8dc1f..29e56271 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -232,6 +232,8 @@ int tracecmd_set_cursor(struct tracecmd_input *handle,
 unsigned long long
 tracecmd_get_cursor(struct tracecmd_input *handle, int cpu);
 
+unsigned long tracecmd_get_file_version(struct tracecmd_input *handle);
+
 int tracecmd_ftrace_overrides(struct tracecmd_input *handle, struct tracecmd_ftrace *finfo);
 bool tracecmd_get_use_trace_clock(struct tracecmd_input *handle);
 tracecmd_show_data_func
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index a00fa982..b2a03ab8 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -4034,6 +4034,15 @@ struct tep_handle *tracecmd_get_tep(struct tracecmd_input *handle)
 	return handle->pevent;
 }
 
+/**
+ * tracecmd_get_file_version - return the trace.dat file version
+ * @handle: input handle for the trace.dat file
+ */
+unsigned long tracecmd_get_file_version(struct tracecmd_input *handle)
+{
+	return handle->file_version;
+}
+
 /**
  * tracecmd_get_use_trace_clock - return use_trace_clock
  * @handle: input handle for the trace.dat file
-- 
2.30.2

[PATCH v2 5/7] trace-cmd library: Select the file version when writing trace file

[Tzvetomir Stoyanov (VMware)]

When a new output handler to trace file is allocated, select the proper
file version. If this output handler is based on an existing input trace
file handler, inherit the trace file version.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-output.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 9231c319..f36718f1 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -57,6 +57,7 @@ struct tracecmd_output {
 	int			nr_options;
 	bool			quiet;
 	unsigned long		file_state;
+	unsigned long		file_version;
 	struct list_head	options;
 	struct tracecmd_msg_handle *msg_handle;
 	char			*trace_clock;
@@ -907,6 +908,17 @@ out_free:
 	return ret;
 }
 
+static int select_file_version(struct tracecmd_output *handle,
+				struct tracecmd_input *ihandle)
+{
+	if (ihandle)
+		handle->file_version = tracecmd_get_file_version(ihandle);
+	else
+		handle->file_version = FILE_VERSION;
+
+	return 0;
+}
+
 static struct tracecmd_output *
 create_file_fd(int fd, struct tracecmd_input *ihandle,
 	       const char *tracing_dir,
@@ -933,6 +945,9 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
 
 	handle->msg_handle = msg_handle;
 
+	if (select_file_version(handle, ihandle))
+		goto out_free;
+
 	list_head_init(&handle->options);
 
 	buf[0] = 23;
@@ -943,7 +958,8 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
 	if (do_write_check(handle, buf, 10))
 		goto out_free;
 
-	if (do_write_check(handle, FILE_VERSION_STRING, strlen(FILE_VERSION_STRING) + 1))
+	sprintf(buf, "%lu", handle->file_version);
+	if (do_write_check(handle, buf, strlen(buf) + 1))
 		goto out_free;
 
 	/* get endian and page size */
@@ -1602,6 +1618,7 @@ struct tracecmd_output *tracecmd_get_output_handle_fd(int fd)
 	handle->pevent = tracecmd_get_tep(ihandle);
 	tep_ref(handle->pevent);
 	handle->page_size = tracecmd_page_size(ihandle);
+	handle->file_version = tracecmd_get_file_version(ihandle);
 	list_head_init(&handle->options);
 
 	tracecmd_close(ihandle);
-- 
2.30.2

Re: [PATCH v2 5/7] trace-cmd library: Select the file version when writing trace file

[Steven Rostedt]

On Thu, 29 Apr 2021 07:01:17 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> index 9231c319..f36718f1 100644
> --- a/lib/trace-cmd/trace-output.c
> +++ b/lib/trace-cmd/trace-output.c
> @@ -57,6 +57,7 @@ struct tracecmd_output {
>  	int			nr_options;
>  	bool			quiet;
>  	unsigned long		file_state;
> +	unsigned long		file_version;
>  	struct list_head	options;
>  	struct tracecmd_msg_handle *msg_handle;
>  	char			*trace_clock;
> @@ -907,6 +908,17 @@ out_free:
>  	return ret;
>  }
>  
> +static int select_file_version(struct tracecmd_output *handle,
> +				struct tracecmd_input *ihandle)
> +{
> +	if (ihandle)
> +		handle->file_version = tracecmd_get_file_version(ihandle);
> +	else
> +		handle->file_version = FILE_VERSION;
> +
> +	return 0;
> +}
> +

This is where we should have the logic for what we write.

We save a string where the version is going to be located. We default it to
the lowest level (6), and then if a feature is recorded that is of a higher
version, we up it, and change the number of disk as well.

If there's a case where we get to two digit strings, we should have it be:

  "6 \0"

which should still work. Then if we use a feature that requires version 10,
it would be updated to:

  "10\0"

We can do this at the end of recording.

-- Steve

[PATCH v2 6/7] trace-cmd library: Extend the create file APIs to support different file version

[Tzvetomir Stoyanov (VMware)]

Added additional parameter for file version to all trace-cmd library
APIs for creating a new trace file. The caller could specify what is the
desired version of the created file:
 tracecmd_create_file_latency
 tracecmd_create_init_file_glob
 tracecmd_create_init_fd_glob
 tracecmd_create_init_fd_msg
 tracecmd_create_init_file
 tracecmd_create_init_file_override

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 .../include/private/trace-cmd-private.h       | 19 ++++---
 lib/trace-cmd/trace-output.c                  | 56 ++++++++++++-------
 tracecmd/trace-record.c                       | 12 ++--
 tracecmd/trace-restore.c                      |  4 +-
 4 files changed, 54 insertions(+), 37 deletions(-)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 29e56271..def01b68 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -266,20 +266,25 @@ struct tracecmd_event_list {
 struct tracecmd_option;
 struct tracecmd_msg_handle;
 
-struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
+struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus,
+						     unsigned long file_version);
 struct tracecmd_output *
-tracecmd_create_init_file_glob(const char *output_file,
-			       struct tracecmd_event_list *list);
+tracecmd_create_init_file_glob(const char *output_file, struct tracecmd_event_list *list,
+			       unsigned long file_version);
 struct tracecmd_output *tracecmd_create_init_fd(int fd);
 struct tracecmd_output *
-tracecmd_create_init_fd_glob(int fd, struct tracecmd_event_list *list);
+tracecmd_create_init_fd_glob(int fd, struct tracecmd_event_list *list,
+			     unsigned long file_version);
 struct tracecmd_output *
 tracecmd_create_init_fd_msg(struct tracecmd_msg_handle *msg_handle,
-			    struct tracecmd_event_list *list);
-struct tracecmd_output *tracecmd_create_init_file(const char *output_file);
+			    struct tracecmd_event_list *list,
+			    unsigned long file_version);
+struct tracecmd_output *tracecmd_create_init_file(const char *output_file,
+						  unsigned long file_version);
 struct tracecmd_output *tracecmd_create_init_file_override(const char *output_file,
 							   const char *tracing_dir,
-							   const char *kallsyms);
+							   const char *kallsyms,
+							   unsigned long file_version);
 struct tracecmd_option *tracecmd_add_option(struct tracecmd_output *handle,
 					    unsigned short id, int size,
 					    const void *data);
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index f36718f1..edff7961 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -908,11 +908,13 @@ out_free:
 	return ret;
 }
 
-static int select_file_version(struct tracecmd_output *handle,
-				struct tracecmd_input *ihandle)
+static int select_file_version(struct tracecmd_output *handle, struct tracecmd_input *ihandle,
+			       unsigned long file_version)
 {
 	if (ihandle)
 		handle->file_version = tracecmd_get_file_version(ihandle);
+	else if (file_version > 0)
+		handle->file_version = file_version;
 	else
 		handle->file_version = FILE_VERSION;
 
@@ -924,7 +926,8 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
 	       const char *tracing_dir,
 	       const char *kallsyms,
 	       struct tracecmd_event_list *list,
-	       struct tracecmd_msg_handle *msg_handle)
+	       struct tracecmd_msg_handle *msg_handle,
+	       unsigned long file_version)
 {
 	struct tracecmd_output *handle;
 	struct tep_handle *pevent;
@@ -945,7 +948,7 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
 
 	handle->msg_handle = msg_handle;
 
-	if (select_file_version(handle, ihandle))
+	if (select_file_version(handle, ihandle, file_version))
 		goto out_free;
 
 	list_head_init(&handle->options);
@@ -1023,7 +1026,8 @@ static struct tracecmd_output *create_file(const char *output_file,
 					   struct tracecmd_input *ihandle,
 					   const char *tracing_dir,
 					   const char *kallsyms,
-					   struct tracecmd_event_list *list)
+					   struct tracecmd_event_list *list,
+					   unsigned long file_version)
 {
 	struct tracecmd_output *handle;
 	int fd;
@@ -1032,7 +1036,7 @@ static struct tracecmd_output *create_file(const char *output_file,
 	if (fd < 0)
 		return NULL;
 
-	handle = create_file_fd(fd, ihandle, tracing_dir, kallsyms, list, NULL);
+	handle = create_file_fd(fd, ihandle, tracing_dir, kallsyms, list, NULL, file_version);
 	if (!handle) {
 		close(fd);
 		unlink(output_file);
@@ -1332,13 +1336,15 @@ int tracecmd_write_cmdlines(struct tracecmd_output *handle)
 	return 0;
 }
 
-struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus)
+struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus,
+						     unsigned long file_version)
 {
 	struct tracecmd_output *handle;
 	char *path;
 	int ret;
 
-	handle = create_file(output_file, NULL, NULL, NULL, &all_event_list);
+	handle = create_file(output_file, NULL, NULL, NULL,
+			     &all_event_list, file_version);
 	if (!handle)
 		return NULL;
 
@@ -1633,39 +1639,46 @@ struct tracecmd_output *tracecmd_get_output_handle_fd(int fd)
 
 struct tracecmd_output *tracecmd_create_init_fd(int fd)
 {
-	return create_file_fd(fd, NULL, NULL, NULL, &all_event_list, NULL);
+	return create_file_fd(fd, NULL, NULL, NULL, &all_event_list, NULL, 0);
 }
 
 struct tracecmd_output *
 tracecmd_create_init_fd_msg(struct tracecmd_msg_handle *msg_handle,
-			    struct tracecmd_event_list *list)
+			    struct tracecmd_event_list *list,
+			    unsigned long file_version)
 {
-	return create_file_fd(msg_handle->fd, NULL, NULL, NULL, list, msg_handle);
+	return create_file_fd(msg_handle->fd, NULL, NULL, NULL,
+			      list, msg_handle, file_version);
 }
 
 struct tracecmd_output *
-tracecmd_create_init_fd_glob(int fd, struct tracecmd_event_list *list)
+tracecmd_create_init_fd_glob(int fd, struct tracecmd_event_list *list,
+			     unsigned long file_version)
 {
-	return create_file_fd(fd, NULL, NULL, NULL, list, NULL);
+	return create_file_fd(fd, NULL, NULL, NULL, list, NULL, file_version);
 }
 
 struct tracecmd_output *
-tracecmd_create_init_file_glob(const char *output_file,
-			       struct tracecmd_event_list *list)
+tracecmd_create_init_file_glob(const char *output_file, struct tracecmd_event_list *list,
+			       unsigned long file_version)
 {
-	return create_file(output_file, NULL, NULL, NULL, list);
+	return create_file(output_file, NULL, NULL, NULL, list, file_version);
 }
 
-struct tracecmd_output *tracecmd_create_init_file(const char *output_file)
+struct tracecmd_output *tracecmd_create_init_file(const char *output_file,
+						  unsigned long file_version)
 {
-	return create_file(output_file, NULL, NULL, NULL, &all_event_list);
+	return create_file(output_file, NULL, NULL, NULL,
+			   &all_event_list, file_version);
 }
 
 struct tracecmd_output *tracecmd_create_init_file_override(const char *output_file,
 							   const char *tracing_dir,
-							   const char *kallsyms)
+							   const char *kallsyms,
+							   unsigned long file_version)
 {
-	return create_file(output_file, NULL, tracing_dir, kallsyms, &all_event_list);
+	return create_file(output_file, NULL, tracing_dir, kallsyms,
+			   &all_event_list, file_version);
 }
 
 /**
@@ -1682,7 +1695,8 @@ struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
 {
 	struct tracecmd_output *handle;
 
-	handle = create_file(file, ihandle, NULL, NULL, &all_event_list);
+	handle = create_file(file, ihandle, NULL, NULL, &all_event_list,
+			     tracecmd_get_file_version(ihandle));
 	if (!handle)
 		return NULL;
 
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index fd03a605..6775338b 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3645,7 +3645,7 @@ setup_connection(struct buffer_instance *instance, struct common_record_context
 
 	/* Now create the handle through this socket */
 	if (msg_handle->version == V3_PROTOCOL) {
-		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events);
+		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events, 0);
 		if (!network_handle)
 			goto error;
 		tracecmd_set_quiet(network_handle, quiet);
@@ -3663,8 +3663,7 @@ setup_connection(struct buffer_instance *instance, struct common_record_context
 		if (ret)
 			goto error;
 	} else {
-		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd,
-							      listed_events);
+		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd, listed_events, 0);
 		if (!network_handle)
 			goto error;
 		tracecmd_set_quiet(network_handle, quiet);
@@ -3851,8 +3850,7 @@ static void setup_agent(struct buffer_instance *instance,
 {
 	struct tracecmd_output *network_handle;
 
-	network_handle = tracecmd_create_init_fd_msg(instance->msg_handle,
-						     listed_events);
+	network_handle = tracecmd_create_init_fd_msg(instance->msg_handle, listed_events, 0);
 	add_options(network_handle, ctx);
 	tracecmd_write_cmdlines(network_handle);
 	tracecmd_write_cpus(network_handle, instance->cpu_count);
@@ -4244,7 +4242,7 @@ static void record_data(struct common_record_context *ctx)
 		return;
 
 	if (latency) {
-		handle = tracecmd_create_file_latency(ctx->output, local_cpu_count);
+		handle = tracecmd_create_file_latency(ctx->output, local_cpu_count, 0);
 		tracecmd_set_quiet(handle, quiet);
 	} else {
 		if (!local_cpu_count)
@@ -4275,7 +4273,7 @@ static void record_data(struct common_record_context *ctx)
 				touch_file(temp_files[i]);
 		}
 
-		handle = tracecmd_create_init_file_glob(ctx->output, listed_events);
+		handle = tracecmd_create_init_file_glob(ctx->output, listed_events, 0);
 		if (!handle)
 			die("Error creating output file");
 		tracecmd_set_quiet(handle, quiet);
diff --git a/tracecmd/trace-restore.c b/tracecmd/trace-restore.c
index 280a37f0..f2b15434 100644
--- a/tracecmd/trace-restore.c
+++ b/tracecmd/trace-restore.c
@@ -91,7 +91,7 @@ void trace_restore (int argc, char **argv)
 		}
 
 		handle = tracecmd_create_init_file_override(output, tracing_dir,
-							    kallsyms);
+							    kallsyms, 0);
 		if (!handle)
 			die("Unabled to create output file %s", output);
 		if (tracecmd_write_cmdlines(handle) < 0)
@@ -128,7 +128,7 @@ void trace_restore (int argc, char **argv)
 		handle = tracecmd_copy(ihandle, output);
 		tracecmd_close(ihandle);
 	} else
-		handle = tracecmd_create_init_file(output);
+		handle = tracecmd_create_init_file(output, 0);
 
 	if (!handle)
 		die("error writing to %s", output);
-- 
2.30.2

Re: [PATCH v2 6/7] trace-cmd library: Extend the create file APIs to support different file version

[Steven Rostedt]

On Thu, 29 Apr 2021 07:01:18 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> --- a/lib/trace-cmd/include/private/trace-cmd-private.h
> +++ b/lib/trace-cmd/include/private/trace-cmd-private.h
> @@ -266,20 +266,25 @@ struct tracecmd_event_list {
>  struct tracecmd_option;
>  struct tracecmd_msg_handle;
>  
> -struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
> +struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus,
> +						     unsigned long file_version);
>  struct tracecmd_output *
> -tracecmd_create_init_file_glob(const char *output_file,
> -			       struct tracecmd_event_list *list);
> +tracecmd_create_init_file_glob(const char *output_file, struct tracecmd_event_list *list,
> +			       unsigned long file_version);
>  struct tracecmd_output *tracecmd_create_init_fd(int fd);
>  struct tracecmd_output *
> -tracecmd_create_init_fd_glob(int fd, struct tracecmd_event_list *list);
> +tracecmd_create_init_fd_glob(int fd, struct tracecmd_event_list *list,
> +			     unsigned long file_version);
>  struct tracecmd_output *
>  tracecmd_create_init_fd_msg(struct tracecmd_msg_handle *msg_handle,
> -			    struct tracecmd_event_list *list);
> -struct tracecmd_output *tracecmd_create_init_file(const char *output_file);
> +			    struct tracecmd_event_list *list,
> +			    unsigned long file_version);
> +struct tracecmd_output *tracecmd_create_init_file(const char *output_file,
> +						  unsigned long file_version);
>  struct tracecmd_output *tracecmd_create_init_file_override(const char *output_file,
>  							   const char *tracing_dir,
> -							   const char *kallsyms);
> +							   const char *kallsyms,
> +							   unsigned long file_version);
>  struct tracecmd_option *tracecmd_add_option(struct tracecmd_output *handle,
>  					    unsigned short id, int size,
>  					    const void *data);


I think this is too complex of an API. The callers should not care what
version the file is used. It should be automatic.

That is, the init function can save the location of where it writes the
version number as I described in the other email. When the user calls an
API that requires a higher version than is recorded, we update it too.

Now this may take some coordination if the versions change the state. But
that too can be detected, as we can build an automata diagram to calculate
what version the user is using to continue the states.

-- Steve

[PATCH v2 7/7] trace-cmd record: Add new parameter --file-version

[Tzvetomir Stoyanov (VMware)]

Added a new optional parameter to "trace-cmd record", can be used to
select the desired file version of the trace output file.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/trace-record.c | 23 ++++++++++++++++++-----
 tracecmd/trace-usage.c  |  1 +
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 6775338b..f95db0e4 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -199,6 +199,7 @@ struct common_record_context {
 	char *date2ts;
 	char *user;
 	const char *clock;
+	unsigned long file_version;
 	struct tsc_nsec tsc2nsec;
 	int data_flags;
 	int tsync_loop_interval;
@@ -3645,7 +3646,8 @@ setup_connection(struct buffer_instance *instance, struct common_record_context
 
 	/* Now create the handle through this socket */
 	if (msg_handle->version == V3_PROTOCOL) {
-		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events, 0);
+		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events,
+							     ctx->file_version);
 		if (!network_handle)
 			goto error;
 		tracecmd_set_quiet(network_handle, quiet);
@@ -3663,7 +3665,8 @@ setup_connection(struct buffer_instance *instance, struct common_record_context
 		if (ret)
 			goto error;
 	} else {
-		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd, listed_events, 0);
+		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd, listed_events,
+							      ctx->file_version);
 		if (!network_handle)
 			goto error;
 		tracecmd_set_quiet(network_handle, quiet);
@@ -3850,7 +3853,8 @@ static void setup_agent(struct buffer_instance *instance,
 {
 	struct tracecmd_output *network_handle;
 
-	network_handle = tracecmd_create_init_fd_msg(instance->msg_handle, listed_events, 0);
+	network_handle = tracecmd_create_init_fd_msg(instance->msg_handle, listed_events,
+						     ctx->file_version);
 	add_options(network_handle, ctx);
 	tracecmd_write_cmdlines(network_handle);
 	tracecmd_write_cpus(network_handle, instance->cpu_count);
@@ -4242,7 +4246,8 @@ static void record_data(struct common_record_context *ctx)
 		return;
 
 	if (latency) {
-		handle = tracecmd_create_file_latency(ctx->output, local_cpu_count, 0);
+		handle = tracecmd_create_file_latency(ctx->output, local_cpu_count,
+						      ctx->file_version);
 		tracecmd_set_quiet(handle, quiet);
 	} else {
 		if (!local_cpu_count)
@@ -4273,7 +4278,8 @@ static void record_data(struct common_record_context *ctx)
 				touch_file(temp_files[i]);
 		}
 
-		handle = tracecmd_create_init_file_glob(ctx->output, listed_events, 0);
+		handle = tracecmd_create_init_file_glob(ctx->output, listed_events,
+							ctx->file_version);
 		if (!handle)
 			die("Error creating output file");
 		tracecmd_set_quiet(handle, quiet);
@@ -5499,6 +5505,7 @@ void init_top_instance(void)
 }
 
 enum {
+	OPT_file_version	= 239,
 	OPT_tsc2nsec		= 240,
 	OPT_fork		= 241,
 	OPT_tsyncinterval	= 242,
@@ -5933,6 +5940,7 @@ static void parse_record_options(int argc,
 			{"tsync-interval", required_argument, NULL, OPT_tsyncinterval},
 			{"fork", no_argument, NULL, OPT_fork},
 			{"tsc2nsec", no_argument, NULL, OPT_tsc2nsec},
+			{"file-version", required_argument, NULL, OPT_file_version},
 			{NULL, 0, NULL, 0}
 		};
 
@@ -6354,6 +6362,11 @@ static void parse_record_options(int argc,
 				die("TSC to nanosecond is not supported");
 			ctx->instance->flags |= BUFFER_FL_TSC2NSEC;
 			break;
+		case OPT_file_version:
+			ctx->file_version = atoi(optarg);
+			if (!tracecmd_is_version_supported(ctx->file_version))
+				die("File version %d is not supported", ctx->file_version);
+			break;
 		case OPT_quiet:
 		case 'q':
 			quiet = true;
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 98247074..e5b54114 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -68,6 +68,7 @@ static struct usage_help usage_help[] = {
 		"               If a negative number is specified, timestamps synchronization is disabled"
 		"               If 0 is specified, no loop is performed - timestamps offset is calculated only twice,"
 		"                                                         at the beginnig and at the end of the trace\n"
+		"          --file-version select the desired version of the trace output file\n"
 	},
 	{
 		"set",
-- 
2.30.2

Re: [PATCH v2 7/7] trace-cmd record: Add new parameter --file-version

[Steven Rostedt]

On Thu, 29 Apr 2021 07:01:19 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Added a new optional parameter to "trace-cmd record", can be used to
> select the desired file version of the trace output file.

I'm not sure record should be where a version is specified. Let the user
decide if they can disable a feature that requires a higher version of the
file. That is, if the user does "--no-compression" or whatever we call it,
then it will just save as version 6.

Now what we should do, is to have a conversion command.

 trace-cmd convert <version> <input.dat> <output.dat>

 $ trace-cmd convert 6 trace-7.dat trace-6.dat


We'll just have to document (probably in the convert man page) what
features are added to which version, and how to set up the record to have
it go to a lower version).

-- Steve

Re: [PATCH v2 0/7] Bump trace file version

[Tzvetomir Stoyanov]

On Thu, Apr 29, 2021 at 7:01 AM Tzvetomir Stoyanov (VMware)
<tz.stoyanov@gmail.com> wrote:
>
> Changed the trace file version to 7 and added support for choosing
> the desired file version.
>
> v2 changes:
>  - Include "Check if file version is supported" patch in the set, as other
>    patches depend on it.
>  - Reorganized the order of the patches, put the clean up patch "Remove
>    unused private APIs for creating trace files" first.
>
> Tzvetomir Stoyanov (VMware) (7):
>   trace-cmd library: Remove unused private APIs for creating trace files
>   trace-cmd: Check if file version is supported
>   trace-cmd library: Bump the trace file version to 7
>   trace-cmd library: Add new API to get file version of input handler
>   trace-cmd library: Select the file version when writing trace file
>   trace-cmd library: Extend the create file APIs to support different
>     file version
>   trace-cmd record: Add new parameter --file-version
>
>  .../include/private/trace-cmd-private.h       | 29 +++---
>  lib/trace-cmd/include/trace-cmd-local.h       |  2 +-
>  lib/trace-cmd/trace-input.c                   | 19 ++++
>  lib/trace-cmd/trace-output.c                  | 98 ++++++++++---------
>  lib/trace-cmd/trace-util.c                    |  7 ++
>  tracecmd/trace-dump.c                         |  7 ++
>  tracecmd/trace-record.c                       | 25 +++--
>  tracecmd/trace-restore.c                      |  4 +-
>  tracecmd/trace-usage.c                        |  1 +
>  9 files changed, 121 insertions(+), 71 deletions(-)
>
> --
> 2.30.2
>

This patchset is superseded by "[PoC PATCH v2 00/21] Add trace file compression"
https://lore.kernel.org/linux-trace-devel/20210510061636.1794357-1-tz.stoyanov@gmail.com/


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