[PATCH v3 0/7] xenstore: enhance runtime debug capabilities

[PATCH v3 0/7] xenstore: enhance runtime debug capabilities

[Juergen Gross]

Today xenstored supports logging only via a command line parameter.
This means that logging is either all the time off (default) or on.
To switch logging on the Xen host has to be rebooted as xenstored
isn't restartable.

This patch series changes this by using the XS_DEBUG wire command of
Xenstore to control various debug functions:

- switch logging on/off
- specify logfile to use
- write memory usage to file

This will enable the host administrator to control above functionality
without the need of specifying any additional command line parameters
of xenstored or to restart the host in case of debug information
needed.

Changes in V3:
- Added new patch 1: fix a bug in reopen_log() (Wei Liu)
- Patch 2 (was 1): add xs_control_command() to libxenstore (Wei Liu)
- Split old patch 2 into two patches (Wei Liu)
- Add comment to patch 6 (was 4) (Wei Liu)

Changes in V2:
- added new patch 1 (rename of XS_DEBUG to XS_CONTROL)

Juergen Gross (7):
  xenstore: correct test for opened logfile in reopen_log()
  xenstore: rename XS_DEBUG wire command
  xenstore: Split out XS_CONTROL action to dedicated source file
  xenstore: enhance control command support
  xenstore: add support for changing log functionality dynamically
  xenstore: make memory report available via XS_CONTROL
  xenstore: remove memory report command line support

 tools/xenstore/Makefile            |   4 +-
 tools/xenstore/include/xenstore.h  |   4 +-
 tools/xenstore/xenstore_control.c  |  65 ++++++++----
 tools/xenstore/xenstored_control.c | 198 +++++++++++++++++++++++++++++++++++++
 tools/xenstore/xenstored_control.h |  19 ++++
 tools/xenstore/xenstored_core.c    |  81 ++++-----------
 tools/xenstore/xenstored_core.h    |   6 +-
 tools/xenstore/xs.c                |  13 ++-
 xen/include/public/io/xs_wire.h    |   3 +-
 9 files changed, 302 insertions(+), 91 deletions(-)
 create mode 100644 tools/xenstore/xenstored_control.c
 create mode 100644 tools/xenstore/xenstored_control.h

-- 
2.10.2


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

[PATCH v3 1/7] xenstore: correct test for opened logfile in reopen_log()

[Juergen Gross]

As 0 is a valid file descriptor testing a descriptor to be valid
should be done via >= 0 instead of > 0.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 1e9b622..7b16338 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -209,7 +209,7 @@ static void trigger_reopen_log(int signal __attribute__((unused)))
 static void reopen_log(void)
 {
 	if (tracefile) {
-		if (tracefd > 0)
+		if (tracefd >= 0)
 			close(tracefd);
 
 		tracefd = open(tracefile, O_WRONLY|O_CREAT|O_APPEND, 0600);
-- 
2.10.2


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

[PATCH v3 2/7] xenstore: rename XS_DEBUG wire command

[Juergen Gross]

In preparation to support other than pure debug functionality via the
Xenstore XS_DEBUG wire command rename it to XS_CONTROL and make
XS_DEBUG an alias of it.

Add an alias xs_control_command for the associated xs_debug_command,
too.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/include/xenstore.h |  4 +++-
 tools/xenstore/xenstored_core.c   |  8 ++++----
 tools/xenstore/xs.c               | 13 +++++++++----
 xen/include/public/io/xs_wire.h   |  3 ++-
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/tools/xenstore/include/xenstore.h b/tools/xenstore/include/xenstore.h
index 0d12c39..f460b8c 100644
--- a/tools/xenstore/include/xenstore.h
+++ b/tools/xenstore/include/xenstore.h
@@ -262,7 +262,9 @@ bool xs_path_is_subpath(const char *parent, const char *child);
  */
 bool xs_is_domain_introduced(struct xs_handle *h, unsigned int domid);
 
-/* Only useful for DEBUG versions */
+char *xs_control_command(struct xs_handle *h, const char *cmd,
+			 void *data, unsigned int len);
+/* Deprecated: use xs_control_command() instead. */
 char *xs_debug_command(struct xs_handle *h, const char *cmd,
 		       void *data, unsigned int len);
 
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 7b16338..97e7e31 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1261,7 +1261,7 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in)
 	return 0;
 }
 
-static int do_debug(struct connection *conn, struct buffered_data *in)
+static int do_control(struct connection *conn, struct buffered_data *in)
 {
 	int num;
 
@@ -1273,13 +1273,13 @@ static int do_debug(struct connection *conn, struct buffered_data *in)
 	if (streq(in->buffer, "print")) {
 		if (num < 2)
 			return EINVAL;
-		xprintf("debug: %s", in->buffer + get_string(in, 0));
+		xprintf("control: %s", in->buffer + get_string(in, 0));
 	}
 
 	if (streq(in->buffer, "check"))
 		check_store();
 
-	send_ack(conn, XS_DEBUG);
+	send_ack(conn, XS_CONTROL);
 
 	return 0;
 }
@@ -1288,7 +1288,7 @@ static struct {
 	const char *str;
 	int (*func)(struct connection *conn, struct buffered_data *in);
 } const wire_funcs[XS_TYPE_COUNT] = {
-	[XS_DEBUG]             = { "DEBUG",             do_debug },
+	[XS_CONTROL]           = { "CONTROL",           do_control },
 	[XS_DIRECTORY]         = { "DIRECTORY",         send_directory },
 	[XS_READ]              = { "READ",              do_read },
 	[XS_GET_PERMS]         = { "GET_PERMS",         do_get_perms },
diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index 6fa1261..56caac7 100644
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -1165,9 +1165,8 @@ out:
     return port;
 }
 
-/* Only useful for DEBUG versions */
-char *xs_debug_command(struct xs_handle *h, const char *cmd,
-		       void *data, unsigned int len)
+char *xs_control_command(struct xs_handle *h, const char *cmd,
+			 void *data, unsigned int len)
 {
 	struct iovec iov[2];
 
@@ -1176,10 +1175,16 @@ char *xs_debug_command(struct xs_handle *h, const char *cmd,
 	iov[1].iov_base = data;
 	iov[1].iov_len = len;
 
-	return xs_talkv(h, XBT_NULL, XS_DEBUG, iov,
+	return xs_talkv(h, XBT_NULL, XS_CONTROL, iov,
 			ARRAY_SIZE(iov), NULL);
 }
 
+char *xs_debug_command(struct xs_handle *h, const char *cmd,
+		       void *data, unsigned int len)
+{
+	return xs_control_command(h, cmd, data, len);
+}
+
 static int read_message(struct xs_handle *h, int nonblocking)
 {
 	/* IMPORTANT: It is forbidden to call this function without
diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
index f9f94f1..4dd6632 100644
--- a/xen/include/public/io/xs_wire.h
+++ b/xen/include/public/io/xs_wire.h
@@ -28,7 +28,8 @@
 
 enum xsd_sockmsg_type
 {
-    XS_DEBUG,
+    XS_CONTROL,
+#define XS_DEBUG XS_CONTROL
     XS_DIRECTORY,
     XS_READ,
     XS_GET_PERMS,
-- 
2.10.2


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

[PATCH v3 3/7] xenstore: Split out XS_CONTROL action to dedicated source file

[Juergen Gross]

Move the XS_CONTROL handling of xenstored to a new source file
xenstored_control.c.

In order to avoid making get_string() in xenstored_core.c globally
visible use strlen() instead, which is save in this context due to
xs_count_strings() before returned a value > 1.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/Makefile            |  4 +++-
 tools/xenstore/xenstored_control.c | 46 ++++++++++++++++++++++++++++++++++++++
 tools/xenstore/xenstored_control.h | 19 ++++++++++++++++
 tools/xenstore/xenstored_core.c    | 27 ++--------------------
 tools/xenstore/xenstored_core.h    |  2 +-
 5 files changed, 71 insertions(+), 27 deletions(-)
 create mode 100644 tools/xenstore/xenstored_control.c
 create mode 100644 tools/xenstore/xenstored_control.h

diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index 36b6fd4..bdca108 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -23,7 +23,9 @@ LDFLAGS += $(LDFLAGS-y)
 CLIENTS := xenstore-exists xenstore-list xenstore-read xenstore-rm xenstore-chmod
 CLIENTS += xenstore-write xenstore-ls xenstore-watch
 
-XENSTORED_OBJS = xenstored_core.o xenstored_watch.o xenstored_domain.o xenstored_transaction.o xs_lib.o talloc.o utils.o tdb.o hashtable.o
+XENSTORED_OBJS = xenstored_core.o xenstored_watch.o xenstored_domain.o
+XENSTORED_OBJS += xenstored_transaction.o xenstored_control.o
+XENSTORED_OBJS += xs_lib.o talloc.o utils.o tdb.o hashtable.o
 
 XENSTORED_OBJS_$(CONFIG_Linux) = xenstored_posix.o
 XENSTORED_OBJS_$(CONFIG_SunOS) = xenstored_solaris.o xenstored_posix.o xenstored_probes.o
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
new file mode 100644
index 0000000..f169d23
--- /dev/null
+++ b/tools/xenstore/xenstored_control.c
@@ -0,0 +1,46 @@
+/*
+    Interactive commands for Xen Store Daemon.
+    Copyright (C) 2017 Juergen Gross, SUSE Linux GmbH
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <errno.h>
+
+#include "utils.h"
+#include "xenstored_core.h"
+#include "xenstored_control.h"
+
+int do_control(struct connection *conn, struct buffered_data *in)
+{
+	int num;
+
+	if (conn->id != 0)
+		return EACCES;
+
+	num = xs_count_strings(in->buffer, in->used);
+
+	if (streq(in->buffer, "print")) {
+		if (num < 2)
+			return EINVAL;
+		xprintf("control: %s", in->buffer + strlen(in->buffer) + 1);
+	}
+
+	if (streq(in->buffer, "check"))
+		check_store();
+
+	send_ack(conn, XS_CONTROL);
+
+	return 0;
+}
diff --git a/tools/xenstore/xenstored_control.h b/tools/xenstore/xenstored_control.h
new file mode 100644
index 0000000..207e0a6
--- /dev/null
+++ b/tools/xenstore/xenstored_control.h
@@ -0,0 +1,19 @@
+/*
+    Interactive commands for Xen Store Daemon.
+    Copyright (C) 2017 Juergen Gross, SUSE Linux GmbH
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; If not, see <http://www.gnu.org/licenses/>.
+*/
+
+int do_control(struct connection *conn, struct buffered_data *in);
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 97e7e31..94c0a1d 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -51,6 +51,7 @@
 #include "xenstored_watch.h"
 #include "xenstored_transaction.h"
 #include "xenstored_domain.h"
+#include "xenstored_control.h"
 #include "tdb.h"
 
 #include "hashtable.h"
@@ -84,7 +85,6 @@ static TDB_CONTEXT *tdb_ctx = NULL;
 static bool trigger_talloc_report = false;
 
 static void corrupt(struct connection *conn, const char *fmt, ...);
-static void check_store(void);
 static const char *sockmsg_string(enum xsd_sockmsg_type type);
 
 #define log(...)							\
@@ -1261,29 +1261,6 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in)
 	return 0;
 }
 
-static int do_control(struct connection *conn, struct buffered_data *in)
-{
-	int num;
-
-	if (conn->id != 0)
-		return EACCES;
-
-	num = xs_count_strings(in->buffer, in->used);
-
-	if (streq(in->buffer, "print")) {
-		if (num < 2)
-			return EINVAL;
-		xprintf("control: %s", in->buffer + get_string(in, 0));
-	}
-
-	if (streq(in->buffer, "check"))
-		check_store();
-
-	send_ack(conn, XS_CONTROL);
-
-	return 0;
-}
-
 static struct {
 	const char *str;
 	int (*func)(struct connection *conn, struct buffered_data *in);
@@ -1764,7 +1741,7 @@ static void clean_store(struct hashtable *reachable)
 }
 
 
-static void check_store(void)
+void check_store(void)
 {
 	char * root = talloc_strdup(NULL, "/");
 	struct hashtable * reachable =
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index f6a56f7..89c1d75 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -158,7 +158,7 @@ TDB_CONTEXT *tdb_context(struct connection *conn);
 bool replace_tdb(const char *newname, TDB_CONTEXT *newtdb);
 
 struct connection *new_connection(connwritefn_t *write, connreadfn_t *read);
-
+void check_store(void);
 
 /* Is this a valid node name? */
 bool is_valid_nodename(const char *node);
-- 
2.10.2


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

[PATCH v3 4/7] xenstore: enhance control command support

[Juergen Gross]

The Xenstore protocol supports the XS_CONTROL command for triggering
various actions in the Xenstore daemon. Enhance that support by using
a command table and adding a help function.

Support multiple control commands in the associated xenstore-control
program used to issue XS_CONTROL commands.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstore_control.c  |  65 +++++++++++++++++-------
 tools/xenstore/xenstored_control.c | 100 +++++++++++++++++++++++++++++++++----
 2 files changed, 135 insertions(+), 30 deletions(-)

diff --git a/tools/xenstore/xenstore_control.c b/tools/xenstore/xenstore_control.c
index 0a108df..e42d478 100644
--- a/tools/xenstore/xenstore_control.c
+++ b/tools/xenstore/xenstore_control.c
@@ -7,29 +7,56 @@
 
 int main(int argc, char **argv)
 {
-  struct xs_handle * xsh;
+    struct xs_handle *xsh;
+    char *par = NULL;
+    char *ret;
+    unsigned int p, len = 0;
+    int rc = 0;
 
-  if (argc < 2 ||
-      strcmp(argv[1], "check"))
-  {
-    fprintf(stderr,
-            "Usage:\n"
-            "\n"
-            "       %s check\n"
-            "\n", argv[0]);
-    return 2;
-  }
+    if (argc < 2) {
+        fprintf(stderr, "Usage:\n"
+                "%s <command> [<arg>...]\n", argv[0]);
+        return 2;
+    }
 
-  xsh = xs_daemon_open();
+    for (p = 2; p < argc; p++)
+        len += strlen(argv[p]) + 1;
+    if (len) {
+        par = malloc(len);
+        if (!par) {
+            fprintf(stderr, "Allocation error.\n");
+            return 1;
+        }
+        len = 0;
+        for (p = 2; p < argc; p++) {
+            memcpy(par + len, argv[p], strlen(argv[p]) + 1);
+            len += strlen(argv[p]) + 1;
+        }
+    }
 
-  if (xsh == NULL) {
-    fprintf(stderr, "Failed to contact Xenstored.\n");
-    return 1;
-  }
+    xsh = xs_open(0);
+    if (xsh == NULL) {
+        fprintf(stderr, "Failed to contact Xenstored.\n");
+        return 1;
+    }
 
-  xs_debug_command(xsh, argv[1], NULL, 0);
+    ret = xs_debug_command(xsh, argv[1], par, len);
+    if (!ret) {
+        rc = 3;
+        if (errno == EINVAL) {
+            ret = xs_debug_command(xsh, "help", NULL, 0);
+            if (ret)
+                fprintf(stderr, "Command not supported. Valid commands are:\n"
+                                "%s\n", ret);
+            else
+                fprintf(stderr, "Error when executing command.\n");
+        } else
+            fprintf(stderr, "Error %d when trying to execute command.\n",
+                    errno);
+    } else if (strlen(ret) > 0)
+        printf("%s\n", ret);
 
-  xs_daemon_close(xsh);
+    xs_close(xsh);
 
-  return 0;
+    return rc;
 }
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index f169d23..3080e47 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -17,30 +17,108 @@
 */
 
 #include <errno.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <string.h>
 
 #include "utils.h"
+#include "talloc.h"
 #include "xenstored_core.h"
 #include "xenstored_control.h"
 
+struct cmd_s {
+	char *cmd;
+	int (*func)(void *, struct connection *, char **, int);
+	char *pars;
+};
+
+static int do_control_check(void *ctx, struct connection *conn,
+			    char **vec, int num)
+{
+	if (num)
+		return EINVAL;
+
+	check_store();
+
+	send_ack(conn, XS_CONTROL);
+	return 0;
+}
+
+static int do_control_print(void *ctx, struct connection *conn,
+			    char **vec, int num)
+{
+	if (num != 1)
+		return EINVAL;
+
+	xprintf("control: %s", vec[0]);
+
+	send_ack(conn, XS_CONTROL);
+	return 0;
+}
+
+static int do_control_help(void *, struct connection *, char **, int);
+
+static struct cmd_s cmds[] = {
+	{ "check", do_control_check, "" },
+	{ "print", do_control_print, "<string>" },
+	{ "help", do_control_help, "" },
+};
+
+static int do_control_help(void *ctx, struct connection *conn,
+			   char **vec, int num)
+{
+	int cmd, len = 0;
+	char *resp;
+
+	if (num)
+		return EINVAL;
+
+	for (cmd = 0; cmd < ARRAY_SIZE(cmds); cmd++) {
+		len += strlen(cmds[cmd].cmd) + 1;
+		len += strlen(cmds[cmd].pars) + 1;
+	}
+	len++;
+
+	resp = talloc_array(ctx, char, len);
+	if (!resp)
+		return ENOMEM;
+
+	len = 0;
+	for (cmd = 0; cmd < ARRAY_SIZE(cmds); cmd++) {
+		strcpy(resp + len, cmds[cmd].cmd);
+		len += strlen(cmds[cmd].cmd);
+		resp[len] = '\t';
+		len++;
+		strcpy(resp + len, cmds[cmd].pars);
+		len += strlen(cmds[cmd].pars);
+		resp[len] = '\n';
+		len++;
+	}
+	resp[len] = 0;
+
+	send_reply(conn, XS_CONTROL, resp, len);
+	return 0;
+}
+
 int do_control(struct connection *conn, struct buffered_data *in)
 {
 	int num;
+	int cmd;
+	char **vec;
 
 	if (conn->id != 0)
 		return EACCES;
 
 	num = xs_count_strings(in->buffer, in->used);
+	vec = talloc_array(in, char *, num);
+	if (!vec)
+		return ENOMEM;
+	if (get_strings(in, vec, num) != num)
+		return EIO;
 
-	if (streq(in->buffer, "print")) {
-		if (num < 2)
-			return EINVAL;
-		xprintf("control: %s", in->buffer + strlen(in->buffer) + 1);
-	}
+	for (cmd = 0; cmd < ARRAY_SIZE(cmds); cmd++)
+		if (streq(vec[0], cmds[cmd].cmd))
+			return cmds[cmd].func(in, conn, vec + 1, num - 1);
 
-	if (streq(in->buffer, "check"))
-		check_store();
-
-	send_ack(conn, XS_CONTROL);
-
-	return 0;
+	return EINVAL;
 }
-- 
2.10.2


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

[PATCH v3 5/7] xenstore: add support for changing log functionality dynamically

[Juergen Gross]

Today Xenstore supports logging only if specified at start of the
Xenstore daemon. As it can't be disabled during runtime it is not
recommended to start xenstored with logging enabled.

Add support for switching logging on and off at runtime and to
specify a (new) logfile. This is done via the XS_CONTROL wire command
which can be sent with xenstore-control.

To switch logging on just use:

xenstore-control log on

To switch it off again:

xenstore-control log off

To specify a (new) logfile:

xenstore-control logfile <file>

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_control.c | 34 ++++++++++++++++++++++++++++++++++
 tools/xenstore/xenstored_core.c    | 15 +++++++++++----
 tools/xenstore/xenstored_core.h    |  3 +++
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index 3080e47..c3587ad 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -44,6 +44,38 @@ static int do_control_check(void *ctx, struct connection *conn,
 	return 0;
 }
 
+static int do_control_log(void *ctx, struct connection *conn,
+			  char **vec, int num)
+{
+	if (num != 1)
+		return EINVAL;
+
+	if (!strcmp(vec[0], "on"))
+		reopen_log();
+	else if (!strcmp(vec[0], "off"))
+		close_log();
+	else
+		return EINVAL;
+
+	send_ack(conn, XS_CONTROL);
+	return 0;
+}
+
+static int do_control_logfile(void *ctx, struct connection *conn,
+			      char **vec, int num)
+{
+	if (num != 1)
+		return EINVAL;
+
+	close_log();
+	talloc_free(tracefile);
+	tracefile = talloc_strdup(NULL, vec[0]);
+	reopen_log();
+
+	send_ack(conn, XS_CONTROL);
+	return 0;
+}
+
 static int do_control_print(void *ctx, struct connection *conn,
 			    char **vec, int num)
 {
@@ -60,6 +92,8 @@ static int do_control_help(void *, struct connection *, char **, int);
 
 static struct cmd_s cmds[] = {
 	{ "check", do_control_check, "" },
+	{ "log", do_control_log, "on|off" },
+	{ "logfile", do_control_logfile, "<file>" },
 	{ "print", do_control_print, "<string>" },
 	{ "help", do_control_help, "" },
 };
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 94c0a1d..1b79a48 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -80,7 +80,7 @@ static int tracefd = -1;
 static bool recovery = true;
 static int reopen_log_pipe[2];
 static int reopen_log_pipe0_pollfd_idx = -1;
-static char *tracefile = NULL;
+char *tracefile = NULL;
 static TDB_CONTEXT *tdb_ctx = NULL;
 static bool trigger_talloc_report = false;
 
@@ -205,12 +205,17 @@ static void trigger_reopen_log(int signal __attribute__((unused)))
 	dummy = write(reopen_log_pipe[1], &c, 1);
 }
 
+void close_log(void)
+{
+	if (tracefd >= 0)
+		close(tracefd);
+	tracefd = -1;
+}
 
-static void reopen_log(void)
+void reopen_log(void)
 {
 	if (tracefile) {
-		if (tracefd >= 0)
-			close(tracefd);
+		close_log();
 
 		tracefd = open(tracefile, O_WRONLY|O_CREAT|O_APPEND, 0600);
 
@@ -2030,6 +2035,8 @@ int main(int argc, char *argv[])
 		finish_daemonize();
 
 	signal(SIGHUP, trigger_reopen_log);
+	if (tracefile)
+		tracefile = talloc_strdup(NULL, tracefile);
 
 	/* Get ready to listen to the tools. */
 	initialize_fds(*sock, &sock_pollfd_idx, *ro_sock, &ro_sock_pollfd_idx,
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 89c1d75..d315568 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -168,7 +168,10 @@ void trace_create(const void *data, const char *type);
 void trace_destroy(const void *data, const char *type);
 void trace(const char *fmt, ...);
 void dtrace_io(const struct connection *conn, const struct buffered_data *data, int out);
+void reopen_log(void);
+void close_log(void);
 
+extern char *tracefile;
 extern int dom0_domid;
 extern int dom0_event;
 extern int priv_domid;
-- 
2.10.2


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

[PATCH v3 6/7] xenstore: make memory report available via XS_CONTROL

[Juergen Gross]

Add a XS_CONTROL command to xenstored for doing a talloc report to a
file. Right now this is supported by specifying a command line option
when starting xenstored and sending a signal to the daemon to trigger
the report.

To dump the report to the standard log file call:

xenstore-control memreport

To dump the report to a new file call:

xenstore-control memreport <file>

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_control.c | 40 ++++++++++++++++++++++++++++++++++++++
 tools/xenstore/xenstored_core.c    |  2 +-
 tools/xenstore/xenstored_core.h    |  1 +
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index c3587ad..7c14911 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -76,6 +76,45 @@ static int do_control_logfile(void *ctx, struct connection *conn,
 	return 0;
 }
 
+static int do_control_memreport(void *ctx, struct connection *conn,
+				char **vec, int num)
+{
+	FILE *fp;
+	int fd;
+
+	if (num > 1)
+		return EINVAL;
+
+	if (num == 0) {
+		if (tracefd < 0) {
+			if (!tracefile)
+				return EBADF;
+			fp = fopen(tracefile, "a");
+		} else {
+			/*
+			 * Use dup() in order to avoid closing the file later
+			 * with fclose() which will release stream resources.
+			 */
+			fd = dup(tracefd);
+			if (fd < 0)
+				return EBADF;
+			fp = fdopen(fd, "a");
+			if (!fp)
+				close(fd);
+		}
+	} else
+		fp = fopen(vec[0], "a");
+
+	if (!fp)
+		return EBADF;
+
+	talloc_report_full(NULL, fp);
+	fclose(fp);
+
+	send_ack(conn, XS_CONTROL);
+	return 0;
+}
+
 static int do_control_print(void *ctx, struct connection *conn,
 			    char **vec, int num)
 {
@@ -94,6 +133,7 @@ static struct cmd_s cmds[] = {
 	{ "check", do_control_check, "" },
 	{ "log", do_control_log, "on|off" },
 	{ "logfile", do_control_logfile, "<file>" },
+	{ "memreport", do_control_memreport, "[<file>]" },
 	{ "print", do_control_print, "<string>" },
 	{ "help", do_control_help, "" },
 };
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 1b79a48..b1f8fbc 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -76,7 +76,7 @@ static unsigned int nr_fds;
 
 static bool verbose = false;
 LIST_HEAD(connections);
-static int tracefd = -1;
+int tracefd = -1;
 static bool recovery = true;
 static int reopen_log_pipe[2];
 static int reopen_log_pipe0_pollfd_idx = -1;
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index d315568..92cccb6 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -172,6 +172,7 @@ void reopen_log(void);
 void close_log(void);
 
 extern char *tracefile;
+extern int tracefd;
 extern int dom0_domid;
 extern int dom0_event;
 extern int priv_domid;
-- 
2.10.2


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

[PATCH v3 7/7] xenstore: remove memory report command line support

[Juergen Gross]

As a memory report can now be triggered via XS_CONTROL support via
command line and signal handler is no longer needed. Remove it.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_core.c | 35 ++---------------------------------
 1 file changed, 2 insertions(+), 33 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index b1f8fbc..5c659d8 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -82,7 +82,6 @@ static int reopen_log_pipe[2];
 static int reopen_log_pipe0_pollfd_idx = -1;
 char *tracefile = NULL;
 static TDB_CONTEXT *tdb_ctx = NULL;
-static bool trigger_talloc_report = false;
 
 static void corrupt(struct connection *conn, const char *fmt, ...);
 static const char *sockmsg_string(enum xsd_sockmsg_type type);
@@ -1792,10 +1791,6 @@ static void init_sockets(int **psock, int **pro_sock)
 	static int minus_one = -1;
 	*psock = *pro_sock = &minus_one;
 }
-
-static void do_talloc_report(int sig)
-{
-}
 #else
 static int destroy_fd(void *_fd)
 {
@@ -1855,11 +1850,6 @@ static void init_sockets(int **psock, int **pro_sock)
 
 
 }
-
-static void do_talloc_report(int sig)
-{
-	trigger_talloc_report = true;
-}
 #endif
 
 static void usage(void)
@@ -1884,7 +1874,6 @@ static void usage(void)
 "  -R, --no-recovery       to request that no recovery should be attempted when\n"
 "                          the store is corrupted (debug only),\n"
 "  -I, --internal-db       store database in memory, not on disk\n"
-"  -M, --memory-debug <file>  support memory debugging to file,\n"
 "  -V, --verbose           to request verbose execution.\n");
 }
 
@@ -1906,7 +1895,6 @@ static struct option options[] = {
 	{ "internal-db", 0, NULL, 'I' },
 	{ "verbose", 0, NULL, 'V' },
 	{ "watch-nb", 1, NULL, 'W' },
-	{ "memory-debug", 1, NULL, 'M' },
 	{ NULL, 0, NULL, 0 } };
 
 extern void dump_conn(struct connection *conn); 
@@ -1922,11 +1910,10 @@ int main(int argc, char *argv[])
 	bool outputpid = false;
 	bool no_domain_init = false;
 	const char *pidfile = NULL;
-	const char *memfile = NULL;
 	int timeout;
 
 
-	while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RVW:M:", options,
+	while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RVW:", options,
 				  NULL)) != -1) {
 		switch (opt) {
 		case 'D':
@@ -1977,9 +1964,6 @@ int main(int argc, char *argv[])
 		case 'p':
 			priv_domid = strtol(optarg, NULL, 10);
 			break;
-		case 'M':
-			memfile = optarg;
-			break;
 		}
 	}
 	if (optind != argc)
