diff mbox series

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

Message ID 20210429040119.843617-7-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series Bump trace file version | expand

Commit Message

Tzvetomir Stoyanov (VMware) April 29, 2021, 4:01 a.m. UTC 
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(-)

Comments

Steven Rostedt April 29, 2021, 1:50 p.m. UTC | #1 
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
diff mbox series

Patch

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);