diff mbox series

[v15,11/18] trace-cmd: Exchange tracing IDs between host and guest

Message ID 20191128085409.289684-12-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series Timestamp synchronization of host - guest tracing session | expand

Commit Message

Tzvetomir Stoyanov (VMware) Nov. 28, 2019, 8:54 a.m. UTC
Extend the trace request and trace reply messages, to include also
the IDs of host and guest tracing sessions. Those IDs are used to
unambiguously match the tracing sessions, when reading trace.dat files.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h  | 12 ++++++++----
 lib/trace-cmd/trace-msg.c      | 31 +++++++++++++++++++++++--------
 tracecmd/include/trace-local.h |  5 +++--
 tracecmd/trace-agent.c         | 12 +++++++-----
 tracecmd/trace-record.c        | 12 +++++++++---
 5 files changed, 50 insertions(+), 22 deletions(-)

Comments

Slavomir Kaslev Nov. 28, 2019, 2:34 p.m. UTC | #1
On Thu, Nov 28, 2019 at 11:01 AM Tzvetomir Stoyanov (VMware)
<tz.stoyanov@gmail.com> wrote:

[...]

> diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c
> index 1c6e0a3..cc330b7 100644
> --- a/tracecmd/trace-agent.c
> +++ b/tracecmd/trace-agent.c
> @@ -128,6 +128,7 @@ cleanup:
>  static void agent_handle(int sd, int nr_cpus, int page_size)
>  {
>         struct tracecmd_msg_handle *msg_handle;
> +       unsigned long long trace_id;
>         unsigned int *ports;
>         char **argv = NULL;
>         int argc = 0;
> @@ -144,7 +145,8 @@ static void agent_handle(int sd, int nr_cpus, int page_size)
>         if (!msg_handle)
>                 die("Failed to allocate message handle");
>
> -       ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv, &use_fifos);
> +       ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv,
> +                                         &use_fifos, &trace_id);

Why is `tracecmd_msg_recv_trace_req` receiving trace_id if it's
getting generated by the agent below?

>         if (ret < 0)
>                 die("Failed to receive trace request");
>
> @@ -153,13 +155,13 @@ static void agent_handle(int sd, int nr_cpus, int page_size)
>
>         if (!use_fifos)
>                 make_vsocks(nr_cpus, fds, ports);
> -
> +       trace_id = tracecmd_generate_traceid();
>         ret = tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size,
> -                                          ports, use_fifos);
> +                                          ports, use_fifos, trace_id);
>         if (ret < 0)
>                 die("Failed to send trace response");
> -
> -       trace_record_agent(msg_handle, nr_cpus, fds, argc, argv, use_fifos);
> +       trace_record_agent(msg_handle, nr_cpus, fds, argc, argv,
> +                          use_fifos, trace_id);
>
>         free(argv[0]);
>         free(argv);

Cheers,

-- Slavi
Tzvetomir Stoyanov (VMware) Nov. 28, 2019, 2:58 p.m. UTC | #2
On Thu, Nov 28, 2019 at 4:34 PM Slavomir Kaslev
<slavomir.kaslev@gmail.com> wrote:
>
 ...
> > -       ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv, &use_fifos);
> > +       ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv,
> > +                                         &use_fifos, &trace_id);
>
> Why is `tracecmd_msg_recv_trace_req` receiving trace_id if it's
> getting generated by the agent below?
>
Each peer generates its own trace id and stores it in its trace.dat
file. tracecmd_msg_recv_trace_req() receives the trace ID of the host
and tracecmd_msg_send_trace_resp() replies its own trace ID.
May be it is a good idea in case of guest to not generate trace ID,
but to use the one generated by the host ?

> >         if (ret < 0)
> >                 die("Failed to receive trace request");
> >
> > @@ -153,13 +155,13 @@ static void agent_handle(int sd, int nr_cpus, int page_size)
> >
> >         if (!use_fifos)
> >                 make_vsocks(nr_cpus, fds, ports);
> > -
> > +       trace_id = tracecmd_generate_traceid();
> >         ret = tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size,
> > -                                          ports, use_fifos);
> > +                                          ports, use_fifos, trace_id);
> >         if (ret < 0)
> >                 die("Failed to send trace response");
> > -
> > -       trace_record_agent(msg_handle, nr_cpus, fds, argc, argv, use_fifos);
> > +       trace_record_agent(msg_handle, nr_cpus, fds, argc, argv,
> > +                          use_fifos, trace_id);
> >
> >         free(argv[0]);
> >         free(argv);
>
> Cheers,
>
> -- Slavi
Tzvetomir Stoyanov (VMware) Nov. 29, 2019, 10:22 a.m. UTC | #3
On Thu, Nov 28, 2019 at 4:58 PM Tzvetomir Stoyanov
<tz.stoyanov@gmail.com> wrote:
>
> On Thu, Nov 28, 2019 at 4:34 PM Slavomir Kaslev
> <slavomir.kaslev@gmail.com> wrote:
> >
>  ...
> > > -       ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv, &use_fifos);
> > > +       ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv,
> > > +                                         &use_fifos, &trace_id);
> >
> > Why is `tracecmd_msg_recv_trace_req` receiving trace_id if it's
> > getting generated by the agent below?
> >
> Each peer generates its own trace id and stores it in its trace.dat
> file. tracecmd_msg_recv_trace_req() receives the trace ID of the host
> and tracecmd_msg_send_trace_resp() replies its own trace ID.
> May be it is a good idea in case of guest to not generate trace ID,
> but to use the one generated by the host ?

Actually each peer in a tracing session must have its own, unique trace ID.
These IDs are used to distinguish guests (in case there is more than one) in
the host context. In the host's trace.dat file a TRACECMD_OPTION_GUEST
option is written for each guest, describing guest VCPU - host PID mapping
and guest's trace ID is used to identify guests.
The host's trace ID is sent to the guest using tracecmd_msg_recv_trace_req()
and indeed it is not used anywhere, but I prefer to keep this exchange
part of the
trace request / reply negotiation, it could be useful in the future.

>
> > >         if (ret < 0)
> > >                 die("Failed to receive trace request");
> > >
> > > @@ -153,13 +155,13 @@ static void agent_handle(int sd, int nr_cpus, int page_size)
> > >
> > >         if (!use_fifos)
> > >                 make_vsocks(nr_cpus, fds, ports);
> > > -
> > > +       trace_id = tracecmd_generate_traceid();
> > >         ret = tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size,
> > > -                                          ports, use_fifos);
> > > +                                          ports, use_fifos, trace_id);
> > >         if (ret < 0)
> > >                 die("Failed to send trace response");
> > > -
> > > -       trace_record_agent(msg_handle, nr_cpus, fds, argc, argv, use_fifos);
> > > +       trace_record_agent(msg_handle, nr_cpus, fds, argc, argv,
> > > +                          use_fifos, trace_id);
> > >
> > >         free(argv[0]);
> > >         free(argv);
> >
> > Cheers,
> >
> > -- Slavi
>
>
>
> --
> Tzvetomir (Ceco) Stoyanov
> VMware Open Source Technology Center
diff mbox series

Patch

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 82def44..90fa146 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -353,16 +353,20 @@  bool tracecmd_msg_done(struct tracecmd_msg_handle *msg_handle);
 void tracecmd_msg_set_done(struct tracecmd_msg_handle *msg_handle);
 
 int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
-				int argc, char **argv, bool use_fifos);
+				int argc, char **argv, bool use_fifos,
+				unsigned long long trace_id);
 int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
-				int *argc, char ***argv, bool *use_fifos);
+				int *argc, char ***argv, bool *use_fifos,
+				unsigned long long *trace_id);
 
 int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int nr_cpus, int page_size,
-				 unsigned int *ports, bool use_fifos);
+				 unsigned int *ports, bool use_fifos,
+				 unsigned long long trace_id);
 int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int *nr_cpus, int *page_size,