@@ -2006,10 +1990,7 @@ int main(int argc, char *argv[])
 	/* Don't kill us with SIGPIPE. */
 	signal(SIGPIPE, SIG_IGN);
 
-	if (memfile) {
-		talloc_enable_null_tracking();
-		signal(SIGUSR1, do_talloc_report);
-	}
+	talloc_enable_null_tracking();
 
 	init_sockets(&sock, &ro_sock);
 
@@ -2054,18 +2035,6 @@ int main(int argc, char *argv[])
 	for (;;) {
 		struct connection *conn, *next;
 
-		if (trigger_talloc_report) {
-			FILE *out;
-
-			assert(memfile);
-			trigger_talloc_report = false;
-			out = fopen(memfile, "a");
-			if (out) {
-				talloc_report_full(NULL, out);
-				fclose(out);
-			}
-		}
-
 		if (poll(fds, nr_fds, timeout) < 0) {
 			if (errno == EINTR)
 				continue;
-- 
2.10.2


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

Re: [PATCH v3 1/7] xenstore: correct test for opened logfile in reopen_log()

[Wei Liu]

On Wed, Feb 22, 2017 at 04:28:45PM +0100, Juergen Gross wrote:
> As 0 is a valid file descriptor testing a descriptor to be valid
> should be done via >= 0 instead of > 0.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---

Acked-by: Wei Liu <wei.liu2@citrix.com>

Ian, this should be backported.

>  tools/xenstore/xenstored_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 1e9b622..7b16338 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -209,7 +209,7 @@ static void trigger_reopen_log(int signal __attribute__((unused)))
>  static void reopen_log(void)
>  {
>  	if (tracefile) {
> -		if (tracefd > 0)
> +		if (tracefd >= 0)
>  			close(tracefd);
>  
>  		tracefd = open(tracefile, O_WRONLY|O_CREAT|O_APPEND, 0600);
> -- 
> 2.10.2
> 

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

Re: [PATCH v3 2/7] xenstore: rename XS_DEBUG wire command

[Wei Liu]

On Wed, Feb 22, 2017 at 04:28:46PM +0100, Juergen Gross wrote:
> In preparation to support other than pure debug functionality via the
> Xenstore XS_DEBUG wire command rename it to XS_CONTROL and make
> XS_DEBUG an alias of it.
> 
> Add an alias xs_control_command for the associated xs_debug_command,
> too.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/xenstore/include/xenstore.h |  4 +++-
>  tools/xenstore/xenstored_core.c   |  8 ++++----
>  tools/xenstore/xs.c               | 13 +++++++++----
>  xen/include/public/io/xs_wire.h   |  3 ++-
>  4 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/xenstore/include/xenstore.h b/tools/xenstore/include/xenstore.h
> index 0d12c39..f460b8c 100644
> --- a/tools/xenstore/include/xenstore.h
> +++ b/tools/xenstore/include/xenstore.h
> @@ -262,7 +262,9 @@ bool xs_path_is_subpath(const char *parent, const char *child);
>   */
>  bool xs_is_domain_introduced(struct xs_handle *h, unsigned int domid);
>  
> -/* Only useful for DEBUG versions */
> +char *xs_control_command(struct xs_handle *h, const char *cmd,
> +			 void *data, unsigned int len);
> +/* Deprecated: use xs_control_command() instead. */
>  char *xs_debug_command(struct xs_handle *h, const char *cmd,
>  		       void *data, unsigned int len);

I would also suggest adding __attribute__((deprecated)) for the
declaration.

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

Re: [PATCH v3 3/7] xenstore: Split out XS_CONTROL action to dedicated source file

[Wei Liu]

On Wed, Feb 22, 2017 at 04:28:47PM +0100, Juergen Gross wrote:
> Move the XS_CONTROL handling of xenstored to a new source file
> xenstored_control.c.
> 
> In order to avoid making get_string() in xenstored_core.c globally
> visible use strlen() instead, which is save in this context due to
> xs_count_strings() before returned a value > 1.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 4/7] xenstore: enhance control command support

[Wei Liu]

On Wed, Feb 22, 2017 at 04:28:48PM +0100, Juergen Gross wrote:
> The Xenstore protocol supports the XS_CONTROL command for triggering
> various actions in the Xenstore daemon. Enhance that support by using
> a command table and adding a help function.
> 
> Support multiple control commands in the associated xenstore-control
> program used to issue XS_CONTROL commands.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  tools/xenstore/xenstore_control.c  |  65 +++++++++++++++++-------
>  tools/xenstore/xenstored_control.c | 100 +++++++++++++++++++++++++++++++++----
>  2 files changed, 135 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/xenstore/xenstore_control.c b/tools/xenstore/xenstore_control.c
> index 0a108df..e42d478 100644
> --- a/tools/xenstore/xenstore_control.c
> +++ b/tools/xenstore/xenstore_control.c
> @@ -7,29 +7,56 @@
>  
>  int main(int argc, char **argv)
>  {
> -  struct xs_handle * xsh;
> +    struct xs_handle *xsh;
> +    char *par = NULL;
> +    char *ret;
> +    unsigned int p, len = 0;
> +    int rc = 0;
>  
> -  if (argc < 2 ||
> -      strcmp(argv[1], "check"))
> -  {
> -    fprintf(stderr,
> -            "Usage:\n"
> -            "\n"
> -            "       %s check\n"
> -            "\n", argv[0]);
> -    return 2;
> -  }
> +    if (argc < 2) {
> +        fprintf(stderr, "Usage:\n"
> +                "%s <command> [<arg>...]\n", argv[0]);
> +        return 2;
> +    }
>  
> -  xsh = xs_daemon_open();
> +    for (p = 2; p < argc; p++)
> +        len += strlen(argv[p]) + 1;
> +    if (len) {
> +        par = malloc(len);

par is never freed.

I would also suggest to use goto style error handling.

> +        if (!par) {
> +            fprintf(stderr, "Allocation error.\n");
> +            return 1;
> +        }
> +        len = 0;
> +        for (p = 2; p < argc; p++) {
> +            memcpy(par + len, argv[p], strlen(argv[p]) + 1);
> +            len += strlen(argv[p]) + 1;
> +        }
> +    }
>  
> -  if (xsh == NULL) {
> -    fprintf(stderr, "Failed to contact Xenstored.\n");
> -    return 1;
> -  }
> +    xsh = xs_open(0);
> +    if (xsh == NULL) {
> +        fprintf(stderr, "Failed to contact Xenstored.\n");
> +        return 1;
> +    }
>  
> -  xs_debug_command(xsh, argv[1], NULL, 0);
> +    ret = xs_debug_command(xsh, argv[1], par, len);

Should be xs_control_command now.

> +    if (!ret) {
> +        rc = 3;
> +        if (errno == EINVAL) {
> +            ret = xs_debug_command(xsh, "help", NULL, 0);

The rest looks good.

Wei.

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

Re: [PATCH v3 6/7] xenstore: make memory report available via XS_CONTROL

[Wei Liu]

On Wed, Feb 22, 2017 at 04:28:50PM +0100, Juergen Gross wrote:
> Add a XS_CONTROL command to xenstored for doing a talloc report to a
> file. Right now this is supported by specifying a command line option
> when starting xenstored and sending a signal to the daemon to trigger
> the report.
> 
> To dump the report to the standard log file call:
> 
> xenstore-control memreport
> 
> To dump the report to a new file call:
> 
> xenstore-control memreport <file>
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 5/7] xenstore: add support for changing log functionality dynamically

[Wei Liu]

On Wed, Feb 22, 2017 at 04:28:49PM +0100, Juergen Gross wrote:
> Today Xenstore supports logging only if specified at start of the
> Xenstore daemon. As it can't be disabled during runtime it is not
> recommended to start xenstored with logging enabled.
> 
> Add support for switching logging on and off at runtime and to
> specify a (new) logfile. This is done via the XS_CONTROL wire command
> which can be sent with xenstore-control.
> 
> To switch logging on just use:
> 
> xenstore-control log on
> 
> To switch it off again:
> 
> xenstore-control log off
> 
> To specify a (new) logfile:
> 
> xenstore-control logfile <file>
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 7/7] xenstore: remove memory report command line support

[Wei Liu]

On Wed, Feb 22, 2017 at 04:28:51PM +0100, Juergen Gross wrote:
> As a memory report can now be triggered via XS_CONTROL support via
> command line and signal handler is no longer needed. Remove it.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 4/7] xenstore: enhance control command support

[Juergen Gross]

On 23/02/17 12:06, Wei Liu wrote:
> On Wed, Feb 22, 2017 at 04:28:48PM +0100, Juergen Gross wrote:
>> The Xenstore protocol supports the XS_CONTROL command for triggering
>> various actions in the Xenstore daemon. Enhance that support by using
>> a command table and adding a help function.
>>
>> Support multiple control commands in the associated xenstore-control
>> program used to issue XS_CONTROL commands.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  tools/xenstore/xenstore_control.c  |  65 +++++++++++++++++-------
>>  tools/xenstore/xenstored_control.c | 100 +++++++++++++++++++++++++++++++++----
>>  2 files changed, 135 insertions(+), 30 deletions(-)
>>
>> diff --git a/tools/xenstore/xenstore_control.c b/tools/xenstore/xenstore_control.c
>> index 0a108df..e42d478 100644
>> --- a/tools/xenstore/xenstore_control.c
>> +++ b/tools/xenstore/xenstore_control.c
>> @@ -7,29 +7,56 @@
>>  
>>  int main(int argc, char **argv)
>>  {
>> -  struct xs_handle * xsh;
>> +    struct xs_handle *xsh;
>> +    char *par = NULL;
>> +    char *ret;
>> +    unsigned int p, len = 0;
>> +    int rc = 0;
>>  
>> -  if (argc < 2 ||
>> -      strcmp(argv[1], "check"))
>> -  {
>> -    fprintf(stderr,
>> -            "Usage:\n"
>> -            "\n"
>> -            "       %s check\n"
>> -            "\n", argv[0]);
>> -    return 2;
>> -  }
>> +    if (argc < 2) {
>> +        fprintf(stderr, "Usage:\n"
>> +                "%s <command> [<arg>...]\n", argv[0]);
>> +        return 2;
>> +    }
>>  
>> -  xsh = xs_daemon_open();
>> +    for (p = 2; p < argc; p++)
>> +        len += strlen(argv[p]) + 1;
>> +    if (len) {
>> +        par = malloc(len);
> 
> par is never freed.

You are right. Will correct.

> I would also suggest to use goto style error handling.

Okay.

>> +        if (!par) {
>> +            fprintf(stderr, "Allocation error.\n");
>> +            return 1;
>> +        }
>> +        len = 0;
>> +        for (p = 2; p < argc; p++) {
>> +            memcpy(par + len, argv[p], strlen(argv[p]) + 1);
>> +            len += strlen(argv[p]) + 1;
>> +        }
>> +    }
>>  
>> -  if (xsh == NULL) {
>> -    fprintf(stderr, "Failed to contact Xenstored.\n");
>> -    return 1;
>> -  }
>> +    xsh = xs_open(0);
>> +    if (xsh == NULL) {
>> +        fprintf(stderr, "Failed to contact Xenstored.\n");
>> +        return 1;
>> +    }
>>  
>> -  xs_debug_command(xsh, argv[1], NULL, 0);
>> +    ret = xs_debug_command(xsh, argv[1], par, len);
> 
> Should be xs_control_command now.

Of course. Will change.

> 
>> +    if (!ret) {
>> +        rc = 3;
>> +        if (errno == EINVAL) {
>> +            ret = xs_debug_command(xsh, "help", NULL, 0);
> 
> The rest looks good.

Thanks,


Juergen


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