[PATCH v2 0/5] xenstore: enhance runtime debug capabilities

[PATCH v2 0/5] 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 V2:
- added new patch 1 (rename of XS_DEBUG to XS_CONTROL)

Juergen Gross (5):
  xenstore: rename XS_DEBUG wire command
  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  |   2 +-
 tools/xenstore/xenstore_control.c  |  65 +++++++++----
 tools/xenstore/xenstored_control.c | 194 +++++++++++++++++++++++++++++++++++++
 tools/xenstore/xenstored_control.h |  19 ++++
 tools/xenstore/xenstored_core.c    |  81 ++++------------
 tools/xenstore/xenstored_core.h    |   6 +-
 tools/xenstore/xs.c                |   7 +-
 xen/include/public/io/xs_wire.h    |   3 +-
 9 files changed, 290 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 v2 1/5] 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 | 2 +-
 tools/xenstore/xenstored_core.c   | 8 ++++----
 tools/xenstore/xs.c               | 7 +++----
 xen/include/public/io/xs_wire.h   | 3 ++-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/xenstore/include/xenstore.h b/tools/xenstore/include/xenstore.h
index 0d12c39..66bb9ed 100644
--- a/tools/xenstore/include/xenstore.h
+++ b/tools/xenstore/include/xenstore.h
@@ -262,9 +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_debug_command(struct xs_handle *h, const char *cmd,
 		       void *data, unsigned int len);
+#define xs_control_command xs_debug_command
 
 int xs_suspend_evtchn_port(int domid);
 #endif /* XENSTORE_H */
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 1e9b622..e332d7f 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..0fdc0bb 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,7 +1175,7 @@ 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);
 }
 
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 v2 2/5] 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. Move all the XS_CONTROL
related code to a new source file xenstored_control.c.

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/Makefile            |   4 +-
 tools/xenstore/xenstore_control.c  |  65 +++++++++++++------
 tools/xenstore/xenstored_control.c | 124 +++++++++++++++++++++++++++++++++++++
 tools/xenstore/xenstored_control.h |  19 ++++++
 tools/xenstore/xenstored_core.c    |  27 +-------
 tools/xenstore/xenstored_core.h    |   2 +-
 6 files changed, 195 insertions(+), 46 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/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
new file mode 100644
index 0000000..3080e47
--- /dev/null
+++ b/tools/xenstore/xenstored_control.c
@@ -0,0 +1,124 @@
+/*
+    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 <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;
+
+	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);
+
+	return EINVAL;
+}
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 e332d7f..e0f1261 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 v2 3/5] 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 e0f1261..aff95f4 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 v2 4/5] 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 | 36 ++++++++++++++++++++++++++++++++++++
 tools/xenstore/xenstored_core.c    |  2 +-
 tools/xenstore/xenstored_core.h    |  1 +
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index c3587ad..b4ec6ce 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -76,6 +76,41 @@ 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 {
+			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 +129,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 aff95f4..e40a725 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 v2 5/5] 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 e40a725..f2b2be5 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 v2 1/5] xenstore: rename XS_DEBUG wire command

[Wei Liu]

On Tue, Feb 21, 2017 at 04:07:33PM +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>
> ---
>  tools/xenstore/include/xenstore.h | 2 +-
>  tools/xenstore/xenstored_core.c   | 8 ++++----
>  tools/xenstore/xs.c               | 7 +++----
>  xen/include/public/io/xs_wire.h   | 3 ++-
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/xenstore/include/xenstore.h b/tools/xenstore/include/xenstore.h
> index 0d12c39..66bb9ed 100644
> --- a/tools/xenstore/include/xenstore.h
> +++ b/tools/xenstore/include/xenstore.h
> @@ -262,9 +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_debug_command(struct xs_handle *h, const char *cmd,
>  		       void *data, unsigned int len);
> +#define xs_control_command xs_debug_command
>  

Should be the other way around?

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

Re: [PATCH v2 2/5] xenstore: enhance control command support

[Wei Liu]

On Tue, Feb 21, 2017 at 04:07:34PM +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. Move all the XS_CONTROL
> related code to a new source file xenstored_control.c.
> 
> Support multiple control commands in the associated xenstore-control
> program used to issue XS_CONTROL commands.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Can you please break the refactoring and enhancement into two patches.
That would be easier for me to review.

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

Re: [PATCH v2 3/5] xenstore: add support for changing log functionality dynamically

[Wei Liu]

On Tue, Feb 21, 2017 at 04:07:35PM +0100, Juergen Gross wrote:
[...]
>  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)

tracefd >= 0

I think this is a bug in the original code though.

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

Re: [PATCH v2 4/5] xenstore: make memory report available via XS_CONTROL

[Wei Liu]

On Tue, Feb 21, 2017 at 04:07:36PM +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>
> ---
>  tools/xenstore/xenstored_control.c | 36 ++++++++++++++++++++++++++++++++++++
>  tools/xenstore/xenstored_core.c    |  2 +-
>  tools/xenstore/xenstored_core.h    |  1 +
>  3 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
> index c3587ad..b4ec6ce 100644
> --- a/tools/xenstore/xenstored_control.c
> +++ b/tools/xenstore/xenstored_control.c
> @@ -76,6 +76,41 @@ 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 {
> +			fd = dup(tracefd);

Why dup() the fd? Is it because you want to avoid tracefd becomes
invalid under your feet?

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

Re: [PATCH v2 1/5] xenstore: rename XS_DEBUG wire command

[Juergen Gross]

On 22/02/17 13:36, Wei Liu wrote:
> On Tue, Feb 21, 2017 at 04:07:33PM +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>
>> ---
>>  tools/xenstore/include/xenstore.h | 2 +-
>>  tools/xenstore/xenstored_core.c   | 8 ++++----
>>  tools/xenstore/xs.c               | 7 +++----
>>  xen/include/public/io/xs_wire.h   | 3 ++-
>>  4 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/xenstore/include/xenstore.h b/tools/xenstore/include/xenstore.h
>> index 0d12c39..66bb9ed 100644
>> --- a/tools/xenstore/include/xenstore.h
>> +++ b/tools/xenstore/include/xenstore.h
>> @@ -262,9 +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_debug_command(struct xs_handle *h, const char *cmd,
>>  		       void *data, unsigned int len);
>> +#define xs_control_command xs_debug_command
>>  
> 
> Should be the other way around?
> 

I did it that way to keep the xs_debug_command symbol in the library.
Otherwise someone using that function would have to rebuild.


Juergen

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

Re: [PATCH v2 2/5] xenstore: enhance control command support

[Juergen Gross]

On 22/02/17 13:36, Wei Liu wrote:
> On Tue, Feb 21, 2017 at 04:07:34PM +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. Move all the XS_CONTROL
>> related code to a new source file xenstored_control.c.
>>
>> Support multiple control commands in the associated xenstore-control
>> program used to issue XS_CONTROL commands.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Can you please break the refactoring and enhancement into two patches.
> That would be easier for me to review.
> 

Okay.


Juergen

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

Re: [PATCH v2 3/5] xenstore: add support for changing log functionality dynamically

[Juergen Gross]

On 22/02/17 13:36, Wei Liu wrote:
> On Tue, Feb 21, 2017 at 04:07:35PM +0100, Juergen Gross wrote:
> [...]
>>  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)
> 
> tracefd >= 0
> 
> I think this is a bug in the original code though.
> 

Okay, so I'll send another patch to correct this.


Juergen

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

Re: [PATCH v2 1/5] xenstore: rename XS_DEBUG wire command

[Wei Liu]

On Wed, Feb 22, 2017 at 01:40:42PM +0100, Juergen Gross wrote:
> On 22/02/17 13:36, Wei Liu wrote:
> > On Tue, Feb 21, 2017 at 04:07:33PM +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>
> >> ---
> >>  tools/xenstore/include/xenstore.h | 2 +-
> >>  tools/xenstore/xenstored_core.c   | 8 ++++----
> >>  tools/xenstore/xs.c               | 7 +++----
> >>  xen/include/public/io/xs_wire.h   | 3 ++-
> >>  4 files changed, 10 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/tools/xenstore/include/xenstore.h b/tools/xenstore/include/xenstore.h
> >> index 0d12c39..66bb9ed 100644
> >> --- a/tools/xenstore/include/xenstore.h
> >> +++ b/tools/xenstore/include/xenstore.h
> >> @@ -262,9 +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_debug_command(struct xs_handle *h, const char *cmd,
> >>  		       void *data, unsigned int len);
> >> +#define xs_control_command xs_debug_command
> >>  
> > 
> > Should be the other way around?
> > 
> 
> I did it that way to keep the xs_debug_command symbol in the library.
> Otherwise someone using that function would have to rebuild.
> 

But then there is no xs_control_command symbol now.

I would just have two functions. xs_debug_command should be implemented
with xs_control_command (plus some extra check to only allow certain
behaviours if you fancy that).

> 
> Juergen

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

Re: [PATCH v2 4/5] xenstore: make memory report available via XS_CONTROL

[Juergen Gross]

On 22/02/17 13:36, Wei Liu wrote:
> On Tue, Feb 21, 2017 at 04:07:36PM +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>
>> ---
>>  tools/xenstore/xenstored_control.c | 36 ++++++++++++++++++++++++++++++++++++
>>  tools/xenstore/xenstored_core.c    |  2 +-
>>  tools/xenstore/xenstored_core.h    |  1 +
>>  3 files changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
>> index c3587ad..b4ec6ce 100644
>> --- a/tools/xenstore/xenstored_control.c
>> +++ b/tools/xenstore/xenstored_control.c
>> @@ -76,6 +76,41 @@ 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 {
>> +			fd = dup(tracefd);
> 
> Why dup() the fd? Is it because you want to avoid tracefd becomes
> invalid under your feet?
> 

I want to be able to fclose() to get rid of the stream resources without
closing the log file.


Juergen

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

Re: [PATCH v2 1/5] xenstore: rename XS_DEBUG wire command

[Juergen Gross]

On 22/02/17 13:43, Wei Liu wrote:
> On Wed, Feb 22, 2017 at 01:40:42PM +0100, Juergen Gross wrote:
>> On 22/02/17 13:36, Wei Liu wrote:
>>> On Tue, Feb 21, 2017 at 04:07:33PM +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>
>>>> ---
>>>>  tools/xenstore/include/xenstore.h | 2 +-
>>>>  tools/xenstore/xenstored_core.c   | 8 ++++----
>>>>  tools/xenstore/xs.c               | 7 +++----
>>>>  xen/include/public/io/xs_wire.h   | 3 ++-
>>>>  4 files changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/xenstore/include/xenstore.h b/tools/xenstore/include/xenstore.h
>>>> index 0d12c39..66bb9ed 100644
>>>> --- a/tools/xenstore/include/xenstore.h
>>>> +++ b/tools/xenstore/include/xenstore.h
>>>> @@ -262,9 +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_debug_command(struct xs_handle *h, const char *cmd,
>>>>  		       void *data, unsigned int len);
>>>> +#define xs_control_command xs_debug_command
>>>>  
>>>
>>> Should be the other way around?
>>>
>>
>> I did it that way to keep the xs_debug_command symbol in the library.
>> Otherwise someone using that function would have to rebuild.
>>
> 
> But then there is no xs_control_command symbol now.
> 
> I would just have two functions. xs_debug_command should be implemented
> with xs_control_command (plus some extra check to only allow certain
> behaviours if you fancy that).

Okay, if you like that better.


Juergen


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

Re: [PATCH v2 4/5] xenstore: make memory report available via XS_CONTROL

[Wei Liu]

On Wed, Feb 22, 2017 at 01:43:27PM +0100, Juergen Gross wrote:
> On 22/02/17 13:36, Wei Liu wrote:
> > On Tue, Feb 21, 2017 at 04:07:36PM +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>
> >> ---
> >>  tools/xenstore/xenstored_control.c | 36 ++++++++++++++++++++++++++++++++++++
> >>  tools/xenstore/xenstored_core.c    |  2 +-
> >>  tools/xenstore/xenstored_core.h    |  1 +
> >>  3 files changed, 38 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
> >> index c3587ad..b4ec6ce 100644
> >> --- a/tools/xenstore/xenstored_control.c
> >> +++ b/tools/xenstore/xenstored_control.c
> >> @@ -76,6 +76,41 @@ 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 {
> >> +			fd = dup(tracefd);
> > 
> > Why dup() the fd? Is it because you want to avoid tracefd becomes
> > invalid under your feet?
> > 
> 
> I want to be able to fclose() to get rid of the stream resources without
> closing the log file.
> 

Oh, right. I missed that aspect. Could you please add a comment for that
please.

> 
> Juergen

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

Re: [PATCH v2 4/5] xenstore: make memory report available via XS_CONTROL

[Juergen Gross]

On 22/02/17 13:47, Wei Liu wrote:
> On Wed, Feb 22, 2017 at 01:43:27PM +0100, Juergen Gross wrote:
>> On 22/02/17 13:36, Wei Liu wrote:
>>> On Tue, Feb 21, 2017 at 04:07:36PM +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>
>>>> ---
>>>>  tools/xenstore/xenstored_control.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>  tools/xenstore/xenstored_core.c    |  2 +-
>>>>  tools/xenstore/xenstored_core.h    |  1 +
>>>>  3 files changed, 38 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
>>>> index c3587ad..b4ec6ce 100644
>>>> --- a/tools/xenstore/xenstored_control.c
>>>> +++ b/tools/xenstore/xenstored_control.c
>>>> @@ -76,6 +76,41 @@ 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 {
>>>> +			fd = dup(tracefd);
>>>
>>> Why dup() the fd? Is it because you want to avoid tracefd becomes
>>> invalid under your feet?
>>>
>>
>> I want to be able to fclose() to get rid of the stream resources without
>> closing the log file.
>>
> 
> Oh, right. I missed that aspect. Could you please add a comment for that
> please.

Sure.


Juergen


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