-				 unsigned int **ports, bool *use_fifos);
+				 unsigned int **ports, bool *use_fifos,
+				 unsigned long long *trace_id);
 /* --- ftrace instances --- */
 
 struct tracecmd_instance {
diff --git a/lib/trace-cmd/trace-msg.c b/lib/trace-cmd/trace-msg.c
index 9c8a690..a34297f 100644
--- a/lib/trace-cmd/trace-msg.c
+++ b/lib/trace-cmd/trace-msg.c
@@ -63,12 +63,14 @@  struct tracecmd_msg_rinit {
 struct tracecmd_msg_trace_req {
 	be32 flags;
 	be32 argc;
+	u64 trace_id;
 } __attribute__((packed));
 
 struct tracecmd_msg_trace_resp {
 	be32 flags;
 	be32 cpus;
 	be32 page_size;
+	u64 trace_id;
 } __attribute__((packed));
 
 struct tracecmd_msg_header {
@@ -811,7 +813,8 @@  int tracecmd_msg_wait_close_resp(struct tracecmd_msg_handle *msg_handle)
 	return tracecmd_msg_wait_for_cmd(msg_handle, MSG_CLOSE_RESP);
 }
 
-static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv, bool use_fifos)
+static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv,
+			  bool use_fifos, unsigned long long trace_id)
 {
 	size_t args_size = 0;
 	char *p;
@@ -823,6 +826,7 @@  static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv, bool
 	msg->hdr.size = htonl(ntohl(msg->hdr.size) + args_size);
 	msg->trace_req.flags = use_fifos ? htonl(MSG_TRACE_USE_FIFOS) : htonl(0);
 	msg->trace_req.argc = htonl(argc);
+	msg->trace_req.trace_id = htonll(trace_id);
 	msg->buf = calloc(args_size, 1);
 	if (!msg->buf)
 		return -ENOMEM;
@@ -835,13 +839,15 @@  static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv, bool
 }
 
 int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
-				int argc, char **argv, bool use_fifos)
+				int argc, char **argv, bool use_fifos,
+				unsigned long long trace_id)
 {
 	struct tracecmd_msg msg;
 	int ret;
 
 	tracecmd_msg_init(MSG_TRACE_REQ, &msg);
-	ret = make_trace_req(&msg, argc, argv, use_fifos);
+	ret = make_trace_req(&msg, argc, argv,
+			     use_fifos, trace_id);
 	if (ret < 0)
 		return ret;
 
@@ -854,7 +860,8 @@  int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
   *     free(argv);
   */
 int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
-				int *argc, char ***argv, bool *use_fifos)
+				int *argc, char ***argv, bool *use_fifos,
+				unsigned long long *trace_id)
 {
 	struct tracecmd_msg msg;
 	char *p, *buf_end, **args;
@@ -901,6 +908,7 @@  int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
 	*argc = nr_args;
 	*argv = args;
 	*use_fifos = ntohl(msg.trace_req.flags) & MSG_TRACE_USE_FIFOS;
+	*trace_id = ntohll(msg.trace_req.trace_id);
 
 	/*
 	 * On success we're passing msg.buf to the caller through argv[0] so we
@@ -921,7 +929,8 @@  out:
 }
 
 static int make_trace_resp(struct tracecmd_msg *msg, int page_size, int nr_cpus,
-			   unsigned int *ports, bool use_fifos)
+			   unsigned int *ports, bool use_fifos,
+			   unsigned long long trace_id)
 {
 	int data_size;
 
@@ -935,19 +944,22 @@  static int make_trace_resp(struct tracecmd_msg *msg, int page_size, int nr_cpus,
 	msg->trace_resp.flags = use_fifos ? htonl(MSG_TRACE_USE_FIFOS) : htonl(0);
 	msg->trace_resp.cpus = htonl(nr_cpus);
 	msg->trace_resp.page_size = htonl(page_size);
+	msg->trace_resp.trace_id = htonll(trace_id);
 
 	return 0;
 }
 
 int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int nr_cpus, int page_size,
-				 unsigned int *ports, bool use_fifos)
+				 unsigned int *ports, bool use_fifos,
+				 unsigned long long trace_id)
 {
 	struct tracecmd_msg msg;
 	int ret;
 
 	tracecmd_msg_init(MSG_TRACE_RESP, &msg);
-	ret = make_trace_resp(&msg, page_size, nr_cpus, ports, use_fifos);
+	ret = make_trace_resp(&msg, page_size, nr_cpus, ports,
+			      use_fifos, trace_id);
 	if (ret < 0)
 		return ret;
 
@@ -956,13 +968,15 @@  int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
 
 int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int *nr_cpus, int *page_size,
-				 unsigned int **ports, bool *use_fifos)
+				 unsigned int **ports, bool *use_fifos,
+				 unsigned long long *trace_id)
 {
 	struct tracecmd_msg msg;
 	char *p, *buf_end;
 	ssize_t buf_len;
 	int i, ret;
 
+	memset(&msg, 0, sizeof(msg));
 	ret = tracecmd_msg_recv(msg_handle->fd, &msg);
 	if (ret < 0)
 		return ret;
@@ -981,6 +995,7 @@  int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
 	*use_fifos = ntohl(msg.trace_resp.flags) & MSG_TRACE_USE_FIFOS;
 	*nr_cpus = ntohl(msg.trace_resp.cpus);
 	*page_size = ntohl(msg.trace_resp.page_size);
+	*trace_id = ntohll(msg.trace_resp.trace_id);
 	*ports = calloc(*nr_cpus, sizeof(**ports));
 	if (!*ports) {
 		ret = -ENOMEM;
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 6203e0d..a6f79c5 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -98,8 +98,9 @@  void trace_usage(int argc, char **argv);
 
 int trace_record_agent(struct tracecmd_msg_handle *msg_handle,
 		       int cpus, int *fds,
-		       int argc, char **argv, bool use_fifos);
-
+		       int argc, char **argv,
+		       bool use_fifos,
+		       unsigned long long trace_id);
 struct hook_list;
 
 void trace_init_profile(struct tracecmd_input *handle, struct hook_list *hooks,
diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c
index 1c6e0a3..cc330b7 100644
--- a/tracecmd/trace-agent.c
+++ b/tracecmd/trace-agent.c
@@ -128,6 +128,7 @@  cleanup:
 static void agent_handle(int sd, int nr_cpus, int page_size)
 {
 	struct tracecmd_msg_handle *msg_handle;
+	unsigned long long trace_id;
 	unsigned int *ports;
 	char **argv = NULL;
 	int argc = 0;
@@ -144,7 +145,8 @@  static void agent_handle(int sd, int nr_cpus, int page_size)
 	if (!msg_handle)
 		die("Failed to allocate message handle");
 
-	ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv, &use_fifos);
+	ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv,
+					  &use_fifos, &trace_id);
 	if (ret < 0)
 		die("Failed to receive trace request");
 
@@ -153,13 +155,13 @@  static void agent_handle(int sd, int nr_cpus, int page_size)
 
 	if (!use_fifos)
 		make_vsocks(nr_cpus, fds, ports);
-
+	trace_id = tracecmd_generate_traceid();
 	ret = tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size,
-					   ports, use_fifos);
+					   ports, use_fifos, trace_id);
 	if (ret < 0)
 		die("Failed to send trace response");
-
-	trace_record_agent(msg_handle, nr_cpus, fds, argc, argv, use_fifos);
+	trace_record_agent(msg_handle, nr_cpus, fds, argc, argv,
+			   use_fifos, trace_id);
 
 	free(argv[0]);
 	free(argv);
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 01a74f1..e51134f 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3507,12 +3507,16 @@  static void connect_to_agent(struct buffer_instance *instance)
 		die("Failed to allocate message handle");
 
 	ret = tracecmd_msg_send_trace_req(msg_handle, instance->argc,
-					  instance->argv, use_fifos);
+					  instance->argv, use_fifos,
+					  top_instance.trace_id);
+
 	if (ret < 0)
 		die("Failed to send trace request");
 
 	ret = tracecmd_msg_recv_trace_resp(msg_handle, &nr_cpus, &page_size,
-					   &ports, &use_fifos);
+					   &ports, &use_fifos,
+					   &instance->trace_id);
+
 	if (ret < 0)
 		die("Failed to receive trace response");
 
@@ -6117,7 +6121,8 @@  void trace_record(int argc, char **argv)
 int trace_record_agent(struct tracecmd_msg_handle *msg_handle,
 		       int cpus, int *fds,
 		       int argc, char **argv,
-		       bool use_fifos)
+		       bool use_fifos,
+		       unsigned long long trace_id)
 {
 	struct common_record_context ctx;
 	char **argv_plus;
@@ -6147,6 +6152,7 @@  int trace_record_agent(struct tracecmd_msg_handle *msg_handle,
 	ctx.instance->flags |= BUFFER_FL_AGENT;
 	ctx.instance->msg_handle = msg_handle;
 	msg_handle->version = V3_PROTOCOL;
+	top_instance.trace_id = trace_id;
 	record_trace(argc, argv, &ctx);
 
 	free(argv_plus);