[Xen-devel] [PATCH 0/4] xenstore domain: improve logging capabilities

[Xen-devel] [PATCH 0/4] xenstore domain: improve logging capabilities

[james-xen]

Hi,

When xenstore is run as a domain there is no way to get log messages from the
process.  This small series adds a new --trace-syslog option which will copy
trace() and xprintf() messages to syslog, in the MiniOS case this directs to
the kernel printk().  When it is enabled in the xenstore domain messages are
recorded on the hypervisor console according to the guest_loglvl.

A new xenstore control is also added so that the functionality can be toggled
dynamically at runtime.

I think there is possibly some room for further enhancements so that it is
more easily possible to log at different priorities rather than a simple
trace() = LOG_DEBUG, xprintf() = LOG_ERR.  Perhaps the addition of some
more macros similar to eprintf could be useful too.  If such change would
be considered useful then some guidance on the most appropriate direction
to take would be helpful.

The series is based on 4.11.3 but I can rebase it to master.

James


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/4] xenstored logging: introduce -Y / --trace-syslog option

[james-xen]

From: James Dingwall <james@dingwall.me.uk>

Add a new command line argument -Y / --trace-syslog which toggles the
boolean tracesyslog variable.
---
 tools/xenstore/xenstored_core.c | 10 ++++++++--
 tools/xenstore/xenstored_core.h |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c8e423700d..d0b383becc 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -79,6 +79,7 @@ static bool recovery = true;
 static int reopen_log_pipe[2];
 static int reopen_log_pipe0_pollfd_idx = -1;
 char *tracefile = NULL;
+bool tracesyslog = false;
 TDB_CONTEXT *tdb_ctx = NULL;
 
 static const char *sockmsg_string(enum xsd_sockmsg_type type);
@@ -1871,7 +1872,8 @@ static void usage(void)
 "  -H, --help              to output this message,\n"
 "  -N, --no-fork           to request that the daemon does not fork,\n"
 "  -P, --output-pid        to request that the pid of the daemon is output,\n"
-"  -T, --trace-file <file> giving the file for logging, and\n"
+"  -T, --trace-file <file> giving the file for logging, and/or\n"
+"  -Y, --trace-syslog writing trace message to syslog,\n"
 "  -E, --entry-nb <nb>     limit the number of entries per domain,\n"
 "  -S, --entry-size <size> limit the size of entry per domain, and\n"
 "  -W, --watch-nb <nb>     limit the number of watches per domain,\n"
@@ -1895,6 +1897,7 @@ static struct option options[] = {
 	{ "output-pid", 0, NULL, 'P' },
 	{ "entry-size", 1, NULL, 'S' },
 	{ "trace-file", 1, NULL, 'T' },
+	{ "trace-syslog", 0, NULL, 'Y' },
 	{ "transaction", 1, NULL, 't' },
 	{ "no-recovery", 0, NULL, 'R' },
 	{ "internal-db", 0, NULL, 'I' },
@@ -1918,7 +1921,7 @@ int main(int argc, char *argv[])
 	int timeout;
 
 
-	while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RVW:", options,
+	while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RVW:Y", options,
 				  NULL)) != -1) {
 		switch (opt) {
 		case 'D':
@@ -1960,6 +1963,9 @@ int main(int argc, char *argv[])
 		case 'W':
 			quota_nb_watch_per_domain = strtol(optarg, NULL, 10);
 			break;
+		case 'Y':
+			tracesyslog = true;
+			break;
 		case 'e':
 			dom0_event = strtol(optarg, NULL, 10);
 			break;
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 3d7eb91254..f5b0d70da4 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -174,6 +174,7 @@ void dtrace_io(const struct connection *conn, const struct buffered_data *data,
 void reopen_log(void);
 void close_log(void);
 
+extern bool tracesyslog;
 extern char *tracefile;
 extern int tracefd;
 
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 2/4] xenstored logging: add control to dynamically toggle tracesyslog flag

[james-xen]

From: James Dingwall <james@dingwall.me.uk>

Add a new xenstore control command which allows the tracesyslog boolean to be
changed at run time.
---
 tools/xenstore/xenstored_control.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index e4b8aa95ab..120dc3aa98 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -76,6 +76,23 @@ static int do_control_logfile(void *ctx, struct connection *conn,
 	return 0;
 }
 
+static int do_control_logsyslog(void *ctx, struct connection *conn,
+			  char **vec, int num)
+{
+	if (num != 1)
+		return EINVAL;
+
+	if (!strcmp(vec[0], "on"))
+		tracesyslog = true;
+	else if (!strcmp(vec[0], "off"))
+		tracesyslog = false;
+	else
+		return EINVAL;
+
+	send_ack(conn, XS_CONTROL);
+	return 0;
+}
+
 static int do_control_memreport(void *ctx, struct connection *conn,
 				char **vec, int num)
 {
@@ -133,6 +150,7 @@ static struct cmd_s cmds[] = {
 	{ "check", do_control_check, "" },
 	{ "log", do_control_log, "on|off" },
 	{ "logfile", do_control_logfile, "<file>" },
+	{ "syslog", do_control_logsyslog, "on|off" },
 	{ "memreport", do_control_memreport, "[<file>]" },
 	{ "print", do_control_print, "<string>" },
 	{ "help", do_control_help, "" },
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 3/4] xenstored logging: send trace messages to syslog

[james-xen]

From: James Dingwall <james@dingwall.me.uk>

Unconditionally openlog() since we allow tracesyslog to be changed at runtime.
Modify the trace() call to send messages to vsyslog() when tracesyslog is
enabled.

Note some trace() messages come in several calls before the '\n'.  This works
well when the output is a file stream but may not suit vsyslog() quite as well.
Primarily this feature is for xenstored in a stubdom which doesn't wrap the
message until '\n' so no attempt to coalesce trace() calls until '\n' is
made.  (Could trace() use vfprintf() to write to the log file?)
---
 tools/xenstore/xenstored_core.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index d0b383becc..5320db2499 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -89,7 +89,7 @@ static const char *sockmsg_string(enum xsd_sockmsg_type type);
 		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
 		if (s) {						\
 			trace("%s\n", s);				\
-			syslog(LOG_ERR, "%s",  s);			\
+			syslog(LOG_ERR, "%s\n", s);			\
 			talloc_free(s);					\
 		} else {						\
 			trace("talloc failure during logging\n");	\
@@ -110,6 +110,12 @@ void trace(const char *fmt, ...)
 	char sbuf[1024];
 	int ret, dummy;
 
+	if (tracesyslog) {
+		va_start(arglist, fmt);
+		vsyslog(LOG_DEBUG, fmt, arglist);
+		va_end(arglist);
+	}
+
 	if (tracefd < 0)
 		return;
 
@@ -1987,10 +1993,9 @@ int main(int argc, char *argv[])
 	mkdir(xs_daemon_rundir(), 0755);
 	mkdir(xs_daemon_rootdir(), 0755);
 
-	if (dofork) {
-		openlog("xenstored", 0, LOG_DAEMON);
+	openlog("xenstored", 0, LOG_DAEMON);
+	if (dofork)
 		daemonize();
-	}
 	if (pidfile)
 		write_pidfile(pidfile);
 
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 4/4] xenstored logging: add minios variant of default_xprintf()

[james-xen]

From: James Dingwall <james@dingwall.me.uk>

stderr is not available in a xenstore domain.  Add a new implementation of
default_xprintf() which sends the message to vsyslog() at LOG_ERROR.
---
 tools/xenstore/utils.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/xenstore/utils.c b/tools/xenstore/utils.c
index a1ac12584a..7ed05b1970 100644
--- a/tools/xenstore/utils.c
+++ b/tools/xenstore/utils.c
@@ -8,8 +8,21 @@
 #include <fcntl.h>
 #include <sys/types.h>
 #include <signal.h>
+#ifdef __MINIOS__
+#include <syslog.h>
+#endif
 #include "utils.h"
 
+#ifdef __MINIOS__
+static void default_xprintf(const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	vsyslog(LOG_ERR, fmt, args);
+	va_end(args);
+}
+#else
 static void default_xprintf(const char *fmt, ...)
 {
 	va_list args;
@@ -19,6 +32,7 @@ static void default_xprintf(const char *fmt, ...)
 	va_end(args);
 	fflush(stderr);
 }
+#endif
 
 void (*xprintf)(const char *fmt, ...) = default_xprintf;
 
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel