* [RFC PATCH v6 01/11] trace-cmd: Detect if vsockets are available
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 02/11] trace-cmd: Add tracecmd_create_recorder_virt function Slavomir Kaslev
` (9 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Detect and define VSOCK if vsockets are available on the system.
This macro is used to disable VM remote tracing features on older kernels.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
Makefile | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Makefile b/Makefile
index f0f7e92..ce16b83 100644
--- a/Makefile
+++ b/Makefile
@@ -204,6 +204,13 @@ CFLAGS ?= -g -Wall
CPPFLAGS ?=
LDFLAGS ?=
+VSOCK_DEFINED := $(shell if (echo "\#include <linux/vm_sockets.h>" | $(CC) -E - >/dev/null 2>&1) ; then echo 1; else echo 0 ; fi)
+
+export VSOCK_DEFINED
+ifeq ($(VSOCK_DEFINED), 1)
+CFLAGS += -DVSOCK
+endif
+
export CFLAGS
export INCLUDES
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC PATCH v6 02/11] trace-cmd: Add tracecmd_create_recorder_virt function
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 01/11] trace-cmd: Detect if vsockets are available Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 03/11] trace-cmd: Add TRACE_REQ and TRACE_RESP messages Slavomir Kaslev
` (8 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
Add tracecmd_create_recorder_virt function that will be used for
tracing VM guests.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
include/trace-cmd/trace-cmd.h | 1 +
lib/trace-cmd/trace-recorder.c | 53 ++++++++++++++++++++++++----------
2 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index ca4452b..5961355 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -277,6 +277,7 @@ enum {
void tracecmd_free_recorder(struct tracecmd_recorder *recorder);
struct tracecmd_recorder *tracecmd_create_recorder(const char *file, int cpu, unsigned flags);
struct tracecmd_recorder *tracecmd_create_recorder_fd(int fd, int cpu, unsigned flags);
+struct tracecmd_recorder *tracecmd_create_recorder_virt(const char *file, int cpu, unsigned flags, int trace_fd);
struct tracecmd_recorder *tracecmd_create_recorder_maxkb(const char *file, int cpu, unsigned flags, int maxkb);
struct tracecmd_recorder *tracecmd_create_buffer_recorder_fd(int fd, int cpu, unsigned flags, const char *buffer);
struct tracecmd_recorder *tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags, const char *buffer);
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index 0b8d98a..fc71180 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -154,16 +154,22 @@ tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
recorder->fd1 = fd;
recorder->fd2 = fd2;
- if (flags & TRACECMD_RECORD_SNAPSHOT)
- ret = asprintf(&path, "%s/per_cpu/cpu%d/snapshot_raw", buffer, cpu);
- else
- ret = asprintf(&path, "%s/per_cpu/cpu%d/trace_pipe_raw", buffer, cpu);
- if (ret < 0)
- goto out_free;
+ if (buffer) {
+ if (flags & TRACECMD_RECORD_SNAPSHOT)
+ ret = asprintf(&path, "%s/per_cpu/cpu%d/snapshot_raw",
+ buffer, cpu);
+ else
+ ret = asprintf(&path, "%s/per_cpu/cpu%d/trace_pipe_raw",
+ buffer, cpu);
+ if (ret < 0)
+ goto out_free;
+
+ recorder->trace_fd = open(path, O_RDONLY);
+ free(path);
- recorder->trace_fd = open(path, O_RDONLY);
- if (recorder->trace_fd < 0)
- goto out_free;
+ if (recorder->trace_fd < 0)
+ goto out_free;
+ }
if ((recorder->flags & TRACECMD_RECORD_NOSPLICE) == 0) {
ret = pipe(recorder->brass);
@@ -183,13 +189,9 @@ tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
recorder->pipe_size = pipe_size;
}
- free(path);
-
return recorder;
out_free:
- free(path);
-
tracecmd_free_recorder(recorder);
return NULL;
}
@@ -200,8 +202,9 @@ tracecmd_create_buffer_recorder_fd(int fd, int cpu, unsigned flags, const char *
return tracecmd_create_buffer_recorder_fd2(fd, -1, cpu, flags, buffer, 0);
}
-struct tracecmd_recorder *
-tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags, const char *buffer)
+static struct tracecmd_recorder *
+__tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
+ const char *buffer)
{
struct tracecmd_recorder *recorder;
int fd;
@@ -264,6 +267,26 @@ tracecmd_create_buffer_recorder_maxkb(const char *file, int cpu, unsigned flags,
goto out;
}
+struct tracecmd_recorder *
+tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
+ const char *buffer)
+{
+ return __tracecmd_create_buffer_recorder(file, cpu, flags, buffer);
+}
+
+struct tracecmd_recorder *
+tracecmd_create_recorder_virt(const char *file, int cpu, unsigned flags,
+ int trace_fd)
+{
+ struct tracecmd_recorder *recorder;
+
+ recorder = __tracecmd_create_buffer_recorder(file, cpu, flags, NULL);
+ if (recorder)
+ recorder->trace_fd = trace_fd;
+
+ return recorder;
+}
+
struct tracecmd_recorder *tracecmd_create_recorder_fd(int fd, int cpu, unsigned flags)
{
const char *tracing;
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC PATCH v6 03/11] trace-cmd: Add TRACE_REQ and TRACE_RESP messages
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 01/11] trace-cmd: Detect if vsockets are available Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 02/11] trace-cmd: Add tracecmd_create_recorder_virt function Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 18:51 ` Steven Rostedt
2019-02-14 14:13 ` [RFC PATCH v6 04/11] trace-cmd: Add buffer instance flags for tracing in guest and agent context Slavomir Kaslev
` (7 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
Add TRACE_REQ and TRACE_RESP messages which are used for initiating guest VM
tracing.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
include/trace-cmd/trace-cmd.h | 12 ++
tracecmd/trace-msg.c | 208 +++++++++++++++++++++++++++++++++-
2 files changed, 219 insertions(+), 1 deletion(-)
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 5961355..6a21e66 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -331,6 +331,18 @@ 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);
+int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
+ int argc, char **argv);
+int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
+ int *argc, char ***argv);
+
+int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
+ int nr_cpus, int page_size,
+ unsigned int *ports);
+int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
+ int *nr_cpus, int *page_size,
+ unsigned int **ports);
+
/* --- Plugin handling --- */
extern struct tep_plugin_option trace_ftrace_options[];
diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c
index 51d0ac8..8b0a4e5 100644
--- a/tracecmd/trace-msg.c
+++ b/tracecmd/trace-msg.c
@@ -16,6 +16,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
+#include <string.h>
#include <unistd.h>
#include <arpa/inet.h>
#include <sys/types.h>
@@ -64,6 +65,17 @@ struct tracecmd_msg_rinit {
be32 cpus;
} __attribute__((packed));
+struct tracecmd_msg_trace_req {
+ be32 flags;
+ be32 argc;
+} __attribute__((packed));
+
+struct tracecmd_msg_trace_resp {
+ be32 flags;
+ be32 cpus;
+ be32 page_size;
+} __attribute__((packed));
+
struct tracecmd_msg_header {
be32 size;
be32 cmd;
@@ -76,7 +88,9 @@ struct tracecmd_msg_header {
C(RINIT, 2, sizeof(struct tracecmd_msg_rinit)), \
C(SEND_DATA, 3, 0), \
C(FIN_DATA, 4, 0), \
- C(NOT_SUPP, 5, 0),
+ C(NOT_SUPP, 5, 0), \
+ C(TRACE_REQ, 6, sizeof(struct tracecmd_msg_trace_req)), \
+ C(TRACE_RESP, 7, sizeof(struct tracecmd_msg_trace_resp)),
#undef C
#define C(a,b,c) MSG_##a = b
@@ -108,6 +122,8 @@ struct tracecmd_msg {
union {
struct tracecmd_msg_tinit tinit;
struct tracecmd_msg_rinit rinit;
+ struct tracecmd_msg_trace_req trace_req;
+ struct tracecmd_msg_trace_resp trace_resp;
};
union {
struct tracecmd_msg_opt *opt;
@@ -723,3 +739,193 @@ error:
msg_free(&msg);
return ret;
}
+
+static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv)
+{
+ size_t args_size = 0;
+ char *p;
+ int i;
+
+ for (i = 0; i < argc; i++)
+ args_size += strlen(argv[i]) + 1;
+
+ msg->hdr.size = htonl(ntohl(msg->hdr.size) + args_size);
+ msg->trace_req.argc = htonl(argc);
+ msg->buf = calloc(args_size, 1);
+ if (!msg->buf)
+ return -ENOMEM;
+
+ p = msg->buf;
+ for (i = 0; i < argc; i++)
+ p = stpcpy(p, argv[i]) + 1;
+
+ return 0;
+}
+
+int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
+ int argc, char **argv)
+{
+ struct tracecmd_msg msg;
+ int ret;
+
+ tracecmd_msg_init(MSG_TRACE_REQ, &msg);
+ ret = make_trace_req(&msg, argc, argv);
+ if (ret < 0)
+ return ret;
+
+ return tracecmd_msg_send(msg_handle->fd, &msg);
+}
+
+ /*
+ * NOTE: On success, the returned `argv` should be freed with:
+ * free(argv[0]);
+ * free(argv);
+ */
+int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
+ int *argc, char ***argv)
+{
+ struct tracecmd_msg msg;
+ char *p, *buf_end, **args;
+ int i, ret, nr_args;
+ ssize_t buf_len;
+
+ ret = tracecmd_msg_recv(msg_handle->fd, &msg);
+ if (ret < 0)
+ return ret;
+
+ if (ntohl(msg.hdr.cmd) != MSG_TRACE_REQ) {
+ ret = -ENOTSUP;
+ goto out;
+ }
+
+ nr_args = ntohl(msg.trace_req.argc);
+ if (nr_args <= 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ buf_len = ntohl(msg.hdr.size) - MSG_HDR_LEN - ntohl(msg.hdr.cmd_size);
+ buf_end = (char *)msg.buf + buf_len;
+ if (buf_len <= 0 && ((char *)msg.buf)[buf_len-1] != '\0') {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ args = calloc(nr_args, sizeof(*args));
+ if (!args) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ p = msg.buf;
+ for (i = 0; i < nr_args; i++) {
+ if (p >= buf_end) {
+ ret = -EINVAL;
+ goto out_args;
+ }
+ args[i] = p;
+ p = strchr(p, '\0');
+ if (!p) {
+ ret = -EINVAL;
+ goto out_args;
+ }
+ p++;
+ }
+
+ /*
+ * On success we're passing msg.buf to the caller through argv[0] so we
+ * don't msg_free in this case.
+ */
+ *argc = nr_args;
+ *argv = args;
+ return 0;
+
+out_args:
+ free(args);
+out:
+ error_operation(&msg);
+ if (ret == -EOPNOTSUPP)
+ handle_unexpected_msg(msg_handle, &msg);
+ msg_free(&msg);
+ return ret;
+}
+
+static int make_trace_resp(struct tracecmd_msg *msg,
+ int page_size, int nr_cpus, unsigned int *ports)
+{
+ int ports_size = nr_cpus * sizeof(*msg->port_array);
+ int i;
+
+ msg->hdr.size = htonl(ntohl(msg->hdr.size) + ports_size);
+ msg->trace_resp.cpus = htonl(nr_cpus);
+ msg->trace_resp.page_size = htonl(page_size);
+
+ msg->port_array = malloc(ports_size);
+ if (!msg->port_array)
+ return -ENOMEM;
+
+ for (i = 0; i < nr_cpus; i++)
+ msg->port_array[i] = htonl(ports[i]);
+
+ return 0;
+}
+
+int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
+ int nr_cpus, int page_size,
+ unsigned int *ports)
+{
+ struct tracecmd_msg msg;
+ int ret;
+
+ tracecmd_msg_init(MSG_TRACE_RESP, &msg);
+ ret = make_trace_resp(&msg, page_size, nr_cpus, ports);
+ if (ret < 0)
+ return ret;
+
+ return tracecmd_msg_send(msg_handle->fd, &msg);
+}
+
+int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
+ int *nr_cpus, int *page_size,
+ unsigned int **ports)
+{
+ struct tracecmd_msg msg;
+ ssize_t buf_len;
+ int i, ret;
+
+ ret = tracecmd_msg_recv(msg_handle->fd, &msg);
+ if (ret < 0)
+ return ret;
+
+ if (ntohl(msg.hdr.cmd) != MSG_TRACE_RESP) {
+ ret = -ENOTSUP;
+ goto out;
+ }
+
+ buf_len = ntohl(msg.hdr.size) - MSG_HDR_LEN - ntohl(msg.hdr.cmd_size);
+ if (buf_len <= 0 ||
+ buf_len != sizeof(*msg.port_array) * ntohl(msg.trace_resp.cpus)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ *nr_cpus = ntohl(msg.trace_resp.cpus);
+ *page_size = ntohl(msg.trace_resp.page_size);
+ *ports = calloc(*nr_cpus, sizeof(**ports));
+ if (!*ports) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ for (i = 0; i < *nr_cpus; i++)
+ (*ports)[i] = ntohl(msg.port_array[i]);
+
+ msg_free(&msg);
+ return 0;
+
+out:
+ error_operation(&msg);
+ if (ret == -EOPNOTSUPP)
+ handle_unexpected_msg(msg_handle, &msg);
+ msg_free(&msg);
+ return ret;
+}
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 03/11] trace-cmd: Add TRACE_REQ and TRACE_RESP messages
2019-02-14 14:13 ` [RFC PATCH v6 03/11] trace-cmd: Add TRACE_REQ and TRACE_RESP messages Slavomir Kaslev
@ 2019-02-14 18:51 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-02-14 18:51 UTC (permalink / raw)
To: Slavomir Kaslev; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, 14 Feb 2019 16:13:27 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:
> + /*
> + * NOTE: On success, the returned `argv` should be freed with:
> + * free(argv[0]);
> + * free(argv);
> + */
> +int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
> + int *argc, char ***argv)
> +{
> + struct tracecmd_msg msg;
> + char *p, *buf_end, **args;
> + int i, ret, nr_args;
> + ssize_t buf_len;
> +
> + ret = tracecmd_msg_recv(msg_handle->fd, &msg);
> + if (ret < 0)
> + return ret;
> +
> + if (ntohl(msg.hdr.cmd) != MSG_TRACE_REQ) {
> + ret = -ENOTSUP;
> + goto out;
> + }
> +
> + nr_args = ntohl(msg.trace_req.argc);
> + if (nr_args <= 0) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + buf_len = ntohl(msg.hdr.size) - MSG_HDR_LEN - ntohl(msg.hdr.cmd_size);
> + buf_end = (char *)msg.buf + buf_len;
> + if (buf_len <= 0 && ((char *)msg.buf)[buf_len-1] != '\0') {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + args = calloc(nr_args, sizeof(*args));
> + if (!args) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + p = msg.buf;
> + for (i = 0; i < nr_args; i++) {
> + if (p >= buf_end) {
> + ret = -EINVAL;
> + goto out_args;
> + }
> + args[i] = p;
> + p = strchr(p, '\0');
> + if (!p) {
> + ret = -EINVAL;
> + goto out_args;
> + }
> + p++;
> + }
> +
> + /*
> + * On success we're passing msg.buf to the caller through argv[0] so we
> + * don't msg_free in this case.
> + */
This is OK for now, but I'm still wondering if we should just add:
msg.buf = NULL;
msg_free(&msg);
For robustness. What happens if for some reason in the future that we
add another allocation to msg? That would cause this to leak memory.
Yeah, yeah, this is highly paranoid, but code like this is exactly how
memory leaks come about and other bugs come about. It's taking advantage
of internals of how msg_free() works to optimize the case. And when the
internals change, things break.
No need to change anything here, I may just add the code on top of this
series.
-- Steve
> + *argc = nr_args;
> + *argv = args;
> + return 0;
> +
> +out_args:
> + free(args);
> +out:
> + error_operation(&msg);
> + if (ret == -EOPNOTSUPP)
> + handle_unexpected_msg(msg_handle, &msg);
> + msg_free(&msg);
> + return ret;
> +}
> +
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH v6 04/11] trace-cmd: Add buffer instance flags for tracing in guest and agent context
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (2 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 03/11] trace-cmd: Add TRACE_REQ and TRACE_RESP messages Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 20:05 ` Steven Rostedt
2019-02-14 14:13 ` [RFC PATCH v6 05/11] trace-cmd: Add VM kernel tracing over vsockets transport Slavomir Kaslev
` (6 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
Add BUFFER_FL_GUEST and BUFFER_FL_AGENT flags to differentiate when
trace-record.c is being called to trace guest or the VM tracing agent.
Also disable functions talking to the local tracefs when called in recording
guest instances context.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
tracecmd/include/trace-local.h | 2 ++
tracecmd/trace-record.c | 55 ++++++++++++++++++++++++++++++++--
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index a1a06e9..f19c8bb 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -149,6 +149,8 @@ char *strstrip(char *str);
enum buffer_instance_flags {
BUFFER_FL_KEEP = 1 << 0,
BUFFER_FL_PROFILE = 1 << 1,
+ BUFFER_FL_GUEST = 1 << 2,
+ BUFFER_FL_AGENT = 1 << 3,
};
struct func_list {
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 8beefab..107d3d1 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -781,6 +781,9 @@ static void __clear_trace(struct buffer_instance *instance)
FILE *fp;
char *path;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
/* reset the trace */
path = get_instance_file(instance, "trace");
fp = fopen(path, "w");
@@ -1264,6 +1267,9 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
char *path;
char zero = '0';
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
path = get_instance_file(instance, "current_tracer");
fp = fopen(path, "w");
if (!fp) {
@@ -1360,6 +1366,9 @@ static void disable_func_stack_trace_instance(struct buffer_instance *instance)
int size;
int ret;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
path = get_instance_file(instance, "current_tracer");
ret = stat(path, &st);
tracecmd_put_tracing_file(path);
@@ -1553,6 +1562,9 @@ reset_events_instance(struct buffer_instance *instance)
int i;
int ret;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
if (use_old_event_method()) {
/* old way only had top instance */
if (!is_top_instance(instance))
@@ -1904,6 +1916,9 @@ static void write_tracing_on(struct buffer_instance *instance, int on)
int ret;
int fd;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
fd = open_tracing_on(instance);
if (fd < 0)
return;
@@ -1923,6 +1938,9 @@ static int read_tracing_on(struct buffer_instance *instance)
char buf[10];
int ret;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return -1;
+
fd = open_tracing_on(instance);
if (fd < 0)
return fd;
@@ -2156,6 +2174,9 @@ static void set_mask(struct buffer_instance *instance)
int fd;
int ret;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
if (!instance->cpumask)
return;
@@ -2186,6 +2207,9 @@ static void enable_events(struct buffer_instance *instance)
{
struct event_list *event;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
for (event = instance->events; event; event = event->next) {
if (!event->neg)
update_event(event, event->filter, 0, '1');
@@ -2209,6 +2233,9 @@ static void set_clock(struct buffer_instance *instance)
char *content;
char *str;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
if (!instance->clock)
return;
@@ -2238,6 +2265,9 @@ static void set_max_graph_depth(struct buffer_instance *instance, char *max_grap
char *path;
int ret;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
path = get_instance_file(instance, "max_graph_depth");
reset_save_file(path, RESET_DEFAULT_PRIO);
tracecmd_put_tracing_file(path);
@@ -2463,6 +2493,9 @@ static void expand_event_instance(struct buffer_instance *instance)
struct event_list *compressed_list = instance->events;
struct event_list *event;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
reset_event_list(instance);
while (compressed_list) {
@@ -3336,6 +3369,9 @@ static void set_funcs(struct buffer_instance *instance)
int set_notrace = 0;
int ret;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
ret = write_func_file(instance, "set_ftrace_filter", &instance->filter_funcs);
if (ret < 0)
die("set_ftrace_filter does not exist. Can not filter functions");
@@ -3632,6 +3668,9 @@ static void set_buffer_size_instance(struct buffer_instance *instance)
int ret;
int fd;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
if (!buffer_size)
return;
@@ -3829,6 +3868,9 @@ static void make_instances(void)
int ret;
for_each_instance(instance) {
+ if (instance->flags & BUFFER_FL_GUEST)
+ continue;
+
path = get_instance_dir(instance);
ret = stat(path, &st);
if (ret < 0) {
@@ -3850,7 +3892,7 @@ void tracecmd_remove_instances(void)
for_each_instance(instance) {
/* Only delete what we created */
- if (instance->flags & BUFFER_FL_KEEP)
+ if (instance->flags & (BUFFER_FL_KEEP | BUFFER_FL_GUEST))
continue;
if (instance->tracing_on_fd > 0) {
close(instance->tracing_on_fd);
@@ -3932,7 +3974,7 @@ static void check_function_plugin(void)
static int __check_doing_something(struct buffer_instance *instance)
{
- return (instance->flags & BUFFER_FL_PROFILE) ||
+ return (instance->flags & (BUFFER_FL_PROFILE | BUFFER_FL_GUEST)) ||
instance->plugin || instance->events;
}
@@ -3954,6 +3996,9 @@ update_plugin_instance(struct buffer_instance *instance,
{
const char *plugin = instance->plugin;
+ if (instance->flags & BUFFER_FL_GUEST)
+ return;
+
if (!plugin)
return;
@@ -4053,6 +4098,9 @@ static void record_stats(void)
int cpu;
for_all_instances(instance) {
+ if (instance->flags & BUFFER_FL_GUEST)
+ continue;
+
s_save = instance->s_save;
s_print = instance->s_print;
for (cpu = 0; cpu < instance->cpu_count; cpu++) {
@@ -4079,6 +4127,9 @@ static void destroy_stats(void)
int cpu;
for_all_instances(instance) {
+ if (instance->flags & BUFFER_FL_GUEST)
+ continue;
+
for (cpu = 0; cpu < instance->cpu_count; cpu++) {
trace_seq_destroy(&instance->s_save[cpu]);
trace_seq_destroy(&instance->s_print[cpu]);
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 04/11] trace-cmd: Add buffer instance flags for tracing in guest and agent context
2019-02-14 14:13 ` [RFC PATCH v6 04/11] trace-cmd: Add buffer instance flags for tracing in guest and agent context Slavomir Kaslev
@ 2019-02-14 20:05 ` Steven Rostedt
2019-02-18 14:24 ` Slavomir Kaslev
0 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2019-02-14 20:05 UTC (permalink / raw)
To: Slavomir Kaslev; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, 14 Feb 2019 16:13:28 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:
> Add BUFFER_FL_GUEST and BUFFER_FL_AGENT flags to differentiate when
> trace-record.c is being called to trace guest or the VM tracing agent.
>
> Also disable functions talking to the local tracefs when called in recording
> guest instances context.
>
> Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
> ---
> tracecmd/include/trace-local.h | 2 ++
> tracecmd/trace-record.c | 55 ++++++++++++++++++++++++++++++++--
> 2 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index a1a06e9..f19c8bb 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -149,6 +149,8 @@ char *strstrip(char *str);
> enum buffer_instance_flags {
> BUFFER_FL_KEEP = 1 << 0,
> BUFFER_FL_PROFILE = 1 << 1,
> + BUFFER_FL_GUEST = 1 << 2,
> + BUFFER_FL_AGENT = 1 << 3,
> };
>
> struct func_list {
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 8beefab..107d3d1 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -781,6 +781,9 @@ static void __clear_trace(struct buffer_instance *instance)
> FILE *fp;
> char *path;
>
> + if (instance->flags & BUFFER_FL_GUEST)
> + return;
> +
> /* reset the trace */
> path = get_instance_file(instance, "trace");
> fp = fopen(path, "w");
> @@ -1264,6 +1267,9 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
> char *path;
> char zero = '0';
>
> + if (instance->flags & BUFFER_FL_GUEST)
> + return;
> +
> path = get_instance_file(instance, "current_tracer");
> fp = fopen(path, "w");
> if (!fp) {
> @@ -1360,6 +1366,9 @@ static void disable_func_stack_trace_instance(struct buffer_instance *instance)
> int size;
> int ret;
>
> + if (instance->flags & BUFFER_FL_GUEST)
> + return;
> +
> path = get_instance_file(instance, "current_tracer");
> ret = stat(path, &st);
> tracecmd_put_tracing_file(path);
I think we should add a:
#define is_guest(instance) ((instance)->flags & BUFFER_FL_GUEST)
Helper macro here, and all these should be turned into:
if (is_guest(instance))
return;
Less error prone, and looks cleaner.
We could add a is_agent() too in later patches.
-- Steve
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 04/11] trace-cmd: Add buffer instance flags for tracing in guest and agent context
2019-02-14 20:05 ` Steven Rostedt
@ 2019-02-18 14:24 ` Slavomir Kaslev
0 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-18 14:24 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, Feb 14, 2019 at 03:05:20PM -0500, Steven Rostedt wrote:
> On Thu, 14 Feb 2019 16:13:28 +0200
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
>
> > Add BUFFER_FL_GUEST and BUFFER_FL_AGENT flags to differentiate when
> > trace-record.c is being called to trace guest or the VM tracing agent.
> >
> > Also disable functions talking to the local tracefs when called in recording
> > guest instances context.
> >
> > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
> > ---
> > tracecmd/include/trace-local.h | 2 ++
> > tracecmd/trace-record.c | 55 ++++++++++++++++++++++++++++++++--
> > 2 files changed, 55 insertions(+), 2 deletions(-)
> >
> > diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> > index a1a06e9..f19c8bb 100644
> > --- a/tracecmd/include/trace-local.h
> > +++ b/tracecmd/include/trace-local.h
> > @@ -149,6 +149,8 @@ char *strstrip(char *str);
> > enum buffer_instance_flags {
> > BUFFER_FL_KEEP = 1 << 0,
> > BUFFER_FL_PROFILE = 1 << 1,
> > + BUFFER_FL_GUEST = 1 << 2,
> > + BUFFER_FL_AGENT = 1 << 3,
> > };
> >
> > struct func_list {
> > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> > index 8beefab..107d3d1 100644
> > --- a/tracecmd/trace-record.c
> > +++ b/tracecmd/trace-record.c
> > @@ -781,6 +781,9 @@ static void __clear_trace(struct buffer_instance *instance)
> > FILE *fp;
> > char *path;
> >
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + return;
> > +
> > /* reset the trace */
> > path = get_instance_file(instance, "trace");
> > fp = fopen(path, "w");
> > @@ -1264,6 +1267,9 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
> > char *path;
> > char zero = '0';
> >
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + return;
> > +
> > path = get_instance_file(instance, "current_tracer");
> > fp = fopen(path, "w");
> > if (!fp) {
> > @@ -1360,6 +1366,9 @@ static void disable_func_stack_trace_instance(struct buffer_instance *instance)
> > int size;
> > int ret;
> >
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + return;
> > +
> > path = get_instance_file(instance, "current_tracer");
> > ret = stat(path, &st);
> > tracecmd_put_tracing_file(path);
>
>
> I think we should add a:
>
> #define is_guest(instance) ((instance)->flags & BUFFER_FL_GUEST)
>
> Helper macro here, and all these should be turned into:
>
> if (is_guest(instance))
> return;
>
> Less error prone, and looks cleaner.
>
> We could add a is_agent() too in later patches.
Makes sense. I added both is_agent and is_guest for the next iteration of this
patchset.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH v6 05/11] trace-cmd: Add VM kernel tracing over vsockets transport
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (3 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 04/11] trace-cmd: Add buffer instance flags for tracing in guest and agent context Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 20:03 ` Steven Rostedt
2019-02-14 14:13 ` [RFC PATCH v6 06/11] trace-cmd: Use splice(2) for vsockets if available Slavomir Kaslev
` (5 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
This patch adds VM tracing over vsockets. The new `trace-cmd agent` command
needs run on each guest we want to trace:
you@guest2 # trace-cmd agent
Then `trace-cmd record` on the host can collect data from both the host and
several guests simultaneously:
you@host $ trace-cmd record -A guest1 -e irq -e sched \
-A guest2 -p function -e all
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
tracecmd/Makefile | 6 +-
tracecmd/include/trace-local.h | 16 +
tracecmd/trace-agent.c | 225 ++++++++++++
tracecmd/trace-cmd.c | 3 +
tracecmd/trace-record.c | 605 ++++++++++++++++++++++++++++++---
tracecmd/trace-usage.c | 13 +-
6 files changed, 819 insertions(+), 49 deletions(-)
create mode 100644 tracecmd/trace-agent.c
diff --git a/tracecmd/Makefile b/tracecmd/Makefile
index 3a11024..865b1c6 100644
--- a/tracecmd/Makefile
+++ b/tracecmd/Makefile
@@ -33,13 +33,17 @@ TRACE_CMD_OBJS += trace-output.o
TRACE_CMD_OBJS += trace-usage.o
TRACE_CMD_OBJS += trace-msg.o
+ifeq ($(VSOCK_DEFINED), 1)
+TRACE_CMD_OBJS += trace-agent.o
+endif
+
ALL_OBJS := $(TRACE_CMD_OBJS:%.o=$(bdir)/%.o)
all_objs := $(sort $(ALL_OBJS))
all_deps := $(all_objs:$(bdir)/%.o=$(bdir)/.%.d)
CONFIG_INCLUDES =
-CONFIG_LIBS =
+CONFIG_LIBS = -lrt
CONFIG_FLAGS =
all: $(TARGETS)
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index f19c8bb..823d323 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -12,6 +12,8 @@
#include "trace-cmd.h"
#include "event-utils.h"
+#define TRACE_AGENT_DEFAULT_PORT 823
+
extern int debug;
extern int quiet;
@@ -64,6 +66,8 @@ void trace_split(int argc, char **argv);
void trace_listen(int argc, char **argv);
+void trace_agent(int argc, char **argv);
+
void trace_restore(int argc, char **argv);
void trace_clear(int argc, char **argv);
@@ -88,6 +92,10 @@ void trace_list(int argc, char **argv);
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);
+
struct hook_list;
void trace_init_profile(struct tracecmd_input *handle, struct hook_list *hooks,
@@ -176,6 +184,7 @@ struct buffer_instance {
struct func_list *notrace_funcs;
const char *clock;
+ unsigned int *client_ports;
struct trace_seq *s_save;
struct trace_seq *s_print;
@@ -190,6 +199,13 @@ struct buffer_instance {
int tracing_on_fd;
int buffer_size;
int cpu_count;
+
+ int argc;
+ char **argv;
+
+ int cid;
+ int port;
+ int *fds;
};
extern struct buffer_instance top_instance;
diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c
new file mode 100644
index 0000000..0c0873b
--- /dev/null
+++ b/tracecmd/trace-agent.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 VMware Inc, Slavomir Kaslev <kaslevs@vmware.com>
+ *
+ * based on prior implementation by Yoshihiro Yunomae
+ * Copyright (C) 2013 Hitachi, Ltd.
+ * Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <linux/vm_sockets.h>
+
+#include "trace-local.h"
+#include "trace-msg.h"
+
+static int make_vsock(unsigned int port)
+{
+ struct sockaddr_vm addr = {
+ .svm_family = AF_VSOCK,
+ .svm_cid = VMADDR_CID_ANY,
+ .svm_port = port,
+ };
+ int sd;
+
+ sd = socket(AF_VSOCK, SOCK_STREAM, 0);
+ if (sd < 0)
+ return -errno;
+
+ setsockopt(sd, SOL_SOCKET, SO_REUSEADDR, &(int){1}, sizeof(int));
+
+ if (bind(sd, (struct sockaddr *)&addr, sizeof(addr)))
+ return -errno;
+
+ if (listen(sd, SOMAXCONN))
+ return -errno;
+
+ return sd;
+}
+
+static int get_vsock_port(int sd, unsigned int *port)
+{
+ struct sockaddr_vm addr;
+ socklen_t addr_len = sizeof(addr);
+
+ if (getsockname(sd, (struct sockaddr *)&addr, &addr_len))
+ return -errno;
+
+ if (addr.svm_family != AF_VSOCK)
+ return -errno;
+
+ if (port)
+ *port = addr.svm_port;
+
+ return 0;
+}
+
+static void make_vsocks(int nr, int *fds, unsigned int *ports)
+{
+ unsigned int port;
+ int i, fd, ret;
+
+ for (i = 0; i < nr; i++) {
+ fd = make_vsock(VMADDR_PORT_ANY);
+ if (fd < 0)
+ die("Failed to open vsocket");
+
+ ret = get_vsock_port(fd, &port);
+ if (ret < 0)
+ die("Failed to get vsocket address");
+
+ fds[i] = fd;
+ ports[i] = port;
+ }
+}
+
+static void agent_handle(int sd, int nr_cpus, int page_size)
+{
+ struct tracecmd_msg_handle *msg_handle;
+ unsigned int *ports;
+ char **argv = NULL;
+ int argc = 0;
+ int *fds;
+ int ret;
+
+ fds = calloc(nr_cpus, sizeof(*fds));
+ ports = calloc(nr_cpus, sizeof(*ports));
+ if (!fds || !ports)
+ die("Failed to allocate memory");
+
+ msg_handle = tracecmd_msg_handle_alloc(sd, 0);
+ if (!msg_handle)
+ die("Failed to allocate message handle");
+
+ ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv);
+ if (ret < 0)
+ die("Failed to receive trace request");
+
+ make_vsocks(nr_cpus, fds, ports);
+
+ ret = tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size, ports);
+ if (ret < 0)
+ die("Failed to send trace response");
+
+ trace_record_agent(msg_handle, nr_cpus, fds, argc, argv);
+
+ free(argv[0]);
+ free(argv);
+ free(ports);
+ free(fds);
+ tracecmd_msg_handle_close(msg_handle);
+ exit(0);
+}
+
+static volatile pid_t handler_pid;
+
+static void handle_sigchld(int sig)
+{
+ int wstatus;
+ pid_t pid;
+
+ for (;;) {
+ pid = waitpid(-1, &wstatus, WNOHANG);
+ if (pid <= 0)
+ break;
+
+ if (pid == handler_pid)
+ handler_pid = 0;
+ }
+}
+
+static void agent_serve(unsigned int port)
+{
+ int sd, cd, nr_cpus;
+ pid_t pid;
+
+ signal(SIGCHLD, handle_sigchld);
+
+ nr_cpus = count_cpus();
+ page_size = getpagesize();
+
+ sd = make_vsock(port);
+ if (sd < 0)
+ die("Failed to open vsocket");
+
+ for (;;) {
+ cd = accept(sd, NULL, NULL);
+ if (cd < 0) {
+ if (errno == EINTR)
+ continue;
+ die("accept");
+ }
+
+ if (handler_pid)
+ goto busy;
+
+ pid = fork();
+ if (pid == 0) {
+ close(sd);
+ signal(SIGCHLD, SIG_DFL);
+ agent_handle(cd, nr_cpus, page_size);
+ }
+ if (pid > 0)
+ handler_pid = pid;
+
+busy:
+ close(cd);
+ }
+}
+
+void trace_agent(int argc, char **argv)
+{
+ bool do_daemon = false;
+ unsigned int port = TRACE_AGENT_DEFAULT_PORT;
+
+ if (argc < 2)
+ usage(argv);
+
+ if (strcmp(argv[1], "agent") != 0)
+ usage(argv);
+
+ for (;;) {
+ int c, option_index = 0;
+ static struct option long_options[] = {
+ {"port", required_argument, NULL, 'p'},
+ {"help", no_argument, NULL, '?'},
+ {NULL, 0, NULL, 0}
+ };
+
+ c = getopt_long(argc-1, argv+1, "+hp:D",
+ long_options, &option_index);
+ if (c == -1)
+ break;
+ switch (c) {
+ case 'h':
+ usage(argv);
+ break;
+ case 'p':
+ port = atoi(optarg);
+ break;
+ case 'D':
+ do_daemon = true;
+ break;
+ default:
+ usage(argv);
+ }
+ }
+
+ if (optind < argc-1)
+ usage(argv);
+
+ if (do_daemon && daemon(1, 0))
+ die("daemon");
+
+ agent_serve(port);
+}
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index 797b303..3ae5e2e 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -83,6 +83,9 @@ struct command commands[] = {
{"hist", trace_hist},
{"mem", trace_mem},
{"listen", trace_listen},
+#ifdef VSOCK
+ {"agent", trace_agent},
+#endif
{"split", trace_split},
{"restore", trace_restore},
{"stack", trace_stack},
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 107d3d1..8a1a665 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -33,6 +33,9 @@
#include <errno.h>
#include <limits.h>
#include <libgen.h>
+#ifdef VSOCK
+#include <linux/vm_sockets.h>
+#endif
#include "trace-local.h"
#include "trace-msg.h"
@@ -74,8 +77,6 @@ static int buffers;
static int clear_function_filters;
static char *host;
-static unsigned int *client_ports;
-static int sfd;
/* Max size to let a per cpu file get */
static int max_kb;
@@ -518,6 +519,22 @@ static char *get_temp_file(struct buffer_instance *instance, int cpu)
return file;
}
+static char *get_guest_file(const char *file, const char *guest)
+{
+ const char *p;
+ char *out = NULL;
+ int base_len;
+
+ p = strrchr(file, '.');
+ if (p && p != file)
+ base_len = p - file;
+ else
+ base_len = strlen(file);
+
+ asprintf(&out, "%.*s-%s%s", base_len, file, guest, file + base_len);
+ return out;
+}
+
static void put_temp_file(char *file)
{
free(file);
@@ -623,11 +640,22 @@ static void delete_thread_data(void)
}
}
+static void tell_guests_to_stop(void)
+{
+ struct buffer_instance *instance;
+
+ for_all_instances(instance) {
+ if (instance->flags & BUFFER_FL_GUEST) {
+ tracecmd_msg_send_close_msg(instance->msg_handle);
+ tracecmd_msg_handle_close(instance->msg_handle);
+ }
+ }
+}
+
static void stop_threads(enum trace_type type)
{
struct timeval tv = { 0, 0 };
- int ret;
- int i;
+ int i, ret;
if (!recorder_threads)
return;
@@ -645,6 +673,11 @@ static void stop_threads(enum trace_type type)
ret = trace_stream_read(pids, recorder_threads, &tv);
} while (ret > 0);
}
+}
+
+static void wait_threads()
+{
+ int i;
for (i = 0; i < recorder_threads; i++) {
if (pids[i].pid > 0) {
@@ -2571,14 +2604,14 @@ static void flush(int sig)
tracecmd_stop_recording(recorder);
}
-static void connect_port(int cpu)
+static int connect_port(const char *host, unsigned int port)
{
struct addrinfo hints;
struct addrinfo *results, *rp;
- int s;
+ int s, sfd;
char buf[BUFSIZ];
- snprintf(buf, BUFSIZ, "%u", client_ports[cpu]);
+ snprintf(buf, BUFSIZ, "%u", port);
memset(&hints, 0, sizeof(hints));
hints.ai_family = AF_UNSPEC;
@@ -2605,7 +2638,190 @@ static void connect_port(int cpu)
freeaddrinfo(results);
- client_ports[cpu] = sfd;
+ return sfd;
+}
+
+#ifdef VSOCK
+static int open_vsock(unsigned int cid, unsigned int port)
+{
+ struct sockaddr_vm addr = {
+ .svm_family = AF_VSOCK,
+ .svm_cid = cid,
+ .svm_port = port,
+ };
+ int sd;
+
+ sd = socket(AF_VSOCK, SOCK_STREAM, 0);
+ if (sd < 0)
+ return -1;
+
+ if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)))
+ return -1;
+
+ return sd;
+}
+#else
+static inline int open_vsock(unsigned int cid, unsigned int port)
+{
+ die("vsock is not supported");
+ return -1;
+}
+#endif
+
+static int do_accept(int sd)
+{
+ int cd;
+
+ for (;;) {
+ cd = accept(sd, NULL, NULL);
+ if (cd < 0) {
+ if (errno == EINTR)
+ continue;
+ die("accept");
+ }
+
+ return cd;
+ }
+
+ return -1;
+}
+
+static bool is_digits(const char *s)
+{
+ const char *p;
+
+ for (p = s; *p; p++)
+ if (!isdigit(*p))
+ return false;
+
+ return true;
+}
+
+struct guest {
+ char *name;
+ int cid;
+ int pid;
+};
+
+static struct guest *guests;
+static size_t guests_len;
+
+static char *get_qemu_guest_name(char *arg)
+{
+ char *tok, *end = arg;
+
+ while ((tok = strsep(&end, ","))) {
+ if (strncmp(tok, "guest=", 6) == 0)
+ return tok + 6;
+ }
+
+ return arg;
+}
+
+static void read_qemu_guests(void)
+{
+ static bool initialized;
+ struct dirent *entry;
+ char path[PATH_MAX];
+ DIR *dir;
+
+ if (initialized)
+ return;
+
+ initialized = true;
+ dir = opendir("/proc");
+ if (!dir)
+ die("opendir");
+
+ for (entry = readdir(dir); entry; entry = readdir(dir)) {
+ bool is_qemu = false, last_was_name = false;
+ struct guest guest = {};
+ char *p, *arg = NULL;
+ size_t arg_size = 0;
+ FILE *f;
+
+ if (!(entry->d_type == DT_DIR && is_digits(entry->d_name)))
+ continue;
+
+ guest.pid = atoi(entry->d_name);
+ snprintf(path, sizeof(path), "/proc/%s/cmdline", entry->d_name);
+ f = fopen(path, "r");
+ if (!f)
+ continue;
+
+ while (getdelim(&arg, &arg_size, 0, f) != -1) {
+ if (!is_qemu && strstr(arg, "qemu-system-")) {
+ is_qemu = true;
+ continue;
+ }
+
+ if (!is_qemu)
+ continue;
+
+ if (strcmp(arg, "-name") == 0) {
+ last_was_name = true;
+ continue;
+ }
+
+ if (last_was_name) {
+ guest.name = strdup(get_qemu_guest_name(arg));
+ last_was_name = false;
+ continue;
+ }
+
+ p = strstr(arg, "guest-cid=");
+ if (p) {
+ guest.cid = atoi(p + 10);
+ continue;
+ }
+ }
+
+ if (!is_qemu)
+ goto next;
+
+ guests = realloc(guests, (guests_len + 1) * sizeof(*guests));
+ if (!guests)
+ die("Can not allocate guest buffer");
+ guests[guests_len++] = guest;
+
+next:
+ free(arg);
+ fclose(f);
+ }
+
+ closedir(dir);
+}
+
+static char *parse_guest_name(char *guest, int *cid, int *port)
+{
+ size_t i;
+ char *p;
+
+ *port = -1;
+ p = strrchr(guest, ':');
+ if (p) {
+ *p = '\0';
+ *port = atoi(p + 1);
+ }
+
+ *cid = -1;
+ p = strrchr(guest, '@');
+ if (p) {
+ *p = '\0';
+ *cid = atoi(p + 1);
+ } else if (is_digits(guest))
+ *cid = atoi(guest);
+
+ read_qemu_guests();
+ for (i = 0; i < guests_len; i++) {
+ if ((*cid > 0 && *cid == guests[i].cid) ||
+ strcmp(guest, guests[i].name) == 0) {
+ *cid = guests[i].cid;
+ return guests[i].name;
+ }
+ }
+
+ return guest;
}
static void set_prio(int prio)
@@ -2652,6 +2868,17 @@ create_recorder_instance(struct buffer_instance *instance, const char *file, int
struct tracecmd_recorder *record;
char *path;
+ if (instance->flags & BUFFER_FL_GUEST) {
+ int sd;
+
+ sd = open_vsock(instance->cid, instance->client_ports[cpu]);
+ if (sd < 0)
+ die("Failed to connect to agent");
+
+ return tracecmd_create_recorder_virt(
+ file, cpu, recorder_flags | TRACECMD_RECORD_NOSPLICE, sd);
+ }
+
if (brass)
return create_recorder_instance_pipe(instance, cpu, brass);
@@ -2676,7 +2903,7 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
{
long ret;
char *file;
- int pid;
+ pid_t pid;
if (type != TRACE_TYPE_EXTRACT) {
signal(SIGUSR1, flush);
@@ -2695,19 +2922,24 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
instance->cpu_count = 0;
}
- if (client_ports) {
- char *path;
+ if ((instance->client_ports && !(instance->flags & BUFFER_FL_GUEST)) ||
+ (instance->flags & BUFFER_FL_AGENT)) {
+ unsigned int flags = recorder_flags;
+ char *path = NULL;
+ int fd;
- connect_port(cpu);
- if (instance->name)
+ if (instance->flags & BUFFER_FL_AGENT)
+ fd = do_accept(instance->fds[cpu]);
+ else
+ fd = connect_port(host, instance->client_ports[cpu]);
+ if (fd < 0)
+ die("Failed connecting to client");
+ if (instance->name && !(instance->flags & BUFFER_FL_AGENT))
path = get_instance_dir(instance);
else
path = tracecmd_find_tracing_dir();
- recorder = tracecmd_create_buffer_recorder_fd(client_ports[cpu],
- cpu, recorder_flags,
- path);
- if (instance->name)
- tracecmd_put_tracing_file(path);
+ recorder = tracecmd_create_buffer_recorder_fd(fd, cpu, flags, path);
+ tracecmd_put_tracing_file(path);
} else {
file = get_temp_file(instance, cpu);
recorder = create_recorder_instance(instance, file, cpu, brass);
@@ -2745,7 +2977,8 @@ static void check_first_msg_from_server(struct tracecmd_msg_handle *msg_handle)
die("server not tracecmd server");
}
-static void communicate_with_listener_v1(struct tracecmd_msg_handle *msg_handle)
+static void communicate_with_listener_v1(struct tracecmd_msg_handle *msg_handle,
+ unsigned int **client_ports)
{
char buf[BUFSIZ];
ssize_t n;
@@ -2788,8 +3021,8 @@ static void communicate_with_listener_v1(struct tracecmd_msg_handle *msg_handle)
/* No options */
write(msg_handle->fd, "0", 2);
- client_ports = malloc(local_cpu_count * sizeof(*client_ports));
- if (!client_ports)
+ *client_ports = malloc(local_cpu_count * sizeof(*client_ports));
+ if (!*client_ports)
die("Failed to allocate client ports for %d cpus", local_cpu_count);
/*
@@ -2807,13 +3040,14 @@ static void communicate_with_listener_v1(struct tracecmd_msg_handle *msg_handle)
if (i == BUFSIZ)
die("read bad port number");
buf[i] = 0;
- client_ports[cpu] = atoi(buf);
+ (*client_ports)[cpu] = atoi(buf);
}
}
-static void communicate_with_listener_v3(struct tracecmd_msg_handle *msg_handle)
+static void communicate_with_listener_v3(struct tracecmd_msg_handle *msg_handle,
+ unsigned int **client_ports)
{
- if (tracecmd_msg_send_init_data(msg_handle, &client_ports) < 0)
+ if (tracecmd_msg_send_init_data(msg_handle, client_ports) < 0)
die("Cannot communicate with server");
}
@@ -2864,7 +3098,7 @@ static void check_protocol_version(struct tracecmd_msg_handle *msg_handle)
}
}
-static struct tracecmd_msg_handle *setup_network(void)
+static struct tracecmd_msg_handle *setup_network(struct buffer_instance *instance)
{
struct tracecmd_msg_handle *msg_handle = NULL;
struct addrinfo hints;
@@ -2934,11 +3168,11 @@ again:
close(sfd);
goto again;
}
- communicate_with_listener_v3(msg_handle);
+ communicate_with_listener_v3(msg_handle, &instance->client_ports);
}
if (msg_handle->version == V1_PROTOCOL)
- communicate_with_listener_v1(msg_handle);
+ communicate_with_listener_v1(msg_handle, &instance->client_ports);
return msg_handle;
}
@@ -2951,7 +3185,7 @@ setup_connection(struct buffer_instance *instance, struct common_record_context
struct tracecmd_msg_handle *msg_handle;
struct tracecmd_output *network_handle;
- msg_handle = setup_network();
+ msg_handle = setup_network(instance);
/* Now create the handle through this socket */
if (msg_handle->version == V3_PROTOCOL) {
@@ -2978,28 +3212,99 @@ static void finish_network(struct tracecmd_msg_handle *msg_handle)
free(host);
}
+static void connect_to_agent(struct buffer_instance *instance)
+{
+ struct tracecmd_msg_handle *msg_handle;
+ int sd, ret, nr_cpus, page_size;
+ unsigned int *ports;
+
+ sd = open_vsock(instance->cid, instance->port);
+ if (sd < 0)
+ die("Failed to connect to vsocket @%d:%d",
+ instance->cid, instance->port);
+
+ msg_handle = tracecmd_msg_handle_alloc(sd, 0);
+ if (!msg_handle)
+ die("Failed to allocate message handle");
+
+ ret = tracecmd_msg_send_trace_req(msg_handle, instance->argc, instance->argv);
+ if (ret < 0)
+ die("Failed to send trace request");
+
+ ret = tracecmd_msg_recv_trace_resp(msg_handle, &nr_cpus, &page_size, &ports);
+ if (ret < 0)
+ die("Failed to receive trace response");
+
+ instance->client_ports = ports;
+ instance->cpu_count = nr_cpus;
+
+ /* the msg_handle now points to the guest fd */
+ instance->msg_handle = msg_handle;
+}
+
+static void setup_guest(struct buffer_instance *instance)
+{
+ struct tracecmd_msg_handle *msg_handle = instance->msg_handle;
+ char *file;
+ int fd;
+
+ /* Create a place to store the guest meta data */
+ file = get_guest_file(output_file, instance->name);
+ if (!file)
+ die("Failed to allocate memory");
+
+ fd = open(file, O_CREAT|O_WRONLY|O_TRUNC, 0644);
+ put_temp_file(file);
+ if (fd < 0)
+ die("Failed to open", file);
+
+ /* Start reading tracing metadata */
+ if (tracecmd_msg_read_data(msg_handle, fd))
+ die("Failed receiving metadata");
+ close(fd);
+}
+
+static void setup_agent(struct buffer_instance *instance, struct common_record_context *ctx)
+{
+ struct tracecmd_output *network_handle;
+
+ network_handle = tracecmd_create_init_fd_msg(instance->msg_handle,
+ listed_events);
+ add_options(network_handle, ctx);
+ tracecmd_write_cpus(network_handle, instance->cpu_count);
+ tracecmd_write_options(network_handle);
+ tracecmd_msg_finish_sending_data(instance->msg_handle);
+ instance->network_handle = network_handle;
+}
+
void start_threads(enum trace_type type, struct common_record_context *ctx)
{
struct buffer_instance *instance;
- int *brass = NULL;
int total_cpu_count = 0;
int i = 0;
int ret;
- for_all_instances(instance)
+ for_all_instances(instance) {
+ /* Start the connection now to find out how many CPUs we need */
+ if (instance->flags & BUFFER_FL_GUEST)
+ connect_to_agent(instance);
total_cpu_count += instance->cpu_count;
+ }
/* make a thread for every CPU we have */
- pids = malloc(sizeof(*pids) * total_cpu_count * (buffers + 1));
+ pids = calloc(total_cpu_count * (buffers + 1), sizeof(*pids));
if (!pids)
- die("Failed to allocat pids for %d cpus", total_cpu_count);
-
- memset(pids, 0, sizeof(*pids) * total_cpu_count * (buffers + 1));
+ die("Failed to allocate pids for %d cpus", total_cpu_count);
for_all_instances(instance) {
+ int *brass = NULL;
int x, pid;
- if (host) {
+ if (instance->flags & BUFFER_FL_AGENT) {
+ setup_agent(instance, ctx);
+ } else if (instance->flags & BUFFER_FL_GUEST) {
+ setup_guest(instance);
+ } else if (host) {
instance->msg_handle = setup_connection(instance, ctx);
if (!instance->msg_handle)
die("Failed to make connection");
@@ -3139,13 +3444,14 @@ static void print_stat(struct buffer_instance *instance)
{
int cpu;
+ if (quiet)
+ return;
+
if (!is_top_instance(instance))
- if (!quiet)
- printf("\nBuffer: %s\n\n", instance->name);
+ printf("\nBuffer: %s\n\n", instance->name);
for (cpu = 0; cpu < instance->cpu_count; cpu++)
- if (!quiet)
- trace_seq_do_printf(&instance->s_print[cpu]);
+ trace_seq_do_printf(&instance->s_print[cpu]);
}
enum {
@@ -3171,7 +3477,44 @@ static void add_options(struct tracecmd_output *handle, struct common_record_con
tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
add_option_hooks(handle);
add_uname(handle);
+}
+
+static void write_guest_file(struct buffer_instance *instance)
+{
+ struct tracecmd_output *handle;
+ int cpu_count = instance->cpu_count;
+ char *file;
+ char **temp_files;
+ int i, fd;
+
+ file = get_guest_file(output_file, instance->name);
+ fd = open(file, O_RDWR);
+ if (fd < 0)
+ die("error opening %s", file);
+ put_temp_file(file);
+
+ handle = tracecmd_get_output_handle_fd(fd);
+ if (!handle)
+ die("error writing to %s", file);
+ temp_files = malloc(sizeof(*temp_files) * cpu_count);
+ if (!temp_files)
+ die("failed to allocate temp_files for %d cpus",
+ cpu_count);
+
+ for (i = 0; i < cpu_count; i++) {
+ temp_files[i] = get_temp_file(instance, i);
+ if (!temp_files[i])
+ die("failed to allocate memory");
+ }
+
+ if (tracecmd_write_cpu_data(handle, cpu_count, temp_files) < 0)
+ die("failed to write CPU data");
+ tracecmd_output_close(handle);
+
+ for (i = 0; i < cpu_count; i++)
+ put_temp_file(temp_files[i]);
+ free(temp_files);
}
static void record_data(struct common_record_context *ctx)
@@ -3185,7 +3528,9 @@ static void record_data(struct common_record_context *ctx)
int i;
for_all_instances(instance) {
- if (instance->msg_handle)
+ if (instance->flags & BUFFER_FL_GUEST)
+ write_guest_file(instance);
+ else if (host && instance->msg_handle)
finish_network(instance->msg_handle);
else
local = true;
@@ -4404,6 +4749,7 @@ void trace_stop(int argc, char **argv)
c = getopt(argc-1, argv+1, "hatB:");
if (c == -1)
break;
+
switch (c) {
case 'h':
usage(argv);
@@ -4566,6 +4912,63 @@ static void init_common_record_context(struct common_record_context *ctx,
#define IS_STREAM(ctx) ((ctx)->curr_cmd == CMD_stream)
#define IS_PROFILE(ctx) ((ctx)->curr_cmd == CMD_profile)
#define IS_RECORD(ctx) ((ctx)->curr_cmd == CMD_record)
+#define IS_AGENT(ctx) ((ctx)->curr_cmd == CMD_record_agent)
+
+static void add_argv(struct buffer_instance *instance, char *arg, bool prepend)
+{
+ instance->argv = realloc(instance->argv,
+ (instance->argc + 1) * sizeof(char *));
+ if (!instance->argv)
+ die("Can not allocate instance args");
+ if (prepend) {
+ memmove(instance->argv + 1, instance->argv,
+ instance->argc * sizeof(*instance->argv));
+ instance->argv[0] = arg;
+ } else {
+ instance->argv[instance->argc] = arg;
+ }
+ instance->argc++;
+}
+
+static void add_arg(struct buffer_instance *instance,
+ int c, const char *opts,
+ struct option *long_options, char *optarg)
+{
+ char *ptr;
+ char *arg;
+ int ret;
+ int i;
+
+ /* Short or long arg */
+ if (!(c & 0x80)) {
+ ret = asprintf(&arg, "-%c", c);
+ if (ret < 0)
+ die("Can not allocate argument");
+ ptr = strstr(opts, arg+1);
+ if (!ptr)
+ return; /* Not found? */
+ add_argv(instance, arg, false);
+ if (ptr[1] == ':')
+ add_argv(instance, optarg, false);
+ return;
+ }
+ for (i = 0; long_options[i].name; i++) {
+ if (c == long_options[i].val) {
+ ret = asprintf(&arg, "--%s", long_options[i].name);
+ if (ret < 0)
+ die("Can not allocate argument");
+ add_argv(instance, arg, false);
+ if (long_options[i].has_arg) {
+ arg = strdup(optarg);
+ if (!arg)
+ die("Can not allocate arguments");
+ add_argv(instance, arg, false);
+ return;
+ }
+ }
+ }
+ /* Not found? */
+}
static void parse_record_options(int argc,
char **argv,
@@ -4607,10 +5010,20 @@ static void parse_record_options(int argc,
if (IS_EXTRACT(ctx))
opts = "+haf:Fp:co:O:sr:g:l:n:P:N:tb:B:ksiT";
else
- opts = "+hae:f:Fp:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
+ opts = "+hae:f:FA:p:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
c = getopt_long (argc-1, argv+1, opts, long_options, &option_index);
if (c == -1)
break;
+
+ /*
+ * If the current instance is to record a guest, then save
+ * all the arguments for this instance.
+ */
+ if (c != 'B' && c != 'A' && ctx->instance->flags & BUFFER_FL_GUEST) {
+ add_arg(ctx->instance, c, opts, long_options, optarg);
+ continue;
+ }
+
switch (c) {
case 'h':
usage(argv);
@@ -4663,6 +5076,26 @@ static void parse_record_options(int argc,
add_trigger(event, optarg);
break;
+ case 'A': {
+ char *name = NULL;
+ int cid = -1, port = -1;
+
+ if (!IS_RECORD(ctx))
+ die("-A is only allowed for record operations");
+
+ name = parse_guest_name(optarg, &cid, &port);
+ if (!name || cid == -1)
+ die("guest %s not found", optarg);
+ if (port == -1)
+ port = TRACE_AGENT_DEFAULT_PORT;
+
+ ctx->instance = create_instance(name);
+ ctx->instance->flags |= BUFFER_FL_GUEST;
+ ctx->instance->cid = cid;
+ ctx->instance->port = port;
+ add_instance(ctx->instance, 0);
+ break;
+ }
case 'F':
test_set_event_pid();
filter_task = 1;
@@ -4733,6 +5166,8 @@ static void parse_record_options(int argc,
ctx->disable = 1;
break;
case 'o':
+ if (IS_AGENT(ctx))
+ die("-o incompatible with agent recording");
if (host)
die("-o incompatible with -N");
if (IS_START(ctx))
@@ -4794,6 +5229,8 @@ static void parse_record_options(int argc,
case 'N':
if (!IS_RECORD(ctx))
die("-N only available with record");
+ if (IS_AGENT(ctx))
+ die("-N incompatible with agent recording");
if (ctx->output)
die("-N incompatible with -o");
host = optarg;
@@ -4890,6 +5327,16 @@ static void parse_record_options(int argc,
}
}
+ /* If --date is specified, prepend it to all guest VM flags */
+ if (ctx->date) {
+ struct buffer_instance *instance;
+
+ for_all_instances(instance) {
+ if (instance->flags & BUFFER_FL_GUEST)
+ add_argv(instance, "--date", true);
+ }
+ }
+
if (!ctx->filtered && ctx->instance->filter_mod)
add_func(&ctx->instance->filter_funcs,
ctx->instance->filter_mod, "*");
@@ -4920,7 +5367,8 @@ static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
{CMD_stream, TRACE_TYPE_STREAM},
{CMD_extract, TRACE_TYPE_EXTRACT},
{CMD_profile, TRACE_TYPE_STREAM},
- {CMD_start, TRACE_TYPE_START}
+ {CMD_start, TRACE_TYPE_START},
+ {CMD_record_agent, TRACE_TYPE_RECORD}
};
for (int i = 0; i < ARRAY_SIZE(trace_type_per_command); i++) {
@@ -4952,12 +5400,28 @@ static void finalize_record_trace(struct common_record_context *ctx)
if (instance->flags & BUFFER_FL_KEEP)
write_tracing_on(instance,
instance->tracing_on_init_val);
+ if (instance->flags & BUFFER_FL_AGENT)
+ tracecmd_output_close(instance->network_handle);
}
if (host)
tracecmd_output_close(ctx->instance->network_handle);
}
+static bool has_local_instances(void)
+{
+ struct buffer_instance *instance;
+
+ for_all_instances(instance) {
+ if (instance->flags & BUFFER_FL_GUEST)
+ continue;
+ if (host && instance->msg_handle)
+ continue;
+ return true;
+ }
+ return false;
+}
+
/*
* This function contains common code for the following commands:
* record, start, stream, profile.
@@ -4986,7 +5450,6 @@ static void record_trace(int argc, char **argv,
/* Save the state of tracing_on before starting */
for_all_instances(instance) {
-
if (!ctx->manual && instance->flags & BUFFER_FL_PROFILE)
enable_profile(instance);
@@ -5003,14 +5466,16 @@ static void record_trace(int argc, char **argv,
page_size = getpagesize();
- fset = set_ftrace(!ctx->disable, ctx->total_disable);
+ if (!(ctx->instance->flags & BUFFER_FL_GUEST))
+ fset = set_ftrace(!ctx->disable, ctx->total_disable);
tracecmd_disable_all_tracing(1);
for_all_instances(instance)
set_clock(instance);
/* Record records the date first */
- if (IS_RECORD(ctx) && ctx->date)
+ if (ctx->date &&
+ ((IS_RECORD(ctx) && has_local_instances()) || IS_AGENT(ctx)))
ctx->date2ts = get_date_to_ts();
for_all_instances(instance) {
@@ -5045,9 +5510,13 @@ static void record_trace(int argc, char **argv,
exit(0);
}
- if (ctx->run_command)
+ if (ctx->run_command) {
run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
- else {
+ } else if (ctx->instance && (ctx->instance->flags & BUFFER_FL_AGENT)) {
+ update_task_filter();
+ tracecmd_enable_tracing();
+ tracecmd_msg_wait_close(ctx->instance->msg_handle);
+ } else {
update_task_filter();
tracecmd_enable_tracing();
/* We don't ptrace ourself */
@@ -5057,6 +5526,8 @@ static void record_trace(int argc, char **argv,
printf("Hit Ctrl^C to stop recording\n");
while (!finished)
trace_or_sleep(type);
+
+ tell_guests_to_stop();
}
tracecmd_disable_tracing();
@@ -5068,6 +5539,9 @@ static void record_trace(int argc, char **argv,
if (!keep)
tracecmd_disable_all_tracing(0);
+ if (!latency)
+ wait_threads();
+
if (IS_RECORD(ctx)) {
record_data(ctx);
delete_thread_data();
@@ -5202,3 +5676,40 @@ void trace_record(int argc, char **argv)
record_trace(argc, argv, &ctx);
exit(0);
}
+
+int trace_record_agent(struct tracecmd_msg_handle *msg_handle,
+ int cpus, int *fds,
+ int argc, char **argv)
+{
+ struct common_record_context ctx;
+ char **argv_plus;
+
+ /* Reset optind for getopt_long */
+ optind = 1;
+ /*
+ * argc is the number of elements in argv, but we need to convert
+ * argc and argv into "trace-cmd", "record", argv.
+ * where argc needs to grow by two.
+ */
+ argv_plus = calloc(argc + 2, sizeof(char *));
+ if (!argv_plus)
+ return -ENOMEM;
+
+ argv_plus[0] = "trace-cmd";
+ argv_plus[1] = "record";
+ memmove(argv_plus + 2, argv, argc * sizeof(char *));
+ argc += 2;
+
+ parse_record_options(argc, argv_plus, CMD_record_agent, &ctx);
+ if (ctx.run_command)
+ return -EINVAL;
+
+ ctx.instance->fds = fds;
+ ctx.instance->flags |= BUFFER_FL_AGENT;
+ ctx.instance->msg_handle = msg_handle;
+ msg_handle->version = V3_PROTOCOL;
+ record_trace(argc, argv, &ctx);
+
+ free(argv_plus);
+ return 0;
+}
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 9ea1906..e845f50 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -231,11 +231,22 @@ static struct usage_help usage_help[] = {
"listen on a network socket for trace clients",
" %s listen -p port[-D][-o file][-d dir][-l logfile]\n"
" Creates a socket to listen for clients.\n"
- " -D create it in daemon mode.\n"
+ " -p port number to listen on.\n"
+ " -D run in daemon mode.\n"
" -o file name to use for clients.\n"
" -d directory to store client files.\n"
" -l logfile to write messages to.\n"
},
+#ifdef VSOCK
+ {
+ "agent",
+ "listen on a vsocket for trace clients",
+ " %s agent -p port[-D]\n"
+ " Creates a vsocket to listen for clients.\n"
+ " -p port number to listen on.\n"
+ " -D run in daemon mode.\n"
+ },
+#endif
{
"list",
"list the available events, plugins or options",
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 05/11] trace-cmd: Add VM kernel tracing over vsockets transport
2019-02-14 14:13 ` [RFC PATCH v6 05/11] trace-cmd: Add VM kernel tracing over vsockets transport Slavomir Kaslev
@ 2019-02-14 20:03 ` Steven Rostedt
2019-02-18 14:26 ` Slavomir Kaslev
2019-02-18 14:28 ` Slavomir Kaslev
0 siblings, 2 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-02-14 20:03 UTC (permalink / raw)
To: Slavomir Kaslev; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, 14 Feb 2019 16:13:29 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:
> void start_threads(enum trace_type type, struct common_record_context *ctx)
> {
> struct buffer_instance *instance;
> - int *brass = NULL;
> int total_cpu_count = 0;
> int i = 0;
> int ret;
>
> - for_all_instances(instance)
> + for_all_instances(instance) {
> + /* Start the connection now to find out how many CPUs we need */
> + if (instance->flags & BUFFER_FL_GUEST)
> + connect_to_agent(instance);
> total_cpu_count += instance->cpu_count;
> + }
>
> /* make a thread for every CPU we have */
> - pids = malloc(sizeof(*pids) * total_cpu_count * (buffers + 1));
> + pids = calloc(total_cpu_count * (buffers + 1), sizeof(*pids));
> if (!pids)
> - die("Failed to allocat pids for %d cpus", total_cpu_count);
> -
> - memset(pids, 0, sizeof(*pids) * total_cpu_count * (buffers + 1));
> + die("Failed to allocate pids for %d cpus", total_cpu_count);
>
> for_all_instances(instance) {
> + int *brass = NULL;
> int x, pid;
>
> - if (host) {
> + if (instance->flags & BUFFER_FL_AGENT) {
> + setup_agent(instance, ctx);
> + } else if (instance->flags & BUFFER_FL_GUEST) {
> + setup_guest(instance);
> + } else if (host) {
> instance->msg_handle = setup_connection(instance, ctx);
> if (!instance->msg_handle)
> die("Failed to make connection");
> @@ -3139,13 +3444,14 @@ static void print_stat(struct buffer_instance *instance)
> {
> int cpu;
>
> + if (quiet)
> + return;
Hmm, this looks unrelated to this patch, and looks like it should have
been in a patch by itself. As a clean up.
> +
> if (!is_top_instance(instance))
> - if (!quiet)
> - printf("\nBuffer: %s\n\n", instance->name);
> + printf("\nBuffer: %s\n\n", instance->name);
>
> for (cpu = 0; cpu < instance->cpu_count; cpu++)
> - if (!quiet)
> - trace_seq_do_printf(&instance->s_print[cpu]);
> + trace_seq_do_printf(&instance->s_print[cpu]);
> }
>
> enum {
> @@ -3171,7 +3477,44 @@ static void add_options(struct tracecmd_output *handle, struct common_record_con
> tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
> add_option_hooks(handle);
> add_uname(handle);
> +}
> +
> +static void write_guest_file(struct buffer_instance *instance)
> +{
> + struct tracecmd_output *handle;
> + int cpu_count = instance->cpu_count;
> + char *file;
> + char **temp_files;
> + int i, fd;
> +
> + file = get_guest_file(output_file, instance->name);
> + fd = open(file, O_RDWR);
> + if (fd < 0)
> + die("error opening %s", file);
> + put_temp_file(file);
> +
> + handle = tracecmd_get_output_handle_fd(fd);
> + if (!handle)
> + die("error writing to %s", file);
>
> + temp_files = malloc(sizeof(*temp_files) * cpu_count);
> + if (!temp_files)
> + die("failed to allocate temp_files for %d cpus",
> + cpu_count);
> +
> + for (i = 0; i < cpu_count; i++) {
> + temp_files[i] = get_temp_file(instance, i);
> + if (!temp_files[i])
> + die("failed to allocate memory");
> + }
> +
> + if (tracecmd_write_cpu_data(handle, cpu_count, temp_files) < 0)
> + die("failed to write CPU data");
> + tracecmd_output_close(handle);
> +
> + for (i = 0; i < cpu_count; i++)
> + put_temp_file(temp_files[i]);
> + free(temp_files);
> }
>
> static void record_data(struct common_record_context *ctx)
> @@ -3185,7 +3528,9 @@ static void record_data(struct common_record_context *ctx)
> int i;
>
> for_all_instances(instance) {
> - if (instance->msg_handle)
> + if (instance->flags & BUFFER_FL_GUEST)
> + write_guest_file(instance);
> + else if (host && instance->msg_handle)
> finish_network(instance->msg_handle);
> else
> local = true;
> @@ -4404,6 +4749,7 @@ void trace_stop(int argc, char **argv)
> c = getopt(argc-1, argv+1, "hatB:");
> if (c == -1)
> break;
> +
> switch (c) {
> case 'h':
> usage(argv);
> @@ -4566,6 +4912,63 @@ static void init_common_record_context(struct common_record_context *ctx,
> #define IS_STREAM(ctx) ((ctx)->curr_cmd == CMD_stream)
> #define IS_PROFILE(ctx) ((ctx)->curr_cmd == CMD_profile)
> #define IS_RECORD(ctx) ((ctx)->curr_cmd == CMD_record)
> +#define IS_AGENT(ctx) ((ctx)->curr_cmd == CMD_record_agent)
> +
> +static void add_argv(struct buffer_instance *instance, char *arg, bool prepend)
> +{
> + instance->argv = realloc(instance->argv,
> + (instance->argc + 1) * sizeof(char *));
> + if (!instance->argv)
> + die("Can not allocate instance args");
> + if (prepend) {
> + memmove(instance->argv + 1, instance->argv,
> + instance->argc * sizeof(*instance->argv));
> + instance->argv[0] = arg;
> + } else {
> + instance->argv[instance->argc] = arg;
> + }
> + instance->argc++;
> +}
> +
> +static void add_arg(struct buffer_instance *instance,
> + int c, const char *opts,
> + struct option *long_options, char *optarg)
> +{
> + char *ptr;
> + char *arg;
> + int ret;
> + int i;
> +
> + /* Short or long arg */
> + if (!(c & 0x80)) {
> + ret = asprintf(&arg, "-%c", c);
> + if (ret < 0)
> + die("Can not allocate argument");
> + ptr = strstr(opts, arg+1);
> + if (!ptr)
> + return; /* Not found? */
This leaks arg, as arg was allocated with asprintf(). Need a
"free(arg)" here.
> + add_argv(instance, arg, false);
> + if (ptr[1] == ':')
> + add_argv(instance, optarg, false);
> + return;
> + }
> + for (i = 0; long_options[i].name; i++) {
> + if (c == long_options[i].val) {
> + ret = asprintf(&arg, "--%s", long_options[i].name);
> + if (ret < 0)
> + die("Can not allocate argument");
> + add_argv(instance, arg, false);
> + if (long_options[i].has_arg) {
> + arg = strdup(optarg);
> + if (!arg)
> + die("Can not allocate arguments");
> + add_argv(instance, arg, false);
> + return;
> + }
> + }
> + }
> + /* Not found? */
> +}
>
> static void parse_record_options(int argc,
> char **argv,
> @@ -4607,10 +5010,20 @@ static void parse_record_options(int argc,
> if (IS_EXTRACT(ctx))
> opts = "+haf:Fp:co:O:sr:g:l:n:P:N:tb:B:ksiT";
> else
> - opts = "+hae:f:Fp:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
> + opts = "+hae:f:FA:p:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
> c = getopt_long (argc-1, argv+1, opts, long_options, &option_index);
> if (c == -1)
> break;
> +
> + /*
> + * If the current instance is to record a guest, then save
> + * all the arguments for this instance.
> + */
> + if (c != 'B' && c != 'A' && ctx->instance->flags & BUFFER_FL_GUEST) {
> + add_arg(ctx->instance, c, opts, long_options, optarg);
> + continue;
> + }
> +
> switch (c) {
> case 'h':
> usage(argv);
> @@ -4663,6 +5076,26 @@ static void parse_record_options(int argc,
> add_trigger(event, optarg);
> break;
>
> + case 'A': {
> + char *name = NULL;
> + int cid = -1, port = -1;
> +
> + if (!IS_RECORD(ctx))
> + die("-A is only allowed for record operations");
> +
> + name = parse_guest_name(optarg, &cid, &port);
> + if (!name || cid == -1)
> + die("guest %s not found", optarg);
> + if (port == -1)
> + port = TRACE_AGENT_DEFAULT_PORT;
> +
> + ctx->instance = create_instance(name);
> + ctx->instance->flags |= BUFFER_FL_GUEST;
> + ctx->instance->cid = cid;
> + ctx->instance->port = port;
> + add_instance(ctx->instance, 0);
> + break;
> + }
> case 'F':
> test_set_event_pid();
> filter_task = 1;
> @@ -4733,6 +5166,8 @@ static void parse_record_options(int argc,
> ctx->disable = 1;
> break;
> case 'o':
> + if (IS_AGENT(ctx))
> + die("-o incompatible with agent recording");
> if (host)
> die("-o incompatible with -N");
> if (IS_START(ctx))
> @@ -4794,6 +5229,8 @@ static void parse_record_options(int argc,
> case 'N':
> if (!IS_RECORD(ctx))
> die("-N only available with record");
> + if (IS_AGENT(ctx))
> + die("-N incompatible with agent recording");
> if (ctx->output)
> die("-N incompatible with -o");
> host = optarg;
> @@ -4890,6 +5327,16 @@ static void parse_record_options(int argc,
> }
> }
>
> + /* If --date is specified, prepend it to all guest VM flags */
> + if (ctx->date) {
> + struct buffer_instance *instance;
> +
> + for_all_instances(instance) {
> + if (instance->flags & BUFFER_FL_GUEST)
> + add_argv(instance, "--date", true);
> + }
> + }
> +
> if (!ctx->filtered && ctx->instance->filter_mod)
> add_func(&ctx->instance->filter_funcs,
> ctx->instance->filter_mod, "*");
> @@ -4920,7 +5367,8 @@ static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
> {CMD_stream, TRACE_TYPE_STREAM},
> {CMD_extract, TRACE_TYPE_EXTRACT},
> {CMD_profile, TRACE_TYPE_STREAM},
> - {CMD_start, TRACE_TYPE_START}
> + {CMD_start, TRACE_TYPE_START},
> + {CMD_record_agent, TRACE_TYPE_RECORD}
> };
>
> for (int i = 0; i < ARRAY_SIZE(trace_type_per_command); i++) {
> @@ -4952,12 +5400,28 @@ static void finalize_record_trace(struct common_record_context *ctx)
> if (instance->flags & BUFFER_FL_KEEP)
> write_tracing_on(instance,
> instance->tracing_on_init_val);
> + if (instance->flags & BUFFER_FL_AGENT)
> + tracecmd_output_close(instance->network_handle);
> }
>
> if (host)
> tracecmd_output_close(ctx->instance->network_handle);
> }
>
> +static bool has_local_instances(void)
> +{
> + struct buffer_instance *instance;
> +
> + for_all_instances(instance) {
> + if (instance->flags & BUFFER_FL_GUEST)
> + continue;
> + if (host && instance->msg_handle)
> + continue;
> + return true;
> + }
> + return false;
> +}
> +
> /*
> * This function contains common code for the following commands:
> * record, start, stream, profile.
> @@ -4986,7 +5450,6 @@ static void record_trace(int argc, char **argv,
>
> /* Save the state of tracing_on before starting */
> for_all_instances(instance) {
> -
> if (!ctx->manual && instance->flags & BUFFER_FL_PROFILE)
> enable_profile(instance);
>
> @@ -5003,14 +5466,16 @@ static void record_trace(int argc, char **argv,
>
> page_size = getpagesize();
>
> - fset = set_ftrace(!ctx->disable, ctx->total_disable);
> + if (!(ctx->instance->flags & BUFFER_FL_GUEST))
> + fset = set_ftrace(!ctx->disable, ctx->total_disable);
> tracecmd_disable_all_tracing(1);
>
> for_all_instances(instance)
> set_clock(instance);
>
> /* Record records the date first */
> - if (IS_RECORD(ctx) && ctx->date)
> + if (ctx->date &&
> + ((IS_RECORD(ctx) && has_local_instances()) || IS_AGENT(ctx)))
> ctx->date2ts = get_date_to_ts();
>
> for_all_instances(instance) {
> @@ -5045,9 +5510,13 @@ static void record_trace(int argc, char **argv,
> exit(0);
> }
>
> - if (ctx->run_command)
> + if (ctx->run_command) {
> run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
> - else {
> + } else if (ctx->instance && (ctx->instance->flags & BUFFER_FL_AGENT)) {
> + update_task_filter();
> + tracecmd_enable_tracing();
> + tracecmd_msg_wait_close(ctx->instance->msg_handle);
> + } else {
> update_task_filter();
> tracecmd_enable_tracing();
> /* We don't ptrace ourself */
> @@ -5057,6 +5526,8 @@ static void record_trace(int argc, char **argv,
> printf("Hit Ctrl^C to stop recording\n");
> while (!finished)
> trace_or_sleep(type);
> +
> + tell_guests_to_stop();
> }
>
> tracecmd_disable_tracing();
> @@ -5068,6 +5539,9 @@ static void record_trace(int argc, char **argv,
> if (!keep)
> tracecmd_disable_all_tracing(0);
>
> + if (!latency)
> + wait_threads();
> +
> if (IS_RECORD(ctx)) {
> record_data(ctx);
> delete_thread_data();
> @@ -5202,3 +5676,40 @@ void trace_record(int argc, char **argv)
> record_trace(argc, argv, &ctx);
> exit(0);
> }
> +
> +int trace_record_agent(struct tracecmd_msg_handle *msg_handle,
> + int cpus, int *fds,
> + int argc, char **argv)
> +{
> + struct common_record_context ctx;
> + char **argv_plus;
> +
> + /* Reset optind for getopt_long */
> + optind = 1;
> + /*
> + * argc is the number of elements in argv, but we need to convert
> + * argc and argv into "trace-cmd", "record", argv.
> + * where argc needs to grow by two.
> + */
> + argv_plus = calloc(argc + 2, sizeof(char *));
> + if (!argv_plus)
> + return -ENOMEM;
> +
> + argv_plus[0] = "trace-cmd";
> + argv_plus[1] = "record";
> + memmove(argv_plus + 2, argv, argc * sizeof(char *));
> + argc += 2;
> +
> + parse_record_options(argc, argv_plus, CMD_record_agent, &ctx);
> + if (ctx.run_command)
> + return -EINVAL;
> +
> + ctx.instance->fds = fds;
> + ctx.instance->flags |= BUFFER_FL_AGENT;
> + ctx.instance->msg_handle = msg_handle;
> + msg_handle->version = V3_PROTOCOL;
> + record_trace(argc, argv, &ctx);
> +
> + free(argv_plus);
> + return 0;
> +}
> diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
> index 9ea1906..e845f50 100644
> --- a/tracecmd/trace-usage.c
> +++ b/tracecmd/trace-usage.c
> @@ -231,11 +231,22 @@ static struct usage_help usage_help[] = {
> "listen on a network socket for trace clients",
> " %s listen -p port[-D][-o file][-d dir][-l logfile]\n"
> " Creates a socket to listen for clients.\n"
> - " -D create it in daemon mode.\n"
> + " -p port number to listen on.\n"
> + " -D run in daemon mode.\n"
> " -o file name to use for clients.\n"
> " -d directory to store client files.\n"
> " -l logfile to write messages to.\n"
> },
> +#ifdef VSOCK
> + {
> + "agent",
In the future, it would be nice to allow agents to work with networks
as well (in case vsock isn't available, but also across machines, where
we would do a p2p to synchronize time stamps.
-- Steve
> + "listen on a vsocket for trace clients",
> + " %s agent -p port[-D]\n"
> + " Creates a vsocket to listen for clients.\n"
> + " -p port number to listen on.\n"
> + " -D run in daemon mode.\n"
> + },
> +#endif
> {
> "list",
> "list the available events, plugins or options",
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 05/11] trace-cmd: Add VM kernel tracing over vsockets transport
2019-02-14 20:03 ` Steven Rostedt
@ 2019-02-18 14:26 ` Slavomir Kaslev
2019-02-18 14:28 ` Slavomir Kaslev
1 sibling, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-18 14:26 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, Feb 14, 2019 at 03:03:48PM -0500, Steven Rostedt wrote:
> On Thu, 14 Feb 2019 16:13:29 +0200
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
>
> > void start_threads(enum trace_type type, struct common_record_context *ctx)
> > {
> > struct buffer_instance *instance;
> > - int *brass = NULL;
> > int total_cpu_count = 0;
> > int i = 0;
> > int ret;
> >
> > - for_all_instances(instance)
> > + for_all_instances(instance) {
> > + /* Start the connection now to find out how many CPUs we need */
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + connect_to_agent(instance);
> > total_cpu_count += instance->cpu_count;
> > + }
> >
> > /* make a thread for every CPU we have */
> > - pids = malloc(sizeof(*pids) * total_cpu_count * (buffers + 1));
> > + pids = calloc(total_cpu_count * (buffers + 1), sizeof(*pids));
> > if (!pids)
> > - die("Failed to allocat pids for %d cpus", total_cpu_count);
> > -
> > - memset(pids, 0, sizeof(*pids) * total_cpu_count * (buffers + 1));
> > + die("Failed to allocate pids for %d cpus", total_cpu_count);
> >
> > for_all_instances(instance) {
> > + int *brass = NULL;
> > int x, pid;
> >
> > - if (host) {
> > + if (instance->flags & BUFFER_FL_AGENT) {
> > + setup_agent(instance, ctx);
> > + } else if (instance->flags & BUFFER_FL_GUEST) {
> > + setup_guest(instance);
> > + } else if (host) {
> > instance->msg_handle = setup_connection(instance, ctx);
> > if (!instance->msg_handle)
> > die("Failed to make connection");
> > @@ -3139,13 +3444,14 @@ static void print_stat(struct buffer_instance *instance)
> > {
> > int cpu;
> >
> > + if (quiet)
> > + return;
>
> Hmm, this looks unrelated to this patch, and looks like it should have
> been in a patch by itself. As a clean up.
OK, I split two such unrelated cleanups as separate patches at the beginning of
the next iteration so that they can be merged independently of the remaining
vsockets work.
>
> > +
> > if (!is_top_instance(instance))
> > - if (!quiet)
> > - printf("\nBuffer: %s\n\n", instance->name);
> > + printf("\nBuffer: %s\n\n", instance->name);
> >
> > for (cpu = 0; cpu < instance->cpu_count; cpu++)
> > - if (!quiet)
> > - trace_seq_do_printf(&instance->s_print[cpu]);
> > + trace_seq_do_printf(&instance->s_print[cpu]);
> > }
> >
> > enum {
> > @@ -3171,7 +3477,44 @@ static void add_options(struct tracecmd_output *handle, struct common_record_con
> > tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
> > add_option_hooks(handle);
> > add_uname(handle);
> > +}
> > +
> > +static void write_guest_file(struct buffer_instance *instance)
> > +{
> > + struct tracecmd_output *handle;
> > + int cpu_count = instance->cpu_count;
> > + char *file;
> > + char **temp_files;
> > + int i, fd;
> > +
> > + file = get_guest_file(output_file, instance->name);
> > + fd = open(file, O_RDWR);
> > + if (fd < 0)
> > + die("error opening %s", file);
> > + put_temp_file(file);
> > +
> > + handle = tracecmd_get_output_handle_fd(fd);
> > + if (!handle)
> > + die("error writing to %s", file);
> >
> > + temp_files = malloc(sizeof(*temp_files) * cpu_count);
> > + if (!temp_files)
> > + die("failed to allocate temp_files for %d cpus",
> > + cpu_count);
> > +
> > + for (i = 0; i < cpu_count; i++) {
> > + temp_files[i] = get_temp_file(instance, i);
> > + if (!temp_files[i])
> > + die("failed to allocate memory");
> > + }
> > +
> > + if (tracecmd_write_cpu_data(handle, cpu_count, temp_files) < 0)
> > + die("failed to write CPU data");
> > + tracecmd_output_close(handle);
> > +
> > + for (i = 0; i < cpu_count; i++)
> > + put_temp_file(temp_files[i]);
> > + free(temp_files);
> > }
> >
> > static void record_data(struct common_record_context *ctx)
> > @@ -3185,7 +3528,9 @@ static void record_data(struct common_record_context *ctx)
> > int i;
> >
> > for_all_instances(instance) {
> > - if (instance->msg_handle)
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + write_guest_file(instance);
> > + else if (host && instance->msg_handle)
> > finish_network(instance->msg_handle);
> > else
> > local = true;
> > @@ -4404,6 +4749,7 @@ void trace_stop(int argc, char **argv)
> > c = getopt(argc-1, argv+1, "hatB:");
> > if (c == -1)
> > break;
> > +
> > switch (c) {
> > case 'h':
> > usage(argv);
> > @@ -4566,6 +4912,63 @@ static void init_common_record_context(struct common_record_context *ctx,
> > #define IS_STREAM(ctx) ((ctx)->curr_cmd == CMD_stream)
> > #define IS_PROFILE(ctx) ((ctx)->curr_cmd == CMD_profile)
> > #define IS_RECORD(ctx) ((ctx)->curr_cmd == CMD_record)
> > +#define IS_AGENT(ctx) ((ctx)->curr_cmd == CMD_record_agent)
> > +
> > +static void add_argv(struct buffer_instance *instance, char *arg, bool prepend)
> > +{
> > + instance->argv = realloc(instance->argv,
> > + (instance->argc + 1) * sizeof(char *));
> > + if (!instance->argv)
> > + die("Can not allocate instance args");
> > + if (prepend) {
> > + memmove(instance->argv + 1, instance->argv,
> > + instance->argc * sizeof(*instance->argv));
> > + instance->argv[0] = arg;
> > + } else {
> > + instance->argv[instance->argc] = arg;
> > + }
> > + instance->argc++;
> > +}
> > +
> > +static void add_arg(struct buffer_instance *instance,
> > + int c, const char *opts,
> > + struct option *long_options, char *optarg)
> > +{
> > + char *ptr;
> > + char *arg;
> > + int ret;
> > + int i;
> > +
> > + /* Short or long arg */
> > + if (!(c & 0x80)) {
> > + ret = asprintf(&arg, "-%c", c);
> > + if (ret < 0)
> > + die("Can not allocate argument");
> > + ptr = strstr(opts, arg+1);
> > + if (!ptr)
> > + return; /* Not found? */
>
> This leaks arg, as arg was allocated with asprintf(). Need a
> "free(arg)" here.
>
>
> > + add_argv(instance, arg, false);
> > + if (ptr[1] == ':')
> > + add_argv(instance, optarg, false);
> > + return;
> > + }
> > + for (i = 0; long_options[i].name; i++) {
> > + if (c == long_options[i].val) {
> > + ret = asprintf(&arg, "--%s", long_options[i].name);
> > + if (ret < 0)
> > + die("Can not allocate argument");
> > + add_argv(instance, arg, false);
> > + if (long_options[i].has_arg) {
> > + arg = strdup(optarg);
> > + if (!arg)
> > + die("Can not allocate arguments");
> > + add_argv(instance, arg, false);
> > + return;
> > + }
> > + }
> > + }
> > + /* Not found? */
> > +}
> >
> > static void parse_record_options(int argc,
> > char **argv,
> > @@ -4607,10 +5010,20 @@ static void parse_record_options(int argc,
> > if (IS_EXTRACT(ctx))
> > opts = "+haf:Fp:co:O:sr:g:l:n:P:N:tb:B:ksiT";
> > else
> > - opts = "+hae:f:Fp:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
> > + opts = "+hae:f:FA:p:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
> > c = getopt_long (argc-1, argv+1, opts, long_options, &option_index);
> > if (c == -1)
> > break;
> > +
> > + /*
> > + * If the current instance is to record a guest, then save
> > + * all the arguments for this instance.
> > + */
> > + if (c != 'B' && c != 'A' && ctx->instance->flags & BUFFER_FL_GUEST) {
> > + add_arg(ctx->instance, c, opts, long_options, optarg);
> > + continue;
> > + }
> > +
> > switch (c) {
> > case 'h':
> > usage(argv);
> > @@ -4663,6 +5076,26 @@ static void parse_record_options(int argc,
> > add_trigger(event, optarg);
> > break;
> >
> > + case 'A': {
> > + char *name = NULL;
> > + int cid = -1, port = -1;
> > +
> > + if (!IS_RECORD(ctx))
> > + die("-A is only allowed for record operations");
> > +
> > + name = parse_guest_name(optarg, &cid, &port);
> > + if (!name || cid == -1)
> > + die("guest %s not found", optarg);
> > + if (port == -1)
> > + port = TRACE_AGENT_DEFAULT_PORT;
> > +
> > + ctx->instance = create_instance(name);
> > + ctx->instance->flags |= BUFFER_FL_GUEST;
> > + ctx->instance->cid = cid;
> > + ctx->instance->port = port;
> > + add_instance(ctx->instance, 0);
> > + break;
> > + }
> > case 'F':
> > test_set_event_pid();
> > filter_task = 1;
> > @@ -4733,6 +5166,8 @@ static void parse_record_options(int argc,
> > ctx->disable = 1;
> > break;
> > case 'o':
> > + if (IS_AGENT(ctx))
> > + die("-o incompatible with agent recording");
> > if (host)
> > die("-o incompatible with -N");
> > if (IS_START(ctx))
> > @@ -4794,6 +5229,8 @@ static void parse_record_options(int argc,
> > case 'N':
> > if (!IS_RECORD(ctx))
> > die("-N only available with record");
> > + if (IS_AGENT(ctx))
> > + die("-N incompatible with agent recording");
> > if (ctx->output)
> > die("-N incompatible with -o");
> > host = optarg;
> > @@ -4890,6 +5327,16 @@ static void parse_record_options(int argc,
> > }
> > }
> >
> > + /* If --date is specified, prepend it to all guest VM flags */
> > + if (ctx->date) {
> > + struct buffer_instance *instance;
> > +
> > + for_all_instances(instance) {
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + add_argv(instance, "--date", true);
> > + }
> > + }
> > +
> > if (!ctx->filtered && ctx->instance->filter_mod)
> > add_func(&ctx->instance->filter_funcs,
> > ctx->instance->filter_mod, "*");
> > @@ -4920,7 +5367,8 @@ static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
> > {CMD_stream, TRACE_TYPE_STREAM},
> > {CMD_extract, TRACE_TYPE_EXTRACT},
> > {CMD_profile, TRACE_TYPE_STREAM},
> > - {CMD_start, TRACE_TYPE_START}
> > + {CMD_start, TRACE_TYPE_START},
> > + {CMD_record_agent, TRACE_TYPE_RECORD}
> > };
> >
> > for (int i = 0; i < ARRAY_SIZE(trace_type_per_command); i++) {
> > @@ -4952,12 +5400,28 @@ static void finalize_record_trace(struct common_record_context *ctx)
> > if (instance->flags & BUFFER_FL_KEEP)
> > write_tracing_on(instance,
> > instance->tracing_on_init_val);
> > + if (instance->flags & BUFFER_FL_AGENT)
> > + tracecmd_output_close(instance->network_handle);
> > }
> >
> > if (host)
> > tracecmd_output_close(ctx->instance->network_handle);
> > }
> >
> > +static bool has_local_instances(void)
> > +{
> > + struct buffer_instance *instance;
> > +
> > + for_all_instances(instance) {
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + continue;
> > + if (host && instance->msg_handle)
> > + continue;
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > /*
> > * This function contains common code for the following commands:
> > * record, start, stream, profile.
> > @@ -4986,7 +5450,6 @@ static void record_trace(int argc, char **argv,
> >
> > /* Save the state of tracing_on before starting */
> > for_all_instances(instance) {
> > -
> > if (!ctx->manual && instance->flags & BUFFER_FL_PROFILE)
> > enable_profile(instance);
> >
> > @@ -5003,14 +5466,16 @@ static void record_trace(int argc, char **argv,
> >
> > page_size = getpagesize();
> >
> > - fset = set_ftrace(!ctx->disable, ctx->total_disable);
> > + if (!(ctx->instance->flags & BUFFER_FL_GUEST))
> > + fset = set_ftrace(!ctx->disable, ctx->total_disable);
> > tracecmd_disable_all_tracing(1);
> >
> > for_all_instances(instance)
> > set_clock(instance);
> >
> > /* Record records the date first */
> > - if (IS_RECORD(ctx) && ctx->date)
> > + if (ctx->date &&
> > + ((IS_RECORD(ctx) && has_local_instances()) || IS_AGENT(ctx)))
> > ctx->date2ts = get_date_to_ts();
> >
> > for_all_instances(instance) {
> > @@ -5045,9 +5510,13 @@ static void record_trace(int argc, char **argv,
> > exit(0);
> > }
> >
> > - if (ctx->run_command)
> > + if (ctx->run_command) {
> > run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
> > - else {
> > + } else if (ctx->instance && (ctx->instance->flags & BUFFER_FL_AGENT)) {
> > + update_task_filter();
> > + tracecmd_enable_tracing();
> > + tracecmd_msg_wait_close(ctx->instance->msg_handle);
> > + } else {
> > update_task_filter();
> > tracecmd_enable_tracing();
> > /* We don't ptrace ourself */
> > @@ -5057,6 +5526,8 @@ static void record_trace(int argc, char **argv,
> > printf("Hit Ctrl^C to stop recording\n");
> > while (!finished)
> > trace_or_sleep(type);
> > +
> > + tell_guests_to_stop();
> > }
> >
> > tracecmd_disable_tracing();
> > @@ -5068,6 +5539,9 @@ static void record_trace(int argc, char **argv,
> > if (!keep)
> > tracecmd_disable_all_tracing(0);
> >
> > + if (!latency)
> > + wait_threads();
> > +
> > if (IS_RECORD(ctx)) {
> > record_data(ctx);
> > delete_thread_data();
> > @@ -5202,3 +5676,40 @@ void trace_record(int argc, char **argv)
> > record_trace(argc, argv, &ctx);
> > exit(0);
> > }
> > +
> > +int trace_record_agent(struct tracecmd_msg_handle *msg_handle,
> > + int cpus, int *fds,
> > + int argc, char **argv)
> > +{
> > + struct common_record_context ctx;
> > + char **argv_plus;
> > +
> > + /* Reset optind for getopt_long */
> > + optind = 1;
> > + /*
> > + * argc is the number of elements in argv, but we need to convert
> > + * argc and argv into "trace-cmd", "record", argv.
> > + * where argc needs to grow by two.
> > + */
> > + argv_plus = calloc(argc + 2, sizeof(char *));
> > + if (!argv_plus)
> > + return -ENOMEM;
> > +
> > + argv_plus[0] = "trace-cmd";
> > + argv_plus[1] = "record";
> > + memmove(argv_plus + 2, argv, argc * sizeof(char *));
> > + argc += 2;
> > +
> > + parse_record_options(argc, argv_plus, CMD_record_agent, &ctx);
> > + if (ctx.run_command)
> > + return -EINVAL;
> > +
> > + ctx.instance->fds = fds;
> > + ctx.instance->flags |= BUFFER_FL_AGENT;
> > + ctx.instance->msg_handle = msg_handle;
> > + msg_handle->version = V3_PROTOCOL;
> > + record_trace(argc, argv, &ctx);
> > +
> > + free(argv_plus);
> > + return 0;
> > +}
> > diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
> > index 9ea1906..e845f50 100644
> > --- a/tracecmd/trace-usage.c
> > +++ b/tracecmd/trace-usage.c
> > @@ -231,11 +231,22 @@ static struct usage_help usage_help[] = {
> > "listen on a network socket for trace clients",
> > " %s listen -p port[-D][-o file][-d dir][-l logfile]\n"
> > " Creates a socket to listen for clients.\n"
> > - " -D create it in daemon mode.\n"
> > + " -p port number to listen on.\n"
> > + " -D run in daemon mode.\n"
> > " -o file name to use for clients.\n"
> > " -d directory to store client files.\n"
> > " -l logfile to write messages to.\n"
> > },
> > +#ifdef VSOCK
> > + {
> > + "agent",
>
> In the future, it would be nice to allow agents to work with networks
> as well (in case vsock isn't available, but also across machines, where
> we would do a p2p to synchronize time stamps.
>
> -- Steve
>
> > + "listen on a vsocket for trace clients",
> > + " %s agent -p port[-D]\n"
> > + " Creates a vsocket to listen for clients.\n"
> > + " -p port number to listen on.\n"
> > + " -D run in daemon mode.\n"
> > + },
> > +#endif
> > {
> > "list",
> > "list the available events, plugins or options",
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 05/11] trace-cmd: Add VM kernel tracing over vsockets transport
2019-02-14 20:03 ` Steven Rostedt
2019-02-18 14:26 ` Slavomir Kaslev
@ 2019-02-18 14:28 ` Slavomir Kaslev
1 sibling, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-18 14:28 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, Feb 14, 2019 at 03:03:48PM -0500, Steven Rostedt wrote:
> On Thu, 14 Feb 2019 16:13:29 +0200
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
>
> > void start_threads(enum trace_type type, struct common_record_context *ctx)
> > {
> > struct buffer_instance *instance;
> > - int *brass = NULL;
> > int total_cpu_count = 0;
> > int i = 0;
> > int ret;
> >
> > - for_all_instances(instance)
> > + for_all_instances(instance) {
> > + /* Start the connection now to find out how many CPUs we need */
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + connect_to_agent(instance);
> > total_cpu_count += instance->cpu_count;
> > + }
> >
> > /* make a thread for every CPU we have */
> > - pids = malloc(sizeof(*pids) * total_cpu_count * (buffers + 1));
> > + pids = calloc(total_cpu_count * (buffers + 1), sizeof(*pids));
> > if (!pids)
> > - die("Failed to allocat pids for %d cpus", total_cpu_count);
> > -
> > - memset(pids, 0, sizeof(*pids) * total_cpu_count * (buffers + 1));
> > + die("Failed to allocate pids for %d cpus", total_cpu_count);
> >
> > for_all_instances(instance) {
> > + int *brass = NULL;
> > int x, pid;
> >
> > - if (host) {
> > + if (instance->flags & BUFFER_FL_AGENT) {
> > + setup_agent(instance, ctx);
> > + } else if (instance->flags & BUFFER_FL_GUEST) {
> > + setup_guest(instance);
> > + } else if (host) {
> > instance->msg_handle = setup_connection(instance, ctx);
> > if (!instance->msg_handle)
> > die("Failed to make connection");
> > @@ -3139,13 +3444,14 @@ static void print_stat(struct buffer_instance *instance)
> > {
> > int cpu;
> >
> > + if (quiet)
> > + return;
>
> Hmm, this looks unrelated to this patch, and looks like it should have
> been in a patch by itself. As a clean up.
>
> > +
> > if (!is_top_instance(instance))
> > - if (!quiet)
> > - printf("\nBuffer: %s\n\n", instance->name);
> > + printf("\nBuffer: %s\n\n", instance->name);
> >
> > for (cpu = 0; cpu < instance->cpu_count; cpu++)
> > - if (!quiet)
> > - trace_seq_do_printf(&instance->s_print[cpu]);
> > + trace_seq_do_printf(&instance->s_print[cpu]);
> > }
> >
> > enum {
> > @@ -3171,7 +3477,44 @@ static void add_options(struct tracecmd_output *handle, struct common_record_con
> > tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
> > add_option_hooks(handle);
> > add_uname(handle);
> > +}
> > +
> > +static void write_guest_file(struct buffer_instance *instance)
> > +{
> > + struct tracecmd_output *handle;
> > + int cpu_count = instance->cpu_count;
> > + char *file;
> > + char **temp_files;
> > + int i, fd;
> > +
> > + file = get_guest_file(output_file, instance->name);
> > + fd = open(file, O_RDWR);
> > + if (fd < 0)
> > + die("error opening %s", file);
> > + put_temp_file(file);
> > +
> > + handle = tracecmd_get_output_handle_fd(fd);
> > + if (!handle)
> > + die("error writing to %s", file);
> >
> > + temp_files = malloc(sizeof(*temp_files) * cpu_count);
> > + if (!temp_files)
> > + die("failed to allocate temp_files for %d cpus",
> > + cpu_count);
> > +
> > + for (i = 0; i < cpu_count; i++) {
> > + temp_files[i] = get_temp_file(instance, i);
> > + if (!temp_files[i])
> > + die("failed to allocate memory");
> > + }
> > +
> > + if (tracecmd_write_cpu_data(handle, cpu_count, temp_files) < 0)
> > + die("failed to write CPU data");
> > + tracecmd_output_close(handle);
> > +
> > + for (i = 0; i < cpu_count; i++)
> > + put_temp_file(temp_files[i]);
> > + free(temp_files);
> > }
> >
> > static void record_data(struct common_record_context *ctx)
> > @@ -3185,7 +3528,9 @@ static void record_data(struct common_record_context *ctx)
> > int i;
> >
> > for_all_instances(instance) {
> > - if (instance->msg_handle)
> > + if (instance->flags & BUFFER_FL_GUEST)
> > + write_guest_file(instance);
> > + else if (host && instance->msg_handle)
> > finish_network(instance->msg_handle);
> > else
> > local = true;
> > @@ -4404,6 +4749,7 @@ void trace_stop(int argc, char **argv)
> > c = getopt(argc-1, argv+1, "hatB:");
> > if (c == -1)
> > break;
> > +
> > switch (c) {
> > case 'h':
> > usage(argv);
> > @@ -4566,6 +4912,63 @@ static void init_common_record_context(struct common_record_context *ctx,
> > #define IS_STREAM(ctx) ((ctx)->curr_cmd == CMD_stream)
> > #define IS_PROFILE(ctx) ((ctx)->curr_cmd == CMD_profile)
> > #define IS_RECORD(ctx) ((ctx)->curr_cmd == CMD_record)
> > +#define IS_AGENT(ctx) ((ctx)->curr_cmd == CMD_record_agent)
> > +
> > +static void add_argv(struct buffer_instance *instance, char *arg, bool prepend)
> > +{
> > + instance->argv = realloc(instance->argv,
> > + (instance->argc + 1) * sizeof(char *));
> > + if (!instance->argv)
> > + die("Can not allocate instance args");
> > + if (prepend) {
> > + memmove(instance->argv + 1, instance->argv,
> > + instance->argc * sizeof(*instance->argv));
> > + instance->argv[0] = arg;
> > + } else {
> > + instance->argv[instance->argc] = arg;
> > + }
> > + instance->argc++;
> > +}
> > +
> > +static void add_arg(struct buffer_instance *instance,
> > + int c, const char *opts,
> > + struct option *long_options, char *optarg)
> > +{
> > + char *ptr;
> > + char *arg;
> > + int ret;
> > + int i;
> > +
> > + /* Short or long arg */
> > + if (!(c & 0x80)) {
> > + ret = asprintf(&arg, "-%c", c);
> > + if (ret < 0)
> > + die("Can not allocate argument");
> > + ptr = strstr(opts, arg+1);
> > + if (!ptr)
> > + return; /* Not found? */
>
> This leaks arg, as arg was allocated with asprintf(). Need a
> "free(arg)" here.
Bah! Another left over from the VIRT-SERVER branch. Reorganized this function
for the next iteration.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH v6 06/11] trace-cmd: Use splice(2) for vsockets if available
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (4 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 05/11] trace-cmd: Add VM kernel tracing over vsockets transport Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command Slavomir Kaslev
` (4 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
Detect if splice(2) reading is supported for vsockets (Linux 4.20 and
later) and use it, or fallback to read/write-ing otherwise.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
lib/trace-cmd/trace-recorder.c | 6 ++--
tracecmd/trace-record.c | 56 ++++++++++++++++++++++++++++++++--
2 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index fc71180..e45e2d1 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -386,10 +386,8 @@ static long splice_data(struct tracecmd_recorder *recorder)
ret = splice(recorder->brass[0], NULL, recorder->fd, NULL,
read, recorder->fd_flags);
if (ret < 0) {
- if (errno != EAGAIN && errno != EINTR) {
- warning("recorder error in splice output");
- return -1;
- }
+ if (errno != EAGAIN && errno != EINTR)
+ return ret;
return total_read;
} else
update_fd(recorder, ret);
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 8a1a665..e48b270 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -2660,12 +2660,61 @@ static int open_vsock(unsigned int cid, unsigned int port)
return sd;
}
+
+static int try_splice_read_vsock(void)
+{
+ int ret, sd, brass[2];
+
+ sd = socket(AF_VSOCK, SOCK_STREAM, 0);
+ if (sd < 0)
+ return sd;
+
+ ret = pipe(brass);
+ if (ret < 0)
+ goto out_close_sd;
+
+ /*
+ * On kernels that don't support splice reading from vsockets
+ * this will fail with EINVAL, or ENOTCONN otherwise.
+ * Technically, it should never succeed but if it does, claim splice
+ * reading is supported.
+ */
+ ret = splice(sd, NULL, brass[1], NULL, 10, 0);
+ if (ret < 0)
+ ret = errno != EINVAL;
+ else
+ ret = 1;
+
+ close(brass[0]);
+ close(brass[1]);
+out_close_sd:
+ close(sd);
+ return ret;
+}
+
+static bool can_splice_read_vsock(void)
+{
+ static bool initialized, res;
+
+ if (initialized)
+ return res;
+
+ res = try_splice_read_vsock() > 0;
+ initialized = true;
+ return res;
+}
+
#else
static inline int open_vsock(unsigned int cid, unsigned int port)
{
die("vsock is not supported");
return -1;
}
+
+static bool can_splice_read_vsock(void)
+{
+ return false;
+}
#endif
static int do_accept(int sd)
@@ -2870,13 +2919,16 @@ create_recorder_instance(struct buffer_instance *instance, const char *file, int
if (instance->flags & BUFFER_FL_GUEST) {
int sd;
+ unsigned int flags;
sd = open_vsock(instance->cid, instance->client_ports[cpu]);
if (sd < 0)
die("Failed to connect to agent");
- return tracecmd_create_recorder_virt(
- file, cpu, recorder_flags | TRACECMD_RECORD_NOSPLICE, sd);
+ flags = recorder_flags;
+ if (!can_splice_read_vsock())
+ flags |= TRACECMD_RECORD_NOSPLICE;
+ return tracecmd_create_recorder_virt(file, cpu, flags, sd);
}
if (brass)
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (5 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 06/11] trace-cmd: Use splice(2) for vsockets if available Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 20:41 ` Steven Rostedt
2019-02-14 14:13 ` [RFC PATCH v6 08/11] trace-cmd: Try to autodetect number of guest CPUs in setup-guest if not specified Slavomir Kaslev
` (3 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
Add `trace-cmd setup-guest` command that creates the necessary FIFOs for tracing
a guest over FIFOs instead of vsockets.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
tracecmd/Makefile | 1 +
tracecmd/include/trace-local.h | 6 ++
tracecmd/trace-cmd.c | 1 +
tracecmd/trace-setup-guest.c | 186 +++++++++++++++++++++++++++++++++
tracecmd/trace-usage.c | 8 ++
5 files changed, 202 insertions(+)
create mode 100644 tracecmd/trace-setup-guest.c
diff --git a/tracecmd/Makefile b/tracecmd/Makefile
index 865b1c6..d3e3080 100644
--- a/tracecmd/Makefile
+++ b/tracecmd/Makefile
@@ -35,6 +35,7 @@ TRACE_CMD_OBJS += trace-msg.o
ifeq ($(VSOCK_DEFINED), 1)
TRACE_CMD_OBJS += trace-agent.o
+TRACE_CMD_OBJS += trace-setup-guest.o
endif
ALL_OBJS := $(TRACE_CMD_OBJS:%.o=$(bdir)/%.o)
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 823d323..b23130e 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -14,6 +14,10 @@
#define TRACE_AGENT_DEFAULT_PORT 823
+#define GUEST_PIPE_NAME "trace-pipe-cpu"
+#define GUEST_DIR_FMT "/var/lib/trace-cmd/virt/%s"
+#define GUEST_FIFO_FMT GUEST_DIR_FMT "/" GUEST_PIPE_NAME "%d"
+
extern int debug;
extern int quiet;
@@ -68,6 +72,8 @@ void trace_listen(int argc, char **argv);
void trace_agent(int argc, char **argv);
+void trace_setup_guest(int argc, char **argv);
+
void trace_restore(int argc, char **argv);
void trace_clear(int argc, char **argv);
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index 3ae5e2e..4da82b4 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -85,6 +85,7 @@ struct command commands[] = {
{"listen", trace_listen},
#ifdef VSOCK
{"agent", trace_agent},
+ {"setup-guest", trace_setup_guest},
#endif
{"split", trace_split},
{"restore", trace_restore},
diff --git a/tracecmd/trace-setup-guest.c b/tracecmd/trace-setup-guest.c
new file mode 100644
index 0000000..875ac0e
--- /dev/null
+++ b/tracecmd/trace-setup-guest.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 VMware Inc, Slavomir Kaslev <kaslevs@vmware.com>
+ *
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <grp.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "trace-local.h"
+#include "trace-msg.h"
+
+static int make_dir(const char *path, mode_t mode)
+{
+ char *buf, *end, *p;
+ int ret = 0;
+
+ buf = strdup(path);
+ end = buf + strlen(buf);
+
+ for (p = buf; p < end; p++) {
+ for (; p < end && *p == '/'; p++);
+ for (; p < end && *p != '/'; p++);
+
+ *p = '\0';
+ ret = mkdir(buf, mode);
+ if (ret < 0) {
+ if (errno != EEXIST) {
+ ret = -errno;
+ break;
+ }
+ ret = 0;
+ }
+ *p = '/';
+ }
+
+ free(buf);
+ return ret;
+}
+
+static int make_fifo(const char *path, mode_t mode)
+{
+ struct stat st;
+ int ret;
+
+ ret = stat(path, &st);
+ if (ret == 0) {
+ if (S_ISFIFO(st.st_mode))
+ return 0;
+ return -EEXIST;
+ }
+
+ return mkfifo(path, mode);
+}
+
+static int make_guest_dir(const char *guest)
+{
+ char path[PATH_MAX];
+
+ snprintf(path, sizeof(path), GUEST_DIR_FMT, guest);
+ return make_dir(path, 0750);
+}
+
+static int make_guest_fifo(const char *guest, int cpu, mode_t mode)
+{
+ static const char *exts[] = {".in", ".out"};
+ char path[PATH_MAX];
+ int i, ret = 0;
+
+ for (i = 0; i < ARRAY_SIZE(exts); i++) {
+ snprintf(path, sizeof(path), GUEST_FIFO_FMT "%s",
+ guest, cpu, exts[i]);
+ ret = make_fifo(path, mode);
+ if (ret < 0)
+ break;
+ }
+
+ return ret;
+}
+
+static int make_guest_fifos(const char *guest, int nr_cpus, mode_t mode)
+{
+ int i, ret = 0;
+ mode_t mask;
+
+ mask = umask(0);
+ for (i = 0; i < nr_cpus; i++) {
+ ret = make_guest_fifo(guest, i, mode);
+ if (ret < 0)
+ break;
+ }
+ umask(mask);
+
+ return ret;
+}
+
+static void do_setup_guest(const char *guest, int nr_cpus, mode_t mode, gid_t gid)
+{
+ gid_t save_egid;
+ int ret;
+
+ if (gid != -1) {
+ save_egid = getegid();
+ ret = setegid(gid);
+ if (ret < 0)
+ pdie("failed to set effective group ID");
+ }
+
+ ret = make_guest_dir(guest);
+ if (ret < 0)
+ pdie("failed to create guest directory for %s", guest);
+
+ ret = make_guest_fifos(guest, nr_cpus, mode);
+ if (ret < 0)
+ pdie("failed to create FIFOs for %s", guest);
+
+ if (gid != -1) {
+ ret = setegid(save_egid);
+ if (ret < 0)
+ pdie("failed to restore effective group ID");
+ }
+}
+
+void trace_setup_guest(int argc, char **argv)
+{
+ struct group *group;
+ mode_t mode = 0660;
+ int nr_cpus = -1;
+ gid_t gid = -1;
+ char *guest;
+
+ if (argc < 2)
+ usage(argv);
+
+ if (strcmp(argv[1], "setup-guest") != 0)
+ usage(argv);
+
+ for (;;) {
+ int c, option_index = 0;
+ static struct option long_options[] = {
+ {"help", no_argument, NULL, '?'},
+ {NULL, 0, NULL, 0}
+ };
+
+ c = getopt_long(argc-1, argv+1, "+hc:p:g:",
+ long_options, &option_index);
+ if (c == -1)
+ break;
+ switch (c) {
+ case 'h':
+ usage(argv);
+ break;
+ case 'c':
+ nr_cpus = atoi(optarg);
+ break;
+ case 'p':
+ mode = strtol(optarg, NULL, 8);
+ break;
+ case 'g':
+ group = getgrnam(optarg);
+ if (!group)
+ pdie("group %s does not exist", optarg);
+ gid = group->gr_gid;
+ break;
+ default:
+ usage(argv);
+ }
+ }
+
+ if (optind != argc-2)
+ usage(argv);
+
+ guest = argv[optind+1];
+
+ if (nr_cpus <= 0)
+ pdie("invalid number of cpus for guest %s", guest);
+
+ do_setup_guest(guest, nr_cpus, mode, gid);
+}
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index e845f50..7a8002f 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -246,6 +246,14 @@ static struct usage_help usage_help[] = {
" -p port number to listen on.\n"
" -D run in daemon mode.\n"
},
+ {
+ "setup-guest",
+ "create FIFOs for tracing guest VMs",
+ " %s setup-guest -c cpus[-p perm][-g group] guest\n"
+ " -c number of guest virtual CPUs\n"
+ " -p FIFOs permissions (default: 0660)\n"
+ " -g FIFOs group owner\n"
+ },
#endif
{
"list",
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command
2019-02-14 14:13 ` [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command Slavomir Kaslev
@ 2019-02-14 20:41 ` Steven Rostedt
2019-02-18 14:37 ` Slavomir Kaslev
0 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2019-02-14 20:41 UTC (permalink / raw)
To: Slavomir Kaslev; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, 14 Feb 2019 16:13:31 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:
> Add `trace-cmd setup-guest` command that creates the necessary FIFOs for tracing
> a guest over FIFOs instead of vsockets.
>
> Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
> ---
> tracecmd/Makefile | 1 +
> tracecmd/include/trace-local.h | 6 ++
> tracecmd/trace-cmd.c | 1 +
> tracecmd/trace-setup-guest.c | 186 +++++++++++++++++++++++++++++++++
> tracecmd/trace-usage.c | 8 ++
> 5 files changed, 202 insertions(+)
> create mode 100644 tracecmd/trace-setup-guest.c
>
> diff --git a/tracecmd/Makefile b/tracecmd/Makefile
> index 865b1c6..d3e3080 100644
> --- a/tracecmd/Makefile
> +++ b/tracecmd/Makefile
> @@ -35,6 +35,7 @@ TRACE_CMD_OBJS += trace-msg.o
>
> ifeq ($(VSOCK_DEFINED), 1)
> TRACE_CMD_OBJS += trace-agent.o
> +TRACE_CMD_OBJS += trace-setup-guest.o
> endif
>
> ALL_OBJS := $(TRACE_CMD_OBJS:%.o=$(bdir)/%.o)
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index 823d323..b23130e 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -14,6 +14,10 @@
>
> #define TRACE_AGENT_DEFAULT_PORT 823
>
> +#define GUEST_PIPE_NAME "trace-pipe-cpu"
> +#define GUEST_DIR_FMT "/var/lib/trace-cmd/virt/%s"
> +#define GUEST_FIFO_FMT GUEST_DIR_FMT "/" GUEST_PIPE_NAME "%d"
> +
> extern int debug;
> extern int quiet;
>
> @@ -68,6 +72,8 @@ void trace_listen(int argc, char **argv);
>
> void trace_agent(int argc, char **argv);
>
> +void trace_setup_guest(int argc, char **argv);
> +
> void trace_restore(int argc, char **argv);
>
> void trace_clear(int argc, char **argv);
> diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
> index 3ae5e2e..4da82b4 100644
> --- a/tracecmd/trace-cmd.c
> +++ b/tracecmd/trace-cmd.c
> @@ -85,6 +85,7 @@ struct command commands[] = {
> {"listen", trace_listen},
> #ifdef VSOCK
> {"agent", trace_agent},
> + {"setup-guest", trace_setup_guest},
> #endif
> {"split", trace_split},
> {"restore", trace_restore},
> diff --git a/tracecmd/trace-setup-guest.c b/tracecmd/trace-setup-guest.c
> new file mode 100644
> index 0000000..875ac0e
> --- /dev/null
> +++ b/tracecmd/trace-setup-guest.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 VMware Inc, Slavomir Kaslev <kaslevs@vmware.com>
> + *
> + */
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <getopt.h>
> +#include <grp.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +
> +#include "trace-local.h"
> +#include "trace-msg.h"
> +
> +static int make_dir(const char *path, mode_t mode)
> +{
> + char *buf, *end, *p;
> + int ret = 0;
> +
> + buf = strdup(path);
> + end = buf + strlen(buf);
> +
> + for (p = buf; p < end; p++) {
> + for (; p < end && *p == '/'; p++);
> + for (; p < end && *p != '/'; p++);
Why not:
for (p = buf; p && *p; p++) {
for (; *p == '/'; p++);
p = index(p, '/');
if (p)
*p = '\0';
> +
> + *p = '\0';
> + ret = mkdir(buf, mode);
> + if (ret < 0) {
> + if (errno != EEXIST) {
> + ret = -errno;
> + break;
> + }
> + ret = 0;
> + }
if (p)
*p = '/';
?
No need for "end".
-- Steve
> + *p = '/';
> + }
> +
> + free(buf);
> + return ret;
> +}
> +
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command
2019-02-14 20:41 ` Steven Rostedt
@ 2019-02-18 14:37 ` Slavomir Kaslev
2019-02-18 17:44 ` Steven Rostedt
0 siblings, 1 reply; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-18 14:37 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, Feb 14, 2019 at 03:41:32PM -0500, Steven Rostedt wrote:
> On Thu, 14 Feb 2019 16:13:31 +0200
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
>
> > Add `trace-cmd setup-guest` command that creates the necessary FIFOs for tracing
> > a guest over FIFOs instead of vsockets.
> >
> > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
> > ---
> > tracecmd/Makefile | 1 +
> > tracecmd/include/trace-local.h | 6 ++
> > tracecmd/trace-cmd.c | 1 +
> > tracecmd/trace-setup-guest.c | 186 +++++++++++++++++++++++++++++++++
> > tracecmd/trace-usage.c | 8 ++
> > 5 files changed, 202 insertions(+)
> > create mode 100644 tracecmd/trace-setup-guest.c
> >
> > diff --git a/tracecmd/Makefile b/tracecmd/Makefile
> > index 865b1c6..d3e3080 100644
> > --- a/tracecmd/Makefile
> > +++ b/tracecmd/Makefile
> > @@ -35,6 +35,7 @@ TRACE_CMD_OBJS += trace-msg.o
> >
> > ifeq ($(VSOCK_DEFINED), 1)
> > TRACE_CMD_OBJS += trace-agent.o
> > +TRACE_CMD_OBJS += trace-setup-guest.o
> > endif
> >
> > ALL_OBJS := $(TRACE_CMD_OBJS:%.o=$(bdir)/%.o)
> > diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> > index 823d323..b23130e 100644
> > --- a/tracecmd/include/trace-local.h
> > +++ b/tracecmd/include/trace-local.h
> > @@ -14,6 +14,10 @@
> >
> > #define TRACE_AGENT_DEFAULT_PORT 823
> >
> > +#define GUEST_PIPE_NAME "trace-pipe-cpu"
> > +#define GUEST_DIR_FMT "/var/lib/trace-cmd/virt/%s"
> > +#define GUEST_FIFO_FMT GUEST_DIR_FMT "/" GUEST_PIPE_NAME "%d"
> > +
> > extern int debug;
> > extern int quiet;
> >
> > @@ -68,6 +72,8 @@ void trace_listen(int argc, char **argv);
> >
> > void trace_agent(int argc, char **argv);
> >
> > +void trace_setup_guest(int argc, char **argv);
> > +
> > void trace_restore(int argc, char **argv);
> >
> > void trace_clear(int argc, char **argv);
> > diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
> > index 3ae5e2e..4da82b4 100644
> > --- a/tracecmd/trace-cmd.c
> > +++ b/tracecmd/trace-cmd.c
> > @@ -85,6 +85,7 @@ struct command commands[] = {
> > {"listen", trace_listen},
> > #ifdef VSOCK
> > {"agent", trace_agent},
> > + {"setup-guest", trace_setup_guest},
> > #endif
> > {"split", trace_split},
> > {"restore", trace_restore},
> > diff --git a/tracecmd/trace-setup-guest.c b/tracecmd/trace-setup-guest.c
> > new file mode 100644
> > index 0000000..875ac0e
> > --- /dev/null
> > +++ b/tracecmd/trace-setup-guest.c
> > @@ -0,0 +1,186 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 VMware Inc, Slavomir Kaslev <kaslevs@vmware.com>
> > + *
> > + */
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <getopt.h>
> > +#include <grp.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <sys/stat.h>
> > +#include <unistd.h>
> > +
> > +#include "trace-local.h"
> > +#include "trace-msg.h"
> > +
> > +static int make_dir(const char *path, mode_t mode)
> > +{
> > + char *buf, *end, *p;
> > + int ret = 0;
> > +
> > + buf = strdup(path);
> > + end = buf + strlen(buf);
> > +
> > + for (p = buf; p < end; p++) {
> > + for (; p < end && *p == '/'; p++);
> > + for (; p < end && *p != '/'; p++);
>
> Why not:
> for (p = buf; p && *p; p++) {
>
> for (; *p == '/'; p++);
> p = index(p, '/');
> if (p)
> *p = '\0';
>
>
> > +
> > + *p = '\0';
> > + ret = mkdir(buf, mode);
> > + if (ret < 0) {
> > + if (errno != EEXIST) {
> > + ret = -errno;
> > + break;
> > + }
> > + ret = 0;
> > + }
>
> if (p)
> *p = '/';
>
> ?
>
> No need for "end".
>
> -- Steve
>
> > + *p = '/';
> > + }
> > +
> > + free(buf);
> > + return ret;
> > +}
> > +
>
This won't work as proposed: `p` will be NULL on the last iteration but will
still get incremented from the outer for-loop and the check (p && *p) won't get
triggered (p == 0x01 in this case).
A fixed version might look like this:
static int make_dir(const char *path, mode_t mode)
{
char buf[PATH_MAX+1], *p;
int ret = 0;
strncpy(buf, path, sizeof(buf));
for (p = buf; *p; p++) {
for (; *p == '/'; p++);
p = strchr(p, '/');
if (p)
*p = '\0';
ret = mkdir(buf, mode);
if (ret < 0) {
if (errno != EEXIST) {
ret = -errno;
break;
}
ret = 0;
}
if (!p)
break;
*p = '/';
}
return ret;
}
OTOH I find the original version much more readable:
static int make_dir(const char *path, mode_t mode)
{
char buf[PATH_MAX+1], *end, *p;
int ret = 0;
end = stpncpy(buf, path, sizeof(buf));
for (p = buf; p < end; p++) {
for (; p < end && *p == '/'; p++);
for (; p < end && *p != '/'; p++);
*p = '\0';
ret = mkdir(buf, mode);
if (ret < 0) {
if (errno != EEXIST) {
ret = -errno;
break;
}
ret = 0;
}
*p = '/';
}
return ret;
}
The intent behind `*p = '\0'; ... *p = '/';` is more clearly expressed in this
version without getting bogged down by strchr() edge case handling.
Since this is not on a performance critical path how about sticking to the more
readable of the two?
-Slavi
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command
2019-02-18 14:37 ` Slavomir Kaslev
@ 2019-02-18 17:44 ` Steven Rostedt
2019-02-18 20:07 ` Slavomir Kaslev
0 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2019-02-18 17:44 UTC (permalink / raw)
To: Slavomir Kaslev; +Cc: linux-trace-devel, slavomir.kaslev
On Mon, 18 Feb 2019 16:37:47 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:
> This won't work as proposed: `p` will be NULL on the last iteration but will
> still get incremented from the outer for-loop and the check (p && *p) won't get
> triggered (p == 0x01 in this case).
I still don't like the "end", it just looks awkward.
>
> A fixed version might look like this:
>
> static int make_dir(const char *path, mode_t mode)
> {
> char buf[PATH_MAX+1], *p;
> int ret = 0;
>
> strncpy(buf, path, sizeof(buf));
> for (p = buf; *p; p++) {
> for (; *p == '/'; p++);
> p = strchr(p, '/');
>
> if (p)
> *p = '\0';
> ret = mkdir(buf, mode);
> if (ret < 0) {
> if (errno != EEXIST) {
> ret = -errno;
> break;
> }
> ret = 0;
> }
> if (!p)
> break;
> *p = '/';
> }
>
> return ret;
> }
>
> OTOH I find the original version much more readable:
>
> static int make_dir(const char *path, mode_t mode)
> {
> char buf[PATH_MAX+1], *end, *p;
> int ret = 0;
>
> end = stpncpy(buf, path, sizeof(buf));
> for (p = buf; p < end; p++) {
> for (; p < end && *p == '/'; p++);
> for (; p < end && *p != '/'; p++);
>
> *p = '\0';
> ret = mkdir(buf, mode);
> if (ret < 0) {
> if (errno != EEXIST) {
> ret = -errno;
> break;
> }
> ret = 0;
> }
> *p = '/';
> }
>
> return ret;
> }
>
> The intent behind `*p = '\0'; ... *p = '/';` is more clearly expressed in this
> version without getting bogged down by strchr() edge case handling.
>
> Since this is not on a performance critical path how about sticking to the more
> readable of the two?
>
I'd still like to use '*p' as that's very common.
Also break up the other for loops into a while loops.
for (p = buf; *p; p++) {
while (*p == '/')
p++;
while (*p && *p != '/')
p++;
if (*p)
*p = '\0';
else
p--; /* for the for loop */
[...]
This would work, and I think is still readable. It matches more the
standard way of the Linux kernel as well.
-- Steve
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command
2019-02-18 17:44 ` Steven Rostedt
@ 2019-02-18 20:07 ` Slavomir Kaslev
2019-02-18 20:54 ` Slavomir Kaslev
0 siblings, 1 reply; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-18 20:07 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, slavomir.kaslev
On Mon, Feb 18, 2019 at 12:44:37PM -0500, Steven Rostedt wrote:
> On Mon, 18 Feb 2019 16:37:47 +0200
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
>
> > This won't work as proposed: `p` will be NULL on the last iteration but will
> > still get incremented from the outer for-loop and the check (p && *p) won't get
> > triggered (p == 0x01 in this case).
>
> I still don't like the "end", it just looks awkward.
If that's the only argument I don't think it stands.
>
> >
> > A fixed version might look like this:
> >
> > static int make_dir(const char *path, mode_t mode)
> > {
> > char buf[PATH_MAX+1], *p;
> > int ret = 0;
> >
> > strncpy(buf, path, sizeof(buf));
> > for (p = buf; *p; p++) {
> > for (; *p == '/'; p++);
> > p = strchr(p, '/');
> >
> > if (p)
> > *p = '\0';
> > ret = mkdir(buf, mode);
> > if (ret < 0) {
> > if (errno != EEXIST) {
> > ret = -errno;
> > break;
> > }
> > ret = 0;
> > }
> > if (!p)
> > break;
> > *p = '/';
> > }
> >
> > return ret;
> > }
> >
> > OTOH I find the original version much more readable:
> >
> > static int make_dir(const char *path, mode_t mode)
> > {
> > char buf[PATH_MAX+1], *end, *p;
> > int ret = 0;
> >
> > end = stpncpy(buf, path, sizeof(buf));
> > for (p = buf; p < end; p++) {
> > for (; p < end && *p == '/'; p++);
> > for (; p < end && *p != '/'; p++);
> >
> > *p = '\0';
> > ret = mkdir(buf, mode);
> > if (ret < 0) {
> > if (errno != EEXIST) {
> > ret = -errno;
> > break;
> > }
> > ret = 0;
> > }
> > *p = '/';
> > }
> >
> > return ret;
> > }
> >
> > The intent behind `*p = '\0'; ... *p = '/';` is more clearly expressed in this
> > version without getting bogged down by strchr() edge case handling.
> >
> > Since this is not on a performance critical path how about sticking to the more
> > readable of the two?
> >
>
> I'd still like to use '*p' as that's very common.
Testing for '*p' is more common since in the common case one doesn't know the
length of the string.
This is not the case here since we first do a copy anyway and hence we know the
length from then on. We also actively manipulate to string sentinel and knowing
where the string actually ends makes reasoning about the code much easier.
>
> Also break up the other for loops into a while loops.
OK switching the for()s to while()s and dropping the first `p < end` check
(which is never true) sounds fine.
>
> for (p = buf; *p; p++) {
>
> while (*p == '/')
> p++;
> while (*p && *p != '/')
> p++;
>
> if (*p)
> *p = '\0';
> else
> p--; /* for the for loop */
>
> [...]
>
>
> This would work, and I think is still readable.
It's really not more readable and having a comment explaining what's going on
only supports this claim.
That being said, I don't have strong feelings one way or another though,
disappointingly, I did want to share my bikeshedding opinion. It takes less
energy to make the proposed change than to explain why imo it's worse. If none
of the above makes any sense or you insist on dropping `end`, I'll send an
updated version of this patch for the v7 patchset tomorrow.
Thanks,
-- Slavi
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command
2019-02-18 20:07 ` Slavomir Kaslev
@ 2019-02-18 20:54 ` Slavomir Kaslev
0 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-18 20:54 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, slavomir.kaslev
On Mon, Feb 18, 2019 at 10:07:06PM +0200, Slavomir Kaslev wrote:
> On Mon, Feb 18, 2019 at 12:44:37PM -0500, Steven Rostedt wrote:
> > On Mon, 18 Feb 2019 16:37:47 +0200
> > Slavomir Kaslev <kaslevs@vmware.com> wrote:
> >
> > > This won't work as proposed: `p` will be NULL on the last iteration but will
> > > still get incremented from the outer for-loop and the check (p && *p) won't get
> > > triggered (p == 0x01 in this case).
> >
> > I still don't like the "end", it just looks awkward.
>
> If that's the only argument I don't think it stands.
>
> >
> > >
> > > A fixed version might look like this:
> > >
> > > static int make_dir(const char *path, mode_t mode)
> > > {
> > > char buf[PATH_MAX+1], *p;
> > > int ret = 0;
> > >
> > > strncpy(buf, path, sizeof(buf));
> > > for (p = buf; *p; p++) {
> > > for (; *p == '/'; p++);
> > > p = strchr(p, '/');
> > >
> > > if (p)
> > > *p = '\0';
> > > ret = mkdir(buf, mode);
> > > if (ret < 0) {
> > > if (errno != EEXIST) {
> > > ret = -errno;
> > > break;
> > > }
> > > ret = 0;
> > > }
> > > if (!p)
> > > break;
> > > *p = '/';
> > > }
> > >
> > > return ret;
> > > }
> > >
> > > OTOH I find the original version much more readable:
> > >
> > > static int make_dir(const char *path, mode_t mode)
> > > {
> > > char buf[PATH_MAX+1], *end, *p;
> > > int ret = 0;
> > >
> > > end = stpncpy(buf, path, sizeof(buf));
> > > for (p = buf; p < end; p++) {
> > > for (; p < end && *p == '/'; p++);
> > > for (; p < end && *p != '/'; p++);
> > >
> > > *p = '\0';
> > > ret = mkdir(buf, mode);
> > > if (ret < 0) {
> > > if (errno != EEXIST) {
> > > ret = -errno;
> > > break;
> > > }
> > > ret = 0;
> > > }
> > > *p = '/';
> > > }
> > >
> > > return ret;
> > > }
> > >
> > > The intent behind `*p = '\0'; ... *p = '/';` is more clearly expressed in this
> > > version without getting bogged down by strchr() edge case handling.
> > >
> > > Since this is not on a performance critical path how about sticking to the more
> > > readable of the two?
> > >
> >
> > I'd still like to use '*p' as that's very common.
>
> Testing for '*p' is more common since in the common case one doesn't know the
> length of the string.
>
> This is not the case here since we first do a copy anyway and hence we know the
> length from then on. We also actively manipulate to string sentinel and knowing
> where the string actually ends makes reasoning about the code much easier.
>
> >
> > Also break up the other for loops into a while loops.
>
> OK switching the for()s to while()s and dropping the first `p < end` check
> (which is never true) sounds fine.
>
> >
> > for (p = buf; *p; p++) {
> >
> > while (*p == '/')
> > p++;
> > while (*p && *p != '/')
> > p++;
> >
> > if (*p)
> > *p = '\0';
> > else
> > p--; /* for the for loop */
> >
> > [...]
> >
> >
> > This would work, and I think is still readable.
>
> It's really not more readable and having a comment explaining what's going on
> only supports this claim.
>
I thing in the end we're comparing this:
static int make_dir(const char *path, mode_t mode)
{
char buf[PATH_MAX+1], *end, *p;
end = stpncpy(buf, path, sizeof(buf));
for (p = buf; p < end; p++) {
while (*p == '/')
p++;
while (p < end && *p != '/')
p++;
*p = '\0';
if (mkdir(buf, mode) < 0 && errno != EEXIST)
return -errno;
*p = '/';
}
return 0;
}
to this version:
static int make_dir(const char *path, mode_t mode)
{
char buf[PATH_MAX+1], *p;
strncpy(buf, path, sizeof(buf));
for (p = buf; *p; p++) {
bool eos = true;
while (*p == '/')
p++;
while (*p && *p != '/')
p++;
if (*p)
*p = '\0';
else
eos = false;
if (mkdir(buf, mode) < 0 && errno != EEXIST)
return -errno;
if (eos)
*p = '/';
}
return 0;
}
Cheers,
-- Slavi
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH v6 08/11] trace-cmd: Try to autodetect number of guest CPUs in setup-guest if not specified
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (6 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 07/11] trace-cmd: Add `trace-cmd setup-guest` command Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 09/11] trace-cmd: Add setup-guest flag for attaching FIFOs to the guest VM config Slavomir Kaslev
` (2 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
When no number of guest CPUs is provided with the -c flag to `trace-cmd
setup-guest`, try to autodetect it using virsh for libvirt managed guests.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
tracecmd/trace-setup-guest.c | 21 +++++++++++++++++++++
tracecmd/trace-usage.c | 2 +-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/tracecmd/trace-setup-guest.c b/tracecmd/trace-setup-guest.c
index 875ac0e..7ee6f06 100644
--- a/tracecmd/trace-setup-guest.c
+++ b/tracecmd/trace-setup-guest.c
@@ -101,6 +101,24 @@ static int make_guest_fifos(const char *guest, int nr_cpus, mode_t mode)
return ret;
}
+static int get_guest_cpu_count(const char *guest)
+{
+ const char *cmd_fmt = "virsh vcpucount --maximum '%s' 2>/dev/null";
+ int nr_cpus = -1;
+ char cmd[1024];
+ FILE *f;
+
+ snprintf(cmd, sizeof(cmd), cmd_fmt, guest);
+ f = popen(cmd, "r");
+ if (!f)
+ return -errno;
+
+ fscanf(f, "%d", &nr_cpus);
+ pclose(f);
+
+ return nr_cpus;
+}
+
static void do_setup_guest(const char *guest, int nr_cpus, mode_t mode, gid_t gid)
{
gid_t save_egid;
@@ -179,6 +197,9 @@ void trace_setup_guest(int argc, char **argv)
guest = argv[optind+1];
+ if (nr_cpus <= 0)
+ nr_cpus = get_guest_cpu_count(guest);
+
if (nr_cpus <= 0)
pdie("invalid number of cpus for guest %s", guest);
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 7a8002f..9a13d93 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -249,7 +249,7 @@ static struct usage_help usage_help[] = {
{
"setup-guest",
"create FIFOs for tracing guest VMs",
- " %s setup-guest -c cpus[-p perm][-g group] guest\n"
+ " %s setup-guest [-c cpus][-p perm][-g group] guest\n"
" -c number of guest virtual CPUs\n"
" -p FIFOs permissions (default: 0660)\n"
" -g FIFOs group owner\n"
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC PATCH v6 09/11] trace-cmd: Add setup-guest flag for attaching FIFOs to the guest VM config
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (7 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 08/11] trace-cmd: Try to autodetect number of guest CPUs in setup-guest if not specified Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 10/11] trace-cmd: Add splice() recording from FIFO without additional pipe buffer Slavomir Kaslev
2019-02-14 14:13 ` [RFC PATCH v6 11/11] trace-cmd: Add VM tracing over FIFOs transport Slavomir Kaslev
10 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
This patch adds a flag for attaching the newly created FIFOs for guest tracing
as virtio serial devices to libvirt managed guests.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
tracecmd/trace-setup-guest.c | 54 ++++++++++++++++++++++++++++++++++--
tracecmd/trace-usage.c | 3 +-
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/tracecmd/trace-setup-guest.c b/tracecmd/trace-setup-guest.c
index 7ee6f06..7898c66 100644
--- a/tracecmd/trace-setup-guest.c
+++ b/tracecmd/trace-setup-guest.c
@@ -119,7 +119,45 @@ static int get_guest_cpu_count(const char *guest)
return nr_cpus;
}
-static void do_setup_guest(const char *guest, int nr_cpus, mode_t mode, gid_t gid)
+static int attach_guest_fifos(const char *guest, int nr_cpus)
+{
+ const char *cmd_fmt =
+ "virsh attach-device --config '%s' '%s' >/dev/null 2>/dev/null";
+ const char *xml_fmt =
+ "<channel type='pipe'>\n"
+ " <source path='%s'/>\n"
+ " <target type='virtio' name='%s%d'/>\n"
+ "</channel>";
+ char tmp_path[PATH_MAX], path[PATH_MAX];
+ char cmd[PATH_MAX], xml[PATH_MAX];
+ int i, fd, ret = 0;
+
+ strcpy(tmp_path, "/tmp/pipexmlXXXXXX");
+ fd = mkstemp(tmp_path);
+ if (fd < 0)
+ return fd;
+
+ for (i = 0; i < nr_cpus; i++) {
+ snprintf(path, sizeof(path), GUEST_FIFO_FMT, guest, i);
+ snprintf(xml, sizeof(xml), xml_fmt, path, GUEST_PIPE_NAME, i);
+ pwrite(fd, xml, strlen(xml), 0);
+
+ snprintf(cmd, sizeof(cmd), cmd_fmt, guest, tmp_path);
+ errno = 0;
+ if (system(cmd) != 0) {
+ ret = -errno;
+ break;
+ }
+ }
+
+ close(fd);
+ unlink(tmp_path);
+
+ return ret;
+}
+
+static void do_setup_guest(const char *guest, int nr_cpus,
+ mode_t mode, gid_t gid, bool attach)
{
gid_t save_egid;
int ret;
@@ -139,6 +177,12 @@ static void do_setup_guest(const char *guest, int nr_cpus, mode_t mode, gid_t gi
if (ret < 0)
pdie("failed to create FIFOs for %s", guest);
+ if (attach) {
+ ret = attach_guest_fifos(guest, nr_cpus);
+ if (ret < 0)
+ pdie("failed to attach FIFOs to %s", guest);
+ }
+
if (gid != -1) {
ret = setegid(save_egid);
if (ret < 0)
@@ -148,6 +192,7 @@ static void do_setup_guest(const char *guest, int nr_cpus, mode_t mode, gid_t gi
void trace_setup_guest(int argc, char **argv)
{
+ bool attach = false;
struct group *group;
mode_t mode = 0660;
int nr_cpus = -1;
@@ -167,7 +212,7 @@ void trace_setup_guest(int argc, char **argv)
{NULL, 0, NULL, 0}
};
- c = getopt_long(argc-1, argv+1, "+hc:p:g:",
+ c = getopt_long(argc-1, argv+1, "+hc:p:g:a",
long_options, &option_index);
if (c == -1)
break;
@@ -187,6 +232,9 @@ void trace_setup_guest(int argc, char **argv)
pdie("group %s does not exist", optarg);
gid = group->gr_gid;
break;
+ case 'a':
+ attach = true;
+ break;
default:
usage(argv);
}
@@ -203,5 +251,5 @@ void trace_setup_guest(int argc, char **argv)
if (nr_cpus <= 0)
pdie("invalid number of cpus for guest %s", guest);
- do_setup_guest(guest, nr_cpus, mode, gid);
+ do_setup_guest(guest, nr_cpus, mode, gid, attach);
}
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 9a13d93..bcfd3dc 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -249,10 +249,11 @@ static struct usage_help usage_help[] = {
{
"setup-guest",
"create FIFOs for tracing guest VMs",
- " %s setup-guest [-c cpus][-p perm][-g group] guest\n"
+ " %s setup-guest [-c cpus][-p perm][-g group][-a] guest\n"
" -c number of guest virtual CPUs\n"
" -p FIFOs permissions (default: 0660)\n"
" -g FIFOs group owner\n"
+ " -a Attach FIFOs to guest VM config\n"
},
#endif
{
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC PATCH v6 10/11] trace-cmd: Add splice() recording from FIFO without additional pipe buffer
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (8 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 09/11] trace-cmd: Add setup-guest flag for attaching FIFOs to the guest VM config Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
2019-02-14 21:07 ` Steven Rostedt
2019-02-14 14:13 ` [RFC PATCH v6 11/11] trace-cmd: Add VM tracing over FIFOs transport Slavomir Kaslev
10 siblings, 1 reply; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
When `trace-cmd record` is reading tracing data over FIFO we can do a direct
splice from the FIFO to the output file descriptor instead of doing two through
an additional pipe buffer. This patch implements specialized tracecmd_recorder
data transfer version for this case.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
include/trace-cmd/trace-cmd.h | 3 +-
lib/trace-cmd/trace-recorder.c | 81 +++++++++++++++++++++++++++-------
2 files changed, 67 insertions(+), 17 deletions(-)
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 6a21e66..52962e9 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -270,8 +270,9 @@ struct tracecmd_output *tracecmd_get_output_handle_fd(int fd);
enum {
TRACECMD_RECORD_NOSPLICE = (1 << 0), /* Use read instead of splice */
- TRACECMD_RECORD_SNAPSHOT = (1 << 1), /* extract from snapshot */
+ TRACECMD_RECORD_SNAPSHOT = (1 << 1), /* Extract from snapshot */
TRACECMD_RECORD_BLOCK = (1 << 2), /* Block on splice write */
+ TRACECMD_RECORD_NOBRASS = (1 << 3), /* Splice directly without a brass pipe */
};
void tracecmd_free_recorder(struct tracecmd_recorder *recorder);
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index e45e2d1..8058d71 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -8,6 +8,7 @@
#include <stdlib.h>
#include <fcntl.h>
#include <time.h>
+#include <poll.h>
#include <unistd.h>
#include <errno.h>
@@ -26,6 +27,8 @@
# define SPLICE_F_GIFT 8
#endif
+#define POLL_TIMEOUT_MS 1000
+
struct tracecmd_recorder {
int fd;
int fd1;
@@ -40,6 +43,7 @@ struct tracecmd_recorder {
int pages;
int count;
unsigned fd_flags;
+ unsigned trace_fd_flags;
unsigned flags;
};
@@ -127,6 +131,8 @@ tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
if (!(recorder->flags & TRACECMD_RECORD_BLOCK))
recorder->fd_flags |= SPLICE_F_NONBLOCK;
+ recorder->trace_fd_flags = SPLICE_F_MOVE;
+
/* Init to know what to free and release */
recorder->trace_fd = -1;
recorder->brass[0] = -1;
@@ -171,7 +177,8 @@ tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
goto out_free;
}
- if ((recorder->flags & TRACECMD_RECORD_NOSPLICE) == 0) {
+ if (!(recorder->flags & (TRACECMD_RECORD_NOSPLICE |
+ TRACECMD_RECORD_NOBRASS))) {
ret = pipe(recorder->brass);
if (ret < 0)
goto out_free;
@@ -372,7 +379,7 @@ static long splice_data(struct tracecmd_recorder *recorder)
long ret;
read = splice(recorder->trace_fd, NULL, recorder->brass[1], NULL,
- recorder->pipe_size, SPLICE_F_MOVE);
+ recorder->pipe_size, recorder->trace_fd_flags);
if (read < 0) {
if (errno != EAGAIN && errno != EINTR) {
warning("recorder error in splice input");
@@ -399,6 +406,39 @@ static long splice_data(struct tracecmd_recorder *recorder)
return total_read;
}
+/*
+ * Returns -1 on error.
+ * or bytes of data read.
+ */
+static long direct_splice_data(struct tracecmd_recorder *recorder)
+{
+ struct pollfd pfd = {
+ .fd = recorder->trace_fd,
+ .events = POLLIN,
+ };
+ long read;
+ int ret;
+
+ ret = poll(&pfd, 1, POLL_TIMEOUT_MS);
+ if (ret < 0)
+ return -1;
+
+ if (!(pfd.revents | POLLIN))
+ return 0;
+
+ read = splice(recorder->trace_fd, NULL, recorder->fd, NULL,
+ recorder->pipe_size, recorder->trace_fd_flags);
+ if (read < 0) {
+ if (errno == EAGAIN || errno == EINTR)
+ return 0;
+
+ warning("recorder error in splice input");
+ return -1;
+ }
+
+ return read;
+}
+
/*
* Returns -1 on error.
* or bytes of data read.
@@ -433,6 +473,17 @@ static long read_data(struct tracecmd_recorder *recorder)
return r;
}
+static long move_data(struct tracecmd_recorder *recorder)
+{
+ if (recorder->flags & TRACECMD_RECORD_NOSPLICE)
+ return read_data(recorder);
+
+ if (recorder->flags & TRACECMD_RECORD_NOBRASS)
+ return direct_splice_data(recorder);
+
+ return splice_data(recorder);
+}
+
static void set_nonblock(struct tracecmd_recorder *recorder)
{
long flags;
@@ -440,8 +491,11 @@ static void set_nonblock(struct tracecmd_recorder *recorder)
/* Do not block on reads for flushing */
flags = fcntl(recorder->trace_fd, F_GETFL);
fcntl(recorder->trace_fd, F_SETFL, flags | O_NONBLOCK);
+ recorder->trace_fd_flags |= SPLICE_F_NONBLOCK;
- /* Do not block on streams for write */
+ /* Do not block on pipes for write */
+ flags = fcntl(recorder->fd, F_GETFL);
+ fcntl(recorder->fd, F_SETFL, flags | O_NONBLOCK);
recorder->fd_flags |= SPLICE_F_NONBLOCK;
}
@@ -455,10 +509,7 @@ long tracecmd_flush_recording(struct tracecmd_recorder *recorder)
set_nonblock(recorder);
do {
- if (recorder->flags & TRACECMD_RECORD_NOSPLICE)
- ret = read_data(recorder);
- else
- ret = splice_data(recorder);
+ ret = move_data(recorder);
if (ret < 0)
return ret;
total += ret;
@@ -487,7 +538,10 @@ long tracecmd_flush_recording(struct tracecmd_recorder *recorder)
int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long sleep)
{
- struct timespec req;
+ struct timespec req = {
+ .tv_sec = sleep / 1000000,
+ .tv_nsec = (sleep % 1000000) * 1000,
+ };
long read = 1;
long ret;
@@ -495,17 +549,12 @@ int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long s
do {
/* Only sleep if we did not read anything last time */
- if (!read && sleep) {
- req.tv_sec = sleep / 1000000;
- req.tv_nsec = (sleep % 1000000) * 1000;
+ if (!read && sleep)
nanosleep(&req, NULL);
- }
+
read = 0;
do {
- if (recorder->flags & TRACECMD_RECORD_NOSPLICE)
- ret = read_data(recorder);
- else
- ret = splice_data(recorder);
+ ret = move_data(recorder);
if (ret < 0)
return ret;
read += ret;
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [RFC PATCH v6 10/11] trace-cmd: Add splice() recording from FIFO without additional pipe buffer
2019-02-14 14:13 ` [RFC PATCH v6 10/11] trace-cmd: Add splice() recording from FIFO without additional pipe buffer Slavomir Kaslev
@ 2019-02-14 21:07 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-02-14 21:07 UTC (permalink / raw)
To: Slavomir Kaslev; +Cc: linux-trace-devel, slavomir.kaslev
On Thu, 14 Feb 2019 16:13:34 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:
> @@ -487,7 +538,10 @@ long tracecmd_flush_recording(struct tracecmd_recorder *recorder)
>
> int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long sleep)
> {
> - struct timespec req;
> + struct timespec req = {
> + .tv_sec = sleep / 1000000,
> + .tv_nsec = (sleep % 1000000) * 1000,
> + };
> long read = 1;
> long ret;
>
> @@ -495,17 +549,12 @@ int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long s
>
> do {
> /* Only sleep if we did not read anything last time */
> - if (!read && sleep) {
> - req.tv_sec = sleep / 1000000;
> - req.tv_nsec = (sleep % 1000000) * 1000;
> + if (!read && sleep)
> nanosleep(&req, NULL);
> - }
> +
The above change is really a clean up and should be a separate patch as
well.
-- Steve
> read = 0;
> do {
> - if (recorder->flags & TRACECMD_RECORD_NOSPLICE)
> - ret = read_data(recorder);
> - else
> - ret = splice_data(recorder);
> + ret = move_data(recorder);
> if (ret < 0)
> return ret;
> read += ret;
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH v6 11/11] trace-cmd: Add VM tracing over FIFOs transport
2019-02-14 14:13 [RFC PATCH v6 00/11] Add VM kernel tracing over vsockets and FIFOs Slavomir Kaslev
` (9 preceding siblings ...)
2019-02-14 14:13 ` [RFC PATCH v6 10/11] trace-cmd: Add splice() recording from FIFO without additional pipe buffer Slavomir Kaslev
@ 2019-02-14 14:13 ` Slavomir Kaslev
10 siblings, 0 replies; 24+ messages in thread
From: Slavomir Kaslev @ 2019-02-14 14:13 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, slavomir.kaslev
FIFOs offer ~3x the throughput of vsockets. This patch adds support for using
FIFOs to stream tracing data back to the host when tracing VMs.
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
include/trace-cmd/trace-cmd.h | 8 +--
tracecmd/include/trace-local.h | 4 +-
tracecmd/trace-agent.c | 40 +++++++++++++--
tracecmd/trace-msg.c | 26 ++++++----
tracecmd/trace-record.c | 93 +++++++++++++++++++++++++++++-----
5 files changed, 140 insertions(+), 31 deletions(-)
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 52962e9..27ba89d 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -333,16 +333,16 @@ 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);
+ int argc, char **argv, bool use_fifos);
int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
- int *argc, char ***argv);
+ int *argc, char ***argv, bool *use_fifos);
int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
int nr_cpus, int page_size,
- unsigned int *ports);
+ unsigned int *ports, bool use_fifos);
int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
int *nr_cpus, int *page_size,
- unsigned int **ports);
+ unsigned int **ports, bool *use_fifos);
/* --- Plugin handling --- */
extern struct tep_plugin_option trace_ftrace_options[];
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index b23130e..8bf7c8e 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -17,6 +17,7 @@
#define GUEST_PIPE_NAME "trace-pipe-cpu"
#define GUEST_DIR_FMT "/var/lib/trace-cmd/virt/%s"
#define GUEST_FIFO_FMT GUEST_DIR_FMT "/" GUEST_PIPE_NAME "%d"
+#define VIRTIO_FIFO_FMT "/dev/virtio-ports/" GUEST_PIPE_NAME "%d"
extern int debug;
extern int quiet;
@@ -100,7 +101,7 @@ 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);
+ int argc, char **argv, bool use_fifos);
struct hook_list;
@@ -212,6 +213,7 @@ struct buffer_instance {
int cid;
int port;
int *fds;
+ bool use_fifos;
};
extern struct buffer_instance top_instance;
diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c
index 0c0873b..7389f72 100644
--- a/tracecmd/trace-agent.c
+++ b/tracecmd/trace-agent.c
@@ -83,12 +83,39 @@ static void make_vsocks(int nr, int *fds, unsigned int *ports)
}
}
+static int open_agent_fifos(int nr_cpus, int *fds)
+{
+ char path[PATH_MAX];
+ int i, fd, ret = 0;
+
+ for (i = 0; i < nr_cpus; i++) {
+ snprintf(path, sizeof(path), VIRTIO_FIFO_FMT, i);
+ fd = open(path, O_WRONLY);
+ if (fd < 0) {
+ ret = -errno;
+ break;
+ }
+
+ fds[i] = fd;
+ }
+
+ if (!ret)
+ return ret;
+
+ /* We failed to open all FIFOs so clean up and return error */
+ while (--i >= 0)
+ close(fds[i]);
+
+ return ret;
+}
+
static void agent_handle(int sd, int nr_cpus, int page_size)
{
struct tracecmd_msg_handle *msg_handle;
unsigned int *ports;
char **argv = NULL;
int argc = 0;
+ bool use_fifos;
int *fds;
int ret;
@@ -101,17 +128,22 @@ 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);
+ ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv, &use_fifos);
if (ret < 0)
die("Failed to receive trace request");
- make_vsocks(nr_cpus, fds, ports);
+ if (use_fifos && open_agent_fifos(nr_cpus, fds))
+ use_fifos = false;
+
+ if (!use_fifos)
+ make_vsocks(nr_cpus, fds, ports);
- ret = tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size, ports);
+ ret = tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size,
+ ports, use_fifos);
if (ret < 0)
die("Failed to send trace response");
- trace_record_agent(msg_handle, nr_cpus, fds, argc, argv);
+ trace_record_agent(msg_handle, nr_cpus, fds, argc, argv, use_fifos);
free(argv[0]);
free(argv);
diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c
index 8b0a4e5..c5e047e 100644
--- a/tracecmd/trace-msg.c
+++ b/tracecmd/trace-msg.c
@@ -159,6 +159,10 @@ static int msg_write(int fd, struct tracecmd_msg *msg)
return __do_write_check(fd, msg->buf, data_size);
}
+enum msg_trace_flags {
+ MSG_TRACE_USE_FIFOS = 1 << 0,
+};
+
enum msg_opt_command {
MSGOPT_USETCP = 1,
};
@@ -740,7 +744,7 @@ error:
return ret;
}
-static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv)
+static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv, bool use_fifos)
{
size_t args_size = 0;
char *p;
@@ -750,6 +754,7 @@ static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv)
args_size += strlen(argv[i]) + 1;
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->buf = calloc(args_size, 1);
if (!msg->buf)
@@ -763,13 +768,13 @@ static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv)
}
int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
- int argc, char **argv)
+ int argc, char **argv, bool use_fifos)
{
struct tracecmd_msg msg;
int ret;
tracecmd_msg_init(MSG_TRACE_REQ, &msg);
- ret = make_trace_req(&msg, argc, argv);
+ ret = make_trace_req(&msg, argc, argv, use_fifos);
if (ret < 0)
return ret;
@@ -782,7 +787,7 @@ 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)
+ int *argc, char ***argv, bool *use_fifos)
{
struct tracecmd_msg msg;
char *p, *buf_end, **args;
@@ -838,6 +843,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;
return 0;
out_args:
@@ -850,13 +856,14 @@ out:
return ret;
}
-static int make_trace_resp(struct tracecmd_msg *msg,
- int page_size, int nr_cpus, unsigned int *ports)
+static int make_trace_resp(struct tracecmd_msg *msg, int page_size, int nr_cpus,
+ unsigned int *ports, bool use_fifos)
{
int ports_size = nr_cpus * sizeof(*msg->port_array);
int i;
msg->hdr.size = htonl(ntohl(msg->hdr.size) + ports_size);
+ 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);
@@ -872,13 +879,13 @@ static int make_trace_resp(struct tracecmd_msg *msg,
int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
int nr_cpus, int page_size,
- unsigned int *ports)
+ unsigned int *ports, bool use_fifos)
{
struct tracecmd_msg msg;
int ret;
tracecmd_msg_init(MSG_TRACE_RESP, &msg);
- ret = make_trace_resp(&msg, page_size, nr_cpus, ports);
+ ret = make_trace_resp(&msg, page_size, nr_cpus, ports, use_fifos);
if (ret < 0)
return ret;
@@ -887,7 +894,7 @@ 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)
+ unsigned int **ports, bool *use_fifos)
{
struct tracecmd_msg msg;
ssize_t buf_len;
@@ -909,6 +916,7 @@ int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
goto out;
}
+ *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);
*ports = calloc(*nr_cpus, sizeof(**ports));
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index e48b270..4b88ab1 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -76,6 +76,8 @@ static int buffers;
/* Clear all function filters */
static int clear_function_filters;
+static bool no_fifos;
+
static char *host;
/* Max size to let a per cpu file get */
@@ -2918,17 +2920,22 @@ create_recorder_instance(struct buffer_instance *instance, const char *file, int
char *path;
if (instance->flags & BUFFER_FL_GUEST) {
- int sd;
+ int fd;
unsigned int flags;
- sd = open_vsock(instance->cid, instance->client_ports[cpu]);
- if (sd < 0)
+ if (instance->use_fifos)
+ fd = instance->fds[cpu];
+ else
+ fd = open_vsock(instance->cid, instance->client_ports[cpu]);
+ if (fd < 0)
die("Failed to connect to agent");
flags = recorder_flags;
- if (!can_splice_read_vsock())
+ if (instance->use_fifos)
+ flags |= TRACECMD_RECORD_NOBRASS;
+ else if (!can_splice_read_vsock())
flags |= TRACECMD_RECORD_NOSPLICE;
- return tracecmd_create_recorder_virt(file, cpu, flags, sd);
+ return tracecmd_create_recorder_virt(file, cpu, flags, fd);
}
if (brass)
@@ -2980,10 +2987,14 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
char *path = NULL;
int fd;
- if (instance->flags & BUFFER_FL_AGENT)
- fd = do_accept(instance->fds[cpu]);
- else
+ if (instance->flags & BUFFER_FL_AGENT) {
+ if (instance->use_fifos)
+ fd = instance->fds[cpu];
+ else
+ fd = do_accept(instance->fds[cpu]);
+ } else {
fd = connect_port(host, instance->client_ports[cpu]);
+ }
if (fd < 0)
die("Failed connecting to client");
if (instance->name && !(instance->flags & BUFFER_FL_AGENT))
@@ -3264,11 +3275,42 @@ static void finish_network(struct tracecmd_msg_handle *msg_handle)
free(host);
}
+static int open_guest_fifos(const char *guest, int **fds)
+{
+ char path[PATH_MAX];
+ int i, fd, flags;
+
+ for (i = 0; ; i++) {
+ snprintf(path, sizeof(path), GUEST_FIFO_FMT ".out", guest, i);
+
+ /* O_NONBLOCK so we don't wait for writers */
+ fd = open(path, O_RDONLY | O_NONBLOCK);
+ if (fd < 0)
+ break;
+
+ /* Success, now clear O_NONBLOCK */
+ flags = fcntl(fd, F_GETFL);
+ fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
+
+ *fds = realloc(*fds, i + 1);
+ (*fds)[i] = fd;
+ }
+
+ return i;
+}
+
static void connect_to_agent(struct buffer_instance *instance)
{
struct tracecmd_msg_handle *msg_handle;
- int sd, ret, nr_cpus, page_size;
+ int sd, ret, nr_fifos, nr_cpus, page_size;
unsigned int *ports;
+ int i, *fds = NULL;
+ bool use_fifos = false;
+
+ if (!no_fifos) {
+ nr_fifos = open_guest_fifos(instance->name, &fds);
+ use_fifos = nr_fifos > 0;
+ }
sd = open_vsock(instance->cid, instance->port);
if (sd < 0)
@@ -3279,15 +3321,33 @@ static void connect_to_agent(struct buffer_instance *instance)
if (!msg_handle)
die("Failed to allocate message handle");
- ret = tracecmd_msg_send_trace_req(msg_handle, instance->argc, instance->argv);
+ ret = tracecmd_msg_send_trace_req(msg_handle, instance->argc,
+ instance->argv, use_fifos);
if (ret < 0)
die("Failed to send trace request");
- ret = tracecmd_msg_recv_trace_resp(msg_handle, &nr_cpus, &page_size, &ports);
+ ret = tracecmd_msg_recv_trace_resp(msg_handle, &nr_cpus, &page_size,
+ &ports, &use_fifos);
if (ret < 0)
die("Failed to receive trace response");
- instance->client_ports = ports;
+ if (use_fifos) {
+ if (nr_cpus != nr_fifos) {
+ warning("number of FIFOs (%d) for guest %s differs "
+ "from number of virtual CPUs (%d)",
+ nr_fifos, instance->name, nr_cpus);
+ nr_cpus = nr_cpus < nr_fifos ? nr_cpus : nr_fifos;
+ }
+ free(ports);
+ instance->fds = fds;
+ } else {
+ for (i = 0; i < nr_fifos; i++)
+ close(fds[i]);
+ free(fds);
+ instance->client_ports = ports;
+ }
+
+ instance->use_fifos = use_fifos;
instance->cpu_count = nr_cpus;
/* the msg_handle now points to the guest fd */
@@ -4786,6 +4846,7 @@ enum {
OPT_funcstack = 254,
OPT_date = 255,
OPT_module = 256,
+ OPT_nofifos = 257,
};
void trace_stop(int argc, char **argv)
@@ -5047,6 +5108,7 @@ static void parse_record_options(int argc,
{"date", no_argument, NULL, OPT_date},
{"func-stack", no_argument, NULL, OPT_funcstack},
{"nosplice", no_argument, NULL, OPT_nosplice},
+ {"nofifos", no_argument, NULL, OPT_nofifos},
{"profile", no_argument, NULL, OPT_profile},
{"stderr", no_argument, NULL, OPT_stderr},
{"by-comm", no_argument, NULL, OPT_bycomm},
@@ -5332,6 +5394,9 @@ static void parse_record_options(int argc,
case OPT_nosplice:
recorder_flags |= TRACECMD_RECORD_NOSPLICE;
break;
+ case OPT_nofifos:
+ no_fifos = true;
+ break;
case OPT_profile:
handle_init = trace_init_profile;
ctx->instance->flags |= BUFFER_FL_PROFILE;
@@ -5731,7 +5796,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)
+ int argc, char **argv,
+ bool use_fifos)
{
struct common_record_context ctx;
char **argv_plus;
@@ -5757,6 +5823,7 @@ int trace_record_agent(struct tracecmd_msg_handle *msg_handle,
return -EINVAL;
ctx.instance->fds = fds;
+ ctx.instance->use_fifos = use_fifos;
ctx.instance->flags |= BUFFER_FL_AGENT;
ctx.instance->msg_handle = msg_handle;
msg_handle->version = V3_PROTOCOL;
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread