[PATCH 0/5] Cleanups of tracecmd_output_ APIs

[PATCH 0/5] Cleanups of tracecmd_output_ APIs

[Tzvetomir Stoyanov (VMware)]

Reduced the APIs for creating output handle to a trace file. Capsulated the
logic for creating the file inside the library. 

Tzvetomir Stoyanov (VMware) (5):
  trace-cmd library: Fix typo in tracecmd_output_set_msg()
  trace-cmd library: New API for creating an output trace handle
  trace-cmd library: Rename tracecmd_output_allocate()
  trace-cmd library: Remove APIs for create and init output handle
  trace-cmd library: Make tracecmd_output_write_init static

 .../include/private/trace-cmd-private.h       |  6 +-
 lib/trace-cmd/trace-output.c                  | 86 ++++++++-----------
 tracecmd/trace-record.c                       | 14 ++-
 tracecmd/trace-restore.c                      | 16 ++--
 tracecmd/trace-stream.c                       |  3 +-
 5 files changed, 50 insertions(+), 75 deletions(-)

-- 
2.33.1

[PATCH 1/5] trace-cmd library: Fix typo in tracecmd_output_set_msg()

[Tzvetomir Stoyanov (VMware)]

Fixed typo in the description of tracecmd_output_set_msg() function.

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

diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 3eaca3e4..31f08751 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -941,7 +941,7 @@ struct tracecmd_output *tracecmd_output_allocate(int fd)
  * All subsequent calls to @handle will send data over the network using @msg_handle
  * instead of writing to a file.
  *
- * This mut be called after the handle file version is set and before calling
+ * This must be called after the handle file version is set and before calling
  * tracecmd_output_write_init().
  *
  * Returns 0 on success, or -1 if the output file handle is not allocated or not
-- 
2.33.1

[PATCH 2/5] trace-cmd library: New API for creating an output trace handle

[Tzvetomir Stoyanov (VMware)]

There is lack of API to create an output trace handle to file with
given name, which is common use case. Existing flow is -  the library
user should create the file, open it and pass a file descriptor to the
library for creating the output handle. The proposed new API
	tracecmd_output_create()
has file name as input parameter. It creates a file with given name (or
truncates existing one) and create an output trace handle to that file.
The file name parameter is optional, if NULL is passed - the created
output trace handle will not be associated with a file.

Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 .../include/private/trace-cmd-private.h       |  1 +
 lib/trace-cmd/trace-output.c                  | 46 +++++++++++++------
 tracecmd/trace-record.c                       | 12 ++---
 tracecmd/trace-restore.c                      | 16 +++----
 4 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index c7ade7ae..80027c9e 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -279,6 +279,7 @@ int tracecmd_output_write_init(struct tracecmd_output *handle);
 int tracecmd_output_write_headers(struct tracecmd_output *handle,
 				  struct tracecmd_event_list *list);
 
