[PATCH v2 0/8] Changes to trace-cmd logs

[PATCH v2 0/8] Changes to trace-cmd logs

[Tzvetomir Stoyanov (VMware)]

Recently changes in traceevent and tracefs libraries were introduced: log
levels and new APIs to modify them. As trace-cmd library uses traceevent
and tracefs libraries, those changes affect trace-cmd as well.

These changes depend on:
"[PATCH v2 0/3] Add API to set severity to tracefs library logs"
https://lore.kernel.org/linux-trace-devel/20210507095154.1080202-1-tz.stoyanov@gmail.com/

v2 changes:
  - Add a parameter to tep_vprint() for errno printing, insted of using the log
    level.

Tzvetomir Stoyanov (VMware) (8):
  trace-cmd library: Use tep_vprint() for logs
  trace-cmd library: Add tracecmd_info() log function
  trace-cmd library: Add log levels
  trace-cmd library: Document the API for log levels
  trace-cmd library: Renamed tracecmd_fatal() to tracecmd_critical()
  trace-cmd library: Set debug log level in debug mode
  trace-cmd report: Set the log level with -V and -q options
  trace-cmd: Add new function to set log level

 Documentation/libtracecmd/libtracecmd-log.txt | 78 +++++++++++++++++++
 Documentation/libtracecmd/libtracecmd.txt     |  3 +
 include/trace-cmd/trace-cmd.h                 |  2 +
 lib/trace-cmd/include/trace-cmd-local.h       |  4 +-
 lib/trace-cmd/trace-input.c                   | 14 ++--
 lib/trace-cmd/trace-plugin.c                  |  2 +-
 lib/trace-cmd/trace-util.c                    | 47 +++++++++--
 python/ctracecmd.i                            | 13 +---
 tracecmd/include/trace-local.h                |  2 +
 tracecmd/trace-cmd.c                          | 55 +++++++++----
 tracecmd/trace-read.c                         |  2 +
 tracecmd/trace-record.c                       | 14 ++++
 12 files changed, 194 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/libtracecmd/libtracecmd-log.txt

-- 
2.31.1

[PATCH v2 1/8] trace-cmd library: Use tep_vprint() for logs

[Tzvetomir Stoyanov (VMware)]

The traceevent library funciton for printing logs is changed from
tep_vwarning to tep_vprint.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-util.c | 5 +++--
 python/ctracecmd.i         | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 2d3bc741..ab77ce2a 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -20,6 +20,7 @@
 #include <sys/stat.h>
 #include <sys/sysinfo.h>
 #include <time.h>
+#include <traceevent/event-parse.h>
 #include <traceevent/event-utils.h>
 
 #include "trace-cmd-private.h"
@@ -359,7 +360,7 @@ void __weak tracecmd_warning(const char *fmt, ...)
 	va_list ap;
 
 	va_start(ap, fmt);
-	tep_vwarning("libtracecmd", fmt, ap);
+	tep_vprint("libtracecmd", TEP_LOG_WARNING, true, fmt, ap);
 	va_end(ap);
 }
 
@@ -369,7 +370,7 @@ void __weak tracecmd_fatal(const char *fmt, ...)
 	va_list ap;
 
 	va_start(ap, fmt);
-	ret = tep_vwarning("libtracecmd", fmt, ap);
+	ret = tep_vprint("libtracecmd", TEP_LOG_CRITICAL, true, fmt, ap);
 	va_end(ap);
 
 	if (debug) {
diff --git a/python/ctracecmd.i b/python/ctracecmd.i
index e938c780..7091b9bd 100644
--- a/python/ctracecmd.i
+++ b/python/ctracecmd.i
@@ -63,7 +63,7 @@ void warning(const char *fmt, ...)
 		return;
 
 	va_start(ap, fmt);
-	tep_vwarning("tracecmd", fmt, ap);
+	tep_vprint("tracecmd", TEP_LOG_WARNING, true, fmt, ap);
 	va_end(ap);
 }
 
-- 
2.31.1

[PATCH v2 2/8] trace-cmd library: Add tracecmd_info() log function

[Tzvetomir Stoyanov (VMware)]

The pr_stat() function is removed from the traceevent library. This function
was used by trace-cmd to print statistics. Introduced tracecmd_info() log
function and replaced pr_stat() with it in the trace-cmd library.
However, pr_stat() is used also in the trace-cmd application. In order no
to change the existing behaviour, added a static pr_stat() implementation for
that usage.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/include/trace-cmd-local.h |  1 +
 lib/trace-cmd/trace-input.c             |  2 +-
 lib/trace-cmd/trace-plugin.c            |  2 +-
 lib/trace-cmd/trace-util.c              | 10 ++++++++++
 python/ctracecmd.i                      | 11 -----------
 tracecmd/trace-cmd.c                    | 14 --------------
 tracecmd/trace-record.c                 | 14 ++++++++++++++
 7 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index e8533b22..cd868f60 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -12,6 +12,7 @@
 /* Can be overridden */
 void tracecmd_warning(const char *fmt, ...);
 void tracecmd_fatal(const char *fmt, ...);
+void tracecmd_info(const char *fmt, ...);
 
 /* trace.dat file format version */
 #define FILE_VERSION 6
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 9726e994..2e519752 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3292,7 +3292,7 @@ struct tracecmd_input *tracecmd_alloc_fd(int fd, int flags)
 	version = read_string(handle);
 	if (!version)
 		goto failed_read;
-	pr_stat("version = %s\n", version);
+	tracecmd_info("version = %s\n", version);
 	free(version);
 
 	if (do_read_check(handle, buf, 1))
diff --git a/lib/trace-cmd/trace-plugin.c b/lib/trace-cmd/trace-plugin.c
index ca7cadae..50c800c2 100644
--- a/lib/trace-cmd/trace-plugin.c
+++ b/lib/trace-cmd/trace-plugin.c
@@ -140,7 +140,7 @@ load_plugin(struct trace_plugin_context *trace, const char *path,
 	list->name = plugin;
 	*plugin_list = list;
 
-	pr_stat("registering plugin: %s", plugin);
+	tracecmd_info("registering plugin: %s", plugin);
 	func(trace);
 	return;
 
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index ab77ce2a..049fe049 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -364,6 +364,16 @@ void __weak tracecmd_warning(const char *fmt, ...)
 	va_end(ap);
 }
 
+void tracecmd_info(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	tep_vprint("libtracecmd", TEP_LOG_INFO, false, fmt, ap);
+	va_end(ap);
+}
+
+
 void __weak tracecmd_fatal(const char *fmt, ...)
 {
 	int ret;
diff --git a/python/ctracecmd.i b/python/ctracecmd.i
index 7091b9bd..6d0179e3 100644
--- a/python/ctracecmd.i
+++ b/python/ctracecmd.i
@@ -44,17 +44,6 @@ static void py_supress_trace_output(void)
 	skip_output = 1;
 }
 
-void pr_stat(const char *fmt, ...)
-{
-        va_list ap;
-
-	if (skip_output)
-		return;
-	va_start(ap, fmt);
-	__vpr_stat(fmt, ap);
-	va_end(ap);
-}
-
 void warning(const char *fmt, ...)
 {
 	va_list ap;
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index 7376c5a5..60cd3ea1 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -35,20 +35,6 @@ void warning(const char *fmt, ...)
 	fprintf(stderr, "\n");
 }
 
-void pr_stat(const char *fmt, ...)
-{
-	va_list ap;
-
-	if (!show_status)
-		return;
-
-	va_start(ap, fmt);
-	vprintf(fmt, ap);
-	va_end(ap);
-
-	printf("\n");
-}
-
 void *malloc_or_die(unsigned int size)
 {
 	void *data;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 5dd8be4a..274d706c 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -2934,6 +2934,20 @@ static void test_event(struct event_list *event, const char *path,
 
 	*save = event;
 }
+static void pr_stat(const char *fmt, ...)
+{
+	va_list ap;
+
+	if (!show_status)
+		return;
+
+	va_start(ap, fmt);
+	vprintf(fmt, ap);
+	va_end(ap);
+
+	printf("\n");
+}
+
 
 static int expand_event_files(struct buffer_instance *instance,
 			      const char *file, struct event_list *old_event)
-- 
2.31.1

[PATCH v2 3/8] trace-cmd library: Add log levels

[Tzvetomir Stoyanov (VMware)]

Add levels to library logs and introduce a new API to set the desired
log severity:
 tracecmd_set_loglevel()
When a new trace-cmd library log level is set, propagate it to tracefs
and traceevent libraries as well.
Removed the "weak" definition of the library log functions. Setting
the desired log level can be used to silence the library logs, instead
of overwriting the log functions.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h           |  2 ++
 lib/trace-cmd/include/trace-cmd-local.h |  1 -
 lib/trace-cmd/trace-util.c              | 26 ++++++++++++++++++++++---
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 7305487c..6984db86 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -43,4 +43,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);
 
+void tracecmd_set_loglevel(enum tep_loglevel level);
+
 #endif /* _TRACE_CMD_H */
diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index cd868f60..76179148 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -9,7 +9,6 @@
 #include <byteswap.h>
 #include "trace-cmd-private.h"
 
-/* Can be overridden */
 void tracecmd_warning(const char *fmt, ...);
 void tracecmd_fatal(const char *fmt, ...);
 void tracecmd_info(const char *fmt, ...);
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 049fe049..b3f1b075 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -30,7 +30,7 @@
 #define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled"
 
 static bool debug;
-
+static int log_level = TEP_LOG_CRITICAL;
 static FILE *logfp;
 
 const static struct {
@@ -355,10 +355,24 @@ trace_load_plugins(struct tep_handle *tep, int flags)
 	return list;
 }
 
-void __weak tracecmd_warning(const char *fmt, ...)
+/**
+ * tracecmd_set_loglevel - set log level of the library
+ * @level: desired level of the library messages
+ */
+void tracecmd_set_loglevel(enum tep_loglevel level)
+{
+	log_level = level;
+	tracefs_set_loglevel(level);
+	tep_set_loglevel(level);
+}
+
+void tracecmd_warning(const char *fmt, ...)
 {
 	va_list ap;
 
+	if (log_level < TEP_LOG_WARNING)
+		return;
+
 	va_start(ap, fmt);
 	tep_vprint("libtracecmd", TEP_LOG_WARNING, true, fmt, ap);
 	va_end(ap);
@@ -368,17 +382,23 @@ void tracecmd_info(const char *fmt, ...)
 {
 	va_list ap;
 
+	if (log_level < TEP_LOG_INFO)
+		return;
+
 	va_start(ap, fmt);
 	tep_vprint("libtracecmd", TEP_LOG_INFO, false, fmt, ap);
 	va_end(ap);
 }
 
 
-void __weak tracecmd_fatal(const char *fmt, ...)
+void tracecmd_fatal(const char *fmt, ...)
 {
 	int ret;
 	va_list ap;
 
+	if (log_level < TEP_LOG_CRITICAL)
+		return;
+
 	va_start(ap, fmt);
 	ret = tep_vprint("libtracecmd", TEP_LOG_CRITICAL, true, fmt, ap);
 	va_end(ap);
-- 
2.31.1

[PATCH v2 4/8] trace-cmd library: Document the API for log levels

[Tzvetomir Stoyanov (VMware)]

Updated trace-cmd library man pages with description of the API for
setting the library log levels:
 tracecmd_set_loglevel()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 Documentation/libtracecmd/libtracecmd-log.txt | 78 +++++++++++++++++++
 Documentation/libtracecmd/libtracecmd.txt     |  3 +
 2 files changed, 81 insertions(+)
 create mode 100644 Documentation/libtracecmd/libtracecmd-log.txt

diff --git a/Documentation/libtracecmd/libtracecmd-log.txt b/Documentation/libtracecmd/libtracecmd-log.txt
new file mode 100644
index 00000000..de5c2776
--- /dev/null
+++ b/Documentation/libtracecmd/libtracecmd-log.txt
@@ -0,0 +1,78 @@
+libtracecmd(3)
+=============
+
+NAME
+----
+tracecmd_set_loglevel - Set log level of the library
+
+SYNOPSIS
+--------
+[verse]
+--
+*#include <trace-cmd.h>*
+
+int *tracecmd_set_loglevel*(enum tep_loglevel _level_);
+--
+
+DESCRIPTION
+-----------
+The _tracecmd_set_loglevel()_ function sets the level of the library logs that will be printed on
+the console. See _libtraceevent(3)_ for detailed desciription of the log levels. Setting the log
+level to specific value means that logs from the previous levels will be printed too. For example
+_TEP_LOG_WARNING_ will print any logs with severity _TEP_LOG_WARNING_, _TEP_LOG_ERROR_ and
+_TEP_LOG_CRITICAL_. The default log level is _TEP_LOG_CRITICAL_.  When a new level is set, it is
+also propagated to the libtracefs and libtraceevent.
+
+EXAMPLE
+-------
+[source,c]
+--
+#include <trace-cmd.h>
+...
+tracecmd_set_loglevel(TEP_LOG_ALL);
+...
+/* call libtracecmd, libtracefs or libtraceevent APIs and observe any logs they produce */
+...
+tracecmd_set_loglevel(TEP_LOG_CRITICAL);
+--
+
+FILES
+-----
+[verse]
+--
+*trace-cmd.h*
+	Header file to include in order to have access to the library APIs.
+*-ltracecmd*
+	Linker switch to add when building a program that uses the library.
+--
+
+SEE ALSO
+--------
+_libtracefs(3)_,
+_libtraceevent(3)_,
+_trace-cmd(1)_
+_trace-cmd.dat(5)_
+
+AUTHOR
+------
+[verse]
+--
+*Steven Rostedt* <rostedt@goodmis.org>
+*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
+--
+REPORTING BUGS
+--------------
+Report bugs to  <linux-trace-devel@vger.kernel.org>
+
+LICENSE
+-------
+libtracecmd is Free Software licensed under the GNU LGPL 2.1
+
+RESOURCES
+---------
+https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/
+
+COPYING
+-------
+Copyright \(C) 2021 VMware, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
diff --git a/Documentation/libtracecmd/libtracecmd.txt b/Documentation/libtracecmd/libtracecmd.txt
index adc9321c..dc528ce0 100644
--- a/Documentation/libtracecmd/libtracecmd.txt
+++ b/Documentation/libtracecmd/libtracecmd.txt
@@ -33,6 +33,9 @@ Read tracing instances from a trace file:
 Get traceing peer information from a trace file:
 	unsigned long long *tracecmd_get_traceid*(struct tracecmd_input pass:[*]_handle_);
 	int *tracecmd_get_guest_cpumap*(struct tracecmd_input pass:[*]_handle_, unsigned long long _trace_id_, const char pass:[*]pass:[*]_name_, int pass:[*]_vcpu_count_, const int pass:[*]pass:[*]_cpu_pid_);
+
+Control library logs:
+	int *tracecmd_set_loglevel*(enum tep_loglevel _level_);
 --
 
 DESCRIPTION
-- 
2.31.1

[PATCH v2 5/8] trace-cmd library: Renamed tracecmd_fatal() to tracecmd_critical()

[Tzvetomir Stoyanov (VMware)]

The trace-cmd library function for pritning critical logs is renamed
from tracecmd_fatal() to tracecmd_critical() to be consistent with the
names of the log levels.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/include/trace-cmd-local.h |  2 +-
 lib/trace-cmd/trace-input.c             | 12 ++++++------
 lib/trace-cmd/trace-util.c              |  5 ++---
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index 76179148..7bf620d4 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -10,7 +10,7 @@
 #include "trace-cmd-private.h"
 
 void tracecmd_warning(const char *fmt, ...);
-void tracecmd_fatal(const char *fmt, ...);
+void tracecmd_critical(const char *fmt, ...);
 void tracecmd_info(const char *fmt, ...);
 
 /* trace.dat file format version */
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 2e519752..5ee69b14 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -1105,7 +1105,7 @@ static void __free_page(struct tracecmd_input *handle, struct page *page)
 	int index;
 
 	if (!page->ref_count) {
-		tracecmd_fatal("Page ref count is zero!\n");
+		tracecmd_critical("Page ref count is zero!\n");
 		return;
 	}
 
@@ -1166,7 +1166,7 @@ void tracecmd_free_record(struct tep_record *record)
 		return;
 
 	if (!record->ref_count) {
-		tracecmd_fatal("record ref count is zero!");
+		tracecmd_critical("record ref count is zero!");
 		return;
 	}
 
@@ -1176,7 +1176,7 @@ void tracecmd_free_record(struct tep_record *record)
 		return;
 
 	if (record->locked) {
-		tracecmd_fatal("freeing record when it is locked!");
+		tracecmd_critical("freeing record when it is locked!");
 		return;
 	}
 
@@ -1375,7 +1375,7 @@ static int get_page(struct tracecmd_input *handle, int cpu,
 
 	if (offset & (handle->page_size - 1)) {
 		errno = -EINVAL;
-		tracecmd_fatal("bad page offset %llx", offset);
+		tracecmd_critical("bad page offset %llx", offset);
 		return -1;
 	}
 
@@ -1383,7 +1383,7 @@ static int get_page(struct tracecmd_input *handle, int cpu,
 	    offset > handle->cpu_data[cpu].file_offset +
 	    handle->cpu_data[cpu].file_size) {
 		errno = -EINVAL;
-		tracecmd_fatal("bad page offset %llx", offset);
+		tracecmd_critical("bad page offset %llx", offset);
 		return -1;
 	}
 
@@ -1949,7 +1949,7 @@ tracecmd_peek_data(struct tracecmd_input *handle, int cpu)
 
 		record = handle->cpu_data[cpu].next;
 		if (!record->data) {
-			tracecmd_fatal("Something freed the record");
+			tracecmd_critical("Something freed the record");
 			return NULL;
 		}
 
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index b3f1b075..3b2a9183 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -390,8 +390,7 @@ void tracecmd_info(const char *fmt, ...)
 	va_end(ap);
 }
 
-
-void tracecmd_fatal(const char *fmt, ...)
+void tracecmd_critical(const char *fmt, ...)
 {
 	int ret;
 	va_list ap;
@@ -560,7 +559,7 @@ int tracecmd_count_cpus(void)
 
 	fp = fopen("/proc/cpuinfo", "r");
 	if (!fp) {
-		tracecmd_fatal("Can not read cpuinfo");
+		tracecmd_critical("Can not read cpuinfo");
 		return 0;
 	}
 
-- 
2.31.1

[PATCH v2 6/8] trace-cmd library: Set debug log level in debug mode

[Tzvetomir Stoyanov (VMware)]

When the library works in debug mode, set the log level to debug.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-util.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 3b2a9183..995f5786 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -93,6 +93,11 @@ const char *tracecmd_clock_id2str(enum tracecmd_clocks clock)
 void tracecmd_set_debug(bool set_debug)
 {
 	debug = set_debug;
+
+	if (set_debug)
+		tracecmd_set_loglevel(TEP_LOG_DEBUG);
+	else
+		tracecmd_set_loglevel(TEP_LOG_CRITICAL);
 }
 
 /**
-- 
2.31.1

[PATCH v2 7/8] trace-cmd report: Set the log level with -V and -q options

[Tzvetomir Stoyanov (VMware)]

When "trace-cmd report -V" is specified, set the log level to INFO.
When "trace-cmd report -q" is specified, set the log level to NONE.

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

diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c
index b315283e..0cf6e773 100644
--- a/tracecmd/trace-read.c
+++ b/tracecmd/trace-read.c
@@ -1703,9 +1703,11 @@ void trace_report (int argc, char **argv)
 			break;
 		case 'V':
 			show_status = 1;
+			tracecmd_set_loglevel(TEP_LOG_INFO);
 			break;
 		case 'q':
 			silence_warnings = 1;
+			tracecmd_set_loglevel(TEP_LOG_NONE);
 			break;
 		case OPT_cpu:
 			parse_cpulist(optarg);
-- 
2.31.1

[PATCH v2 8/8] trace-cmd: Add new function to set log level

[Tzvetomir Stoyanov (VMware)]

Introduce a new trace-cmd internal function to set the application log
level.
 trace_set_verbose()
The log level can be set using string with log id or log name.

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

diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index b3997d00..8da07c83 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -39,6 +39,8 @@ void usage(char **argv);
 extern int silence_warnings;
 extern int show_status;
 
+int trace_set_verbose(char *level);
+
 struct pid_record_data {
 	int			pid;
 	int			brass[2];
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index 60cd3ea1..4e4a73af 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -45,6 +45,47 @@ void *malloc_or_die(unsigned int size)
 	return data;
 }
 
+static struct trace_log_severity {
+	int		id;
+	const char	*name;
+} log_severtity[] = {
+	{ .id = TEP_LOG_NONE, .name = "none" },
+	{ .id = TEP_LOG_CRITICAL, .name = "crit" },
+	{ .id = TEP_LOG_ERROR, .name = "err" },
+	{ .id = TEP_LOG_WARNING, .name = "warn" },
+	{ .id = TEP_LOG_INFO, .name = "info" },
+	{ .id = TEP_LOG_DEBUG, .name = "debug" },
+	{ .id = TEP_LOG_ALL, .name = "all" },
+};
+
+int trace_set_verbose(char *level)
+{
+	int id;
+
+	if (!level)
+		return -1;
+
+	if (isdigit(level[0])) {
+		id = atoi(level);
+		if (id >= TEP_LOG_NONE && id <= TEP_LOG_ALL) {
+			tracecmd_set_loglevel(id);
+			return 0;
+		}
+	} else {
+		int size = sizeof(log_severtity) / sizeof(log_severtity[0]);
+		int i;
+
+		for (i = 0; i < size; i++) {
+			if (strlen(level) >= strlen(log_severtity[i].name) &&
+			    !strncmp(level, log_severtity[i].name, strlen(log_severtity[i].name))) {
+				tracecmd_set_loglevel(log_severtity[i].id);
+				return 0;
+			}
+		}
+	}
+
+	return -1;
+}
 
 /**
  * struct command
-- 
2.31.1

Re: [PATCH v2 2/8] trace-cmd library: Add tracecmd_info() log function

[Steven Rostedt]

On Fri,  7 May 2021 12:53:27 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -2934,6 +2934,20 @@ static void test_event(struct event_list *event, const char *path,
>  
>  	*save = event;
>  }
> +static void pr_stat(const char *fmt, ...)
> +{
> +	va_list ap;
> +
> +	if (!show_status)
> +		return;
> +
> +	va_start(ap, fmt);
> +	vprintf(fmt, ap);
> +	va_end(ap);
> +
> +	printf("\n");
> +}
> +
>  

Since we need to keep the library pr_stat() for backward compatibility, I
rename this to "print_event()" as that's what it was doing locally anyway.

Otherwise it conflicted with the old pr_stat() that I added back to
libtraceevent.

-- Steve

Re: [PATCH v2 3/8] trace-cmd library: Add log levels

[Steven Rostedt]

On Fri,  7 May 2021 12:53:28 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Removed the "weak" definition of the library log functions. Setting
> the desired log level can be used to silence the library logs, instead
> of overwriting the log functions.

That's not the purpose of the weak definition. It is so that a GUI (like
kernelshark) can turn it into a pop up if need be or show in a status
window. It was not for quieting the function.

Please do not remove the weak symbols.

-- Steve

Re: [PATCH v2 8/8] trace-cmd: Add new function to set log level

[Steven Rostedt]

On Fri,  7 May 2021 12:53:33 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Introduce a new trace-cmd internal function to set the application log
> level.
>  trace_set_verbose()
> The log level can be set using string with log id or log name.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  tracecmd/include/trace-local.h |  2 ++
>  tracecmd/trace-cmd.c           | 41 ++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index b3997d00..8da07c83 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -39,6 +39,8 @@ void usage(char **argv);
>  extern int silence_warnings;
>  extern int show_status;
>  
> +int trace_set_verbose(char *level);
> +
>  struct pid_record_data {
>  	int			pid;
>  	int			brass[2];
> diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
> index 60cd3ea1..4e4a73af 100644
> --- a/tracecmd/trace-cmd.c
> +++ b/tracecmd/trace-cmd.c
> @@ -45,6 +45,47 @@ void *malloc_or_die(unsigned int size)
>  	return data;
>  }
>  
> +static struct trace_log_severity {
> +	int		id;
> +	const char	*name;
> +} log_severtity[] = {

Is there a reason for the extra 't' in severity?

> +	{ .id = TEP_LOG_NONE, .name = "none" },
> +	{ .id = TEP_LOG_CRITICAL, .name = "crit" },
> +	{ .id = TEP_LOG_ERROR, .name = "err" },
> +	{ .id = TEP_LOG_WARNING, .name = "warn" },
> +	{ .id = TEP_LOG_INFO, .name = "info" },
> +	{ .id = TEP_LOG_DEBUG, .name = "debug" },
> +	{ .id = TEP_LOG_ALL, .name = "all" },
> +};
> +
> +int trace_set_verbose(char *level)
> +{
> +	int id;
> +
> +	if (!level)
> +		return -1;
> +
> +	if (isdigit(level[0])) {
> +		id = atoi(level);
> +		if (id >= TEP_LOG_NONE && id <= TEP_LOG_ALL) {
> +			tracecmd_set_loglevel(id);
> +			return 0;
> +		}
> +	} else {
> +		int size = sizeof(log_severtity) / sizeof(log_severtity[0]);

This can be shortened to:

		int size = ARRAY_SIZE(log_severity);

Or at least we need to copy that macro to something that trace-cmd core has
access to (see lib/trace-cmd/include/private/trace-cmd-private.h)


> +		int i;
> +
> +		for (i = 0; i < size; i++) {
> +			if (strlen(level) >= strlen(log_severtity[i].name) &&

Why the strlen test? It's OK to pass in "debugignorme"? What's the use case
for this?

Anyway, I applied the first two patches of this series.

-- Steve


> +			    !strncmp(level, log_severtity[i].name, strlen(log_severtity[i].name))) {
> +				tracecmd_set_loglevel(log_severtity[i].id);
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -1;
> +}
>  
>  /**
>   * struct command

Re: [PATCH v2 3/8] trace-cmd library: Add log levels

[Tzvetomir Stoyanov]

On Fri, May 14, 2021 at 12:23 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri,  7 May 2021 12:53:28 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Removed the "weak" definition of the library log functions. Setting
> > the desired log level can be used to silence the library logs, instead
> > of overwriting the log functions.
>
> That's not the purpose of the weak definition. It is so that a GUI (like
> kernelshark) can turn it into a pop up if need be or show in a status
> window. It was not for quieting the function.

The description is not correct. All these log functions use
tep_vprint(), which is implemented as weak. This function has
information for the library and log severity and should be used by
kernelshark for that purpose. That's why I removed weak library
specific functions.

>
> Please do not remove the weak symbols.
>
> -- Steve



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

Re: [PATCH v2 8/8] trace-cmd: Add new function to set log level

[Tzvetomir Stoyanov]

On Fri, May 14, 2021 at 12:36 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri,  7 May 2021 12:53:33 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Introduce a new trace-cmd internal function to set the application log
> > level.
> >  trace_set_verbose()
> > The log level can be set using string with log id or log name.
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  tracecmd/include/trace-local.h |  2 ++
> >  tracecmd/trace-cmd.c           | 41 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> >
> > diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> > index b3997d00..8da07c83 100644
> > --- a/tracecmd/include/trace-local.h
> > +++ b/tracecmd/include/trace-local.h
> > @@ -39,6 +39,8 @@ void usage(char **argv);
> >  extern int silence_warnings;
> >  extern int show_status;
> >
> > +int trace_set_verbose(char *level);
> > +
> >  struct pid_record_data {
> >       int                     pid;
> >       int                     brass[2];
> > diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
> > index 60cd3ea1..4e4a73af 100644
> > --- a/tracecmd/trace-cmd.c
> > +++ b/tracecmd/trace-cmd.c
> > @@ -45,6 +45,47 @@ void *malloc_or_die(unsigned int size)
> >       return data;
> >  }
> >
> > +static struct trace_log_severity {
> > +     int             id;
> > +     const char      *name;
> > +} log_severtity[] = {
>
> Is there a reason for the extra 't' in severity?

It's a typo.

>
> > +     { .id = TEP_LOG_NONE, .name = "none" },
> > +     { .id = TEP_LOG_CRITICAL, .name = "crit" },
> > +     { .id = TEP_LOG_ERROR, .name = "err" },
> > +     { .id = TEP_LOG_WARNING, .name = "warn" },
> > +     { .id = TEP_LOG_INFO, .name = "info" },
> > +     { .id = TEP_LOG_DEBUG, .name = "debug" },
> > +     { .id = TEP_LOG_ALL, .name = "all" },
> > +};
> > +
> > +int trace_set_verbose(char *level)
> > +{
> > +     int id;
> > +
> > +     if (!level)
> > +             return -1;
> > +
> > +     if (isdigit(level[0])) {
> > +             id = atoi(level);
> > +             if (id >= TEP_LOG_NONE && id <= TEP_LOG_ALL) {
> > +                     tracecmd_set_loglevel(id);
> > +                     return 0;
> > +             }
> > +     } else {
> > +             int size = sizeof(log_severtity) / sizeof(log_severtity[0]);
>
> This can be shortened to:
>
>                 int size = ARRAY_SIZE(log_severity);
>
> Or at least we need to copy that macro to something that trace-cmd core has
> access to (see lib/trace-cmd/include/private/trace-cmd-private.h)
>
>
> > +             int i;
> > +
> > +             for (i = 0; i < size; i++) {
> > +                     if (strlen(level) >= strlen(log_severtity[i].name) &&
>
> Why the strlen test? It's OK to pass in "debugignorme"? What's the use case
> for this?
>
> Anyway, I applied the first two patches of this series.
>
> -- Steve
>
>
> > +                         !strncmp(level, log_severtity[i].name, strlen(log_severtity[i].name))) {
> > +                             tracecmd_set_loglevel(log_severtity[i].id);
> > +                             return 0;
> > +                     }
> > +             }
> > +     }
> > +
> > +     return -1;
> > +}
> >
> >  /**
> >   * struct command
>


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

Re: [PATCH v2 3/8] trace-cmd library: Add log levels

[Steven Rostedt]

On Fri, 14 May 2021 05:51:24 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> On Fri, May 14, 2021 at 12:23 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Fri,  7 May 2021 12:53:28 +0300
> > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> >  
> > > Removed the "weak" definition of the library log functions. Setting
> > > the desired log level can be used to silence the library logs, instead
> > > of overwriting the log functions.  
> >
> > That's not the purpose of the weak definition. It is so that a GUI (like
> > kernelshark) can turn it into a pop up if need be or show in a status
> > window. It was not for quieting the function.  
> 
> The description is not correct. All these log functions use
> tep_vprint(), which is implemented as weak. This function has
> information for the library and log severity and should be used by
> kernelshark for that purpose. That's why I removed weak library
> specific functions.

Yes, tep_vprintk() could offer this as well, but there's still no
reason to not keep them as weak. Adding log levels is still unrelated
to removing the weak attribute.

I still do not see the rationale for removing the weak attribute of
these functions. It gives more flexibility for the users, without any
downsides.

Or is there a downside that you see for keeping them weak?

-- Steve