linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] trace-cmd: Resend record --date fix
@ 2018-12-14 13:57 Slavomir Kaslev
  2018-12-14 13:57 ` [PATCH v4 1/3] trace-cmd: Prepare for protocol bump to version 3 Slavomir Kaslev
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Slavomir Kaslev @ 2018-12-14 13:57 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: rostedt, ykaradzhov, tstoyanov

Resending "trace-cmd record --date" fix while keeping backward compatibility.

Ideally those patches should land together since the last one assumes that all
v3 listeners follow new behavior and don't write options in the trace file.

Changes in v4:
  - split tracecmd_attach_cpu_data_fd into smaller functions
  - reorder the patches per Steven's request

Changes in v3:
  - added Signed-off-by

Changes in v2:
  - split into 3 patches for easier reviewing
  - fix tracecmd_msg_recv not reading cmd_size bytes it doesn't know about

Slavomir Kaslev (3):
  trace-cmd: Prepare for protocol bump to version 3
  trace-cmd: Fix record --date flag when sending tracing data to a
    listener
  trace-cmd: Bump protocol version to v3

 include/trace-cmd/trace-cmd.h |  16 ++-
 tracecmd/include/trace-msg.h  |   6 +-
 tracecmd/trace-listen.c       |  43 +++++---
 tracecmd/trace-msg.c          | 198 +++++++++++++++-------------------
 tracecmd/trace-output.c       |  78 +++++++-------
 tracecmd/trace-record.c       |  79 +++++++-------
 6 files changed, 210 insertions(+), 210 deletions(-)

-- 
2.19.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 1/3] trace-cmd: Prepare for protocol bump to version 3
  2018-12-14 13:57 [PATCH v4 0/3] trace-cmd: Resend record --date fix Slavomir Kaslev
@ 2018-12-14 13:57 ` Slavomir Kaslev
  2018-12-14 13:57 ` [PATCH v4 2/3] trace-cmd: Fix record --date flag when sending tracing data to a listener Slavomir Kaslev
  2018-12-14 13:57 ` [PATCH v4 3/3] trace-cmd: Bump protocol version to v3 Slavomir Kaslev
  2 siblings, 0 replies; 5+ messages in thread
From: Slavomir Kaslev @ 2018-12-14 13:57 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: rostedt, ykaradzhov, tstoyanov

This patch does the necessary renames to bump protocol to version 3.

No changes in behavior are introduced.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 include/trace-cmd/trace-cmd.h |  6 +++---
 tracecmd/include/trace-msg.h  |  6 +++---
 tracecmd/trace-listen.c       | 16 ++++++++--------
 tracecmd/trace-msg.c          |  8 ++++----
 tracecmd/trace-output.c       |  2 +-
 tracecmd/trace-record.c       | 24 ++++++++++++------------
 6 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 684ddb7..7cce592 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -316,16 +316,16 @@ void tracecmd_msg_handle_close(struct tracecmd_msg_handle *msg_handle);
 /* for clients */
 int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 				int **client_ports);
-int tracecmd_msg_metadata_send(struct tracecmd_msg_handle *msg_handle,
+int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle,
 			       const char *buf, int size);
-int tracecmd_msg_finish_sending_metadata(struct tracecmd_msg_handle *msg_handle);
+int tracecmd_msg_finish_sending_data(struct tracecmd_msg_handle *msg_handle);
 void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle);
 
 /* for server */
 int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle);
 int tracecmd_msg_send_port_array(struct tracecmd_msg_handle *msg_handle,
 				 int *ports);
-int tracecmd_msg_collect_metadata(struct tracecmd_msg_handle *msg_handle, int ofd);
+int tracecmd_msg_collect_data(struct tracecmd_msg_handle *msg_handle, int ofd);
 bool tracecmd_msg_done(struct tracecmd_msg_handle *msg_handle);
 void tracecmd_msg_set_done(struct tracecmd_msg_handle *msg_handle);
 
diff --git a/tracecmd/include/trace-msg.h b/tracecmd/include/trace-msg.h
index bfd065c..722548a 100644
--- a/tracecmd/include/trace-msg.h
+++ b/tracecmd/include/trace-msg.h
@@ -4,11 +4,11 @@
 #include <stdbool.h>
 
 #define UDP_MAX_PACKET	(65536 - 20)
-#define V2_MAGIC	"677768\0"
-#define V2_CPU		"-1V2"
+#define V3_MAGIC	"677768\0"
+#define V3_CPU		"-1V2"
 
 #define V1_PROTOCOL	1
-#define V2_PROTOCOL	2
+#define V3_PROTOCOL	2
 
 extern unsigned int page_size;
 
diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c
index 5727c0e..a13b83b 100644
--- a/tracecmd/trace-listen.c
+++ b/tracecmd/trace-listen.c
@@ -366,7 +366,7 @@ static int communicate_with_client(struct tracecmd_msg_handle *msg_handle)
 
 	/* Is the client using the new protocol? */
 	if (cpus == -1) {
-		if (memcmp(buf, V2_CPU, n) != 0) {
+		if (memcmp(buf, V3_CPU, n) != 0) {
 			/* If it did not send a version, then bail */
 			if (memcmp(buf, "-1V", 3)) {
 				plog("Unknown string %s\n", buf);
@@ -391,18 +391,18 @@ static int communicate_with_client(struct tracecmd_msg_handle *msg_handle)
 			goto try_again;
 		}
 
-		/* Let the client know we use v2 protocol */
+		/* Let the client know we use v3 protocol */
 		write(fd, "V2", 3);
 
 		/* read the rest of dummy data */
-		n = read(fd, buf, sizeof(V2_MAGIC));
-		if (memcmp(buf, V2_MAGIC, n) != 0)
+		n = read(fd, buf, sizeof(V3_MAGIC));
+		if (memcmp(buf, V3_MAGIC, n) != 0)
 			goto out;
 
 		/* We're off! */
 		write(fd, "OK", 2);
 
-		msg_handle->version = V2_PROTOCOL;
+		msg_handle->version = V3_PROTOCOL;
 
 		/* read the CPU count, the page size, and options */
 		if ((pagesize = tracecmd_msg_initial_setting(msg_handle)) < 0)
@@ -557,7 +557,7 @@ static int *create_all_readers(const char *node, const char *port,
 		start_port = udp_port + 1;
 	}
 
-	if (msg_handle->version == V2_PROTOCOL) {
+	if (msg_handle->version == V3_PROTOCOL) {
 		/* send set of port numbers to the client */
 		if (tracecmd_msg_send_port_array(msg_handle, port_array) < 0) {
 			plog("Failed sending port array\n");
@@ -674,8 +674,8 @@ static int process_client(struct tracecmd_msg_handle *msg_handle,
 	stop_msg_handle = msg_handle;
 
 	/* Now we are ready to start reading data from the client */
-	if (msg_handle->version == V2_PROTOCOL)
-		tracecmd_msg_collect_metadata(msg_handle, ofd);
+	if (msg_handle->version == V3_PROTOCOL)
+		tracecmd_msg_collect_data(msg_handle, ofd);
 	else
 		collect_metadata_from_client(msg_handle, ofd);
 
diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c
index 8e1226b..b496935 100644
--- a/tracecmd/trace-msg.c
+++ b/tracecmd/trace-msg.c
@@ -599,8 +599,8 @@ void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle)
 	tracecmd_msg_send(msg_handle->fd, &msg);
 }
 
-int tracecmd_msg_metadata_send(struct tracecmd_msg_handle *msg_handle,
-			       const char *buf, int size)
+int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle,
+			   const char *buf, int size)
 {
 	struct tracecmd_msg msg;
 	int fd = msg_handle->fd;
@@ -638,7 +638,7 @@ int tracecmd_msg_metadata_send(struct tracecmd_msg_handle *msg_handle,
 	return ret;
 }
 
-int tracecmd_msg_finish_sending_metadata(struct tracecmd_msg_handle *msg_handle)
+int tracecmd_msg_finish_sending_data(struct tracecmd_msg_handle *msg_handle)
 {
 	struct tracecmd_msg msg;
 	int ret;
@@ -650,7 +650,7 @@ int tracecmd_msg_finish_sending_metadata(struct tracecmd_msg_handle *msg_handle)
 	return 0;
 }
 
-int tracecmd_msg_collect_metadata(struct tracecmd_msg_handle *msg_handle, int ofd)
+int tracecmd_msg_collect_data(struct tracecmd_msg_handle *msg_handle, int ofd)
 {
 	struct tracecmd_msg msg;
 	u32 t, n, cmd;
diff --git a/tracecmd/trace-output.c b/tracecmd/trace-output.c
index 99493e6..78a8fe6 100644
--- a/tracecmd/trace-output.c
+++ b/tracecmd/trace-output.c
@@ -74,7 +74,7 @@ static stsize_t
 do_write_check(struct tracecmd_output *handle, const void *data, tsize_t size)
 {
 	if (handle->msg_handle)
-		return tracecmd_msg_metadata_send(handle->msg_handle, data, size);
+		return tracecmd_msg_data_send(handle->msg_handle, data, size);
 
 	return __do_write_check(handle->fd, data, size);
 }
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 24580a4..129f36a 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -2747,7 +2747,7 @@ static void communicate_with_listener_v1(struct tracecmd_msg_handle *msg_handle)
 	}
 }
 
-static void communicate_with_listener_v2(struct tracecmd_msg_handle *msg_handle)
+static void communicate_with_listener_v3(struct tracecmd_msg_handle *msg_handle)
 {
 	if (tracecmd_msg_send_init_data(msg_handle, &client_ports) < 0)
 		die("Cannot communicate with server");
@@ -2764,8 +2764,8 @@ static void check_protocol_version(struct tracecmd_msg_handle *msg_handle)
 	/*
 	 * Write the protocol version, the magic number, and the dummy
 	 * option(0) (in ASCII). The client understands whether the client
-	 * uses the v2 protocol or not by checking a reply message from the
-	 * server. If the message is "V2", the server uses v2 protocol. On the
+	 * uses the v3 protocol or not by checking a reply message from the
+	 * server. If the message is "V3", the server uses v3 protocol. On the
 	 * other hands, if the message is just number strings, the server
 	 * returned port numbers. So, in that time, the client understands the
 	 * server uses the v1 protocol. However, the old server tells the
@@ -2773,7 +2773,7 @@ static void check_protocol_version(struct tracecmd_msg_handle *msg_handle)
 	 * So, we add the dummy number (the magic number and 0 option) to the
 	 * first client message.
 	 */
-	write(fd, V2_CPU, sizeof(V2_CPU));
+	write(fd, V3_CPU, sizeof(V3_CPU));
 
 	buf[0] = 0;
 
@@ -2787,8 +2787,8 @@ static void check_protocol_version(struct tracecmd_msg_handle *msg_handle)
 	} else {
 		if (memcmp(buf, "V2", n) != 0)
 			die("Cannot handle the protocol %s", buf);
-		/* OK, let's use v2 protocol */
-		write(fd, V2_MAGIC, sizeof(V2_MAGIC));
+		/* OK, let's use v3 protocol */
+		write(fd, V3_MAGIC, sizeof(V3_MAGIC));
 
 		n = read(fd, buf, BUFSIZ - 1);
 		if (n != 2 || memcmp(buf, "OK", 2) != 0) {
@@ -2857,20 +2857,20 @@ again:
 			die("Failed to allocate message handle");
 
 		msg_handle->cpu_count = local_cpu_count;
-		msg_handle->version = V2_PROTOCOL;
+		msg_handle->version = V3_PROTOCOL;
 	}
 
 	if (use_tcp)
 		msg_handle->flags |= TRACECMD_MSG_FL_USE_TCP;
 
-	if (msg_handle->version == V2_PROTOCOL) {
+	if (msg_handle->version == V3_PROTOCOL) {
 		check_protocol_version(msg_handle);
 		if (msg_handle->version == V1_PROTOCOL) {
 			/* reconnect to the server for using the v1 protocol */
 			close(sfd);
 			goto again;
 		}
-		communicate_with_listener_v2(msg_handle);
+		communicate_with_listener_v3(msg_handle);
 	}
 
 	if (msg_handle->version == V1_PROTOCOL)
@@ -2888,9 +2888,9 @@ setup_connection(struct buffer_instance *instance)
 	msg_handle = setup_network();
 
 	/* Now create the handle through this socket */
-	if (msg_handle->version == V2_PROTOCOL) {
+	if (msg_handle->version == V3_PROTOCOL) {
 		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events);
-		tracecmd_msg_finish_sending_metadata(msg_handle);
+		tracecmd_msg_finish_sending_data(msg_handle);
 	} else
 		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd,
 							      listed_events);
@@ -2903,7 +2903,7 @@ setup_connection(struct buffer_instance *instance)
 
 static void finish_network(struct tracecmd_msg_handle *msg_handle)
 {
-	if (msg_handle->version == V2_PROTOCOL)
+	if (msg_handle->version == V3_PROTOCOL)
 		tracecmd_msg_send_close_msg(msg_handle);
 	tracecmd_msg_handle_close(msg_handle);
 	free(host);
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 2/3] trace-cmd: Fix record --date flag when sending tracing data to a listener
  2018-12-14 13:57 [PATCH v4 0/3] trace-cmd: Resend record --date fix Slavomir Kaslev
  2018-12-14 13:57 ` [PATCH v4 1/3] trace-cmd: Prepare for protocol bump to version 3 Slavomir Kaslev
@ 2018-12-14 13:57 ` Slavomir Kaslev
  2018-12-14 17:47   ` Steven Rostedt
  2018-12-14 13:57 ` [PATCH v4 3/3] trace-cmd: Bump protocol version to v3 Slavomir Kaslev
  2 siblings, 1 reply; 5+ messages in thread
From: Slavomir Kaslev @ 2018-12-14 13:57 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: rostedt, ykaradzhov, tstoyanov

Currently the `trace-cmd record` --date is not taken into account when tracing
data is sent to a remote host with the -N flag.

This patch fixes this by the writing output buffer options from the recording
side instead of on the listener side.

We also provide backward compatibility by falling back to previous behavior when
we're not using protocol v3.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 include/trace-cmd/trace-cmd.h | 10 ++++-
 tracecmd/trace-listen.c       | 23 ++++++++---
 tracecmd/trace-output.c       | 76 +++++++++++++++++------------------
 tracecmd/trace-record.c       | 55 ++++++++++++++-----------
 4 files changed, 96 insertions(+), 68 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 7cce592..26c1180 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -245,6 +245,9 @@ struct tracecmd_option *tracecmd_add_option(struct tracecmd_output *handle,
 					    const void *data);
 struct tracecmd_option *tracecmd_add_buffer_option(struct tracecmd_output *handle,
 						   const char *name, int cpus);
+
+int tracecmd_write_cpus(struct tracecmd_output *handle, int cpus);
+int tracecmd_write_options(struct tracecmd_output *handle);
 int tracecmd_update_option(struct tracecmd_output *handle,
 			   struct tracecmd_option *option, int size,
 			   const void *data);
@@ -252,13 +255,16 @@ void tracecmd_output_close(struct tracecmd_output *handle);
 void tracecmd_output_free(struct tracecmd_output *handle);
 struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
 				      const char *file);
+
+int tracecmd_write_cpu_data(struct tracecmd_output *handle,
+			    int cpus, char * const *cpu_data_files);
 int tracecmd_append_cpu_data(struct tracecmd_output *handle,
 			     int cpus, char * const *cpu_data_files);
 int tracecmd_append_buffer_cpu_data(struct tracecmd_output *handle,
 				    struct tracecmd_option *option,
 				    int cpus, char * const *cpu_data_files);
-int tracecmd_attach_cpu_data(char *file, int cpus, char * const *cpu_data_files);
-int tracecmd_attach_cpu_data_fd(int fd, int cpus, char * const *cpu_data_files);
+
+struct tracecmd_output *tracecmd_get_output_handle_fd(int fd);
 
 /* --- Reading the Fly Recorder Trace --- */
 
diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c
index a13b83b..57151ba 100644
--- a/tracecmd/trace-listen.c
+++ b/tracecmd/trace-listen.c
@@ -624,8 +624,9 @@ static void stop_all_readers(int cpus, int *pid_array)
 }
 
 static int put_together_file(int cpus, int ofd, const char *node,
-			      const char *port)
+			     const char *port, bool write_options)
 {
+	struct tracecmd_output *handle;
 	char **temp_files;
 	int cpu;
 	int ret = -ENOMEM;
@@ -641,9 +642,20 @@ static int put_together_file(int cpus, int ofd, const char *node,
 			goto out;
 	}
 
-	tracecmd_attach_cpu_data_fd(ofd, cpus, temp_files);
-	ret = 0;
- out:
+	handle = tracecmd_get_output_handle_fd(ofd);
+	if (!handle) {
+		ret = -1;
+		goto out;
+	}
+
+	if (write_options) {
+		tracecmd_write_cpus(handle, cpus);
+		tracecmd_write_options(handle);
+	}
+	ret = tracecmd_write_cpu_data(handle, cpus, temp_files);
+
+out:
+	tracecmd_output_close(handle);
 	for (cpu--; cpu >= 0; cpu--) {
 		put_temp_file(temp_files[cpu]);
 	}
@@ -692,7 +704,8 @@ static int process_client(struct tracecmd_msg_handle *msg_handle,
 	/* wait a little to have the readers clean up */
 	sleep(1);
 
-	ret = put_together_file(cpus, ofd, node, port);
+	ret = put_together_file(cpus, ofd, node, port,
+				msg_handle->version != 3);
 
 	destroy_all_readers(cpus, pid_array, node, port);
 
diff --git a/tracecmd/trace-output.c b/tracecmd/trace-output.c
index 78a8fe6..656b064 100644
--- a/tracecmd/trace-output.c
+++ b/tracecmd/trace-output.c
@@ -932,7 +932,13 @@ tracecmd_add_option(struct tracecmd_output *handle,
 	return option;
 }
 
-static int add_options(struct tracecmd_output *handle)
+int tracecmd_write_cpus(struct tracecmd_output *handle, int cpus)
+{
+	cpus = convert_endian_4(handle, cpus);
+	return do_write_check(handle, &cpus, 4);
+}
+
+int tracecmd_write_options(struct tracecmd_output *handle)
 {
 	struct tracecmd_option *options;
 	unsigned short option;
@@ -1052,11 +1058,11 @@ struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, in
 	if (!handle)
 		return NULL;
 
-	cpus = convert_endian_4(handle, cpus);
-	if (do_write_check(handle, &cpus, 4))
+
+	if (tracecmd_write_cpus(handle, cpus) < 0)
 		goto out_free;
 
-	if (add_options(handle) < 0)
+	if (tracecmd_write_options(handle) < 0)
 		goto out_free;
 
 	if (do_write_check(handle, "latency  ", 10))
@@ -1077,8 +1083,8 @@ out_free:
 	return NULL;
 }
 
-static int __tracecmd_append_cpu_data(struct tracecmd_output *handle,
-				      int cpus, char * const *cpu_data_files)
+int tracecmd_write_cpu_data(struct tracecmd_output *handle,
+			    int cpus, char * const *cpu_data_files)
 {
 	off64_t *offsets = NULL;
 	unsigned long long *sizes = NULL;
@@ -1186,16 +1192,17 @@ static int __tracecmd_append_cpu_data(struct tracecmd_output *handle,
 int tracecmd_append_cpu_data(struct tracecmd_output *handle,
 			     int cpus, char * const *cpu_data_files)
 {
-	int endian4;
+	int ret;
 
-	endian4 = convert_endian_4(handle, cpus);
-	if (do_write_check(handle, &endian4, 4))
-		return -1;
+	ret = tracecmd_write_cpus(handle, cpus);
+	if (ret)
+		return ret;
 
-	if (add_options(handle) < 0)
-		return -1;
+	ret = tracecmd_write_options(handle);
+	if (ret)
+		return ret;
 
-	return __tracecmd_append_cpu_data(handle, cpus, cpu_data_files);
+	return tracecmd_write_cpu_data(handle, cpus, cpu_data_files);
 }
 
 int tracecmd_append_buffer_cpu_data(struct tracecmd_output *handle,
@@ -1224,35 +1231,38 @@ int tracecmd_append_buffer_cpu_data(struct tracecmd_output *handle,
 		return -1;
 	}
 
-	return __tracecmd_append_cpu_data(handle, cpus, cpu_data_files);
+	return tracecmd_write_cpu_data(handle, cpus, cpu_data_files);
 }
 
-int tracecmd_attach_cpu_data_fd(int fd, int cpus, char * const *cpu_data_files)
+struct tracecmd_output *tracecmd_get_output_handle_fd(int fd)
 {
+	struct tracecmd_output *handle = NULL;
 	struct tracecmd_input *ihandle;
-	struct tracecmd_output *handle;
 	struct tep_handle *pevent;
-	int ret = -1;
+	int fd2;
 
 	/* Move the file descriptor to the beginning */
 	if (lseek(fd, 0, SEEK_SET) == (off_t)-1)
-		return -1;
+		return NULL;
+
+	/* dup fd to be used by the ihandle bellow */
+	fd2 = dup(fd);
+	if (fd2 < 0)
+		return NULL;
 
 	/* get a input handle from this */
-	ihandle = tracecmd_alloc_fd(fd);
+	ihandle = tracecmd_alloc_fd(fd2);
 	if (!ihandle)
-		return -1;
+		return NULL;
 
 	/* move the file descriptor to the end */
 	if (lseek(fd, 0, SEEK_END) == (off_t)-1)
 		goto out_free;
 
 	/* create a partial output handle */
-
-	handle = malloc(sizeof(*handle));
+	handle = calloc(1, sizeof(*handle));
 	if (!handle)
 		goto out_free;
-	memset(handle, 0, sizeof(*handle));
 
 	handle->fd = fd;
 
@@ -1264,24 +1274,14 @@ int tracecmd_attach_cpu_data_fd(int fd, int cpus, char * const *cpu_data_files)
 	handle->page_size = tracecmd_page_size(ihandle);
 	list_head_init(&handle->options);
 
-	if (tracecmd_append_cpu_data(handle, cpus, cpu_data_files) >= 0)
-		ret = 0;
-
-	tracecmd_output_close(handle);
- out_free:
 	tracecmd_close(ihandle);
-	return ret;
-}
-
-int tracecmd_attach_cpu_data(char *file, int cpus, char * const *cpu_data_files)
-{
-	int fd;
 
-	fd = open(file, O_RDWR);
-	if (fd < 0)
-		return -1;
+	return handle;
 
-	return tracecmd_attach_cpu_data_fd(fd, cpus, cpu_data_files);
+ out_free:
+	tracecmd_close(ihandle);
+	free(handle);
+	return NULL;
 }
 
 struct tracecmd_output *
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 129f36a..f19341b 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -2879,8 +2879,10 @@ again:
 	return msg_handle;
 }
 
+static void add_options(struct tracecmd_output *handle, char *date2ts, int flags);
+
 static struct tracecmd_msg_handle *
-setup_connection(struct buffer_instance *instance)
+setup_connection(struct buffer_instance *instance, char *date2ts, int flags)
 {
 	struct tracecmd_msg_handle *msg_handle;
 	struct tracecmd_output *network_handle;
@@ -2890,6 +2892,11 @@ setup_connection(struct buffer_instance *instance)
 	/* Now create the handle through this socket */
 	if (msg_handle->version == V3_PROTOCOL) {
 		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events);
+		if (msg_handle->version == 3) {
+			add_options(network_handle, date2ts, flags);
+			tracecmd_write_cpus(network_handle, instance->cpu_count);
+			tracecmd_write_options(network_handle);
+		}
 		tracecmd_msg_finish_sending_data(msg_handle);
 	} else
 		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd,
@@ -2909,7 +2916,7 @@ static void finish_network(struct tracecmd_msg_handle *msg_handle)
 	free(host);
 }
 
-void start_threads(enum trace_type type, int global)
+void start_threads(enum trace_type type, int global, char *date2ts, int flags)
 {
 	struct buffer_instance *instance;
 	int *brass = NULL;
@@ -2931,7 +2938,7 @@ void start_threads(enum trace_type type, int global)
 		int x, pid;
 
 		if (host) {
-			instance->msg_handle = setup_connection(instance);
+			instance->msg_handle = setup_connection(instance, date2ts, flags);
 			if (!instance->msg_handle)
 				die("Failed to make connection");
 		}
@@ -3085,6 +3092,26 @@ enum {
 	DATA_FL_OFFSET		= 2,
 };
 
+static void add_options(struct tracecmd_output *handle, char *date2ts, int flags)
+{
+	int type = 0;
+
+	if (date2ts) {
+		if (flags & DATA_FL_DATE)
+			type = TRACECMD_OPTION_DATE;
+		else if (flags & DATA_FL_OFFSET)
+			type = TRACECMD_OPTION_OFFSET;
+	}
+
+	if (type)
+		tracecmd_add_option(handle, type, strlen(date2ts)+1, date2ts);
+
+	tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
+	add_option_hooks(handle);
+	add_uname(handle);
+
+}
+
 static void record_data(char *date2ts, int flags)
 {
 	struct tracecmd_option **buffer_options;
@@ -3140,18 +3167,7 @@ static void record_data(char *date2ts, int flags)
 		if (!handle)
 			die("Error creating output file");
 
-		if (date2ts) {
-			int type = 0;
-
-			if (flags & DATA_FL_DATE)
-				type = TRACECMD_OPTION_DATE;
-			else if (flags & DATA_FL_OFFSET)
-				type = TRACECMD_OPTION_OFFSET;
-
-			if (type)
-				tracecmd_add_option(handle, type,
-						    strlen(date2ts)+1, date2ts);
-		}
+		add_options(handle, date2ts, flags);
 
 		/* Only record the top instance under TRACECMD_OPTION_CPUSTAT*/
 		if (!no_top_instance() && !top_instance.msg_handle) {
@@ -3162,13 +3178,6 @@ static void record_data(char *date2ts, int flags)
 						    s[i].len+1, s[i].buffer);
 		}
 
-		tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK,
-				    0, NULL);
-
-		add_option_hooks(handle);
-
-		add_uname(handle);
-
 		if (buffers) {
 			buffer_options = malloc(sizeof(*buffer_options) * buffers);
 			if (!buffer_options)
@@ -4977,7 +4986,7 @@ static void record_trace(int argc, char **argv,
 	if (type & (TRACE_TYPE_RECORD | TRACE_TYPE_STREAM)) {
 		signal(SIGINT, finish);
 		if (!latency)
-			start_threads(type, ctx->global);
+			start_threads(type, ctx->global, ctx->date2ts, ctx->data_flags);
 	} else {
 		update_task_filter();
 		tracecmd_enable_tracing();
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 3/3] trace-cmd: Bump protocol version to v3
  2018-12-14 13:57 [PATCH v4 0/3] trace-cmd: Resend record --date fix Slavomir Kaslev
  2018-12-14 13:57 ` [PATCH v4 1/3] trace-cmd: Prepare for protocol bump to version 3 Slavomir Kaslev
  2018-12-14 13:57 ` [PATCH v4 2/3] trace-cmd: Fix record --date flag when sending tracing data to a listener Slavomir Kaslev
@ 2018-12-14 13:57 ` Slavomir Kaslev
  2 siblings, 0 replies; 5+ messages in thread
From: Slavomir Kaslev @ 2018-12-14 13:57 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: rostedt, ykaradzhov, tstoyanov

This patch bumps trace-cmd protocol to v3 while keeping backward compatibility
by falling back to v1.

The new protocol works similarly to v2 but allows future extension of commands
by saving the command size on the wire and having the client on the other side
to only read the bytes it understands.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 tracecmd/include/trace-msg.h |   6 +-
 tracecmd/trace-listen.c      |   6 +-
 tracecmd/trace-msg.c         | 190 +++++++++++++++--------------------
 tracecmd/trace-record.c      |  10 +-
 4 files changed, 92 insertions(+), 120 deletions(-)

diff --git a/tracecmd/include/trace-msg.h b/tracecmd/include/trace-msg.h
index 722548a..b7fe10b 100644
--- a/tracecmd/include/trace-msg.h
+++ b/tracecmd/include/trace-msg.h
@@ -4,11 +4,11 @@
 #include <stdbool.h>
 
 #define UDP_MAX_PACKET	(65536 - 20)
-#define V3_MAGIC	"677768\0"
-#define V3_CPU		"-1V2"
+#define V3_MAGIC	"766679\0"
+#define V3_CPU		"-1V3"
 
 #define V1_PROTOCOL	1
-#define V3_PROTOCOL	2
+#define V3_PROTOCOL	3
 
 extern unsigned int page_size;
 
diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c
index 57151ba..8394b0f 100644
--- a/tracecmd/trace-listen.c
+++ b/tracecmd/trace-listen.c
@@ -387,12 +387,12 @@ static int communicate_with_client(struct tracecmd_msg_handle *msg_handle)
 				last_proto[n] = 0;
 			}
 			/* Return the highest protocol we can use */
-			write(fd, "V2", 3);
+			write(fd, "V3", 3);
 			goto try_again;
 		}
 
 		/* Let the client know we use v3 protocol */
-		write(fd, "V2", 3);
+		write(fd, "V3", 3);
 
 		/* read the rest of dummy data */
 		n = read(fd, buf, sizeof(V3_MAGIC));
@@ -705,7 +705,7 @@ static int process_client(struct tracecmd_msg_handle *msg_handle,
 	sleep(1);
 
 	ret = put_together_file(cpus, ofd, node, port,
-				msg_handle->version != 3);
+				msg_handle->version != V3_PROTOCOL);
 
 	destroy_all_readers(cpus, pid_array, node, port);
 
diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c
index b496935..6b69a65 100644
--- a/tracecmd/trace-msg.c
+++ b/tracecmd/trace-msg.c
@@ -45,21 +45,7 @@ static inline void dprint(const char *fmt, ...)
 
 #define MSG_HDR_LEN			sizeof(struct tracecmd_msg_header)
 
-#define MSG_DATA_LEN			(MSG_MAX_LEN - MSG_HDR_LEN)
-
-					/* - header size for error msg */
-#define MSG_META_MAX_LEN		(MSG_MAX_LEN - MIN_META_SIZE)
-
-
-#define MIN_TINIT_SIZE	(sizeof(struct tracecmd_msg_header) + \
-			 sizeof(struct tracecmd_msg_tinit))
-
-/* Not really the minimum, but I couldn't think of a better name */
-#define MIN_RINIT_SIZE	(sizeof(struct tracecmd_msg_header) + \
-			 sizeof(struct tracecmd_msg_rinit))
-
-#define MIN_META_SIZE	(sizeof(struct tracecmd_msg_header) + \
-			 sizeof(struct tracecmd_msg_meta))
+#define MSG_MAX_DATA_LEN		(MSG_MAX_LEN - MSG_HDR_LEN)
 
 unsigned int page_size;
 
@@ -81,8 +67,7 @@ make_server(struct tracecmd_msg_handle *msg_handle)
 struct tracecmd_msg_opt {
 	be32 size;
 	be32 opt_cmd;
-	be32 padding;	/* for backward compatibility */
-};
+} __attribute__((packed));
 
 struct tracecmd_msg_tinit {
 	be32 cpus;
@@ -94,36 +79,31 @@ struct tracecmd_msg_rinit {
 	be32 cpus;
 } __attribute__((packed));
 
-struct tracecmd_msg_meta {
-	be32 size;
-} __attribute__((packed));
-
 struct tracecmd_msg_header {
 	be32	size;
 	be32	cmd;
+	be32	cmd_size;
 } __attribute__((packed));
 
-#define MSG_MAP						\
-	C(UNUSED_0,	0,	-1),			\
-	C(CLOSE,	1,	0),			\
-	C(USUSED_2,	2,	-1),			\
-	C(UNUSED_3,	3,	-1),			\
-	C(TINIT,	4,	MIN_TINIT_SIZE),	\
-	C(RINIT,	5,	MIN_RINIT_SIZE),	\
-	C(SENDMETA,	6,	MIN_META_SIZE),		\
-	C(FINMETA,	7,	0),
+#define MSG_MAP								\
+	C(CLOSE,	0,	0),					\
+	C(TINIT,	1,	sizeof(struct tracecmd_msg_tinit)),	\
+	C(RINIT,	2,	sizeof(struct tracecmd_msg_rinit)),	\
+	C(SEND_DATA,	3,	0),					\
+	C(FIN_DATA,	4,	0),
 
 #undef C
 #define C(a,b,c)	MSG_##a = b
 
 enum tracecmd_msg_cmd {
 	MSG_MAP
+	MSG_NR_COMMANDS
 };
 
 #undef C
 #define C(a,b,c)	c
 
-static be32 msg_min_sizes[] = { MSG_MAP };
+static be32 msg_cmd_sizes[] = { MSG_MAP };
 
 #undef C
 #define C(a,b,c)	#a
@@ -132,9 +112,9 @@ static const char *msg_names[] = { MSG_MAP };
 
 static const char *cmd_to_name(int cmd)
 {
-	if (cmd <= MSG_FINMETA)
-		return msg_names[cmd];
-	return "Unkown";
+	if (cmd < 0 || cmd >= MSG_NR_COMMANDS)
+		return "Unknown";
+	return msg_names[cmd];
 }
 
 struct tracecmd_msg {
@@ -142,7 +122,6 @@ struct tracecmd_msg {
 	union {
 		struct tracecmd_msg_tinit	tinit;
 		struct tracecmd_msg_rinit	rinit;
-		struct tracecmd_msg_meta	meta;
 	};
 	union {
 		struct tracecmd_msg_opt		*opt;
@@ -156,24 +135,27 @@ struct tracecmd_msg *errmsg;
 static int msg_write(int fd, struct tracecmd_msg *msg)
 {
 	int cmd = ntohl(msg->hdr.cmd);
-	int size;
+	int msg_size, data_size;
 	int ret;
 
-	if (cmd > MSG_FINMETA)
+	if (cmd < 0 || cmd >= MSG_NR_COMMANDS)
 		return -EINVAL;
 
 	dprint("msg send: %d (%s)\n", cmd, cmd_to_name(cmd));
 
-	size = msg_min_sizes[cmd];
-	if (!size)
-		size = ntohl(msg->hdr.size);
+	msg_size = MSG_HDR_LEN + ntohl(msg->hdr.cmd_size);
+	data_size = ntohl(msg->hdr.size) - msg_size;
+	if (data_size < 0)
+		return -EINVAL;
 
-	ret = __do_write_check(fd, msg, size);
+	ret = __do_write_check(fd, msg, msg_size);
 	if (ret < 0)
 		return ret;
-	if (ntohl(msg->hdr.size) <= size)
+
+	if (!data_size)
 		return 0;
-	return __do_write_check(fd, msg->buf, ntohl(msg->hdr.size) - size);
+
+	return __do_write_check(fd, msg->buf, data_size);
 }
 
 enum msg_opt_command {
@@ -186,7 +168,7 @@ static int make_tinit(struct tracecmd_msg_handle *msg_handle,
 	struct tracecmd_msg_opt *opt;
 	int cpu_count = msg_handle->cpu_count;
 	int opt_num = 0;
-	int size = MIN_TINIT_SIZE;
+	int data_size = 0;
 
 	if (msg_handle->flags & TRACECMD_MSG_FL_USE_TCP) {
 		opt_num++;
@@ -196,43 +178,31 @@ static int make_tinit(struct tracecmd_msg_handle *msg_handle,
 		opt->size = htonl(sizeof(*opt));
 		opt->opt_cmd = htonl(MSGOPT_USETCP);
 		msg->opt = opt;
-		size += sizeof(*opt);
+		data_size += sizeof(*opt);
 	}
 
 	msg->tinit.cpus = htonl(cpu_count);
 	msg->tinit.page_size = htonl(page_size);
 	msg->tinit.opt_num = htonl(opt_num);
 
-	msg->hdr.size = htonl(size);
+	msg->hdr.size = htonl(ntohl(msg->hdr.size) + data_size);
 
 	return 0;
 }
 
-static int make_rinit(struct tracecmd_msg *msg, int total_cpus, int *ports)
+static int make_rinit(struct tracecmd_msg *msg, int cpus, int *ports)
 {
-	int size = MIN_RINIT_SIZE;
-	be32 *ptr;
-	be32 port;
 	int i;
 
-	msg->rinit.cpus = htonl(total_cpus);
-
-	msg->port_array = malloc(sizeof(*ports) * total_cpus);
+	msg->rinit.cpus = htonl(cpus);
+	msg->port_array = malloc(sizeof(*ports) * cpus);
 	if (!msg->port_array)
 		return -ENOMEM;
 
-	size += sizeof(*ports) * total_cpus;
-
-	ptr = msg->port_array;
-
-	for (i = 0; i < total_cpus; i++) {
-		/* + rrqports->cpus or rrqports->port_array[i] */
-		port = htonl(ports[i]);
-		*ptr = port;
-		ptr++;
-	}
+	for (i = 0; i < cpus; i++)
+		msg->port_array[i] = htonl(ports[i]);
 
-	msg->hdr.size = htonl(size);
+	msg->hdr.size = htonl(ntohl(msg->hdr.size) + sizeof(*ports) * cpus);
 
 	return 0;
 }
@@ -240,21 +210,14 @@ static int make_rinit(struct tracecmd_msg *msg, int total_cpus, int *ports)
 static void tracecmd_msg_init(u32 cmd, struct tracecmd_msg *msg)
 {
 	memset(msg, 0, sizeof(*msg));
+	msg->hdr.size = htonl(MSG_HDR_LEN + msg_cmd_sizes[cmd]);
 	msg->hdr.cmd = htonl(cmd);
-	if (!msg_min_sizes[cmd])
-		msg->hdr.size = htonl(MSG_HDR_LEN);
-	else
-		msg->hdr.size = htonl(msg_min_sizes[cmd]);
+	msg->hdr.cmd_size = htonl(msg_cmd_sizes[cmd]);
 }
 
 static void msg_free(struct tracecmd_msg *msg)
 {
-	int cmd = ntohl(msg->hdr.cmd);
-
-	/* If a min size is defined, then the buf needs to be freed */
-	if (cmd < MSG_FINMETA && (msg_min_sizes[cmd] > 0))
-		free(msg->buf);
-
+	free(msg->buf);
 	memset(msg, 0, sizeof(*msg));
 }
 
@@ -290,30 +253,42 @@ static int msg_read(int fd, void *buf, u32 size, int *n)
 	return 0;
 }
 
+static char scratch_buf[MSG_MAX_LEN];
+
 static int msg_read_extra(int fd, struct tracecmd_msg *msg,
 			  int *n, int size)
 {
-	u32 cmd;
-	int rsize;
+	int cmd, cmd_size, rsize;
 	int ret;
 
 	cmd = ntohl(msg->hdr.cmd);
-	if (cmd > MSG_FINMETA)
+	if (cmd < 0 || cmd >= MSG_NR_COMMANDS)
 		return -EINVAL;
 
-	rsize = msg_min_sizes[cmd] - *n;
-	if (rsize <= 0)
-		return 0;
+	cmd_size = ntohl(msg->hdr.cmd_size);
+	if (cmd_size < 0)
+		return -EINVAL;
 
-	ret = msg_read(fd, msg, rsize, n);
-	if (ret < 0)
-		return ret;
+	if (cmd_size > 0) {
+		rsize = cmd_size;
+		if (rsize > msg_cmd_sizes[cmd])
+			rsize = msg_cmd_sizes[cmd];
+
+		ret = msg_read(fd, msg, rsize, n);
+		if (ret < 0)
+			return ret;
+
+		ret = msg_read(fd, scratch_buf, cmd_size - rsize, n);
+		if (ret < 0)
+			return ret;
+	}
 
 	if (size > *n) {
 		size -= *n;
 		msg->buf = malloc(size);
 		if (!msg->buf)
 			return -ENOMEM;
+
 		*n = 0;
 		return msg_read(fd, msg->buf, size, n);
 	}
@@ -437,7 +412,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 		return -EINVAL;
 
 	cpus = ntohl(recv_msg.rinit.cpus);
-	ports = malloc_or_die(sizeof(int) * cpus);
+	ports = malloc_or_die(sizeof(*ports) * cpus);
 	for (i = 0; i < cpus; i++)
 		ports[i] = ntohl(recv_msg.port_array[i]);
 
@@ -503,7 +478,7 @@ int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle)
 	int cpus;
 	int ret;
 	int offset = 0;
-	u32 size = MIN_TINIT_SIZE;
+	u32 size;
 	u32 cmd;
 
 	ret = tracecmd_msg_recv_wait(msg_handle->fd, &msg);
@@ -535,6 +510,7 @@ int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle)
 		goto error;
 	}
 
+	size = MSG_HDR_LEN + ntohl(msg.hdr.cmd_size);
 	options = ntohl(msg.tinit.opt_num);
 	for (i = 0; i < options; i++) {
 		if (size + sizeof(*opt) > ntohl(msg.hdr.size)) {
@@ -608,31 +584,29 @@ int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle,
 	int ret;
 	int count = 0;
 
-	tracecmd_msg_init(MSG_SENDMETA, &msg);
+	tracecmd_msg_init(MSG_SEND_DATA, &msg);
 
-	msg.buf = malloc(MSG_META_MAX_LEN);
+	msg.buf = malloc(MSG_MAX_DATA_LEN);
 	if (!msg.buf)
 		return -ENOMEM;
 
-	msg.meta.size = htonl(MSG_META_MAX_LEN);
-	msg.hdr.size = htonl(MIN_META_SIZE + MSG_META_MAX_LEN);
+	msg.hdr.size = htonl(MSG_MAX_LEN);
 
 	n = size;
-	do {
-		if (n > MSG_META_MAX_LEN) {
-			memcpy(msg.buf, buf+count, MSG_META_MAX_LEN);
-			n -= MSG_META_MAX_LEN;
-			count += MSG_META_MAX_LEN;
+	while (n) {
+		if (n > MSG_MAX_DATA_LEN) {
+			memcpy(msg.buf, buf + count, MSG_MAX_DATA_LEN);
+			n -= MSG_MAX_DATA_LEN;
+			count += MSG_MAX_DATA_LEN;
 		} else {
-			msg.hdr.size = htonl(MIN_META_SIZE + n);
-			msg.meta.size = htonl(n);
-			memcpy(msg.buf, buf+count, n);
+			msg.hdr.size = htonl(MSG_HDR_LEN + n);
+			memcpy(msg.buf, buf + count, n);
 			n = 0;
 		}
 		ret = msg_write(fd, &msg);
 		if (ret < 0)
 			break;
-	} while (n);
+	}
 
 	msg_free(&msg);
 	return ret;
@@ -643,7 +617,7 @@ int tracecmd_msg_finish_sending_data(struct tracecmd_msg_handle *msg_handle)
 	struct tracecmd_msg msg;
 	int ret;
 
-	tracecmd_msg_init(MSG_FINMETA, &msg);
+	tracecmd_msg_init(MSG_FIN_DATA, &msg);
 	ret = tracecmd_msg_send(msg_handle->fd, &msg);
 	if (ret < 0)
 		return ret;
@@ -653,11 +627,11 @@ int tracecmd_msg_finish_sending_data(struct tracecmd_msg_handle *msg_handle)
 int tracecmd_msg_collect_data(struct tracecmd_msg_handle *msg_handle, int ofd)
 {
 	struct tracecmd_msg msg;
-	u32 t, n, cmd;
+	int t, n, cmd;
 	ssize_t s;
 	int ret;
 
-	do {
+	for (;;) {
 		ret = tracecmd_msg_recv_wait(msg_handle->fd, &msg);
 		if (ret < 0) {
 			if (ret == -ETIMEDOUT)
@@ -668,16 +642,16 @@ int tracecmd_msg_collect_data(struct tracecmd_msg_handle *msg_handle, int ofd)
 		}
 
 		cmd = ntohl(msg.hdr.cmd);
-		if (cmd == MSG_FINMETA) {
-			/* Finish receiving meta data */
+		if (cmd == MSG_FIN_DATA) {
+			/* Finish receiving data */
 			break;
-		} else if (cmd != MSG_SENDMETA)
+		} else if (cmd != MSG_SEND_DATA)
 			goto error;
 
-		n = ntohl(msg.meta.size);
+		n = ntohl(msg.hdr.size) - MSG_HDR_LEN - ntohl(msg.hdr.cmd_size);
 		t = n;
 		s = 0;
-		do {
+		while (t > 0) {
 			s = write(ofd, msg.buf+s, t);
 			if (s < 0) {
 				if (errno == EINTR)
@@ -687,8 +661,8 @@ int tracecmd_msg_collect_data(struct tracecmd_msg_handle *msg_handle, int ofd)
 			}
 			t -= s;
 			s = n - t;
-		} while (t);
-	} while (cmd == MSG_SENDMETA);
+		}
+	}
 
 	/* check the finish message of the client */
 	while (!tracecmd_msg_done(msg_handle)) {
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index f19341b..e45a1f8 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -2785,7 +2785,7 @@ static void check_protocol_version(struct tracecmd_msg_handle *msg_handle)
 		msg_handle->version = V1_PROTOCOL;
 		plog("Use the v1 protocol\n");
 	} else {
-		if (memcmp(buf, "V2", n) != 0)
+		if (memcmp(buf, "V3", n) != 0)
 			die("Cannot handle the protocol %s", buf);
 		/* OK, let's use v3 protocol */
 		write(fd, V3_MAGIC, sizeof(V3_MAGIC));
@@ -2892,11 +2892,9 @@ setup_connection(struct buffer_instance *instance, char *date2ts, int flags)
 	/* Now create the handle through this socket */
 	if (msg_handle->version == V3_PROTOCOL) {
 		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events);
-		if (msg_handle->version == 3) {
-			add_options(network_handle, date2ts, flags);
-			tracecmd_write_cpus(network_handle, instance->cpu_count);
-			tracecmd_write_options(network_handle);
-		}
+		add_options(network_handle, date2ts, flags);
+		tracecmd_write_cpus(network_handle, instance->cpu_count);
+		tracecmd_write_options(network_handle);
 		tracecmd_msg_finish_sending_data(msg_handle);
 	} else
 		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd,
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 2/3] trace-cmd: Fix record --date flag when sending tracing data to a listener
  2018-12-14 13:57 ` [PATCH v4 2/3] trace-cmd: Fix record --date flag when sending tracing data to a listener Slavomir Kaslev
@ 2018-12-14 17:47   ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2018-12-14 17:47 UTC (permalink / raw)
  To: Slavomir Kaslev; +Cc: linux-trace-devel, ykaradzhov, tstoyanov

On Fri, 14 Dec 2018 15:57:48 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:

> diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c
> index a13b83b..57151ba 100644
> --- a/tracecmd/trace-listen.c
> +++ b/tracecmd/trace-listen.c
> @@ -624,8 +624,9 @@ static void stop_all_readers(int cpus, int *pid_array)
>  }
>  
>  static int put_together_file(int cpus, int ofd, const char *node,
> -			      const char *port)
> +			     const char *port, bool write_options)
>  {
> +	struct tracecmd_output *handle;
>  	char **temp_files;
>  	int cpu;
>  	int ret = -ENOMEM;
> @@ -641,9 +642,20 @@ static int put_together_file(int cpus, int ofd, const char *node,
>  			goto out;
>  	}
>  
> -	tracecmd_attach_cpu_data_fd(ofd, cpus, temp_files);
> -	ret = 0;
> - out:
> +	handle = tracecmd_get_output_handle_fd(ofd);
> +	if (!handle) {
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	if (write_options) {
> +		tracecmd_write_cpus(handle, cpus);
> +		tracecmd_write_options(handle);
> +	}
> +	ret = tracecmd_write_cpu_data(handle, cpus, temp_files);
> +
> +out:
> +	tracecmd_output_close(handle);
>  	for (cpu--; cpu >= 0; cpu--) {
>  		put_temp_file(temp_files[cpu]);
>  	}
> @@ -692,7 +704,8 @@ static int process_client(struct tracecmd_msg_handle *msg_handle,
>  	/* wait a little to have the readers clean up */
>  	sleep(1);
>  
> -	ret = put_together_file(cpus, ofd, node, port);
> +	ret = put_together_file(cpus, ofd, node, port,
> +				msg_handle->version != 3);

Couple of thing here. Just to have it be a bit more self explanatory,
let's create a "write_options" variable for process_client(), set it,
and pass that to put_together_file(). The compiler should optimize it
out, so it's not going to affect code execution, but still is good to
see why we are doing the above test.

Second, let's make it "< 3" instead of "!= 3", because I'm sure v4 will
still do this too.

	write_options = msg_handle->version < 3;

	ret = put_together_file(cpus, ofd, node, port, write_options);


>  
>  	destroy_all_readers(cpus, pid_array, node, port);
> 


> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 129f36a..f19341b 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -2879,8 +2879,10 @@ again:
>  	return msg_handle;
>  }
>  
> +static void add_options(struct tracecmd_output *handle, char *date2ts, int flags);
> +
>  static struct tracecmd_msg_handle *
> -setup_connection(struct buffer_instance *instance)
> +setup_connection(struct buffer_instance *instance, char *date2ts, int flags)
>  {
>  	struct tracecmd_msg_handle *msg_handle;
>  	struct tracecmd_output *network_handle;
> @@ -2890,6 +2892,11 @@ setup_connection(struct buffer_instance *instance)
>  	/* Now create the handle through this socket */
>  	if (msg_handle->version == V3_PROTOCOL) {
>  		network_handle = tracecmd_create_init_fd_msg(msg_handle, listed_events);
> +		if (msg_handle->version == 3) {

Probably should add a comment here, as to why we are checking (for
bisectability).

Since these are slight changes, I'll make the changes and add the
patch, as Yordan needs these changes quickly. Unless...

I haven't taken a look at patch 3 yet, so if there's an issue there,
then we can make these updates for v5.

-- Steve


> +			add_options(network_handle, date2ts, flags);
> +			tracecmd_write_cpus(network_handle, instance->cpu_count);
> +			tracecmd_write_options(network_handle);
> +		}
>  		tracecmd_msg_finish_sending_data(msg_handle);
>  	} else
>  		network_handle = tracecmd_create_init_fd_glob(msg_handle->fd,
> @@ -2909,7 +2916,7 @@ static void finish_network(struct tracecmd_msg_handle *msg_handle)
>  	free(host);
>  }
>  
> -void start_threads(enum trace_type type, int global)
> +void start_threads(enum trace_type type, int global, char *date2ts, int flags)
>  {
>  	struct buffer_instance *instance;
>  	int *brass = NULL;
> @@ -2931,7 +2938,7 @@ void start_threads(enum trace_type type, int global)
>  		int x, pid;
>  
>  		if (host) {
> -			instance->msg_handle = setup_connection(instance);
> +			instance->msg_handle = setup_connection(instance, date2ts, flags);
>  			if (!instance->msg_handle)
>  				die("Failed to make connection");
>  		}
> @@ -3085,6 +3092,26 @@ enum {
>  	DATA_FL_OFFSET		= 2,
>  };
>  
> +static void add_options(struct tracecmd_output *handle, char *date2ts, int flags)
> +{
> +	int type = 0;
> +
> +	if (date2ts) {
> +		if (flags & DATA_FL_DATE)
> +			type = TRACECMD_OPTION_DATE;
> +		else if (flags & DATA_FL_OFFSET)
> +			type = TRACECMD_OPTION_OFFSET;
> +	}
> +
> +	if (type)
> +		tracecmd_add_option(handle, type, strlen(date2ts)+1, date2ts);
> +
> +	tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
> +	add_option_hooks(handle);
> +	add_uname(handle);
> +
> +}
> +
>  static void record_data(char *date2ts, int flags)
>  {
>  	struct tracecmd_option **buffer_options;
> @@ -3140,18 +3167,7 @@ static void record_data(char *date2ts, int flags)
>  		if (!handle)
>  			die("Error creating output file");
>  
> -		if (date2ts) {
> -			int type = 0;
> -
> -			if (flags & DATA_FL_DATE)
> -				type = TRACECMD_OPTION_DATE;
> -			else if (flags & DATA_FL_OFFSET)
> -				type = TRACECMD_OPTION_OFFSET;
> -
> -			if (type)
> -				tracecmd_add_option(handle, type,
> -						    strlen(date2ts)+1, date2ts);
> -		}
> +		add_options(handle, date2ts, flags);
>  
>  		/* Only record the top instance under TRACECMD_OPTION_CPUSTAT*/
>  		if (!no_top_instance() && !top_instance.msg_handle) {
> @@ -3162,13 +3178,6 @@ static void record_data(char *date2ts, int flags)
>  						    s[i].len+1, s[i].buffer);
>  		}
>  
> -		tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK,
> -				    0, NULL);
> -
> -		add_option_hooks(handle);
> -
> -		add_uname(handle);
> -
>  		if (buffers) {
>  			buffer_options = malloc(sizeof(*buffer_options) * buffers);
>  			if (!buffer_options)
> @@ -4977,7 +4986,7 @@ static void record_trace(int argc, char **argv,
>  	if (type & (TRACE_TYPE_RECORD | TRACE_TYPE_STREAM)) {
>  		signal(SIGINT, finish);
>  		if (!latency)
> -			start_threads(type, ctx->global);
> +			start_threads(type, ctx->global, ctx->date2ts, ctx->data_flags);
>  	} else {
>  		update_task_filter();
>  		tracecmd_enable_tracing();

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-12-14 17:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-14 13:57 [PATCH v4 0/3] trace-cmd: Resend record --date fix Slavomir Kaslev
2018-12-14 13:57 ` [PATCH v4 1/3] trace-cmd: Prepare for protocol bump to version 3 Slavomir Kaslev
2018-12-14 13:57 ` [PATCH v4 2/3] trace-cmd: Fix record --date flag when sending tracing data to a listener Slavomir Kaslev
2018-12-14 17:47   ` Steven Rostedt
2018-12-14 13:57 ` [PATCH v4 3/3] trace-cmd: Bump protocol version to v3 Slavomir Kaslev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).