+struct tracecmd_output *tracecmd_output_create(const char *output_file);
 struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
 struct tracecmd_output *tracecmd_create_init_fd(int fd);
 
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 31f08751..2f8ebf25 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1442,17 +1442,11 @@ struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, in
 {
 	struct tracecmd_output *handle;
 	char *path;
-	int fd;
 
-	fd = open(output_file, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
-	if (fd < 0)
+	handle = tracecmd_output_create(output_file);
+	if (!handle)
 		return NULL;
 
-	handle = tracecmd_output_allocate(fd);
-	if (!handle)
-		goto out_free;
-	if (tracecmd_output_write_init(handle))
-		goto out_free;
 	if (tracecmd_output_write_headers(handle, NULL))
 		goto out_free;
 	/*
@@ -1806,6 +1800,34 @@ struct tracecmd_output *tracecmd_get_output_handle_fd(int fd)
 	return NULL;
 }
 
+/**
+ * tracecmd_output_create - Create new output handle to a trace file with given name
+ * @output_file: Name of the trace file that will be created.
+ *
+ * The @output_file parameter can be NULL. In this case the output handle is created
+ * and initialized, but is not associated with a file.
+ *
+ * Returns pointer to created outpuy handle, or NULL in case of an error.
+ */
+struct tracecmd_output *tracecmd_output_create(const char *output_file)
+{
+	struct tracecmd_output *out;
+	int fd = -1;
+
+	if (output_file) {
+		fd = open(output_file, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
+		if (fd < 0)
+			return NULL;
+	}
+	out = tracecmd_output_allocate(fd);
+	if (!out && fd >= 0) {
+		close(fd);
+		unlink(output_file);
+	}
+
+	return out;
+}
+
 struct tracecmd_output *tracecmd_create_init_fd(int fd)
 {
 	struct tracecmd_output *out;
@@ -1855,15 +1877,11 @@ struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
 				      const char *file)
 {
 	struct tracecmd_output *handle;
-	int fd;
 
-	fd = open(file, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
-	if (fd < 0)
+	handle = tracecmd_output_create(file);
+	if (!handle)
 		return NULL;
 
-	handle = tracecmd_output_allocate(fd);
-	if (!handle)
-		goto out_free;
 	if (tracecmd_output_set_from_input(handle, ihandle))
 		goto out_free;
 	tracecmd_output_write_init(handle);
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 3a84e116..677a4bd9 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3694,7 +3694,7 @@ static struct tracecmd_output *create_net_output(struct common_record_context *c
 {
 	struct tracecmd_output *out;
 
-	out = tracecmd_output_allocate(-1);
+	out = tracecmd_output_create(NULL);
 	if (!out)
 		return NULL;
 	if (tracecmd_output_set_msg(out, msg_handle))
@@ -4458,23 +4458,21 @@ static void write_guest_file(struct buffer_instance *instance)
 static struct tracecmd_output *create_output(struct common_record_context *ctx)
 {
 	struct tracecmd_output *out;
-	int fd;
 
-	fd = open(ctx->output, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
-	if (fd < 0)
+	if (!ctx->output)
 		return NULL;
 
-	out = tracecmd_output_allocate(fd);
+	out = tracecmd_output_create(ctx->output);
 	if (!out)
 		goto error;
+
 	if (tracecmd_output_write_headers(out, listed_events))
 		goto error;
+
 	return out;
 error:
 	if (out)
 		tracecmd_output_close(out);
-	else
-		close(fd);
 	unlink(ctx->output);
 	return NULL;
 }
diff --git a/tracecmd/trace-restore.c b/tracecmd/trace-restore.c
index 8d2fcae8..23a7f4af 100644
--- a/tracecmd/trace-restore.c
+++ b/tracecmd/trace-restore.c
@@ -26,15 +26,11 @@ static struct tracecmd_output *create_output(const char *file,
 					     const char *tracing_dir, const char *kallsyms)
 {
 	struct tracecmd_output *out;
-	int fd;
 
-	fd = open(file, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
-	if (fd < 0)
-		return NULL;
-
-	out = tracecmd_output_allocate(fd);
+	out = tracecmd_output_create(file);
 	if (!out)
 		goto error;
+
 	if (tracing_dir && tracecmd_output_set_trace_dir(out, tracing_dir))
 		goto error;
 	if (kallsyms && tracecmd_output_set_kallsyms(out, kallsyms))
@@ -45,8 +41,6 @@ static struct tracecmd_output *create_output(const char *file,
 error:
 	if (out)
 		tracecmd_output_close(out);
-	else
-		close(fd);
 	unlink(file);
 	return NULL;
 }
@@ -155,8 +149,10 @@ void trace_restore (int argc, char **argv)
 
 		handle = tracecmd_copy(ihandle, output);
 		tracecmd_close(ihandle);
-	} else
-		handle = tracecmd_create_init_file(output);
+	} else {
+		handle = tracecmd_output_create(output);
+		tracecmd_output_write_headers(handle, NULL);
+	}
 
 	if (!handle)
 		die("error writing to %s", output);
-- 
2.33.1

[PATCH 3/5] trace-cmd library: Rename tracecmd_output_allocate()

[Tzvetomir Stoyanov (VMware)]

The existing tracecmd_output_allocate() API is renamed to
tracecmd_output_create_fd(), to be consistent with
tracecmd_output_create() API.

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

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 80027c9e..79196147 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -269,7 +269,6 @@ struct tracecmd_event_list {
 struct tracecmd_option;
 struct tracecmd_msg_handle;
 
-struct tracecmd_output *tracecmd_output_allocate(int fd);
 int tracecmd_output_set_msg(struct tracecmd_output *handle,
 			    struct tracecmd_msg_handle *msg_handle);
 int tracecmd_output_set_trace_dir(struct tracecmd_output *handle, const char *tracing_dir);
@@ -280,6 +279,7 @@ int tracecmd_output_write_headers(struct tracecmd_output *handle,
 				  struct tracecmd_event_list *list);
 
 struct tracecmd_output *tracecmd_output_create(const char *output_file);
+struct tracecmd_output *tracecmd_output_create_fd(int fd);
 struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
 struct tracecmd_output *tracecmd_create_init_fd(int fd);
 
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 2f8ebf25..067deae8 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -898,7 +898,7 @@ out_free:
 }
 
 /**
- * tracecmd_output_allocate - allocate new output handle to a trace file
+ * tracecmd_output_create_fd - allocate new output handle to a trace file
  * @fd: File descriptor for the handle to write to.
  *
  * Allocate a tracecmd_output descriptor and perform minimal initialization.
@@ -909,7 +909,7 @@ out_free:
  * a tracecmd data file. In case of an error, NULL is returned. The returned
  * handle must be freed with tracecmd_output_close() or tracecmd_output_free()
  */
-struct tracecmd_output *tracecmd_output_allocate(int fd)
+struct tracecmd_output *tracecmd_output_create_fd(int fd)
 {
 	struct tracecmd_output *handle;
 
@@ -1819,7 +1819,7 @@ struct tracecmd_output *tracecmd_output_create(const char *output_file)
 		if (fd < 0)
 			return NULL;
 	}
-	out = tracecmd_output_allocate(fd);
+	out = tracecmd_output_create_fd(fd);
 	if (!out && fd >= 0) {
 		close(fd);
 		unlink(output_file);
@@ -1832,7 +1832,7 @@ struct tracecmd_output *tracecmd_create_init_fd(int fd)
 {
 	struct tracecmd_output *out;
 
-	out = tracecmd_output_allocate(fd);
+	out = tracecmd_output_create_fd(fd);
 	if (!out)
 		return NULL;
 	if (tracecmd_output_write_init(out))
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 677a4bd9..b4200db1 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3737,7 +3737,7 @@ setup_connection(struct buffer_instance *instance, struct common_record_context
 		if (ret)
 			goto error;
 	} else {
-		network_handle = tracecmd_output_allocate(msg_handle->fd);
+		network_handle = tracecmd_output_create_fd(msg_handle->fd);
 		if (!network_handle)
 			goto error;
 		if (tracecmd_output_write_headers(network_handle, listed_events))
-- 
2.33.1

[PATCH 4/5] trace-cmd library: Remove APIs for create and init output handle

[Tzvetomir Stoyanov (VMware)]

These APIs are redundant, their functionality can be replaced by
existing library APIs. Removed them, to simplify the API set:
  tracecmd_create_init_fd()
  tracecmd_create_init_file()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 .../include/private/trace-cmd-private.h       |  2 --
 lib/trace-cmd/trace-output.c                  | 36 -------------------
 tracecmd/trace-stream.c                       |  3 +-
 3 files changed, 2 insertions(+), 39 deletions(-)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 79196147..069283c7 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -281,9 +281,7 @@ int tracecmd_output_write_headers(struct tracecmd_output *handle,
 struct tracecmd_output *tracecmd_output_create(const char *output_file);
 struct tracecmd_output *tracecmd_output_create_fd(int fd);
 struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
-struct tracecmd_output *tracecmd_create_init_fd(int fd);
 
-struct tracecmd_output *tracecmd_create_init_file(const char *output_file);
 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 067deae8..a20e42df 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1828,42 +1828,6 @@ struct tracecmd_output *tracecmd_output_create(const char *output_file)
 	return out;
 }
 
-struct tracecmd_output *tracecmd_create_init_fd(int fd)
-{
-	struct tracecmd_output *out;
-
-	out = tracecmd_output_create_fd(fd);
-	if (!out)
-		return NULL;
-	if (tracecmd_output_write_init(out))
-		goto error;
-	if (tracecmd_output_write_headers(out, NULL))
-		goto error;
-
-	return out;
-error:
-	tracecmd_output_close(out);
-	return NULL;
-}
-
-struct tracecmd_output *tracecmd_create_init_file(const char *output_file)
-{
-	struct tracecmd_output *handle;
-	int fd;
-
-	fd = open(output_file, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
-	if (fd < 0)
-		return NULL;
-	handle = tracecmd_create_init_fd(fd);
-	if (!handle) {
-		close(fd);
-		unlink(output_file);
-		return NULL;
-	}
-
-	return handle;
-}
-
 /**
  * tracecmd_copy - copy the headers of one trace.dat file for another
  * @ihandle: input handle of the trace.dat file to copy
diff --git a/tracecmd/trace-stream.c b/tracecmd/trace-stream.c
index b47b208c..ee310f3d 100644
--- a/tracecmd/trace-stream.c
+++ b/tracecmd/trace-stream.c
@@ -43,11 +43,12 @@ trace_stream_init(struct buffer_instance *instance, int cpu, int fd, int cpus,
 		tfd = fileno(fp);
 
 		ofd = dup(tfd);
-		trace_output = tracecmd_create_init_fd(ofd);
+		trace_output = tracecmd_output_create_fd(ofd);
 		if (!trace_output) {
 			fclose(fp);
 			return NULL;
 		}
+		tracecmd_output_write_headers(trace_output, NULL);
 		tracecmd_output_free(trace_output);
 	}
 
-- 
2.33.1

[PATCH 5/5] trace-cmd library: Make tracecmd_output_write_init static

[Tzvetomir Stoyanov (VMware)]

That function is used only inside the library, it should not be exposed
as library API:
 tracecmd_output_write_init()

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

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 069283c7..a26076de 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -274,7 +274,6 @@ int tracecmd_output_set_msg(struct tracecmd_output *handle,
 int tracecmd_output_set_trace_dir(struct tracecmd_output *handle, const char *tracing_dir);
 int tracecmd_output_set_kallsyms(struct tracecmd_output *handle, const char *kallsyms);
 int tracecmd_output_set_from_input(struct tracecmd_output *handle, struct tracecmd_input *ihandle);
-int tracecmd_output_write_init(struct tracecmd_output *handle);
 int tracecmd_output_write_headers(struct tracecmd_output *handle,
 				  struct tracecmd_event_list *list);
 
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index a20e42df..f5e2574c 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -942,7 +942,7 @@ struct tracecmd_output *tracecmd_output_create_fd(int fd)
  * instead of writing to a file.
  *
  * This must be called after the handle file version is set and before calling
- * tracecmd_output_write_init().
+ * tracecmd_output_write_headers().
  *
  * Returns 0 on success, or -1 if the output file handle is not allocated or not
  * in the expected state.
@@ -966,7 +966,7 @@ int tracecmd_output_set_msg(struct tracecmd_output *handle, struct tracecmd_msg_
  * (@tracing_dir), to be used when creating the trace file instead of using the
  * system default tracig directory.
  *
- * Must be called before tracecmd_output_write_init().
+ * Must be called before tracecmd_output_write_headers().
  *
  * Returns 0 on success, or -1 if the output file handle is not allocated or not
  * in the expected state.
@@ -995,7 +995,7 @@ int tracecmd_output_set_trace_dir(struct tracecmd_output *handle, const char *tr
  * Have the output file handle (@handle) use a custom kernel symbols file instead
  * of the default /proc/kallsyms.
  *
- * Must be called before tracecmd_output_write_init().
+ * Must be called before tracecmd_output_write_headers().
  *
  * Returns 0 on success, or -1 if the output file handle is not allocated or
  * not in the expected state.
@@ -1031,7 +1031,7 @@ int tracecmd_output_set_kallsyms(struct tracecmd_output *handle, const char *kal
  *  - file version
  *  - file compression protocol
  *
- * Must be called before tracecmd_output_write_init().
+ * Must be called before tracecmd_output_write_headers().
  *
  * Returns 0 on success, or -1 if the output file handle is not allocated or
  * not in expected state.
@@ -1053,7 +1053,7 @@ int tracecmd_output_set_from_input(struct tracecmd_output *handle, struct tracec
 }
 
 /**
- * tracecmd_output_write_init - Write the initial data into the trace file
+ * output_write_init - Write the initial data into the trace file
  * @handle: output handle to a trace file.
  *
  * Must be called after tracecmd_output_set_*() functions and before writing
@@ -1070,7 +1070,7 @@ int tracecmd_output_set_from_input(struct tracecmd_output *handle, struct tracec
  * Returns 0 on success, or -1 if the output file handle is not allocated or
  * not in the expected state.
  */
-int tracecmd_output_write_init(struct tracecmd_output *handle)
+static int output_write_init(struct tracecmd_output *handle)
 {
 	char buf[BUFSIZ];
 	int endian4;
@@ -1131,7 +1131,7 @@ int tracecmd_output_write_headers(struct tracecmd_output *handle,
 		return -1;
 
 	/* Write init data, if not written yet */
-	if (handle->file_state < TRACECMD_FILE_INIT && tracecmd_output_write_init(handle))
+	if (handle->file_state < TRACECMD_FILE_INIT && output_write_init(handle))
 		return -1;
 	if (read_header_files(handle))
 		return -1;
@@ -1848,7 +1848,7 @@ struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
 
 	if (tracecmd_output_set_from_input(handle, ihandle))
 		goto out_free;
-	tracecmd_output_write_init(handle);
+	output_write_init(handle);
 
 	if (tracecmd_copy_headers(ihandle, handle->fd, 0, 0) < 0)
 		goto out_free;
-- 
2.33.1