[PATCH v2 00/10] Refactor APIs for creating output handler

[PATCH v2 00/10] Refactor APIs for creating output handler

[Tzvetomir Stoyanov (VMware)]

In the trace-cmd library there are various APIs for allocating and
initializing output handler to a trace file. The existing APIs are use
case oriented, with a lot of parameters. Extending them for new use
cases, adding more input parameters, will make the library more complex
and not easy to use.
Almost all use case oriented APIs for output handler creation are
removed and replaced with new flow, which is easier to be extended with
new parameters.

Removed APIs:
 tracecmd_create_init_fd_msg()
 tracecmd_create_init_file_glob()
 tracecmd_create_init_fd_glob()
 tracecmd_create_init_file_override()

New APIs:
 tracecmd_output_allocate()
 tracecmd_output_set_msg()
 tracecmd_output_set_trace_dir()
 tracecmd_output_set_kallsyms()
 tracecmd_output_set_from_input()
 tracecmd_output_write_init()
 tracecmd_output_write_headers()

The new tracecmd_output_allocate() API allocates memory and performs
minimal initialization of an output handler to a trace file. No data
is written in the file.
The tracecmd_output_set_...() APIs can be used to set various
parameters to the newly allocated output handler, that affect the way
the data is written into the file.
When the output handler is configured for the desired use case, the
tracecmd_output_write_init() is used to start writing to the file, it
writes initial magic bytes.
The tracecmd_output_write_headers() API is used to write the initial
headers into the file.

This patch-set depends on:
  "trace-cmd fixes and clean-ups" patch set.

v2 changes:
 - Rebased on top of the latest master.

Tzvetomir Stoyanov (VMware) (10):
  trace-cmd library: New API for allocating an output handler
  trace-cmd library: New API for setting a message context to an output
    handler
  trace-cmd library: New API for setting a custom trace directory to an
    output handler
  trace-cmd library: New API for setting a custom kallsyms to an output
    handler
  trace-cmd library: New API to inherit parameters from an existing
    trace file
  trace-cmd library: New API tracecmd_output_write_init
  trace-cmd library: New API to write headers of a trace file
  trace-cmd library: Use the new flow when creating output handler
  trace-cmd: Use the new flow when creating output handler
  trace-cmd library: Remove deprecated APIs for creating an output
    handler

 .../include/private/trace-cmd-private.h       |  25 +-
 lib/trace-cmd/trace-output.c                  | 397 +++++++++++-------
 tracecmd/trace-record.c                       |  55 ++-
 tracecmd/trace-restore.c                      |  32 +-
 4 files changed, 347 insertions(+), 162 deletions(-)

-- 
2.33.1

[PATCH v2 01/10] trace-cmd library: New API for allocating an output handler

[Tzvetomir Stoyanov (VMware)]

The API only allocates a handler and performs minimal initialization.
No data is written in the file.
  tracecmd_output_allocate()

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

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index f40babef..bed96657 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -102,7 +102,8 @@ static inline int tracecmd_host_bigendian(void)
 /* --- Opening and Reading the trace.dat file --- */
 
 enum tracecmd_file_states {
-	TRACECMD_FILE_INIT = 1,
+	TRACECMD_FILE_ALLOCATED = 0,
+	TRACECMD_FILE_INIT,
 	TRACECMD_FILE_HEADERS,
 	TRACECMD_FILE_FTRACE_EVENTS,
 	TRACECMD_FILE_ALL_EVENTS,
@@ -268,6 +269,7 @@ struct tracecmd_event_list {
 struct tracecmd_option;
 struct tracecmd_msg_handle;
 
+struct tracecmd_output *tracecmd_output_allocate(int fd);
 struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
 struct tracecmd_output *
 tracecmd_create_init_file_glob(const char *output_file,
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index fe1d1aaa..3878b963 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -59,6 +59,7 @@ struct tracecmd_output {
 	unsigned long		file_state;
 	unsigned long		file_version;
 	size_t			options_start;
+	bool			big_endian;
 
 	struct list_head	options;
 	struct tracecmd_msg_handle *msg_handle;
@@ -883,6 +884,41 @@ out_free:
 	return ret;
 }
 
+/**
+ * tracecmd_output_allocate - allocate new output handler to a trace file
+ * @handle: file descriptor to an empty file, it can be -1 if the handler
+ *	    will not write to a file
+ *
+ * This API only allocates a handler and performs minimal initialization.
+ * No data is written in the file.
+ *
+ * Returns pointer to a newly allocated file descriptor, that can be used
+ * with other library APIs. In case of an error, NULL is returned. The returned
+ * handler must be freed with tracecmd_output_close() or tracecmd_output_free()
+ */
+struct tracecmd_output *tracecmd_output_allocate(int fd)
+{
+	struct tracecmd_output *handle;
+
+	handle = calloc(1, sizeof(*handle));
+	if (!handle)
+		return NULL;
+
+	handle->fd = fd;
+
+	handle->file_version = FILE_VERSION;
+
+	handle->page_size = getpagesize();
+	handle->big_endian = tracecmd_host_bigendian();
+
+	list_head_init(&handle->options);
+
+	handle->file_state = TRACECMD_FILE_ALLOCATED;
+
+	return handle;
+}
+
+
 static int select_file_version(struct tracecmd_output *handle,
 				struct tracecmd_input *ihandle)
 {
-- 
2.33.1

Re: [PATCH v2 01/10] trace-cmd library: New API for allocating an output handler

[Steven Rostedt]

On Thu, 11 Nov 2021 17:03:12 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> +/**
> + * tracecmd_output_allocate - allocate new output handler to a trace file
> + * @handle: file descriptor to an empty file, it can be -1 if the handler
> + *	    will not write to a file

Nit, but technically, the parameter description should fit on one line.
If more needs to be said about it, it can be done in the description,
where every reference of the name has the "@" attached to it.

I only mentioned this because I noticed it in a few places already.

-- Steve

> + *
> + * This API only allocates a handler and performs minimal initialization.
> + * No data is written in the file.
> + *
> + * Returns pointer to a newly allocated file descriptor, that can be used
> + * with other library APIs. In case of an error, NULL is returned. The returned
> + * handler must be freed with tracecmd_output_close() or tracecmd_output_free()
> + */
> +struct tracecmd_output *tracecmd_output_allocate(int fd)
> +{
> +	struct tracecmd_output *handle;
> +
> +	handle = calloc(1, sizeof(*handle));
> +	if (!handle)
> +		return NULL;
> +
> +	handle->fd = fd;
> +
> +	handle->file_version = FILE_VERSION;
> +
> +	handle->page_size = getpagesize();
> +	handle->big_endian = tracecmd_host_bigendian();
> +
> +	list_head_init(&handle->options);
> +
> +	handle->file_state = TRACECMD_FILE_ALLOCATED;
> +
> +	return handle;
> +}
> +
> +
>  static int select_file_version(struct tracecmd_output *handle,
>  				struct tracecmd_input *ihandle)
>  {

[PATCH v2 02/10] trace-cmd library: New API for setting a message context to an output handler

[Tzvetomir Stoyanov (VMware)]

The new API associates previously created handler to message context
with given output handler. The message context is used by the output
handler to send data over a network, instead writing to a local file.
 tracecmd_output_allocate()

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

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index bed96657..49c8c138 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -270,6 +270,8 @@ struct tracecmd_option;
 struct tracecmd_msg_handle;
 
 struct tracecmd_output *tracecmd_output_allocate(int fd);
+int tracecmd_output_set_msg(struct tracecmd_output *handler,
+			    struct tracecmd_msg_handle *msg_handle);
 struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
 struct tracecmd_output *
 tracecmd_create_init_file_glob(const char *output_file,
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 3878b963..d73c4f99 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -918,6 +918,28 @@ struct tracecmd_output *tracecmd_output_allocate(int fd)
 	return handle;
 }
 
+/**
+ * tracecmd_output_set_msg - associated an output file handler with network message handler
+ * @handle: output handler to a trace file.
+ * @msg_handle: network handler, allocated by tracecmd_msg_handle_alloc()
+ *
+ * This API associates an output file handler with a network stream. All subsequent API calls
+ * with this output file handler will send data over the network using the @msg_handle, instead
+ * of writing to a file.
+ * This API must be called after the handler file version is set and before
+ * tracecmd_output_write_init().
+ *
+ * Returns 0 on success, or -1 if the output file handler is not allocated or not in expected state.
+ */
+int tracecmd_output_set_msg(struct tracecmd_output *handler, struct tracecmd_msg_handle *msg_handle)
+{
+	if (!handler || handler->file_state != TRACECMD_FILE_ALLOCATED)
+		return -1;
+
+	handler->msg_handle = msg_handle;
+
+	return 0;
+}
 
 static int select_file_version(struct tracecmd_output *handle,
 				struct tracecmd_input *ihandle)
-- 
2.33.1

Re: [PATCH v2 02/10] trace-cmd library: New API for setting a message context to an output handler

[Steven Rostedt]

On Thu, 11 Nov 2021 17:03:13 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> +/**
> + * tracecmd_output_set_msg - associated an output file handler with network message handler
> + * @handle: output handler to a trace file.
> + * @msg_handle: network handler, allocated by tracecmd_msg_handle_alloc()
> + *
> + * This API associates an output file handler with a network stream. All subsequent API calls

Nit. You don't need to say "API". We know it's an API ;-)

  "Associate the output file handle @handle with a network stream @msg_handle.
   All transactions of @handle after this will send data over the network ...


> + * with this output file handler will send data over the network using the @msg_handle, instead
> + * of writing to a file.
> + * This API must be called after the handler file version is set and before

   "This must be called after the handler ..."

No need to update for this. I'll leave it be. But more FYI.

We could always clean up the comments at a later time. This doesn't
affect the API ;-)

-- Steve

> + * tracecmd_output_write_init().
> + *
> + * Returns 0 on success, or -1 if the output file handler is not allocated or not in expected state.
> + */
> +int tracecmd_output_set_msg(struct tracecmd_output *handler, struct tracecmd_msg_handle *msg_handle)
> +{
> +	if (!handler || handler->file_state != TRACECMD_FILE_ALLOCATED)
> +		return -1;
> +
> +	handler->msg_handle = msg_handle;
> +
> +	return 0;
> +}
>  
>  static int select_file_version(struct tracecmd_output *handle,
>  				struct tracecmd_input *ihandle)

[PATCH v2 03/10] trace-cmd library: New API for setting a custom trace directory to an output handler

[Tzvetomir Stoyanov (VMware)]

The new API associates a custom tracing directory with an output file
handler. It is used when creating the trace file instead of the system
default tracing directory.
 tracecmd_output_set_trace_dir()

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

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 49c8c138..eb173a39 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -272,6 +272,7 @@ struct tracecmd_msg_handle;
 struct tracecmd_output *tracecmd_output_allocate(int fd);
 int tracecmd_output_set_msg(struct tracecmd_output *handler,
 			    struct tracecmd_msg_handle *msg_handle);
+int tracecmd_output_set_trace_dir(struct tracecmd_output *handler, const char *tracing_dir);
 struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus);
 struct tracecmd_output *
 tracecmd_create_init_file_glob(const char *output_file,
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index d73c4f99..a5d7ed5f 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -941,6 +941,33 @@ int tracecmd_output_set_msg(struct tracecmd_output *handler, struct tracecmd_msg
 	return 0;
 }
 
+/**
+ * tracecmd_output_set_trace_dir - Set a custom tracing dir, instead of system default
+ * @handle: output handler to a trace file.
+ * @tracing_dir: full path to a directory with tracing files
+ *
+ * This API associates an output file handler with a custom tracing directory, to be used when
+ * creating the trace file instead of the system default tracing directory.
+ * This API must be called before tracecmd_output_write_init().
+ *
+ * Returns 0 on success, or -1 if the output file handler is not allocated or not in expected state.
+ */
+int tracecmd_output_set_trace_dir(struct tracecmd_output *handler, const char *tracing_dir)
+{
+	if (!handler || handler->file_state != TRACECMD_FILE_ALLOCATED)
+		return -1;
+
+	free(handler->tracing_dir);
+	if (tracing_dir) {
+		handler->tracing_dir = strdup(tracing_dir);
+		if (!handler->tracing_dir)
+			return -1;
+	} else
+		handler->tracing_dir = NULL;
+
+	return 0;
+}
+
 static int select_file_version(struct tracecmd_output *handle,
 				struct tracecmd_input *ihandle)
 {
-- 
2.33.1

Re: [PATCH v2 00/10] Refactor APIs for creating output handler

[Steven Rostedt]

On Thu, 11 Nov 2021 17:03:11 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> This patch-set depends on:
>   "trace-cmd fixes and clean-ups" patch set.

FYI, please have a link to the patches that are dependent. Ideally a
mailing list archive link, or at a minimum, a link to patchwork.

But just a subject line is not easy to find, especially if it's been
archived.

-- Steve