On Mon, 14 Jan 2019 17:28:00 +0200
Slavomir Kaslev <kaslevs@xxxxxxxxxx> wrote:
\
> --- /dev/null
> +++ b/tracecmd/trace-agent.c
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +/*
> + * Copyright (C) 2018 VMware Inc, Slavomir Kaslev <kaslevs@xxxxxxxxxx>
> + *
> + * based on prior implementation by Yoshihiro Yunomae
> + * Copyright (C) 2013 Hitachi, Ltd.
> + * Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@xxxxxxxxxxx>
> + */
> +
> +#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 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 -1;
> +
> + setsockopt(sd, SOL_SOCKET, SO_REUSEADDR, &(int){1}, sizeof(int));
> +
> + if (bind(sd, (struct sockaddr *)&addr, sizeof(addr)))
> + return -1;
> +
> + if (listen(sd, SOMAXCONN))
> + return -1;
> +
> + return sd;
> +}
> +
> +static int get_vsock_port(int sd)
> +{
> + struct sockaddr_vm addr;
> + socklen_t addr_len = sizeof(addr);
> +
> + if (getsockname(sd, (struct sockaddr *)&addr, &addr_len))
> + return -1;
> +
> + if (addr.svm_family != AF_VSOCK)
> + return -1;
> +
> + return addr.svm_port;
> +}
> +
> +static void make_vsocks(int nr, int **fds, int **ports)
> +{
> + int i, fd, port;
> +
> + *fds = calloc(nr, sizeof(*fds));
> + *ports = calloc(nr, sizeof(*ports));
> + if (!*fds || !*ports)
> + die("Failed to allocate memory");
> +
> + for (i = 0; i < nr; i++) {
> + fd = make_vsock(VMADDR_PORT_ANY);
> + if (fd < 0)
> + die("Failed to open vsock socket");
> +
> + port = get_vsock_port(fd);
> + if (port < 0)
> + die("Failed to get vsock socket address");
> +
> + (*fds)[i] = fd;
> + (*ports)[i] = port;
> + }
> +}
> +
> +static void free_vsocks(int nr, int *fds, int *ports)
> +{
> + int i;
> +
> + for (i = 0; i < nr; i++)
> + close(fds[i]);
> + free(fds);
> + free(ports);
> +}
> +
> +static void agent_handle(int sd, int nr_cpus, int page_size)
> +{
> + struct tracecmd_msg_handle *msg_handle;
> + int *fds, *ports;
> + char **argv = NULL;
> + int argc = 0;
> +
> + msg_handle = tracecmd_msg_handle_alloc(sd, TRACECMD_MSG_FL_CLIENT);
> + if (!msg_handle)
> + die("Failed to allocate message handle");
> +
> + if (tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv))
> + die("Failed to receive trace request");
> +
> + make_vsocks(nr_cpus, &fds, &ports);
> +
> + if (tracecmd_msg_send_trace_resp(msg_handle, nr_cpus, page_size, ports))
> + die("Failed to send trace response");
> +
> + trace_record_agent(msg_handle, nr_cpus, fds, argc, argv);
> +
> + free_vsocks(nr_cpus, fds, ports);
> + free(argv[0]);
> + free(argv);
> + 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 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 vsock socket");
> +
> + 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);
> + }
> +
Hmm, I don't see any break from the above for (;;) and that's an
infinite loop. That makes this dead code below.
> + close(sd);
> + signal(SIGCHLD, SIG_DFL);
> +}
> +
> +void trace_agent(int argc, char **argv)
> +{
> + bool do_daemon = false;
> + unsigned 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 ((argc - optind) >= 2)
> + 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 012371c..6c8754e 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 int *client_ports;
> -static int sfd;
>
> /* Max size to let a per cpu file get */
> static int max_kb;
> @@ -171,6 +172,15 @@ static struct tracecmd_recorder *recorder;
>
> static int ignore_event_not_found = 0;
>
> +static inline size_t grow_cap(size_t old_cap)
> +{
> + size_t cap = 3 * old_cap / 2;
> +
> + if (cap < 16)
> + cap = 16;
> + return cap;
> +}
I'm a bit confused by what the grow_cap() is for.
> +
> static inline int is_top_instance(struct buffer_instance *instance)
> {
> return instance == &top_instance;
> @@ -518,6 +528,36 @@ static char *get_temp_file(struct buffer_instance *instance, int cpu)
> return file;
> }
>
> +static char *get_guest_file(const char *file, const char *guest)
> +{
> + size_t guest_len = strlen(guest);
> + size_t file_len = strlen(file);
> + size_t base_len, idx = 0;
> + const char *p;
> + char *out;
> +
> + out = malloc(file_len + guest_len + 2 /* dash and \0 */);
> + if (!out)
> + return NULL;
> +
> + p = strrchr(file, '.');
> + if (p && p != file)
> + base_len = p - file;
> + else
> + base_len = file_len;
> +
> + memcpy(out, file, base_len);
> + idx += base_len;
> + out[idx++] = '-';
> + memcpy(out + idx, guest, guest_len);
> + idx += guest_len;
> + memcpy(out + idx, p, file_len - base_len);
> + idx += file_len - base_len;
> + out[idx] = '\0';
Can't we pretty much replace this whole function with:
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,6 +663,16 @@ 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_handle_close(instance->msg_handle);
> + }
> +}
> +
> static void stop_threads(enum trace_type type)
> {
> struct timeval tv = { 0, 0 };
> @@ -632,6 +682,8 @@ static void stop_threads(enum trace_type type)
> if (!recorder_threads)
> return;
>
> + tell_guests_to_stop();
> +
> /* Tell all threads to finish up */
> for (i = 0; i < recorder_threads; i++) {
> if (pids[i].pid > 0) {
> @@ -2571,14 +2623,14 @@ static void flush(int sig)
> tracecmd_stop_recording(recorder);
> }
>
> -static void connect_port(int cpu)
> +static int connect_port(const char *host, unsigned port)
> {
> struct addrinfo hints;
> struct addrinfo *results, *rp;
> - int s;
> + int s, sfd;
> char buf[BUFSIZ];
>
> - snprintf(buf, BUFSIZ, "%d", client_ports[cpu]);
> + snprintf(buf, BUFSIZ, "%d", port);
>
> memset(&hints, 0, sizeof(hints));
> hints.ai_family = AF_UNSPEC;
> @@ -2605,7 +2657,190 @@ static void connect_port(int cpu)
>
> freeaddrinfo(results);
>
> - client_ports[cpu] = sfd;
> + return sfd;
> +}
> +
> +#ifdef VSOCK
> +static int open_vsock(unsigned cid, unsigned 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 cid, unsigned 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 size_t guests_cap, guests_len;
> +static struct guest *guests;
> +
> +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 = false;
> + 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) {
> + if (guests_cap == guests_len) {
> + guests_cap = grow_cap(guests_cap);
This isn't a fast path is it? I'm not sure why we are using
guests_cap() here. Note, realloc allocs more than required and is
pretty much a nop if the allocated amount is still within its
allocation that it made the first time.
-- Steve
> + guests = realloc(guests,
> + guests_cap * sizeof(*guests));
> + }
> + guests[guests_len++] = guest;
> + }
> +
> + free(arg);
> + fclose(f);
> + }
> +
> + closedir(dir);
> +}
> +
>
|