[PATCH v2 0/8] Separate trace-cmd and libtracecmd code

[PATCH v2 0/8] Separate trace-cmd and libtracecmd code

[Tzvetomir Stoyanov (VMware)]

libtracecmd is a library, containing functions that can be
used without the trace-cmd application. However, some of the
functions declared as libtracecmd APIs in trace-cmd.h
depend on trace-cmd context. That causes a problem when
other application uses the library. The problem can be
observed when running kerneshark and there is a python
module, loaded by the python plugin - there is a bunch
of warnings.
To resolve the problem, implementations of all trace-cmd
independent functions are moved into libtracecmd. All
libtracecmd functions, that depend on trace-cmd context
are removed from the library and from trace-cmd.h file.

[
  v2 changes:
   - Added a new patch: 
        Move trace-cmd-local.h from the application to the library.
   - Remove trace-output.c dependency of version.h. Moved FILE_VERSION_STRING
     define from top Makefile to trace-cmd-local.h.
]

Tzvetomir Stoyanov (VMware) (8):
  trace-cmd: Move trace-cmd-local.h from the application to the library
  trace-cmd: Move trace-output.c into the library code
  trace-cmd: Move trace-msg.c into the library.
  trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd
  trace-cmd: Move trace-cmd global variable "debug" to libtracecmd
  trace-cmd: Move plog() function to libtracecmd.
  trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h
  trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd

 Makefile                                      |   3 -
 include/trace-cmd/trace-cmd.h                 |  19 +-
 .../include => include/trace-cmd}/trace-msg.h |   3 -
 include/version.h                             |   5 -
 lib/trace-cmd/Makefile                        |   2 +
 .../trace-cmd}/include/trace-cmd-local.h      |   7 +-
 {tracecmd => lib/trace-cmd}/trace-msg.c       |   4 +-
 {tracecmd => lib/trace-cmd}/trace-output.c    |   5 +-
 lib/trace-cmd/trace-util.c                    | 162 ++++++++++++++++++
 tracecmd/Makefile                             |   2 -
 tracecmd/include/trace-local.h                |  14 +-
 tracecmd/trace-cmd.c                          |   3 -
 tracecmd/trace-list.c                         |   2 +-
 tracecmd/trace-listen.c                       |  77 ++-------
 tracecmd/trace-read.c                         |   8 +-
 tracecmd/trace-record.c                       |   8 +-
 tracecmd/trace-stack.c                        |  56 +-----
 17 files changed, 218 insertions(+), 162 deletions(-)
 rename {tracecmd/include => include/trace-cmd}/trace-msg.h (79%)
 rename {tracecmd => lib/trace-cmd}/include/trace-cmd-local.h (83%)
 rename {tracecmd => lib/trace-cmd}/trace-msg.c (99%)
 rename {tracecmd => lib/trace-cmd}/trace-output.c (99%)

-- 
2.21.0

[PATCH v2 1/8] trace-cmd: Move trace-cmd-local.h from the application to the library

[Tzvetomir Stoyanov (VMware)]

The trace-cmd-local.h file is used by trace-input.c, trace-output.c
and trace-msg.c files. The first one is part of the libtracecmd library.
The others will be moved to the same library, so this header file should be
part of the library too.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 {tracecmd => lib/trace-cmd}/include/trace-cmd-local.h | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename {tracecmd => lib/trace-cmd}/include/trace-cmd-local.h (100%)

diff --git a/tracecmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
similarity index 100%
rename from tracecmd/include/trace-cmd-local.h
rename to lib/trace-cmd/include/trace-cmd-local.h
-- 
2.21.0

[PATCH v2 2/8] trace-cmd: Move trace-output.c into the library code

[Tzvetomir Stoyanov (VMware)]

Functions, implemented in trace-output.c file, do not depend on
trace-cmd application context and can be used standalone. The file
is moved from trace-cmd to libtracecmd. It also fixes a warning
while loading python modules from kernelshark:
ImportError: ctracecmd.so: undefined symbol: tracecmd_append_cpu_data

Since in trace-output.c is the only code, that uses FILE_VERSION_STRING
define (the version of trace.dat file format), this define is moved from
version.h in trace-cmd-local.h.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
[
 v2 changes:
   - Removed trace-output.c dependency of version.h. Moved FILE_VERSION_STRING
     define from top Makefile to trace-cmd-local.h.
]
 Makefile                                   | 3 ---
 include/version.h                          | 5 -----
 lib/trace-cmd/Makefile                     | 1 +
 lib/trace-cmd/include/trace-cmd-local.h    | 7 +++++++
 {tracecmd => lib/trace-cmd}/trace-output.c | 1 -
 tracecmd/Makefile                          | 1 -
 6 files changed, 8 insertions(+), 10 deletions(-)
 rename {tracecmd => lib/trace-cmd}/trace-output.c (99%)

diff --git a/Makefile b/Makefile
index d34c615..a848394 100644
--- a/Makefile
+++ b/Makefile
@@ -10,9 +10,6 @@ export TC_PATCHLEVEL
 export TC_EXTRAVERSION
 export TRACECMD_VERSION
 
-# file format version
-FILE_VERSION = 6
-
 MAKEFLAGS += --no-print-directory
 
 # Makefiles suck: This macro sets a default value of $(2) for the
diff --git a/include/version.h b/include/version.h
index 68ce79e..fcf7ba0 100644
--- a/include/version.h
+++ b/include/version.h
@@ -9,9 +9,4 @@
 #include "tc_version.h"
 #endif
 
-#define _STR(x)	#x
-#define STR(x)	_STR(x)
-
-#define FILE_VERSION_STRING STR(FILE_VERSION)
-
 #endif /* _VERSION_H */
diff --git a/lib/trace-cmd/Makefile b/lib/trace-cmd/Makefile
index 44c1332..0543b91 100644
--- a/lib/trace-cmd/Makefile
+++ b/lib/trace-cmd/Makefile
@@ -10,6 +10,7 @@ OBJS =
 OBJS += trace-hash.o
 OBJS += trace-hooks.o
 OBJS += trace-input.o
+OBJS += trace-output.o
 OBJS += trace-recorder.o
 OBJS += trace-util.o
 OBJS += trace-filter-hash.o
diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index fa96d4f..bad325f 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -11,6 +11,13 @@
 #include "trace-cmd.h"
 #include "event-utils.h"
 
+/* trace.dat file format version */
+#define FILE_VERSION 6
+
+#define _STR(x)	#x
+#define STR(x)	_STR(x)
+#define FILE_VERSION_STRING STR(FILE_VERSION)
+
 extern int quiet;
 
 static ssize_t __do_write(int fd, const void *data, size_t size)
diff --git a/tracecmd/trace-output.c b/lib/trace-cmd/trace-output.c
similarity index 99%
rename from tracecmd/trace-output.c
rename to lib/trace-cmd/trace-output.c
index 33d6ce3..1f94346 100644
--- a/tracecmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -23,7 +23,6 @@
 #include "trace-cmd-local.h"
 #include "list.h"
 #include "trace-msg.h"
-#include "version.h"
 
 /* We can't depend on the host size for size_t, all must be 64 bit */
 typedef unsigned long long	tsize_t;
diff --git a/tracecmd/Makefile b/tracecmd/Makefile
index bcd437a..6968f83 100644
--- a/tracecmd/Makefile
+++ b/tracecmd/Makefile
@@ -29,7 +29,6 @@ TRACE_CMD_OBJS += trace-restore.o
 TRACE_CMD_OBJS += trace-check-events.o
 TRACE_CMD_OBJS += trace-show.o
 TRACE_CMD_OBJS += trace-list.o
-TRACE_CMD_OBJS += trace-output.o
 TRACE_CMD_OBJS += trace-usage.o
 TRACE_CMD_OBJS += trace-msg.o
 
-- 
2.21.0

[PATCH v2 3/8] trace-cmd: Move trace-msg.c into the library.

[Tzvetomir Stoyanov (VMware)]

Functions, implemented in trace-msg.c file, do not depend on
trace-cmd application context and can be used standalone.
The file is moved from trace-cmd to libtracecmd.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 {tracecmd/include => include/trace-cmd}/trace-msg.h | 0
 lib/trace-cmd/Makefile                              | 1 +
 {tracecmd => lib/trace-cmd}/trace-msg.c             | 0
 tracecmd/Makefile                                   | 1 -
 4 files changed, 1 insertion(+), 1 deletion(-)
 rename {tracecmd/include => include/trace-cmd}/trace-msg.h (100%)
 rename {tracecmd => lib/trace-cmd}/trace-msg.c (100%)

diff --git a/tracecmd/include/trace-msg.h b/include/trace-cmd/trace-msg.h
similarity index 100%
rename from tracecmd/include/trace-msg.h
rename to include/trace-cmd/trace-msg.h
diff --git a/lib/trace-cmd/Makefile b/lib/trace-cmd/Makefile
index 0543b91..78875e4 100644
--- a/lib/trace-cmd/Makefile
+++ b/lib/trace-cmd/Makefile
@@ -14,6 +14,7 @@ OBJS += trace-output.o
 OBJS += trace-recorder.o
 OBJS += trace-util.o
 OBJS += trace-filter-hash.o
+OBJS += trace-msg.o
 
 # Additional util objects
 OBJS += trace-blk-hack.o
diff --git a/tracecmd/trace-msg.c b/lib/trace-cmd/trace-msg.c
similarity index 100%
rename from tracecmd/trace-msg.c
rename to lib/trace-cmd/trace-msg.c
diff --git a/tracecmd/Makefile b/tracecmd/Makefile
index 6968f83..d491aae 100644
--- a/tracecmd/Makefile
+++ b/tracecmd/Makefile
@@ -30,7 +30,6 @@ TRACE_CMD_OBJS += trace-check-events.o
 TRACE_CMD_OBJS += trace-show.o
 TRACE_CMD_OBJS += trace-list.o
 TRACE_CMD_OBJS += trace-usage.o
-TRACE_CMD_OBJS += trace-msg.o
 
 ALL_OBJS := $(TRACE_CMD_OBJS:%.o=$(bdir)/%.o)
 
-- 
2.21.0

[PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd

[Tzvetomir Stoyanov (VMware)]

A trace-cmd global variable "quiet" is used from libtracecmd and
should be defined there. A new library APIs are implemented to
access it:
 void tracecmd_set_quiet(int quiet);
 int tracecmd_get_quiet(void);

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h           |  3 +++
 lib/trace-cmd/include/trace-cmd-local.h |  2 --
 lib/trace-cmd/trace-output.c            |  4 ++--
 lib/trace-cmd/trace-util.c              | 21 +++++++++++++++++++++
 tracecmd/include/trace-local.h          |  1 -
 tracecmd/trace-cmd.c                    |  1 -
 tracecmd/trace-record.c                 |  6 +++---
 7 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index c4a437a..5ce8fb3 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -49,6 +49,9 @@ enum {
 void tracecmd_record_ref(struct tep_record *record);
 void free_record(struct tep_record *record);
 
+void tracecmd_set_quiet(int quiet);
+int tracecmd_get_quiet(void);
+
 struct tracecmd_input;
 struct tracecmd_output;
 struct tracecmd_recorder;
diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index bad325f..09574db 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -18,8 +18,6 @@
 #define STR(x)	_STR(x)
 #define FILE_VERSION_STRING STR(FILE_VERSION)
 
-extern int quiet;
-
 static ssize_t __do_write(int fd, const void *data, size_t size)
 {
 	ssize_t tot = 0;
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 1f94346..35252ef 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1157,7 +1157,7 @@ int tracecmd_write_cpu_data(struct tracecmd_output *handle,
 		goto out_free;
 
 	for (i = 0; i < cpus; i++) {
-		if (!quiet)
+		if (!tracecmd_get_quiet())
 			fprintf(stderr, "CPU%d data recorded at offset=0x%llx\n",
 				i, (unsigned long long) offsets[i]);
 		offset = lseek64(handle->fd, offsets[i], SEEK_SET);
@@ -1172,7 +1172,7 @@ int tracecmd_write_cpu_data(struct tracecmd_output *handle,
 			    check_size, sizes[i]);
 			goto out_free;
 		}
-		if (!quiet)
+		if (!tracecmd_get_quiet())
 			fprintf(stderr, "    %llu bytes in size\n",
 				(unsigned long long)check_size);
 	}
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 7c74bae..26b9a18 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -28,6 +28,7 @@
 
 int tracecmd_disable_sys_plugins;
 int tracecmd_disable_plugins;
+static int tracecmd_quiet;
 
 static struct registered_plugin_options {
 	struct registered_plugin_options	*next;
@@ -96,6 +97,26 @@ char **trace_util_list_plugin_options(void)
 	return list;
 }
 
+/**
+ * tracecmd_set_quiet - Set if to print output to the screen
+ * @quiet: If non zero, print no output to the screen
+ *
+ */
+void tracecmd_set_quiet(int quiet)
+{
+	tracecmd_quiet = quiet;
+}
+
+/**
+ * tracecmd_get_quiet - Get if to print output to the screen
+ * Returns non zero, if no output to the screen should be printed
+ *
+ */
+int tracecmd_get_quiet(void)
+{
+	return tracecmd_quiet;
+}
+
 void trace_util_free_plugin_options_list(char **list)
 {
 	tracecmd_free_list(list);
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 78c52dc..23a3a29 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -13,7 +13,6 @@
 #include "event-utils.h"
 
 extern int debug;
-extern int quiet;
 
 /* fix stupid glib guint64 typecasts and printf formats */
 typedef unsigned long long u64;
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index 797b303..5283ba7 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -17,7 +17,6 @@ int silence_warnings;
 int show_status;
 
 int debug;
-int quiet;
 
 void warning(const char *fmt, ...)
 {
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index b2ed6bf..b25b659 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3387,7 +3387,7 @@ static void print_stat(struct buffer_instance *instance)
 {
 	int cpu;
 
-	if (quiet)
+	if (tracecmd_get_quiet())
 		return;
 
 	if (!is_top_instance(instance))
@@ -4207,7 +4207,7 @@ static void check_plugin(const char *plugin)
 	}
 	die ("Plugin '%s' does not exist", plugin);
  out:
-	if (!quiet)
+	if (!tracecmd_get_quiet())
 		fprintf(stderr, "  plugin '%s'\n", plugin);
 	free(buf);
 }
@@ -5154,7 +5154,7 @@ static void parse_record_options(int argc,
 			break;
 		case OPT_quiet:
 		case 'q':
-			quiet = 1;
+			tracecmd_set_quiet(1);
 			break;
 		default:
 			usage(argv);
-- 
2.21.0

[PATCH v2 5/8] trace-cmd: Move trace-cmd global variable "debug" to libtracecmd

[Tzvetomir Stoyanov (VMware)]

A trace-cmd global variable "debug" is used from libtracecmd and
should be defined there. A new library APIs are implemented to
access it:
 void tracecmd_set_debug(bool debug);
 bool tracecmd_get_debug(void);

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h  |  3 +++
 lib/trace-cmd/trace-msg.c      |  4 ++--
 lib/trace-cmd/trace-util.c     | 21 +++++++++++++++++++++
 tracecmd/include/trace-local.h |  2 --
 tracecmd/trace-cmd.c           |  2 --
 tracecmd/trace-list.c          |  2 +-
 tracecmd/trace-listen.c        |  8 ++++----
 tracecmd/trace-read.c          |  8 ++++----
 tracecmd/trace-record.c        |  2 +-
 9 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 5ce8fb3..b96de04 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -51,6 +51,9 @@ void free_record(struct tep_record *record);
 
 void tracecmd_set_quiet(int quiet);
 int tracecmd_get_quiet(void);
+void tracecmd_set_debug(bool debug);
+bool tracecmd_get_debug(void);
+
 
 struct tracecmd_input;
 struct tracecmd_output;
diff --git a/lib/trace-cmd/trace-msg.c b/lib/trace-cmd/trace-msg.c
index e2dd188..92562c7 100644
--- a/lib/trace-cmd/trace-msg.c
+++ b/lib/trace-cmd/trace-msg.c
@@ -32,7 +32,7 @@ static inline void dprint(const char *fmt, ...)
 {
 	va_list ap;
 
-	if (!debug)
+	if (!tracecmd_get_debug())
 		return;
 
 	va_start(ap, fmt);
@@ -351,7 +351,7 @@ static int tracecmd_msg_recv_wait(int fd, struct tracecmd_msg *msg)
 
 	pfd.fd = fd;
 	pfd.events = POLLIN;
-	ret = poll(&pfd, 1, debug ? -1 : msg_wait_to);
+	ret = poll(&pfd, 1, tracecmd_get_debug() ? -1 : msg_wait_to);
 	if (ret < 0)
 		return -errno;
 	else if (ret == 0)
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 26b9a18..b5ce84f 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -29,6 +29,7 @@
 int tracecmd_disable_sys_plugins;
 int tracecmd_disable_plugins;
 static int tracecmd_quiet;
+static bool tracecmd_debug;
 
 static struct registered_plugin_options {
 	struct registered_plugin_options	*next;
@@ -117,6 +118,26 @@ int tracecmd_get_quiet(void)
 	return tracecmd_quiet;
 }
 
+/**
+ * tracecmd_set_quiet - Set if to print output to the screen
+ * @quiet: If non zero, print no output to the screen
+ *
+ */
+void tracecmd_set_debug(bool debug)
+{
+	tracecmd_debug = debug;
+}
+
+/**
+ * tracecmd_get_quiet - Get if to print output to the screen
+ * Returns non zero, if no output to the screen should be printed
+ *
+ */
+bool tracecmd_get_debug(void)
+{
+	return tracecmd_debug;
+}
+
 void trace_util_free_plugin_options_list(char **list)
 {
 	tracecmd_free_list(list);
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 23a3a29..e2d5506 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -12,8 +12,6 @@
 #include "trace-cmd.h"
 #include "event-utils.h"
 
-extern int debug;
-
 /* fix stupid glib guint64 typecasts and printf formats */
 typedef unsigned long long u64;
 
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index 5283ba7..30691b6 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -16,8 +16,6 @@
 int silence_warnings;
 int show_status;
 
-int debug;
-
 void warning(const char *fmt, ...)
 {
 	va_list ap;
diff --git a/tracecmd/trace-list.c b/tracecmd/trace-list.c
index 00c6073..832540e 100644
--- a/tracecmd/trace-list.c
+++ b/tracecmd/trace-list.c
@@ -427,7 +427,7 @@ void trace_list(int argc, char **argv)
 				break;
 			case '-':
 				if (strcmp(argv[i], "--debug") == 0) {
-					debug = true;
+					tracecmd_set_debug(true);
 					break;
 				}
 				fprintf(stderr, "list: invalid option -- '%s'\n",
diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c
index 3106022..3cbee67 100644
--- a/tracecmd/trace-listen.c
+++ b/tracecmd/trace-listen.c
@@ -717,7 +717,7 @@ static int do_fork(int cfd)
 	pid_t pid;
 
 	/* in debug mode, we do not fork off children */
-	if (debug)
+	if (tracecmd_get_debug())
 		return 0;
 
 	pid = fork();
@@ -769,7 +769,7 @@ static int do_connection(int cfd, struct sockaddr_storage *peer_addr,
 
 	tracecmd_msg_handle_close(msg_handle);
 
-	if (!debug)
+	if (!tracecmd_get_debug())
 		exit(0);
 
 	return 0;
@@ -910,7 +910,7 @@ static void do_listen(char *port)
 	struct addrinfo *result, *rp;
 	int sfd, s;
 
-	if (!debug)
+	if (!tracecmd_get_debug())
 		signal_setup(SIGCHLD, sigstub);
 
 	make_pid_file();
@@ -1009,7 +1009,7 @@ void trace_listen(int argc, char **argv)
 			daemon = 1;
 			break;
 		case OPT_debug:
-			debug = 1;
+			tracecmd_set_debug(true);
 			break;
 		default:
 			usage(argv);
diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c
index d22c723..12b8b3d 100644
--- a/tracecmd/trace-read.c
+++ b/tracecmd/trace-read.c
@@ -782,10 +782,10 @@ void trace_show_data(struct tracecmd_input *handle, struct tep_record *record)
 				 cpu, record->missed_events);
 	else if (record->missed_events < 0)
 		trace_seq_printf(&s, "CPU:%d [EVENTS DROPPED]\n", cpu);
-	if (buffer_breaks || debug) {
+	if (buffer_breaks || tracecmd_get_debug()) {
 		if (tracecmd_record_at_buffer_start(handle, record)) {
 			trace_seq_printf(&s, "CPU:%d [SUBBUFFER START]", cpu);
-			if (debug)
+			if (tracecmd_get_debug())
 				trace_seq_printf(&s, " [%lld:0x%llx]",
 						 tracecmd_page_ts(handle, record),
 						 record->offset & ~(page_size - 1));
@@ -816,7 +816,7 @@ void trace_show_data(struct tracecmd_input *handle, struct tep_record *record)
 		tep_print_event(pevent, &s, record, use_trace_clock);
 	if (s.len && *(s.buffer + s.len - 1) == '\n')
 		s.len--;
-	if (debug) {
+	if (tracecmd_get_debug()) {
 		struct kbuffer *kbuf;
 		struct kbuffer_raw_info info;
 		void *page;
@@ -1616,7 +1616,7 @@ void trace_report (int argc, char **argv)
 			break;
 		case OPT_debug:
 			buffer_breaks = 1;
-			debug = 1;
+			tracecmd_set_debug(true);
 			break;
 		case OPT_profile:
 			profile = 1;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index b25b659..32793b3 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -5143,7 +5143,7 @@ static void parse_record_options(int argc,
 			no_filter = true;
 			break;
 		case OPT_debug:
-			debug = 1;
+			tracecmd_set_debug(true);
 			break;
 		case OPT_module:
 			if (ctx->instance->filter_mod)
-- 
2.21.0

[PATCH v2 6/8] trace-cmd: Move plog() function to libtracecmd.

[Tzvetomir Stoyanov (VMware)]

plog() function writes logs into a log file. It is used in
libtracecmd and its implementation should be there.
The function is moved from trace-cmd into the library, and 2
additional APIs are implemented:
	int trace_set_log_file(char *logfile); - use it to set
the log file.
	void plog_error(const char *fmt, ...); - use it to log
an error message into the file.

The plog() function is used also from pdie() in trace-cmd.
pdie() depends on trace-cmd context and cannot be moved to
the library. It is reimplemented as macros, in order to utilize
the new plog() library function.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h |  4 ++
 include/trace-cmd/trace-msg.h |  3 --
 lib/trace-cmd/trace-util.c    | 71 +++++++++++++++++++++++++++++++++++
 tracecmd/trace-listen.c       | 69 ++++------------------------------
 4 files changed, 83 insertions(+), 64 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index b96de04..8db0686 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -398,6 +398,10 @@ struct hook_list {
 struct hook_list *tracecmd_create_event_hook(const char *arg);
 void tracecmd_free_hooks(struct hook_list *hooks);
 
+void plog(const char *fmt, ...);
+void plog_error(const char *fmt, ...);
+int trace_set_log_file(char *logfile);
+
 /* --- Hack! --- */
 int tracecmd_blk_hack(struct tracecmd_input *handle);
 
diff --git a/include/trace-cmd/trace-msg.h b/include/trace-cmd/trace-msg.h
index b7fe10b..aab8a69 100644
--- a/include/trace-cmd/trace-msg.h
+++ b/include/trace-cmd/trace-msg.h
@@ -12,7 +12,4 @@
 
 extern unsigned int page_size;
 
-void plog(const char *fmt, ...);
-void pdie(const char *fmt, ...);
-
 #endif /* _TRACE_MSG_H_ */
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index b5ce84f..8c1a0a0 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -31,6 +31,8 @@ int tracecmd_disable_plugins;
 static int tracecmd_quiet;
 static bool tracecmd_debug;
 
+static FILE *trace_logfp;
+
 static struct registered_plugin_options {
 	struct registered_plugin_options	*next;
 	struct tep_plugin_option			*options;
@@ -1716,3 +1718,72 @@ void __weak *malloc_or_die(unsigned int size)
 		die("malloc");
 	return data;
 }
+
+#define LOG_BUF_SIZE 1024
+static void __plog(const char *prefix, const char *fmt, va_list ap, FILE *fp)
+{
+	static int newline = 1;
+	char buf[LOG_BUF_SIZE];
+	int r;
+
+	r = vsnprintf(buf, LOG_BUF_SIZE, fmt, ap);
+
+	if (r > LOG_BUF_SIZE)
+		r = LOG_BUF_SIZE;
+
+	if (trace_logfp) {
+		if (newline)
+			fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf);
+		else
+			fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf);
+		newline = buf[r - 1] == '\n';
+		fflush(trace_logfp);
+		return;
+	}
+
+	fprintf(fp, "%.*s", r, buf);
+}
+
+void plog(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	__plog("", fmt, ap, stdout);
+	va_end(ap);
+	/* Make sure it gets to the screen, in case we crash afterward */
+	fflush(stdout);
+}
+
+void plog_error(const char *fmt, ...)
+{
+	va_list ap;
+	char *str = "";
+
+	va_start(ap, fmt);
+	__plog("Error: ", fmt, ap, stderr);
+	va_end(ap);
+	if (errno)
+		str = strerror(errno);
+	if (trace_logfp)
+		fprintf(trace_logfp, "\n%s\n", str);
+	else
+		fprintf(stderr, "\n%s\n", str);
+}
+
+/**
+ * trace_set_log_file - Set file for logging
+ * @logfile: Name of the log file
+ *
+ * Returns 0 on successful completion or -1 in case of error
+ */
+int trace_set_log_file(char *logfile)
+{
+	if (trace_logfp)
+		fclose(trace_logfp);
+	trace_logfp = fopen(logfile, "w");
+	if (!trace_logfp)
+		return -1;
+	return 0;
+}
+
diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c
index 3cbee67..e0dcd71 100644
--- a/tracecmd/trace-listen.c
+++ b/tracecmd/trace-listen.c
@@ -34,8 +34,6 @@ static char *output_dir;
 static char *default_output_file = "trace";
 static char *output_file;
 
-static FILE *logfp;
-
 static int backlog = 5;
 
 static int do_daemon;
@@ -44,6 +42,13 @@ static int do_daemon;
 static struct tracecmd_msg_handle *stop_msg_handle;
 static bool done;
 
+#define pdie(fmt, ...)				\
+	do {					\
+		plog_error(fmt, ##__VA_ARGS__);	\
+		remove_pid_file();		\
+		exit(-1);			\
+	} while (0)
+
 #define  TEMP_FILE_STR "%s.%s:%s.cpu%d", output_file, host, port, cpu
 static char *get_temp_file(const char *host, const char *port, int cpu)
 {
@@ -114,43 +119,6 @@ static void finish(int sig)
 	done = true;
 }
 
-#define LOG_BUF_SIZE 1024
-static void __plog(const char *prefix, const char *fmt, va_list ap,
-		   FILE *fp)
-{
-	static int newline = 1;
-	char buf[LOG_BUF_SIZE];
-	int r;
-
-	r = vsnprintf(buf, LOG_BUF_SIZE, fmt, ap);
-
-	if (r > LOG_BUF_SIZE)
-		r = LOG_BUF_SIZE;
-
-	if (logfp) {
-		if (newline)
-			fprintf(logfp, "[%d]%s%.*s", getpid(), prefix, r, buf);
-		else
-			fprintf(logfp, "[%d]%s%.*s", getpid(), prefix, r, buf);
-		newline = buf[r - 1] == '\n';
-		fflush(logfp);
-		return;
-	}
-
-	fprintf(fp, "%.*s", r, buf);
-}
-
-void plog(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	__plog("", fmt, ap, stdout);
-	va_end(ap);
-	/* Make sure it gets to the screen, in case we crash afterward */
-	fflush(stdout);
-}
-
 static void make_pid_name(int mode, char *buf)
 {
 	snprintf(buf, PATH_MAX, VAR_RUN_DIR "/trace-cmd-net.pid");
@@ -169,26 +137,6 @@ static void remove_pid_file(void)
 	unlink(buf);
 }
 
-void pdie(const char *fmt, ...)
-{
-	va_list ap;
-	char *str = "";
-
-	va_start(ap, fmt);
-	__plog("Error: ", fmt, ap, stderr);
-	va_end(ap);
-	if (errno)
-		str = strerror(errno);
-	if (logfp)
-		fprintf(logfp, "\n%s\n", str);
-	else
-		fprintf(stderr, "\n%s\n", str);
-
-	remove_pid_file();
-
-	exit(-1);
-}
-
 static int process_udp_child(int sfd, const char *host, const char *port,
 			     int cpu, int page_size, int use_tcp)
 {
@@ -1030,8 +978,7 @@ void trace_listen(int argc, char **argv)
 
 	if (logfile) {
 		/* set the writes to a logfile instead */
-		logfp = fopen(logfile, "w");
-		if (!logfp)
+		if (trace_set_log_file(logfile) < 0)
 			die("creating log file %s", logfile);
 	}
 
-- 
2.21.0

[PATCH v2 7/8] trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h

[Tzvetomir Stoyanov (VMware)]

These APIs depend on trace-cmd context and cannot be used standalone:
	tracecmd_create_top_instance()
	tracecmd_remove_instances()
	tracecmd_filter_pid()
	tracecmd_add_event()
	tracecmd_enable_events()
	tracecmd_disable_all_tracing()
	tracecmd_disable_tracing()
	tracecmd_enable_tracing()
	tracecmd_stat_cpu()
They are implemented in trace-cmd application, but declared as APIs in
trace-cmd.h. The declarations are moved from trace-cmd.h to trace-local.h,
local header file visible only to trace-cmd application.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h  |  9 ---------
 tracecmd/include/trace-local.h | 11 +++++++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 8db0686..2c8d798 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -137,8 +137,6 @@ int tracecmd_buffer_instances(struct tracecmd_input *handle);
 const char *tracecmd_buffer_instance_name(struct tracecmd_input *handle, int indx);
 struct tracecmd_input *tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx);
 int tracecmd_is_buffer_instance(struct tracecmd_input *handle);
-void tracecmd_create_top_instance(char *name);
-void tracecmd_remove_instances(void);
 
 void tracecmd_set_ts_offset(struct tracecmd_input *handle, unsigned long long offset);
 void tracecmd_set_ts2secs(struct tracecmd_input *handle, unsigned long long hz);
@@ -304,14 +302,7 @@ struct tracecmd_recorder *tracecmd_create_buffer_recorder_maxkb(const char *file
 
 int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long sleep);
 void tracecmd_stop_recording(struct tracecmd_recorder *recorder);
-void tracecmd_stat_cpu(struct trace_seq *s, int cpu);
 long tracecmd_flush_recording(struct tracecmd_recorder *recorder);
-void tracecmd_filter_pid(int pid, int exclude);
-int tracecmd_add_event(const char *event_str, int stack);
-void tracecmd_enable_events(void);
-void tracecmd_disable_all_tracing(int disable_tracer);
-void tracecmd_disable_tracing(void);
-void tracecmd_enable_tracing(void);
 
 enum tracecmd_msg_flags {
 	TRACECMD_MSG_FL_USE_TCP		= 1 << 0,
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index e2d5506..05760d8 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -216,6 +216,17 @@ void show_instance_file(struct buffer_instance *instance, const char *name);
 
 int count_cpus(void);
 
+/* moved from trace-cmd.h */
+void tracecmd_create_top_instance(char *name);
+void tracecmd_remove_instances(void);
+void tracecmd_filter_pid(int pid, int exclude);
+int tracecmd_add_event(const char *event_str, int stack);
+void tracecmd_enable_events(void);
+void tracecmd_disable_all_tracing(int disable_tracer);
+void tracecmd_disable_tracing(void);
+void tracecmd_enable_tracing(void);
+void tracecmd_stat_cpu(struct trace_seq *s, int cpu);
+
 /* No longer in event-utils.h */
 void __noreturn die(const char *fmt, ...); /* Can be overriden */
 void *malloc_or_die(unsigned int size); /* Can be overridden */
-- 
2.21.0

[PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd

[Tzvetomir Stoyanov (VMware)]

tracecmd_stack_tracer_status() function reads the stack tracer status
from the proc file system. It does not depend on trace-cmd context and
can be used standalone. The function is moved from trace-cmd application
into libtracecmd.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-util.c | 49 +++++++++++++++++++++++++++++++++
 tracecmd/trace-stack.c     | 56 ++------------------------------------
 2 files changed, 51 insertions(+), 54 deletions(-)

diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 8c1a0a0..d16a018 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -25,6 +25,7 @@
 #define LOCAL_PLUGIN_DIR ".trace-cmd/plugins"
 #define TRACEFS_PATH "/sys/kernel/tracing"
 #define DEBUGFS_PATH "/sys/kernel/debug"
+#define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled"
 
 int tracecmd_disable_sys_plugins;
 int tracecmd_disable_plugins;
@@ -1787,3 +1788,51 @@ int trace_set_log_file(char *logfile)
 	return 0;
 }
 
+/**
+ * tracecmd_stack_tracer_status - Check stack trace status
+ * @status: Returned stack trace status:
+ *		0 - not configured, disabled
+ *		non 0 - enabled
+ *
+ * Returns -1 in case of an error, 0 if file does not exist
+ *  (stack tracer not enabled) or 1 on successful completion.
+ */
+int tracecmd_stack_tracer_status(int *status)
+{
+	struct stat stat_buf;
+	char buf[64];
+	long num;
+	int fd;
+	int n;
+
+	if (stat(PROC_STACK_FILE, &stat_buf) < 0) {
+		/* stack tracer not configured on running kernel */
+		*status = 0; /* not configured means disabled */
+		return 0;
+	}
+
+	fd = open(PROC_STACK_FILE, O_RDONLY);
+
+	if (fd < 0)
+		return -1;
+
+	n = read(fd, buf, sizeof(buf));
+	close(fd);
+
+	if (n <= 0)
+		return -1;
+
+	if (n >= sizeof(buf))
+		return -1;
+
+	buf[n] = 0;
+	errno = 0;
+	num = strtol(buf, NULL, 10);
+
+	/* Check for various possible errors */
+	if (num > INT_MAX || num < INT_MIN || (!num && errno))
+		return -1;
+
+	*status = num;
+	return 1; /* full success */
+}
diff --git a/tracecmd/trace-stack.c b/tracecmd/trace-stack.c
index 34b3b58..bb002c0 100644
--- a/tracecmd/trace-stack.c
+++ b/tracecmd/trace-stack.c
@@ -36,58 +36,6 @@ static void test_available(void)
 		die("stack tracer not configured on running kernel");
 }
 
-/*
- * Returns:
- *   -1 - Something went wrong
- *    0 - File does not exist (stack tracer not enabled)
- *    1 - Success
- */
-static int read_proc(int *status)
-{
-	struct stat stat_buf;
-	char buf[64];
-	long num;
-	int fd;
-	int n;
-
-	if (stat(PROC_FILE, &stat_buf) < 0) {
-		/* stack tracer not configured on running kernel */
-		*status = 0; /* not configured means disabled */
-		return 0;
-	}
-
-	fd = open(PROC_FILE, O_RDONLY);
-
-	if (fd < 0)
-		return -1;
-
-	n = read(fd, buf, sizeof(buf));
-	close(fd);
-
-	if (n <= 0)
-		return -1;
-
-	if (n >= sizeof(buf))
-		return -1;
-
-	buf[n] = 0;
-	errno = 0;
-	num = strtol(buf, NULL, 10);
-
-	/* Check for various possible errors */
-	if (num > INT_MAX || num < INT_MIN || (!num && errno))
-		return -1;
-
-	*status = num;
-	return 1; /* full success */
-}
-
-/* Public wrapper of read_proc() */
-int tracecmd_stack_tracer_status(int *status)
-{
-	return read_proc(status);
-}
-
 /* NOTE: this implementation only accepts new_status in the range [0..9]. */
 static void change_stack_tracer_status(unsigned new_status)
 {
@@ -102,7 +50,7 @@ static void change_stack_tracer_status(unsigned new_status)
 		return;
 	}
 
-	ret = read_proc(&status);
+	ret = tracecmd_stack_tracer_status(&status);
 	if (ret < 0)
 		die("error reading %s", PROC_FILE);
 
@@ -160,7 +108,7 @@ static void read_trace(void)
 	size_t n;
 	int r;
 
-	if (read_proc(&status) <= 0)
+	if (tracecmd_stack_tracer_status(&status) <= 0)
 		die("Invalid stack tracer state");
 
 	if (status > 0)
-- 
2.21.0

[PATCH v5 0/3] Add new trace-cmd record options: "--proc-map"

[Tzvetomir Stoyanov (VMware)]

New options to trace-cmd record are added:
  - "--proc-map" - Saves traced process(es) address map in trace.dat
    file.
  - "--user" - Executes the traced process as given user.

As "--proc-map" option uses trace-cmd ptrace logic, part of this code is
rewritten. There were some leftovers related to this logic, the code is
upadted.

[
   v5 changes:
        - Added new patch: 
            "Extend ptrace logic to work with multiple filtered pids"
          It resolves "filter_pid" leftover in ptrace related logic.
        - "--proc-map" does not depend on option -F, it works with any command,
          specified as trace-cmd argument or option -P.
        - Renamed "mmap" to "proc-map" - the option name and the names of
          the functions, variables and defines related to this feature.
   v4 changes:
	- Added check for strdup() failure.
	- Made input user string argument of change_user() and run_cmd()
	 constant.
        - Added description of the new "--mmap" trace-cmd option in the 
          program's help and the man page. (Suggested by Slavomir Kaslev)

      Problems, reported by Yordan Karadzhov:
        - Improved the parsing of /proc/<pid>/maps. Made it not so strict, as it
          failed on some machines due to different size of fields. 
        - Implemented trace_pid_mmap_free() cleanup function to free mmap
          related resources at trace-cmd exit.
        - Fixed potential problem with non-terminated string, returned by
          readlink().
        - Coding style fixes.
   v3 changes:
      - "--user" does not depend on option -F, it works with any command,
        specified as trace-cmd argument.
      - Changed tracecmd_search_task_mmap() API to return not only the library
     name, but also the start and end memory addresses.
      - Renamed *tracee* to *task*
      - Improved resources cleanup, in case of an error.
      - Removed (this) changelog from the commit message.
   v2 changes:
     - Check for errors in change_user(). If an error occurs while
       changing the user, the message is printed and the traced 
       process is not executed.
     - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API.
     - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used.
     - Return error in case memory allocation fails.
     - Return error if option string is not in the expected format.
     - Sort memory maps and use binary search to find matching library in the map.
]

Tzvetomir Stoyanov (VMware) (3):
  trace-cmd: Extend ptrace logic to work with multiple filtered pids
  trace-cmd: Save the tracee address map into the trace.dat file.
  trace-cmd: Add option to execute traced process as given user

 Documentation/trace-cmd-record.1.txt |   6 +
 include/trace-cmd/trace-cmd.h        |  10 +
 lib/trace-cmd/trace-input.c          | 172 +++++++++++++++-
 tracecmd/include/trace-local.h       |  10 +
 tracecmd/trace-record.c              | 282 +++++++++++++++++++++++++--
 tracecmd/trace-usage.c               |   2 +
 6 files changed, 464 insertions(+), 18 deletions(-)

-- 
2.21.0

[PATCH v5 1/3] trace-cmd: Extend ptrace logic to work with multiple filtered pids

[Tzvetomir Stoyanov (VMware)]

In the trace-record.c file there is a global variable named "filter_pid".
It is not set anywhere in the code, but there is a logic which relies on it.
This variable is a leftover from the past implementation of trace-cmd
record "-P" option, when it was designed to filter only a single PID.
Now "-P" option works with a list of PIDs, stored in filter_pids global
list. The code is modified to work with filter_pids instead of filter_pid.
This logic is used only if there is no ftrace "options/event-fork" on the
system and we have ptrace support. There is one significant change in
the trace-cmd record behavior in this specific use case:
  - filtered pids are specified with the "-P" option.
  - there is no ftrace "options/event-fork" on the system.
  - there is ptrace support.
The trace continues until Ctrl^C is hit or all filtered PIDs exit,
whatever comes first.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/trace-record.c | 65 ++++++++++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 13 deletions(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 5dc6f17..e0fa07d 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -86,7 +86,6 @@ static bool use_tcp;
 static int do_ptrace;
 
 static int filter_task;
-static int filter_pid = -1;
 static bool no_filter = false;
 
 static int local_cpu_count;
@@ -866,6 +865,13 @@ static void add_filter_pid(int pid, int exclude)
 	struct filter_pids *p;
 	char buf[100];
 
+	for (p = filter_pids; p; p = p->next) {
+		if (p->pid == pid) {
+			p->exclude = exclude;
+			return;
+		}
+	}
+
 	p = malloc(sizeof(*p));
 	if (!p)
 		die("Failed to allocate pid filter");
@@ -1223,17 +1229,34 @@ static void enable_ptrace(void)
 	ptrace(PTRACE_TRACEME, 0, NULL, 0);
 }
 
-static void ptrace_wait(enum trace_type type, int main_pid)
+static void ptrace_wait(enum trace_type type)
 {
+	struct filter_pids *fpid;
 	unsigned long send_sig;
 	unsigned long child;
 	siginfo_t sig;
+	int main_pids;
 	int cstatus;
 	int status;
+	int i = 0;
+	int *pids;
 	int event;
 	int pid;
 	int ret;
 
+	pids = calloc(nr_filter_pids, sizeof(int));
+	if (!pids)
+		return;
+
+	for (fpid = filter_pids; fpid; fpid = fpid->next) {
+		if (fpid->exclude)
+			continue;
+		pids[i++] = fpid->pid;
+		if (i >= nr_filter_pids)
+			break;
+	}
+	main_pids = i;
+
 	do {
 		ret = trace_waitpid(type, -1, &status, WSTOPPED | __WALL);
 		if (ret < 0)
@@ -1275,11 +1298,24 @@ static void ptrace_wait(enum trace_type type, int main_pid)
 			       PTRACE_O_TRACEEXIT);
 			ptrace(PTRACE_CONT, pid, NULL, send_sig);
 		}
-	} while (!finished && ret > 0 &&
-		 (!WIFEXITED(status) || pid != main_pid));
+		if (WIFEXITED(status) ||
+		   (WIFSTOPPED(status) && event == PTRACE_EVENT_EXIT)) {
+			for (i = 0; i < nr_filter_pids; i++) {
+				if (pid == pids[i]) {
+					pids[i] = 0;
+					main_pids--;
+					if (!main_pids)
+						finished = 1;
+					break;
+				}
+			}
+		}
+	} while (!finished && ret > 0);
+
+	free(pids);
 }
 #else
-static inline void ptrace_wait(enum trace_type type, int main_pid) { }
+static inline void ptrace_wait(enum trace_type type) { }
 static inline void enable_ptrace(void) { }
 static inline void ptrace_attach(int pid) { }
 
@@ -1289,8 +1325,8 @@ static void trace_or_sleep(enum trace_type type)
 {
 	struct timeval tv = { 1 , 0 };
 
-	if (do_ptrace && filter_pid >= 0)
-		ptrace_wait(type, filter_pid);
+	if (do_ptrace && filter_pids)
+		ptrace_wait(type);
 	else if (type & TRACE_TYPE_STREAM)
 		trace_stream_read(pids, recorder_threads, &tv);
 	else
@@ -1327,7 +1363,7 @@ static void run_cmd(enum trace_type type, int argc, char **argv)
 	}
 	if (do_ptrace) {
 		add_filter_pid(pid, 0);
-		ptrace_wait(type, pid);
+		ptrace_wait(type);
 	} else
 		trace_waitpid(type, pid, &status, 0);
 }
@@ -2318,7 +2354,7 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
 	*event = *old_event;
 	add_event(instance, event);
 
-	if (event->filter || filter_task || filter_pid) {
+	if (event->filter || filter_task || filter_pids) {
 		event->filter_file = strdup(path);
 		if (!event->filter_file)
 			die("malloc filter file");
@@ -4924,9 +4960,9 @@ static void parse_record_options(int argc,
 		add_func(&ctx->instance->filter_funcs,
 			 ctx->instance->filter_mod, "*");
 
-	if (do_ptrace && !filter_task && (filter_pid < 0))
+	if (do_ptrace && !filter_task && !nr_filter_pids)
 		die(" -c can only be used with -F (or -P with event-fork support)");
-	if (ctx->do_child && !filter_task &&! filter_pid)
+	if (ctx->do_child && !filter_task && !nr_filter_pids)
 		die(" -c can only be used with -P or -F");
 
 	if ((argc - optind) >= 2) {
@@ -4997,6 +5033,7 @@ static void record_trace(int argc, char **argv,
 {
 	enum trace_type type = get_trace_cmd_type(ctx->curr_cmd);
 	struct buffer_instance *instance;
+	struct filter_pids *pid;
 
 	/*
 	 * If top_instance doesn't have any plugins or events, then
@@ -5083,8 +5120,10 @@ static void record_trace(int argc, char **argv,
 		update_task_filter();
 		tracecmd_enable_tracing();
 		/* We don't ptrace ourself */
-		if (do_ptrace && filter_pid >= 0)
-			ptrace_attach(filter_pid);
+		if (do_ptrace && filter_pids)
+			for (pid = filter_pids; pid; pid = pid->next)
+				if (!pid->exclude)
+					ptrace_attach(pid->pid);
 		/* sleep till we are woken with Ctrl^C */
 		printf("Hit Ctrl^C to stop recording\n");
 		while (!finished)
-- 
2.21.0

[PATCH v5 2/3] trace-cmd: Save the tracee address map into the trace.dat file.

[Tzvetomir Stoyanov (VMware)]

A new trace-cmd record option is added: "--proc-map". When it is set
the address map of the traced applications is stored in the trace.dat
file. The traced applications can be specified using the option -P,
or as a given 'command'. A new API tracecmd_search_task_map() can be
used to look up into stored address maps. The map is retrieved from
/proc/<pid>/maps file.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
[
 v5 changes:
   - Added new patch:
        "Extend ptrace logic to work with multiple filtered pids"
     It resolves "filter_pid" leftover in ptrace related logic.
   - "--proc-map" does not depend on option -F, it works with any command,
     specified as trace-cmd argument or option -P.
   - Renamed "mmap" to "proc-map" - the option name and the names of
     the functions, variables and defines related to this feature. 
 v4 changes:
   - Added description of the new "--mmap" trace-cmd option in the 
    program's help and the man page. (Suggested by Slavomir Kaslev)

  Problems, reported by Yordan Karadzhov:
   - Improved the parsing of /proc/<pid>/maps. Made it not so strict, as it
     failed on some machines due to different size of fields. 
   - Implemented trace_pid_mmap_free() cleanup function to free mmap
     related resources at trace-cmd exit.
   - Fixed potential problem with non-terminated string, returned by
     readlink().
   - Coding style fixes.
 v3 changes:
   - Changed tracecmd_search_task_mmap() API to return not only the library
     name, but also the start and end memory addresses.
   - Renamed *tracee* to *task*
   - Improved resources cleanup, in case of an error.
   - Removed (this) changelog from the commit message.

 v2 changes:
   - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API.
   - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used.
   - Return error in case memory allocation fails.
   - Return error if option string is not in the expected format.
   - Sort memory maps and use binary search to find matching library in the map.
]

 Documentation/trace-cmd-record.1.txt |   3 +
 include/trace-cmd/trace-cmd.h        |  10 ++
 lib/trace-cmd/trace-input.c          | 172 ++++++++++++++++++++++++++-
 tracecmd/include/trace-local.h       |  10 ++
 tracecmd/trace-record.c              | 172 ++++++++++++++++++++++++++-
 tracecmd/trace-usage.c               |   1 +
 6 files changed, 364 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
index 26a8299..e697f03 100644
--- a/Documentation/trace-cmd-record.1.txt
+++ b/Documentation/trace-cmd-record.1.txt
@@ -288,6 +288,9 @@ OPTIONS
 
     '--module snd -n "*"' is equivalent to '-n :mod:snd'
 
+*--proc-map*::
+     Save the traced process address map into the trace.dat file. The traced
+     processes can be specified using the option *-P*, or as a given 'command'.
 
 *--profile*::
     With the *--profile* option, "trace-cmd" will enable tracing that can
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 6f62ab9..c4a437a 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -82,6 +82,7 @@ enum {
 	TRACECMD_OPTION_OFFSET,
 	TRACECMD_OPTION_CPUCOUNT,
 	TRACECMD_OPTION_VERSION,
+	TRACECMD_OPTION_PROCMAPS,
 };
 
 enum {
@@ -97,6 +98,12 @@ struct tracecmd_ftrace {
 	int long_size;
 };
 
+struct tracecmd_proc_addr_map {
+	unsigned long long	start;
+	unsigned long long	end;
+	char			*lib_name;
+};
+
 typedef void (*tracecmd_show_data_func)(struct tracecmd_input *handle,
 					struct tep_record *record);
 typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
@@ -208,6 +215,9 @@ unsigned long long tracecmd_page_ts(struct tracecmd_input *handle,
 unsigned int tracecmd_record_ts_delta(struct tracecmd_input *handle,
 				      struct tep_record *record);
 
+struct tracecmd_proc_addr_map *
+tracecmd_search_task_map(struct tracecmd_input *handle,
+			 int pid, unsigned long long addr);
 #ifndef SWIG
 /* hack for function graph work around */
 extern __thread struct tracecmd_input *tracecmd_curr_thread_handle;
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 654101f..a6fa7f5 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -101,6 +101,7 @@ struct tracecmd_input {
 	struct tracecmd_ftrace	finfo;
 
 	struct hook_list	*hooks;
+	struct pid_addr_maps	*pid_maps;
 	/* file information */
 	size_t			header_files_start;
 	size_t			ftrace_files_start;
@@ -2136,6 +2137,167 @@ void tracecmd_set_ts2secs(struct tracecmd_input *handle,
 	handle->use_trace_clock = false;
 }
 
+static int trace_pid_map_cmp(const void *a, const void *b)
+{
+	struct tracecmd_proc_addr_map *m_a = (struct tracecmd_proc_addr_map *)a;
+	struct tracecmd_proc_addr_map *m_b = (struct tracecmd_proc_addr_map *)b;
+
+	if (m_a->start > m_b->start)
+		return 1;
+	if (m_a->start < m_b->start)
+		return -1;
+	return 0;
+}
+
+static void procmap_free(struct pid_addr_maps *maps)
+{
+	int i;
+
+	if (!maps)
+		return;
+	if (maps->lib_maps) {
+		for (i = 0; i < maps->nr_lib_maps; i++)
+			free(maps->lib_maps[i].lib_name);
+		free(maps->lib_maps);
+	}
+	free(maps->proc_name);
+	free(maps);
+}
+
+#define STR_PROCMAP_LINE_MAX	(PATH_MAX+22)
+static int trace_pid_map_load(struct tracecmd_input *handle, char *buf)
+{
+	struct pid_addr_maps *maps = NULL;
+	char mapname[STR_PROCMAP_LINE_MAX];
+	char *line;
+	int res;
+	int ret;
+	int i;
+
+	maps = calloc(1, sizeof(*maps));
+	if (!maps)
+		return -ENOMEM;
+
+	ret  = -EINVAL;
+	line = strchr(buf, '\n');
+	if (!line)
+		goto out_fail;
+
+	*line = '\0';
+	if (strlen(buf) > STR_PROCMAP_LINE_MAX)
+		goto out_fail;
+
+	res = sscanf(buf, "%x %x %s", &maps->pid, &maps->nr_lib_maps, mapname);
+	if (res != 3)
+		goto out_fail;
+
+	ret  = -ENOMEM;
+	maps->proc_name = strdup(mapname);
+	if (!maps->proc_name)
+		goto out_fail;
+
+	maps->lib_maps = calloc(maps->nr_lib_maps, sizeof(struct tracecmd_proc_addr_map));
+	if (!maps->lib_maps)
+		goto out_fail;
+
+	buf = line + 1;
+	line = strchr(buf, '\n');
+	for (i = 0; i < maps->nr_lib_maps; i++) {
+		if (!line)
+			break;
+		*line = '\0';
+		if (strlen(buf) > STR_PROCMAP_LINE_MAX)
+			break;
+		res = sscanf(buf, "%llx %llx %s", &maps->lib_maps[i].start,
+			     &maps->lib_maps[i].end, mapname);
+		if (res != 3)
+			break;
+		maps->lib_maps[i].lib_name = strdup(mapname);
+		if (!maps->lib_maps[i].lib_name)
+			goto out_fail;
+		buf = line + 1;
+		line = strchr(buf, '\n');
+	}
+
+	ret  = -EINVAL;
+	if (i != maps->nr_lib_maps)
+		goto out_fail;
+
+	qsort(maps->lib_maps, maps->nr_lib_maps,
+	      sizeof(*maps->lib_maps), trace_pid_map_cmp);
+
+	maps->next = handle->pid_maps;
+	handle->pid_maps = maps;
+
+	return 0;
+
+out_fail:
+	procmap_free(maps);
+	return ret;
+}
+
+static void trace_pid_map_free(struct pid_addr_maps *maps)
+{
+	struct pid_addr_maps *del;
+
+	while (maps) {
+		del = maps;
+		maps = maps->next;
+		procmap_free(del);
+	}
+}
+
+static int trace_pid_map_search(const void *a, const void *b)
+{
+	struct tracecmd_proc_addr_map *key = (struct tracecmd_proc_addr_map *)a;
+	struct tracecmd_proc_addr_map *map = (struct tracecmd_proc_addr_map *)b;
+
+	if (key->start >= map->end)
+		return 1;
+	if (key->start < map->start)
+		return -1;
+	return 0;
+}
+
+/**
+ * tracecmd_search_task_map - Search task memory address map
+ * @handle: input handle to the trace.dat file
+ * @pid: pid of the task
+ * @addr: address from the task memory space.
+ *
+ * Map of the task memory can be saved in the trace.dat file, using the option
+ * "--proc-map". If there is such information, this API can be used to look up
+ * into this memory map to find what library is loaded at the given @addr.
+ *
+ * A pointer to struct tracecmd_proc_addr_map is returned, containing the name
+ * of the library at given task @addr and the library start and end addresses.
+ */
+struct tracecmd_proc_addr_map *
+tracecmd_search_task_map(struct tracecmd_input *handle,
+			 int pid, unsigned long long addr)
+{
+	struct tracecmd_proc_addr_map *lib;
+	struct tracecmd_proc_addr_map key;
+	struct pid_addr_maps *maps;
+
+	if (!handle || !handle->pid_maps)
+		return NULL;
+
+	maps = handle->pid_maps;
+	while (maps) {
+		if (maps->pid == pid)
+			break;
+		maps = maps->next;
+	}
+	if (!maps || !maps->nr_lib_maps || !maps->lib_maps)
+		return NULL;
+	key.start = addr;
+	lib = bsearch(&key, maps->lib_maps, maps->nr_lib_maps,
+		      sizeof(*maps->lib_maps), trace_pid_map_search);
+
+	return lib;
+}
+
 static int handle_options(struct tracecmd_input *handle)
 {
 	unsigned long long offset;
@@ -2223,9 +2385,6 @@ static int handle_options(struct tracecmd_input *handle)
 		case TRACECMD_OPTION_UNAME:
 			handle->uname = strdup(buf);
 			break;
-		case TRACECMD_OPTION_VERSION:
-			handle->version = strdup(buf);
-			break;
 		case TRACECMD_OPTION_HOOK:
 			hook = tracecmd_create_event_hook(buf);
 			hook->next = handle->hooks;
@@ -2235,6 +2394,10 @@ static int handle_options(struct tracecmd_input *handle)
 			cpus = *(int *)buf;
 			handle->cpus = tep_read_number(handle->pevent, &cpus, 4);
 			break;
+		case TRACECMD_OPTION_PROCMAPS:
+			if (buf[size-1] == '\0')
+				trace_pid_map_load(handle, buf);
+			break;
 		default:
 			warning("unknown option %d", option);
 			break;
@@ -2848,6 +3011,9 @@ void tracecmd_close(struct tracecmd_input *handle)
 	tracecmd_free_hooks(handle->hooks);
 	handle->hooks = NULL;
 
+	trace_pid_map_free(handle->pid_maps);
+	handle->pid_maps = NULL;
+
 	if (handle->flags & TRACECMD_FL_BUFFER_INSTANCE)
 		tracecmd_close(handle->parent);
 	else {
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 1cad3cc..78c52dc 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -157,6 +157,14 @@ struct func_list {
 	const char *mod;
 };
 
+struct pid_addr_maps {
+	struct pid_addr_maps		*next;
+	struct tracecmd_proc_addr_map	*lib_maps;
+	unsigned int			nr_lib_maps;
+	char				*proc_name;
+	int				pid;
+};
+
 struct buffer_instance {
 	struct buffer_instance	*next;
 	const char		*name;
@@ -183,6 +191,8 @@ struct buffer_instance {
 	struct tracecmd_msg_handle *msg_handle;
 	struct tracecmd_output *network_handle;
 
+	struct pid_addr_maps	*pid_maps;
+
 	char			*max_graph_depth;
 
 	int			flags;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index e0fa07d..17c676c 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -84,6 +84,8 @@ static int max_kb;
 static bool use_tcp;
 
 static int do_ptrace;
+static int do_children;
+static int get_procmap;
 
 static int filter_task;
 static bool no_filter = false;
@@ -1068,6 +1070,121 @@ static char *make_pid_filter(char *curr_filter, const char *field)
 	return filter;
 }
 
+#define _STRINGIFY(x) #x
+#define STRINGIFY(x) _STRINGIFY(x)
+
+static int get_pid_addr_maps(int pid)
+{
+	struct buffer_instance *instance = &top_instance;
+	struct pid_addr_maps *maps = instance->pid_maps;
+	struct tracecmd_proc_addr_map *map;
+	unsigned long long begin, end;
+	struct pid_addr_maps *m;
+	char mapname[PATH_MAX+1];
+	char fname[PATH_MAX+1];
+	char buf[PATH_MAX+100];
+	FILE *f;
+	int ret;
+	int res;
+	int i;
+
+	sprintf(fname, "/proc/%d/exe", pid);
+	ret = readlink(fname, mapname, PATH_MAX);
+	if (ret >= PATH_MAX || ret < 0)
+		return -ENOENT;
+	mapname[ret] = 0;
+
+	sprintf(fname, "/proc/%d/maps", pid);
+	f = fopen(fname, "r");
+	if (!f)
+		return -ENOENT;
+
+	while (maps) {
+		if (pid == maps->pid)
+			break;
+		maps = maps->next;
+	}
+
+	ret = -ENOMEM;
+	if (!maps) {
+		maps = calloc(1, sizeof(*maps));
+		if (!maps)
+			goto out_fail;
+		maps->pid = pid;
+		maps->next = instance->pid_maps;
+		instance->pid_maps = maps;
+	} else {
+		for (i = 0; i < maps->nr_lib_maps; i++)
+			free(maps->lib_maps[i].lib_name);
+		free(maps->lib_maps);
+		maps->lib_maps = NULL;
+		maps->nr_lib_maps = 0;
+		free(maps->proc_name);
+	}
+
+	maps->proc_name = strdup(mapname);
+	if (!maps->proc_name)
+		goto out;
+
+	while (fgets(buf, sizeof(buf), f)) {
+		mapname[0] = '\0';
+		res = sscanf(buf, "%llx-%llx %*s %*x %*s %*d %"STRINGIFY(PATH_MAX)"s",
+			     &begin, &end, mapname);
+		if (res == 3 && mapname[0] != '\0') {
+			map = realloc(maps->lib_maps,
+				      (maps->nr_lib_maps + 1) * sizeof(*map));
+			if (!map)
+				goto out_fail;
+			map[maps->nr_lib_maps].end = end;
+			map[maps->nr_lib_maps].start = begin;
+			map[maps->nr_lib_maps].lib_name = strdup(mapname);
+			if (!map[maps->nr_lib_maps].lib_name)
+				goto out_fail;
+			maps->lib_maps = map;
+			maps->nr_lib_maps++;
+		}
+	}
+out:
+	fclose(f);
+	return 0;
+
+out_fail:
+	fclose(f);
+	if (maps) {
+		for (i = 0; i < maps->nr_lib_maps; i++)
+			free(maps->lib_maps[i].lib_name);
+		if (instance->pid_maps != maps) {
+			m = instance->pid_maps;
+			while (m) {
+				if (m->next == maps) {
+					m->next = maps->next;
+					break;
+				}
+				m = m->next;
+			}
+		} else
+			instance->pid_maps = maps->next;
+		free(maps->lib_maps);
+		maps->lib_maps = NULL;
+		maps->nr_lib_maps = 0;
+		free(maps->proc_name);
+		maps->proc_name = NULL;
+		free(maps);
+	}
+	return ret;
+}
+
+static void get_filter_pid_maps(void)
+{
+	struct filter_pids *p;
+
+	for (p = filter_pids; p; p = p->next) {
+		if (p->exclude)
+			continue;
+		get_pid_addr_maps(p->pid);
+	}
+}
+
 static void update_task_filter(void)
 {
 	struct buffer_instance *instance;
@@ -1076,6 +1193,9 @@ static void update_task_filter(void)
 	if (no_filter)
 		return;
 
+	if (get_procmap && filter_pids)
+		get_filter_pid_maps();
+
 	if (filter_task)
 		add_filter_pid(pid, 0);
 
@@ -1287,6 +1407,8 @@ static void ptrace_wait(enum trace_type type)
 				break;
 
 			case PTRACE_EVENT_EXIT:
+				if (get_procmap)
+					get_pid_addr_maps(pid);
 				ptrace(PTRACE_GETEVENTMSG, pid, NULL, &cstatus);
 				ptrace(PTRACE_DETACH, pid, NULL, NULL);
 				break;
@@ -1363,6 +1485,7 @@ static void run_cmd(enum trace_type type, int argc, char **argv)
 	}
 	if (do_ptrace) {
 		add_filter_pid(pid, 0);
+		ptrace_attach(pid);
 		ptrace_wait(type);
 	} else
 		trace_waitpid(type, pid, &status, 0);
@@ -3130,6 +3253,36 @@ static void append_buffer(struct tracecmd_output *handle,
 	}
 }
 
+
+static void
+add_pid_maps(struct tracecmd_output *handle, struct buffer_instance *instance)
+{
+	struct pid_addr_maps *maps = instance->pid_maps;
+	struct trace_seq s;
+	int i;
+
+	trace_seq_init(&s);
+	while (maps) {
+		if (!maps->nr_lib_maps) {
+			maps = maps->next;
+			continue;
+		}
+		trace_seq_reset(&s);
+		trace_seq_printf(&s, "%x %x %s\n",
+				 maps->pid, maps->nr_lib_maps, maps->proc_name);
+		for (i = 0; i < maps->nr_lib_maps; i++)
+			trace_seq_printf(&s, "%llx %llx %s\n",
+					maps->lib_maps[i].start,
+					maps->lib_maps[i].end,
+					maps->lib_maps[i].lib_name);
+		trace_seq_terminate(&s);
+		tracecmd_add_option(handle, TRACECMD_OPTION_PROCMAPS,
+				    s.len + 1, s.buffer);
+		maps = maps->next;
+	}
+	trace_seq_destroy(&s);
+}
+
 static void
 add_buffer_stat(struct tracecmd_output *handle, struct buffer_instance *instance)
 {
@@ -3323,6 +3476,10 @@ static void record_data(struct common_record_context *ctx)
 		if (!no_top_instance() && !top_instance.msg_handle)
 			print_stat(&top_instance);
 
+		for_all_instances(instance) {
+			add_pid_maps(handle, instance);
+		}
+
 		tracecmd_append_cpu_data(handle, local_cpu_count, temp_files);
 
 		for (i = 0; i < max_cpu_count; i++)
@@ -4433,6 +4590,7 @@ void update_first_instance(struct buffer_instance *instance, int topt)
 }
 
 enum {
+	OPT_procmap		= 244,
 	OPT_quiet		= 245,
 	OPT_debug		= 246,
 	OPT_no_filter		= 247,
@@ -4663,6 +4821,7 @@ static void parse_record_options(int argc,
 			{"debug", no_argument, NULL, OPT_debug},
 			{"quiet", no_argument, NULL, OPT_quiet},
 			{"help", no_argument, NULL, '?'},
+			{"proc-map", no_argument, NULL, OPT_procmap},
 			{"module", required_argument, NULL, OPT_module},
 			{NULL, 0, NULL, 0}
 		};
@@ -4752,6 +4911,7 @@ static void parse_record_options(int argc,
 				die("-c invalid: ptrace not supported");
 #endif
 				do_ptrace = 1;
+				do_children = 1;
 			} else {
 				save_option("event-fork");
 				ctx->do_child = 1;
@@ -4894,6 +5054,9 @@ static void parse_record_options(int argc,
 		case 'i':
 			ignore_event_not_found = 1;
 			break;
+		case OPT_procmap:
+			get_procmap = 1;
+			break;
 		case OPT_date:
 			ctx->date = 1;
 			if (ctx->data_flags & DATA_FL_OFFSET)
@@ -4960,7 +5123,7 @@ static void parse_record_options(int argc,
 		add_func(&ctx->instance->filter_funcs,
 			 ctx->instance->filter_mod, "*");
 
-	if (do_ptrace && !filter_task && !nr_filter_pids)
+	if (do_children && !filter_task && !nr_filter_pids)
 		die(" -c can only be used with -F (or -P with event-fork support)");
 	if (ctx->do_child && !filter_task && !nr_filter_pids)
 		die(" -c can only be used with -P or -F");
@@ -4974,6 +5137,13 @@ static void parse_record_options(int argc,
 			    "Did you mean 'record'?");
 		ctx->run_command = 1;
 	}
+
+	if (get_procmap) {
+		if (!ctx->run_command && !nr_filter_pids)
+			warning("--proc-map is ignored, no command or filtered PIDs are specified.");
+		else
+			do_ptrace = 1;
+	}
 }
 
 static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 406384c..7a67784 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -57,6 +57,7 @@ static struct usage_help usage_help[] = {
 		"             (use with caution)\n"
 		"          --max-graph-depth limit function_graph depth\n"
 		"          --no-filter include trace-cmd threads in the trace\n"
+		"          --proc-map save the traced processes address map into the trace.dat file\n"
 	},
 	{
 		"start",
-- 
2.21.0

[PATCH v5 3/3] trace-cmd: Add option to execute traced process as given user

[Tzvetomir Stoyanov (VMware)]

A new trace-cmd record option is added: "--user". When it is set with
combination of option -F, the traced process is executed in the context
of the specified user.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
[
   v4 changes:
	- Added check for strdup() failure.
	- Made input user string argument of change_user() and run_cmd()
	 constant.
   v3 changes:
      "--user" does not depend on option -F, it works with any command,
       specified as trace-cmd argument.
   v2 changes:
     - Check for errors in change_user(). If an error occurs while
       changing the user, the message is printed and the traced 
       process is not executed.
]

 Documentation/trace-cmd-record.1.txt |  3 ++
 tracecmd/trace-record.c              | 49 ++++++++++++++++++++++++++--
 tracecmd/trace-usage.c               |  1 +
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
index e697f03..bb18e88 100644
--- a/Documentation/trace-cmd-record.1.txt
+++ b/Documentation/trace-cmd-record.1.txt
@@ -119,6 +119,9 @@ OPTIONS
      Used with either *-F* (or *-P* if kernel supports it) to trace the process'
      children too.
 
+*--user*::
+     Execute the specified *command* as given user.
+
 *-C* 'clock'::
      Set the trace clock to "clock".
 
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 17c676c..088aa28 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -33,6 +33,8 @@
 #include <errno.h>
 #include <limits.h>
 #include <libgen.h>
+#include <pwd.h>
+#include <grp.h>
 
 #include "version.h"
 #include "trace-local.h"
@@ -208,6 +210,7 @@ struct common_record_context {
 	struct buffer_instance *instance;
 	const char *output;
 	char *date2ts;
+	char *user;
 	int data_flags;
 
 	int record_all;
@@ -1455,7 +1458,34 @@ static void trace_or_sleep(enum trace_type type)
 		sleep(10);
 }
 
-static void run_cmd(enum trace_type type, int argc, char **argv)
+static int change_user(const char *user)
+{
+	struct passwd *pwd;
+
+	if (!user)
+		return 0;
+
+	pwd = getpwnam(user);
+	if (!pwd)
+		return -1;
+	if (initgroups(user, pwd->pw_gid) < 0)
+		return -1;
+	if (setgid(pwd->pw_gid) < 0)
+		return -1;
+	if (setuid(pwd->pw_uid) < 0)
+		return -1;
+
+	if (setenv("HOME", pwd->pw_dir, 1) < 0)
+		return -1;
+	if (setenv("USER", pwd->pw_name, 1) < 0)
+		return -1;
+	if (setenv("LOGNAME", pwd->pw_name, 1) < 0)
+		return -1;
+
+	return 0;
+}
+
+static void run_cmd(enum trace_type type, const char *user, int argc, char **argv)
 {
 	int status;
 	int pid;
@@ -1476,6 +1506,10 @@ static void run_cmd(enum trace_type type, int argc, char **argv)
 			dup2(save_stdout, 1);
 			close(save_stdout);
 		}
+
+		if (change_user(user) < 0)
+			die("Failed to change user to %s", user);
+
 		if (execvp(argv[0], argv)) {
 			fprintf(stderr, "\n********************\n");
 			fprintf(stderr, " Unable to exec %s\n", argv[0]);
@@ -4590,6 +4624,7 @@ void update_first_instance(struct buffer_instance *instance, int topt)
 }
 
 enum {
+	OPT_user		= 243,
 	OPT_procmap		= 244,
 	OPT_quiet		= 245,
 	OPT_debug		= 246,
@@ -4822,6 +4857,7 @@ static void parse_record_options(int argc,
 			{"quiet", no_argument, NULL, OPT_quiet},
 			{"help", no_argument, NULL, '?'},
 			{"proc-map", no_argument, NULL, OPT_procmap},
+			{"user", required_argument, NULL, OPT_user},
 			{"module", required_argument, NULL, OPT_module},
 			{NULL, 0, NULL, 0}
 		};
@@ -5054,6 +5090,11 @@ static void parse_record_options(int argc,
 		case 'i':
 			ignore_event_not_found = 1;
 			break;
+		case OPT_user:
+			ctx->user = strdup(optarg);
+			if (!ctx->user)
+				die("Failed to allocate user name");
+			break;
 		case OPT_procmap:
 			get_procmap = 1;
 			break;
@@ -5137,7 +5178,9 @@ static void parse_record_options(int argc,
 			    "Did you mean 'record'?");
 		ctx->run_command = 1;
 	}
-
+	if (ctx->user && !ctx->run_command)
+		warning("--user %s is ignored, no command is specified",
+			ctx->user);
 	if (get_procmap) {
 		if (!ctx->run_command && !nr_filter_pids)
 			warning("--proc-map is ignored, no command or filtered PIDs are specified.");
@@ -5285,7 +5328,7 @@ static void record_trace(int argc, char **argv,
 	}
 
 	if (ctx->run_command)
-		run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
+		run_cmd(type, ctx->user, (argc - optind) - 1, &argv[optind + 1]);
 	else {
 		update_task_filter();
 		tracecmd_enable_tracing();
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 7a67784..20cb66f 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -58,6 +58,7 @@ static struct usage_help usage_help[] = {
 		"          --max-graph-depth limit function_graph depth\n"
 		"          --no-filter include trace-cmd threads in the trace\n"
 		"          --proc-map save the traced processes address map into the trace.dat file\n"
+		"          --user execute the specified [command ...] as given user\n"
 	},
 	{
 		"start",
-- 
2.21.0

Re: [PATCH v5 0/3] Add new trace-cmd record options: "--proc-map"

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:09 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> New options to trace-cmd record are added:
>   - "--proc-map" - Saves traced process(es) address map in trace.dat
>     file.
>   - "--user" - Executes the traced process as given user.
> 
> As "--proc-map" option uses trace-cmd ptrace logic, part of this code is
> rewritten. There were some leftovers related to this logic, the code is
> upadted.

Hi Tzvetomir,

For some reason, this patch series ended up as part of the: Separate
trace-cmd and libtracecmd code series.

-- Steve

> 
> [
>    v5 changes:
>         - Added new patch: 
>             "Extend ptrace logic to work with multiple filtered pids"
>           It resolves "filter_pid" leftover in ptrace related logic.
>         - "--proc-map" does not depend on option -F, it works with any command,
>           specified as trace-cmd argument or option -P.
>         - Renamed "mmap" to "proc-map" - the option name and the names of
>           the functions, variables and defines related to this feature.
>    v4 changes:
> 	- Added check for strdup() failure.
> 	- Made input user string argument of change_user() and run_cmd()
> 	 constant.
>         - Added description of the new "--mmap" trace-cmd option in the 
>           program's help and the man page. (Suggested by Slavomir Kaslev)
> 
>       Problems, reported by Yordan Karadzhov:
>         - Improved the parsing of /proc/<pid>/maps. Made it not so strict, as it
>           failed on some machines due to different size of fields. 
>         - Implemented trace_pid_mmap_free() cleanup function to free mmap
>           related resources at trace-cmd exit.
>         - Fixed potential problem with non-terminated string, returned by
>           readlink().
>         - Coding style fixes.
>    v3 changes:
>       - "--user" does not depend on option -F, it works with any command,
>         specified as trace-cmd argument.
>       - Changed tracecmd_search_task_mmap() API to return not only the library
>      name, but also the start and end memory addresses.
>       - Renamed *tracee* to *task*
>       - Improved resources cleanup, in case of an error.
>       - Removed (this) changelog from the commit message.
>    v2 changes:
>      - Check for errors in change_user(). If an error occurs while
>        changing the user, the message is printed and the traced 
>        process is not executed.
>      - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API.
>      - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used.
>      - Return error in case memory allocation fails.
>      - Return error if option string is not in the expected format.
>      - Sort memory maps and use binary search to find matching library in the map.
> ]
> 
> Tzvetomir Stoyanov (VMware) (3):
>   trace-cmd: Extend ptrace logic to work with multiple filtered pids
>   trace-cmd: Save the tracee address map into the trace.dat file.
>   trace-cmd: Add option to execute traced process as given user
> 
>  Documentation/trace-cmd-record.1.txt |   6 +
>  include/trace-cmd/trace-cmd.h        |  10 +
>  lib/trace-cmd/trace-input.c          | 172 +++++++++++++++-
>  tracecmd/include/trace-local.h       |  10 +
>  tracecmd/trace-record.c              | 282 +++++++++++++++++++++++++--
>  tracecmd/trace-usage.c               |   2 +
>  6 files changed, 464 insertions(+), 18 deletions(-)
> 

Re: [PATCH v5 1/3] trace-cmd: Extend ptrace logic to work with multiple filtered pids

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:10 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> In the trace-record.c file there is a global variable named "filter_pid".
> It is not set anywhere in the code, but there is a logic which relies on it.
> This variable is a leftover from the past implementation of trace-cmd
> record "-P" option, when it was designed to filter only a single PID.
> Now "-P" option works with a list of PIDs, stored in filter_pids global
> list. The code is modified to work with filter_pids instead of filter_pid.
> This logic is used only if there is no ftrace "options/event-fork" on the
> system and we have ptrace support. There is one significant change in
> the trace-cmd record behavior in this specific use case:
>   - filtered pids are specified with the "-P" option.
>   - there is no ftrace "options/event-fork" on the system.
>   - there is ptrace support.
> The trace continues until Ctrl^C is hit or all filtered PIDs exit,
> whatever comes first.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  tracecmd/trace-record.c | 65 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 52 insertions(+), 13 deletions(-)
> 
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 5dc6f17..e0fa07d 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -86,7 +86,6 @@ static bool use_tcp;
>  static int do_ptrace;
>  
>  static int filter_task;
> -static int filter_pid = -1;
>  static bool no_filter = false;
>  
>  static int local_cpu_count;
> @@ -866,6 +865,13 @@ static void add_filter_pid(int pid, int exclude)
>  	struct filter_pids *p;
>  	char buf[100];
>  
> +	for (p = filter_pids; p; p = p->next) {
> +		if (p->pid == pid) {
> +			p->exclude = exclude;
> +			return;
> +		}
> +	}
> +
>  	p = malloc(sizeof(*p));
>  	if (!p)
>  		die("Failed to allocate pid filter");
> @@ -1223,17 +1229,34 @@ static void enable_ptrace(void)
>  	ptrace(PTRACE_TRACEME, 0, NULL, 0);
>  }
>  
> -static void ptrace_wait(enum trace_type type, int main_pid)
> +static void ptrace_wait(enum trace_type type)
>  {
> +	struct filter_pids *fpid;
>  	unsigned long send_sig;
>  	unsigned long child;
>  	siginfo_t sig;
> +	int main_pids;
>  	int cstatus;
>  	int status;
> +	int i = 0;
> +	int *pids;
>  	int event;
>  	int pid;
>  	int ret;
>  
> +	pids = calloc(nr_filter_pids, sizeof(int));
> +	if (!pids)

Probably at the minimum, we should add a warning here that it didn't
get allocated.

> +		return;
> +
> +	for (fpid = filter_pids; fpid; fpid = fpid->next) {
> +		if (fpid->exclude)
> +			continue;
> +		pids[i++] = fpid->pid;
> +		if (i >= nr_filter_pids)
> +			break;
> +	}
> +	main_pids = i;
> +
>  	do {
>  		ret = trace_waitpid(type, -1, &status, WSTOPPED | __WALL);
>  		if (ret < 0)
> @@ -1275,11 +1298,24 @@ static void ptrace_wait(enum trace_type type, int main_pid)
>  			       PTRACE_O_TRACEEXIT);
>  			ptrace(PTRACE_CONT, pid, NULL, send_sig);
>  		}
> -	} while (!finished && ret > 0 &&
> -		 (!WIFEXITED(status) || pid != main_pid));
> +		if (WIFEXITED(status) ||
> +		   (WIFSTOPPED(status) && event == PTRACE_EVENT_EXIT)) {
> +			for (i = 0; i < nr_filter_pids; i++) {
> +				if (pid == pids[i]) {
> +					pids[i] = 0;
> +					main_pids--;
> +					if (!main_pids)
> +						finished = 1;
> +					break;
> +				}
> +			}
> +		}
> +	} while (!finished && ret > 0);
> +
> +	free(pids);
>  }
>  #else
> -static inline void ptrace_wait(enum trace_type type, int main_pid) { }
> +static inline void ptrace_wait(enum trace_type type) { }
>  static inline void enable_ptrace(void) { }
>  static inline void ptrace_attach(int pid) { }
>  
> @@ -1289,8 +1325,8 @@ static void trace_or_sleep(enum trace_type type)
>  {
>  	struct timeval tv = { 1 , 0 };
>  
> -	if (do_ptrace && filter_pid >= 0)
> -		ptrace_wait(type, filter_pid);
> +	if (do_ptrace && filter_pids)
> +		ptrace_wait(type);
>  	else if (type & TRACE_TYPE_STREAM)
>  		trace_stream_read(pids, recorder_threads, &tv);
>  	else
> @@ -1327,7 +1363,7 @@ static void run_cmd(enum trace_type type, int argc, char **argv)
>  	}
>  	if (do_ptrace) {
>  		add_filter_pid(pid, 0);
> -		ptrace_wait(type, pid);
> +		ptrace_wait(type);
>  	} else
>  		trace_waitpid(type, pid, &status, 0);
>  }
> @@ -2318,7 +2354,7 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
>  	*event = *old_event;
>  	add_event(instance, event);
>  
> -	if (event->filter || filter_task || filter_pid) {
> +	if (event->filter || filter_task || filter_pids) {
>  		event->filter_file = strdup(path);
>  		if (!event->filter_file)
>  			die("malloc filter file");
> @@ -4924,9 +4960,9 @@ static void parse_record_options(int argc,
>  		add_func(&ctx->instance->filter_funcs,
>  			 ctx->instance->filter_mod, "*");
>  
> -	if (do_ptrace && !filter_task && (filter_pid < 0))
> +	if (do_ptrace && !filter_task && !nr_filter_pids)
>  		die(" -c can only be used with -F (or -P with event-fork support)");
> -	if (ctx->do_child && !filter_task &&! filter_pid)
> +	if (ctx->do_child && !filter_task && !nr_filter_pids)
>  		die(" -c can only be used with -P or -F");
>  
>  	if ((argc - optind) >= 2) {
> @@ -4997,6 +5033,7 @@ static void record_trace(int argc, char **argv,
>  {
>  	enum trace_type type = get_trace_cmd_type(ctx->curr_cmd);
>  	struct buffer_instance *instance;
> +	struct filter_pids *pid;
>  
>  	/*
>  	 * If top_instance doesn't have any plugins or events, then
> @@ -5083,8 +5120,10 @@ static void record_trace(int argc, char **argv,
>  		update_task_filter();
>  		tracecmd_enable_tracing();
>  		/* We don't ptrace ourself */
> -		if (do_ptrace && filter_pid >= 0)
> -			ptrace_attach(filter_pid);
> +		if (do_ptrace && filter_pids)
> +			for (pid = filter_pids; pid; pid = pid->next)
> +				if (!pid->exclude)
> +					ptrace_attach(pid->pid);

Just a nit, the above should have brackets. Leaving off brackets is
fine for non complex blocks. That is, only the internal if should have
no brackets:

	if (do_ptrace && filter_pids) {
		for (pid = filter_pids; pid; pid = pid->next) {
			if (!pid->exclude)
				ptrace_attach(pid->pid);
		}
	}

Otherwise mistakes can easily be made.

-- Steve


>  		/* sleep till we are woken with Ctrl^C */
>  		printf("Hit Ctrl^C to stop recording\n");
>  		while (!finished)

Re: [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:04 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

>  void tracecmd_set_quiet(int quiet);
>  int tracecmd_get_quiet(void);

I rather make this a parameter to the descriptor:

	tracecmd_set_quiet(struct tracecmd_output *handle, bool quiet);
	tracecmd_get_quiet(struct tracecmd output *handle);

As we may have multiple handles and perhaps we don't want all of them
quiet.

-- Steve

Re: [PATCH v2 5/8] trace-cmd: Move trace-cmd global variable "debug" to libtracecmd

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:05 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> A trace-cmd global variable "debug" is used from libtracecmd and
> should be defined there. A new library APIs are implemented to
> access it:
>  void tracecmd_set_debug(bool debug);
>  bool tracecmd_get_debug(void);
> 

I was thinking of doing a descriptor for this too, but screw it. Debug
is debug ;-)

-- Steve

Re: [PATCH v2 6/8] trace-cmd: Move plog() function to libtracecmd.

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:06 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> plog() function writes logs into a log file. It is used in
> libtracecmd and its implementation should be there.
> The function is moved from trace-cmd into the library, and 2
> additional APIs are implemented:
> 	int trace_set_log_file(char *logfile); - use it to set
> the log file.
> 	void plog_error(const char *fmt, ...); - use it to log
> an error message into the file.
> 
> The plog() function is used also from pdie() in trace-cmd.
> pdie() depends on trace-cmd context and cannot be moved to
> the library. It is reimplemented as macros, in order to utilize
> the new plog() library function.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h |  4 ++
>  include/trace-cmd/trace-msg.h |  3 --
>  lib/trace-cmd/trace-util.c    | 71 +++++++++++++++++++++++++++++++++++
>  tracecmd/trace-listen.c       | 69 ++++------------------------------
>  4 files changed, 83 insertions(+), 64 deletions(-)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index b96de04..8db0686 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -398,6 +398,10 @@ struct hook_list {
>  struct hook_list *tracecmd_create_event_hook(const char *arg);
>  void tracecmd_free_hooks(struct hook_list *hooks);
>  
> +void plog(const char *fmt, ...);
> +void plog_error(const char *fmt, ...);

If these are going to become visible in the library, then they need to
have a prefix "tracecmd_" attached to them.

> +int trace_set_log_file(char *logfile);
> +
>  /* --- Hack! --- */
>  int tracecmd_blk_hack(struct tracecmd_input *handle);
>  
> diff --git a/include/trace-cmd/trace-msg.h b/include/trace-cmd/trace-msg.h
> index b7fe10b..aab8a69 100644
> --- a/include/trace-cmd/trace-msg.h
> +++ b/include/trace-cmd/trace-msg.h
> @@ -12,7 +12,4 @@
>  
>  extern unsigned int page_size;
>  
> -void plog(const char *fmt, ...);
> -void pdie(const char *fmt, ...);
> -
>  #endif /* _TRACE_MSG_H_ */
> diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
> index b5ce84f..8c1a0a0 100644
> --- a/lib/trace-cmd/trace-util.c
> +++ b/lib/trace-cmd/trace-util.c
> @@ -31,6 +31,8 @@ int tracecmd_disable_plugins;
>  static int tracecmd_quiet;
>  static bool tracecmd_debug;
>  
> +static FILE *trace_logfp;

As this is a static variable, we only need to call it logfp.

> +
>  static struct registered_plugin_options {
>  	struct registered_plugin_options	*next;
>  	struct tep_plugin_option			*options;
> @@ -1716,3 +1718,72 @@ void __weak *malloc_or_die(unsigned int size)
>  		die("malloc");
>  	return data;
>  }
> +
> +#define LOG_BUF_SIZE 1024
> +static void __plog(const char *prefix, const char *fmt, va_list ap, FILE *fp)
> +{
> +	static int newline = 1;
> +	char buf[LOG_BUF_SIZE];
> +	int r;
> +
> +	r = vsnprintf(buf, LOG_BUF_SIZE, fmt, ap);
> +
> +	if (r > LOG_BUF_SIZE)
> +		r = LOG_BUF_SIZE;
> +
> +	if (trace_logfp) {
> +		if (newline)
> +			fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf);
> +		else
> +			fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf);
> +		newline = buf[r - 1] == '\n';
> +		fflush(trace_logfp);
> +		return;
> +	}
> +
> +	fprintf(fp, "%.*s", r, buf);
> +}
> +
> +void plog(const char *fmt, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, fmt);
> +	__plog("", fmt, ap, stdout);
> +	va_end(ap);
> +	/* Make sure it gets to the screen, in case we crash afterward */
> +	fflush(stdout);
> +}
> +
> +void plog_error(const char *fmt, ...)
> +{
> +	va_list ap;
> +	char *str = "";
> +
> +	va_start(ap, fmt);
> +	__plog("Error: ", fmt, ap, stderr);
> +	va_end(ap);
> +	if (errno)
> +		str = strerror(errno);
> +	if (trace_logfp)
> +		fprintf(trace_logfp, "\n%s\n", str);
> +	else
> +		fprintf(stderr, "\n%s\n", str);
> +}
> +
> +/**
> + * trace_set_log_file - Set file for logging
> + * @logfile: Name of the log file
> + *
> + * Returns 0 on successful completion or -1 in case of error
> + */
> +int trace_set_log_file(char *logfile)

Should it be called tracecmd_set_logfile()?

-- Steve

> +{
> +	if (trace_logfp)
> +		fclose(trace_logfp);
> +	trace_logfp = fopen(logfile, "w");
> +	if (!trace_logfp)
> +		return -1;
> +	return 0;
> +}
> +

Re: [PATCH v2 7/8] trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:07 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> These APIs depend on trace-cmd context and cannot be used standalone:
> 	tracecmd_create_top_instance()
> 	tracecmd_remove_instances()
> 	tracecmd_filter_pid()
> 	tracecmd_add_event()
> 	tracecmd_enable_events()
> 	tracecmd_disable_all_tracing()
> 	tracecmd_disable_tracing()
> 	tracecmd_enable_tracing()
> 	tracecmd_stat_cpu()
> They are implemented in trace-cmd application, but declared as APIs in
> trace-cmd.h. The declarations are moved from trace-cmd.h to trace-local.h,
> local header file visible only to trace-cmd application.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h  |  9 ---------
>  tracecmd/include/trace-local.h | 11 +++++++++++
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index 8db0686..2c8d798 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -137,8 +137,6 @@ int tracecmd_buffer_instances(struct tracecmd_input *handle);
>  const char *tracecmd_buffer_instance_name(struct tracecmd_input *handle, int indx);
>  struct tracecmd_input *tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx);
>  int tracecmd_is_buffer_instance(struct tracecmd_input *handle);
> -void tracecmd_create_top_instance(char *name);
> -void tracecmd_remove_instances(void);
>  
>  void tracecmd_set_ts_offset(struct tracecmd_input *handle, unsigned long long offset);
>  void tracecmd_set_ts2secs(struct tracecmd_input *handle, unsigned long long hz);
> @@ -304,14 +302,7 @@ struct tracecmd_recorder *tracecmd_create_buffer_recorder_maxkb(const char *file
>  
>  int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long sleep);
>  void tracecmd_stop_recording(struct tracecmd_recorder *recorder);
> -void tracecmd_stat_cpu(struct trace_seq *s, int cpu);
>  long tracecmd_flush_recording(struct tracecmd_recorder *recorder);
> -void tracecmd_filter_pid(int pid, int exclude);
> -int tracecmd_add_event(const char *event_str, int stack);
> -void tracecmd_enable_events(void);
> -void tracecmd_disable_all_tracing(int disable_tracer);
> -void tracecmd_disable_tracing(void);
> -void tracecmd_enable_tracing(void);
>  
>  enum tracecmd_msg_flags {
>  	TRACECMD_MSG_FL_USE_TCP		= 1 << 0,
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index e2d5506..05760d8 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -216,6 +216,17 @@ void show_instance_file(struct buffer_instance *instance, const char *name);
>  
>  int count_cpus(void);
>  
> +/* moved from trace-cmd.h */
> +void tracecmd_create_top_instance(char *name);
> +void tracecmd_remove_instances(void);
> +void tracecmd_filter_pid(int pid, int exclude);
> +int tracecmd_add_event(const char *event_str, int stack);
> +void tracecmd_enable_events(void);
> +void tracecmd_disable_all_tracing(int disable_tracer);
> +void tracecmd_disable_tracing(void);
> +void tracecmd_enable_tracing(void);
> +void tracecmd_stat_cpu(struct trace_seq *s, int cpu);
> +

I'll take this patch, but I wonder if we should change "tracecmd_" to
"trace_" to let it be known that these are local functions. That can be
a later patch.

-- Steve

>  /* No longer in event-utils.h */
>  void __noreturn die(const char *fmt, ...); /* Can be overriden */
>  void *malloc_or_die(unsigned int size); /* Can be overridden */

Re: [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:08 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> tracecmd_stack_tracer_status() function reads the stack tracer status
> from the proc file system. It does not depend on trace-cmd context and
> can be used standalone. The function is moved from trace-cmd application
> into libtracecmd.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-util.c | 49 +++++++++++++++++++++++++++++++++
>  tracecmd/trace-stack.c     | 56 ++------------------------------------
>  2 files changed, 51 insertions(+), 54 deletions(-)
> 
> diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
> index 8c1a0a0..d16a018 100644
> --- a/lib/trace-cmd/trace-util.c
> +++ b/lib/trace-cmd/trace-util.c
> @@ -25,6 +25,7 @@
>  #define LOCAL_PLUGIN_DIR ".trace-cmd/plugins"
>  #define TRACEFS_PATH "/sys/kernel/tracing"
>  #define DEBUGFS_PATH "/sys/kernel/debug"
> +#define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled"
>  
>  int tracecmd_disable_sys_plugins;
>  int tracecmd_disable_plugins;
> @@ -1787,3 +1788,51 @@ int trace_set_log_file(char *logfile)
>  	return 0;
>  }
>  
> +/**
> + * tracecmd_stack_tracer_status - Check stack trace status
> + * @status: Returned stack trace status:
> + *		0 - not configured, disabled
> + *		non 0 - enabled
> + *
> + * Returns -1 in case of an error, 0 if file does not exist
> + *  (stack tracer not enabled) or 1 on successful completion.

  "(stack tracer not configured in kernel)"

Saying "enabled" is ambiguous.

> + */
> +int tracecmd_stack_tracer_status(int *status)

If we are making this a library function, shouldn't it be declared in
the trace-cmd.h file?

-- Steve

Re: [PATCH v2 0/8] Separate trace-cmd and libtracecmd code

[Steven Rostedt]

On Wed, 14 Aug 2019 11:47:00 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> libtracecmd is a library, containing functions that can be
> used without the trace-cmd application. However, some of the
> functions declared as libtracecmd APIs in trace-cmd.h
> depend on trace-cmd context. That causes a problem when
> other application uses the library. The problem can be
> observed when running kerneshark and there is a python
> module, loaded by the python plugin - there is a bunch
> of warnings.
> To resolve the problem, implementations of all trace-cmd
> independent functions are moved into libtracecmd. All
> libtracecmd functions, that depend on trace-cmd context
> are removed from the library and from trace-cmd.h file.
> 

I pulled in some but not all of these patches (the ones I had comments
on). Feel free to rebase on the git repo and you only need to send the
ones that have not been applied.

-- Steve

Re: [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd

[Tzvetomir Stoyanov]

On Wed, Aug 28, 2019 at 10:59 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 14 Aug 2019 11:47:04 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> >  void tracecmd_set_quiet(int quiet);
> >  int tracecmd_get_quiet(void);
>
> I rather make this a parameter to the descriptor:
>
>         tracecmd_set_quiet(struct tracecmd_output *handle, bool quiet);
>         tracecmd_get_quiet(struct tracecmd output *handle);
>
> As we may have multiple handles and perhaps we don't want all of them
> quiet.
>
It makes sense to have "quiet" per tracecmd_output handler, but when I
started to modify the code,
I noticed one flow where tracecmd_get_quiet() is used and there is no
tracecmd_output handler available -
in check_plugin(). We have either to drop the usage of
tracecmd_get_quiet() in check_plugin(), or leave the scope of
"quiet" to be for the whole library.

> -- Steve


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd

[Steven Rostedt]

On Thu, 29 Aug 2019 14:39:43 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> On Wed, Aug 28, 2019 at 10:59 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Wed, 14 Aug 2019 11:47:04 +0300
> > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> >  
> > >  void tracecmd_set_quiet(int quiet);
> > >  int tracecmd_get_quiet(void);  
> >
> > I rather make this a parameter to the descriptor:
> >
> >         tracecmd_set_quiet(struct tracecmd_output *handle, bool quiet);
> >         tracecmd_get_quiet(struct tracecmd output *handle);
> >
> > As we may have multiple handles and perhaps we don't want all of them
> > quiet.
> >  
> It makes sense to have "quiet" per tracecmd_output handler, but when I
> started to modify the code,
> I noticed one flow where tracecmd_get_quiet() is used and there is no
> tracecmd_output handler available -
> in check_plugin(). We have either to drop the usage of
> tracecmd_get_quiet() in check_plugin(), or leave the scope of
> "quiet" to be for the whole library.

We can still have a global variable in trace-record.c called quiet. And
that gets set by the parameter:

	case OPT_quite:
	case 'q':
		quiet = 1;
		break;

[..]

	tracecmd_set_quiet(handle, quiet);

This is the proper way to handle it. The functions local to
trace-record.c, can just use its own quiet state variable, but when we
set quiet, we use the tracecmd_set_quiet() to notify the library that
it too should be quiet.

-- Steve

Re: [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd

[Tzvetomir Stoyanov]

Hi Steven,

On Wed, Aug 28, 2019 at 11:21 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
...
> > +/**
> > + * tracecmd_stack_tracer_status - Check stack trace status
> > + * @status: Returned stack trace status:
> > + *           0 - not configured, disabled
> > + *           non 0 - enabled
> > + *
> > + * Returns -1 in case of an error, 0 if file does not exist
> > + *  (stack tracer not enabled) or 1 on successful completion.
>
>   "(stack tracer not configured in kernel)"
>
> Saying "enabled" is ambiguous.
>
> > + */
> > +int tracecmd_stack_tracer_status(int *status)
>
> If we are making this a library function, shouldn't it be declared in
> the trace-cmd.h file?
>

It is already part of trace-cmd.h, that was one of the problems - it was defined
as library API, but implemented in the trace-cmd application. This patch moves
the implementation in the library, the declaration in trace-cmd.h
remains the same.

> -- Steve

-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center