thr3ads.net - Xen devel - [Xen-devel] [PATCH resend] allow connecting to xenconsole from remote hosts [Sep 2006]

If this information is useful, please help other people find it:
Share via:

Muli Ben-Yehuda

2006-Sep-26 15:26 UTC

[Xen-devel] [PATCH resend] allow connecting to xenconsole from remote hosts

The attached patch makes xenconsole send and receive console messages
over a remote connection, instead of via stdin/stdout - if given the
--remote switch. It is useful for proxy''ing real console message or
other protocol messages (such as gdb) between dom0 and a remote
host. We''re currently using it for proxying gdb between gdbstub in a
partition that talks gdb over the console page to a remote host
running gdb.

Changed since last version:
- fixed compile warning - type of ''buf'' in handle_read_fd()
should be
  char*, not unsigned char*.

Signed-off-by: Muli Ben-Yehuda <muli@il.ibm.com>

diff -Naurp -X /home/muli/w/dontdiff vanilla/tools/console/client/main.c
gdbconsole.hg/tools/console/client/main.c
--- vanilla/tools/console/client/main.c	2006-09-07 11:16:07.000000000 +0300
+++ gdbconsole.hg/tools/console/client/main.c	2006-09-25 11:17:32.000000000
+0300
@@ -1,8 +1,9 @@
 /*\
- *  Copyright (C) International Business Machines  Corp., 2005
- *  Author(s): Anthony Liguori <aliguori@us.ibm.com>
+ *  Copyright (C) International Business Machines  Corp., 2005, 2006
+ *  Author: Anthony Liguori <aliguori@us.ibm.com>
+ *  Author: Muli Ben-Yehuda <muli@il.ibm.com>
  *
- *  Xen Console Daemon
+ *  Xen Console Client
  *
  *  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
@@ -35,32 +36,110 @@
 #include <err.h>
 #include <errno.h>
 #include <pty.h>
+#include <netinet/in.h>
+#include <netdb.h>
+#include <stdarg.h>
 
 #include "xs.h"
 
+#define min(x,y) ({ \
+        const typeof(x) _x = (x);       \
+        const typeof(y) _y = (y);       \
+        (void) (&_x == &_y);            \
+        _x < _y ? _x : _y; })
+
 #define ESCAPE_CHARACTER 0x1d
+#define DEFAULT_LISTEN_PORT 7890
 
-static volatile sig_atomic_t received_signal = 0;
+struct remote {
+	long port;
+	int do_listen;
+	int gateway;
+	int server; /* server socket */
+};
+
+#define RING_SIZE (1 << 16)
+#define RING_IDX_MASK(i) ((i) & (RING_SIZE - 1))
+
+struct ring {
+	size_t prod;
+	size_t cons;
+	unsigned char ring[RING_SIZE];
+};
 
-static void sighandler(int signum)
+static void ring_put(struct ring* ring, char* buf, size_t len)
 {
-	received_signal = 1;
+	size_t i;
+
+	for (i = 0; i < len; i++)
+		ring->ring[RING_IDX_MASK(ring->prod++)] = buf[i];
 }
 
-static bool write_sync(int fd, const void *data, size_t size)
+static void ring_get(struct ring* ring, char* buf, size_t *pcount)
 {
-	size_t offset = 0;
-	ssize_t len;
+	size_t idx, len, max, sofar, c;
 
-	while (offset < size) {
-		len = write(fd, data + offset, size - offset);
-		if (len < 1) {
-			return false;
-		}
-		offset += len;
+	max   = *pcount;
+	sofar = 0;
+
+	c = ring->cons;
+	while ((c != ring->prod) && (sofar < max)) {
+		idx = RING_IDX_MASK(c);
+		len = ring->prod - c;
+		if ((idx + len) > RING_SIZE)
+			len = RING_SIZE - idx;
+		if ((sofar + len) > max)
+			len = max - sofar;
+		memcpy(buf, &ring->ring[idx], len);
+		buf += len;
+		sofar += len;
+		c += len;
 	}
 
-	return true;
+	ring->cons = c;
+	*pcount = sofar;
+}
+
+static size_t ring_prod_available(struct ring *ring)
+{
+	size_t distance = ring->prod - ring->cons;
+
+	if (distance >= RING_SIZE)
+		return 0;
+
+	return (RING_SIZE - distance);
+}
+
+static size_t ring_cons_available(struct ring *ring)
+{
+	if (ring->cons == ring->prod)
+		return 0;
+
+	return ring->prod - ring->cons;
+}
+
+static volatile sig_atomic_t received_signal = 0;
+
+static int debug;
+
+#define dbg(fmt, args...) do {                       \
+        if (debug)                                   \
+                _dbg("[%s] " fmt, __func__, ##args); \
+} while (0)
+
+static void _dbg(const char *fmt, ...)
+{
+        va_list args;
+
+        va_start(args, fmt);
+        vfprintf(stderr, fmt, args);
+        va_end(args);
+        fflush(stderr);
+}
+
+static void sighandler(int signum)
+{
+	received_signal = 1;
 }
 
 static void usage(const char *program) {
@@ -68,9 +147,35 @@ static void usage(const char *program) {
 	       "Attaches to a virtual domain console\n"
 	       "\n"
 	       "  -h, --help       display this help and exit\n"
+	       "  -r, --remote     wait for connections from local
clients\n"
+	       "  -g, --gateway    allow connections from any host\n"
+	       "  -d, --debug      enable debug output\n"
 	       , program);
 }
 
+ssize_t write_all(int fd, const void *buf, size_t count)
+{
+	const unsigned char* b = (const unsigned char*)buf;
+	ssize_t sum = 0;
+	ssize_t ret = 0;
+
+	while (count) {
+		ret = write(fd, b, count);
+		if (ret == -1) {
+			if (errno == EINTR || errno == EAGAIN)
+				continue;
+			ret = -errno;
+			break;
+		}
+		if (ret == 0)
+			break;
+		count -= ret;
+		b += ret;
+		sum += ret;
+	}
+	return (ret >= 0 ? sum : ret);
+}
+
 /* don''t worry too much if setting terminal attributes fail */
 static void init_term(int fd, struct termios *old)
 {
@@ -91,79 +196,256 @@ static void restore_term(int fd, struct 
 	tcsetattr(fd, TCSAFLUSH, old);
 }
 
-static int console_loop(int fd)
+static inline void dump_buf(char* prefix, int fd, const char *data, size_t len)
+{
+	dbg("%s: %u bytes on %d:\n", prefix, len, fd);
+	dbg("buf: `%s''\n", data);
+}
+
+static int handle_read_fd(int fd, struct ring* ring)
+{
+	int ret;
+	ssize_t len;
+	char* buf;
+	size_t avail;
+
+	avail = ring_prod_available(ring);
+	if (avail == 0)
+		return 0;
+
+	buf = malloc(avail);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	len = read(fd, buf, avail);
+	if (len == 1 && buf[0] == ESCAPE_CHARACTER) {
+		ret = ECONNRESET;
+		goto free;
+	}
+
+	if (len == -1) {
+		if (errno == EINTR || errno == EAGAIN)
+			ret = EINTR;
+		else
+			ret = -errno;
+		goto free;
+	}
+	if (len == 0) {
+		/* try to reconnect */
+		ret = EAGAIN;
+		goto free;
+	}
+
+	dump_buf("read", fd, buf, len);
+	ring_put(ring, buf, len);
+	ret = 0;
+free:
+	free(buf);
+done:
+	return ret;
+}
+
+static int handle_write_fd(int fd, struct ring* ring)
+{
+	int ret;
+	size_t len;
+	char buf[4096];
+
+	len = min(ring_cons_available(ring), sizeof(buf));
+
+	while (len) {
+		ring_get(ring, buf, &len);
+
+		dump_buf("write", fd, buf, len);
+
+		ret = write_all(fd, buf, len);
+		if (ret < 0)
+			return ret;
+
+		len = min(ring_cons_available(ring), sizeof(buf));
+	}
+	return 0;
+}
+
+static int console_loop(int conspty, int infd, int outfd)
 {
 	int ret;
+	int max;
+
+	struct ring console = {
+		.prod = 0,
+		.cons = 0,
+		.ring = { ''\0'' }
+	};
+
+	struct ring out = {
+		.prod = 0,
+		.cons = 0,
+		.ring = { ''\0'' }
+	};
 
 	do {
-		fd_set fds;
+		fd_set rfds, wfds;
+
+		FD_ZERO(&rfds);
+		FD_SET(infd, &rfds);
+		FD_SET(conspty, &rfds);
+
+		FD_ZERO(&wfds);
+		FD_SET(conspty, &wfds);
+		FD_SET(outfd, &wfds);
+
+		max = (conspty | infd | outfd) + 1;
 
-		FD_ZERO(&fds);
-		FD_SET(STDIN_FILENO, &fds);
-		FD_SET(fd, &fds);
+		ret = select(max, &rfds, &wfds, NULL, NULL);
 
-		ret = select(fd + 1, &fds, NULL, NULL, NULL);
 		if (ret == -1) {
 			if (errno == EINTR || errno == EAGAIN) {
 				continue;
 			}
-			return -1;
+			return -errno;
 		}
 
-		if (FD_ISSET(STDIN_FILENO, &fds)) {
-			ssize_t len;
-			char msg[60];
-
-			len = read(STDIN_FILENO, msg, sizeof(msg));
-			if (len == 1 && msg[0] == ESCAPE_CHARACTER) {
+		if (FD_ISSET(infd, &rfds)) {
+			ret = handle_read_fd(infd, &console);
+			if (ret < 0)
+				return ret;
+			if (ret == EINTR)
+				continue;
+			if (ret == EAGAIN)
+				return EAGAIN;
+			if (ret == ECONNRESET)
 				return 0;
-			} 
-
-			if (len == 0 || len == -1) {
-				if (len == -1 &&
-				    (errno == EINTR || errno == EAGAIN)) {
-					continue;
-				}
-				return -1;
-			}
+		}
 
-			if (!write_sync(fd, msg, len)) {
-				perror("write() failed");
-				return -1;
-			}
+		if (FD_ISSET(conspty, &rfds)) {
+			ret = handle_read_fd(conspty, &out);
+			if (ret < 0)
+				return ret;
+			if (ret == EINTR)
+				continue;
+			if (ret == EAGAIN)
+				return EAGAIN;
+			if (ret == ECONNRESET)
+				return 0;
 		}
 
-		if (FD_ISSET(fd, &fds)) {
-			ssize_t len;
-			char msg[512];
-
-			len = read(fd, msg, sizeof(msg));
-			if (len == 0 || len == -1) {
-				if (len == -1 &&
-				    (errno == EINTR || errno == EAGAIN)) {
-					continue;
-				}
-				return -1;
-			}
+		if (FD_ISSET(outfd, &wfds)) {
+			ret = handle_write_fd(outfd, &out);
+			if (ret < 0)
+				return ret;
+		}
 
-			if (!write_sync(STDOUT_FILENO, msg, len)) {
-				perror("write() failed");
-				return -1;
-			}
+		if (FD_ISSET(conspty, &wfds)) {
+			ret = handle_write_fd(conspty, &console);
+			if (ret < 0)
+				return ret;
 		}
 	} while (received_signal == 0);
 
 	return 0;
 }
 
-int main(int argc, char **argv)
+static int start_server(struct remote *remote)
+{
+	struct sockaddr_in sa;
+	struct hostent *he = NULL;
+	int ret;
+	int on;
+
+	memset(&sa, 0, sizeof(sa));
+
+	sa.sin_family = AF_INET;
+	sa.sin_port = htons(remote->port);
+
+	if (!remote->gateway) {
+		he = gethostbyname("localhost");
+		if (!he)
+			err(h_errno, "could not get localhost address\n");
+		memcpy(&sa.sin_addr, he->h_addr_list[0], he->h_length);
+	} else
+		sa.sin_addr.s_addr = htonl(INADDR_ANY);
+
+	remote->server = socket(AF_INET, SOCK_STREAM, 0);
+	if (remote->server < 0)
+		err(errno, "socket failed\n");
+
+	on = 1;
+	ret = setsockopt(remote->server, SOL_SOCKET, SO_REUSEADDR, &on,
sizeof(on));
+	if (ret)
+		err(errno, "setsockopt(SO_REUSEADDR) failed\n");
+
+	ret = bind(remote->server, (struct sockaddr *)&sa, sizeof(sa));
+	if (ret)
+		err(errno, "bind failed\n");
+
+	ret = listen(remote->server, 5);
+	if (ret)
+		err(errno, "listen failed\n");
+
+	return ret;
+}
+
+static int remote_loop(int conspty, struct remote *remote)
 {
+	int in;
+	int out;
+	int socket;
+	int ret;
+
+	ret = start_server(remote);
+	if (ret)
+		return ret;
+
+	do {
+		socket = accept(remote->server, NULL, 0);
+		if (socket < 0)
+			err(errno, "accept failed\n");
+
+		in = socket;
+		out = socket;
+	
+		ret = console_loop(conspty, in, out);
+	} while (ret == EAGAIN);
+
+	return ret;
+}
+
+static int main_loop(int conspty, struct remote *remote)
+{
+	int in;
+	int out;
+	int ret;
 	struct termios attr;
+
+	if (remote->do_listen)
+		return remote_loop(conspty, remote);
+
+	init_term(fileno(stdin), &attr);
+
+	in = fileno(stdin);
+	out = fileno(stdout);
+
+	ret = console_loop(conspty, in, out);
+
+	restore_term(in, &attr);
+
+	return ret;
+}
+
+int main(int argc, char **argv)
+{
 	int domid;
-	char *sopt = "h";
+	char *sopt = "rp:gdh";
 	int ch;
 	int opt_ind=0;
 	struct option lopt[] = {
+		{ "remote",  0, NULL, ''r'' },
+		{ "port",    1, NULL, ''p'' },
+		{ "gateway", 0, NULL, ''g'' },
+		{ "debug",   0, NULL, ''d'' },
 		{ "help",    0, 0, ''h'' },
 		{ 0 },
 
@@ -174,6 +456,13 @@ int main(int argc, char **argv)
 	struct xs_handle *xs;
 	char *end;
 	time_t now;
+	struct remote remote = {
+		.port = DEFAULT_LISTEN_PORT,
+		.do_listen = 0,
+		.gateway = 0,
+		.server = -1,
+	};
+	int ret;
 
 	while((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
 		switch(ch) {
@@ -181,6 +470,25 @@ int main(int argc, char **argv)
 			usage(argv[0]);
 			exit(0);
 			break;
+		case ''r'':
+			remote.do_listen = 1;
+			break;
+		case ''p'':
+			remote.port = strtol(optarg, &end, 10);
+			if (end && *end) {
+				fprintf(stderr, "Invalid port `%s'' specified\n",
+					optarg);
+				fprintf(stderr, "Try `%s --help'' for more "
+					"information.\n", argv[0]);
+				exit(EINVAL);
+			}
+			break;
+		case ''g'':
+			remote.gateway = 1;
+			break;
+		case ''d'':
+			debug = 1;
+			break;
 		}
 	}
 	
@@ -199,6 +507,14 @@ int main(int argc, char **argv)
 		exit(EINVAL);
 	}
 
+	if (remote.gateway && !remote.do_listen) {
+		fprintf(stderr, "setting `gateway'' requires also setting "
+			"`remote''\n");
+		fprintf(stderr, "Try `%s --help'' for more information.\n",
+			argv[0]);
+		exit(EINVAL);
+	}
+
 	xs = xs_daemon_open();
 	if (xs == NULL) {
 		err(errno, "Could not contact XenStore");
@@ -253,9 +569,9 @@ int main(int argc, char **argv)
 	free(str_pty);
 	free(path);
 
-	init_term(STDIN_FILENO, &attr);
-	console_loop(spty);
-	restore_term(STDIN_FILENO, &attr);
+	ret = main_loop(spty, &remote);
 
-	return 0;
- }
+	xs_daemon_close(xs);
+
+	return abs(ret);
+}

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Sep-26 15:50 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On 26/9/06 16:26, "Muli Ben-Yehuda" <muli@il.ibm.com> wrote:
> The attached patch makes xenconsole send and receive console messages
> over a remote connection, instead of via stdin/stdout - if given the
> --remote switch. It is useful for proxy''ing real console message
or
> other protocol messages (such as gdb) between dom0 and a remote
> host. We''re currently using it for proxying gdb between gdbstub in
a
> partition that talks gdb over the console page to a remote host
> running gdb.
> 
> Changed since last version:
> - fixed compile warning - type of ''buf'' in
handle_read_fd() should be
>   char*, not unsigned char*.
> 
> Signed-off-by: Muli Ben-Yehuda <muli@il.ibm.com>
I should have spotted this before, but I''m unsure whether the message
buffer
ring stuff is really needed. On the write (to the guest) side, we only read
from the tty_fd (or socket in your case) if there is space in the shared
console ring. On the read (from the guest) side, we already have a buffer
(see e.g., buffer_append() in console/daemon/io.c). It may not be the best
buffer code, and we may want to change it in future to support logging to
files, but it does work! So can you not just send a minimal patch to add
socket support? It should be really small (like 10s of lines).

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Muli Ben-Yehuda

2006-Sep-26 16:08 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On Tue, Sep 26, 2006 at 04:50:37PM +0100, Keir Fraser
wrote:> On 26/9/06 16:26, "Muli Ben-Yehuda" <muli@il.ibm.com>
wrote:
> 
> > The attached patch makes xenconsole send and receive console messages
> > over a remote connection, instead of via stdin/stdout - if given the
> > --remote switch. It is useful for proxy''ing real console
message or
> > other protocol messages (such as gdb) between dom0 and a remote
> > host. We''re currently using it for proxying gdb between
gdbstub in a
> > partition that talks gdb over the console page to a remote host
> > running gdb.
> > 
> > Changed since last version:
> > - fixed compile warning - type of ''buf'' in
handle_read_fd() should be
> >   char*, not unsigned char*.
> > 
> > Signed-off-by: Muli Ben-Yehuda <muli@il.ibm.com>
> 
> I should have spotted this before, but I''m unsure whether the
message buffer
> ring stuff is really needed.
It is necessary in theory, although we might be able to get around it
in practice. The theory says that an fd is not guaranteed to be
writable, so we need a place to store stuff we want to write to it
until it becomes writable. The code isn''t that complex (it''s
even
using Xen style rings now! :-)) so I don''t see what we lose by being
correct both in theory and in practice.

If you prefer I can split the buffering support into a separate
patch to make it easier to bisect should the need arise.
> On the write (to the guest) side, we only read from the tty_fd (or
> socket in your case) if there is space in the shared console
> ring. On the read (from the guest) side, we already have a buffer
> (see e.g., buffer_append() in console/daemon/io.c). It may not be
> the best buffer code, and we may want to change it in future to
> support logging to files, but it does work! So can you not just send
> a minimal patch to add socket support? It should be really small
> (like 10s of lines).
I can certainly do that - in fact, that''s what I started with. But
even the current xenconsole code suffers from the theoretical problem
mentioned above of writing to an fd without checking that it is
writable first. All it takes to exploit it is to run `xenconsole |
<socket>'' and make the system run out of memory so that the
socket is
temporarily not writable. Granted, if this happens you have bigger
problems, but why not do things right?

Cheers,
Muli


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Sep-26 17:43 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On 26/9/06 17:08, "Muli Ben-Yehuda" <muli@il.ibm.com> wrote:
> I can certainly do that - in fact, that''s what I started with. But
> even the current xenconsole code suffers from the theoretical problem
> mentioned above of writing to an fd without checking that it is
> writable first. All it takes to exploit it is to run `xenconsole |
> <socket>'' and make the system run out of memory so that the
socket is
> temporarily not writable. Granted, if this happens you have bigger
> problems, but why not do things right?
I don''t believe such a problem exists with the current console code.

We only write() to the tty_fd if it is in the set of writefds after the
select() call in handle_io(). This means it can take at least one byte of
data. We may offer the write() call more than this but, since we set it
O_NONBLOCK, it will simply bite off what it can chew right now and we keep
the rest for later.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Muli Ben-Yehuda

2006-Sep-26 18:50 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On Tue, Sep 26, 2006 at 06:43:32PM +0100, Keir Fraser
wrote:> On 26/9/06 17:08, "Muli Ben-Yehuda" <muli@il.ibm.com>
wrote:
> 
> > I can certainly do that - in fact, that''s what I started
with. But
> > even the current xenconsole code suffers from the theoretical problem
> > mentioned above of writing to an fd without checking that it is
> > writable first. All it takes to exploit it is to run `xenconsole |
> > <socket>'' and make the system run out of memory so that
the socket is
> > temporarily not writable. Granted, if this happens you have bigger
> > problems, but why not do things right?
> 
> I don''t believe such a problem exists with the current console
code.
> 
> We only write() to the tty_fd if it is in the set of writefds after the
> select() call in handle_io().
Hmm? you''re looking at the console daemon code and referring to the
pty between the console daemon and client. I''m talking about the
console client and referring the to other fd in use, which is either
stdin/stdout (tty mode) or a socket (--remote mode). Sorry if I wasn''t
clear before - the buffering on the console client side isn''t
necessary between the daemon and the client, it''s necessary between
the client and the "outside world". The code buffers both fds since
it''s simpler than special casing one of them.

Cheers,
Muli

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Sep-26 19:47 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On 26/9/06 7:50 pm, "Muli Ben-Yehuda" <muli@il.ibm.com> wrote:
>> I don''t believe such a problem exists with the current console
code.
>> 
>> We only write() to the tty_fd if it is in the set of writefds after the
>> select() call in handle_io().
> 
> Hmm? you''re looking at the console daemon code and referring to
the
> pty between the console daemon and client. I''m talking about the
> console client and referring the to other fd in use, which is either
> stdin/stdout (tty mode) or a socket (--remote mode). Sorry if I
wasn''t
> clear before - the buffering on the console client side isn''t
> necessary between the daemon and the client, it''s necessary
between
> the client and the "outside world". The code buffers both fds
since
> it''s simpler than special casing one of them.
Oops. Okay, I''ll take another look but couldn''t you simply
only read from
the source fd if you have write space in the destination fd? Ultimately
you''ll fill the ring buffer and need to apply feedback to the data
sender
anyway (by not reading from the fd)? So why have the buffer at all?

I guess I considered this patch as being to the daemon because isn''t
that
really where it would make most sense (e.g., a domain config option asking
console to be exported via a TCP port, which then causes xend to pass
appropriate options to the console daemon)?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Sep-27 09:17 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On 26/9/06 16:26, "Muli Ben-Yehuda" <muli@il.ibm.com> wrote:
> The attached patch makes xenconsole send and receive console messages
> over a remote connection, instead of via stdin/stdout - if given the
> --remote switch. It is useful for proxy''ing real console message
or
> other protocol messages (such as gdb) between dom0 and a remote
> host. We''re currently using it for proxying gdb between gdbstub in
a
> partition that talks gdb over the console page to a remote host
> running gdb.
Reconsidering this patch, I think it''s okay to put socket connection
capability into the client. *But*:
 1. It can''t go in until development reopens for 3.0.4.
 2. I think the buffering code is not quite there yet. What you want is to
make all fd''s non-blocking, have small (e.g., 1kB) buffers, and then
only
add an fd to the read-set if its buffer is not full,  and only add an fd to
the write-set if its buffer is not empty. The buffers can stay as ring
buffers, so a bunch of that code probably stays.
 3. The networking options are confusing. What are the
''remote'' and
''gateway'' options intended for?

I think that the networking and buffering-fixes parts of the patch should be
split. We can put in the buffering/select() changes first.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Muli Ben-Yehuda

2006-Sep-27 21:14 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On Wed, Sep 27, 2006 at 10:17:42AM +0100, Keir Fraser
wrote:> On 26/9/06 16:26, "Muli Ben-Yehuda" <muli@il.ibm.com>
wrote:
> 
> > The attached patch makes xenconsole send and receive console messages
> > over a remote connection, instead of via stdin/stdout - if given the
> > --remote switch. It is useful for proxy''ing real console
message or
> > other protocol messages (such as gdb) between dom0 and a remote
> > host. We''re currently using it for proxying gdb between
gdbstub in a
> > partition that talks gdb over the console page to a remote host
> > running gdb.
> 
> Reconsidering this patch, I think it''s okay to put socket
connection
> capability into the client. *But*:
>  1. It can''t go in until development reopens for 3.0.4.
Sure, that''s completely understandable.
>  2. I think the buffering code is not quite there yet. What you want is to
> make all fd''s non-blocking, have small (e.g., 1kB) buffers, and
then only
> add an fd to the read-set if its buffer is not full,  and only add an fd to
> the write-set if its buffer is not empty. The buffers can stay as ring
> buffers, so a bunch of that code probably stays.
Ok. I think it will end up being more code than what we do right now,
but if you prefer it this way, fine.
>  3. The networking options are confusing. What are the
''remote'' and
> ''gateway'' options intended for?
--remote = activate the networking.
--gateway = allow connections from any host, not just localhost

suggestions for better names will be appreciated.
> I think that the networking and buffering-fixes parts of the patch should
be
> split. We can put in the buffering/select() changes first.
Ok. I''ll implement and resubmit in the 3.0.4 timeframe, other things
require my attention for the time being.

Cheers,
Muli

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2006-Sep-28 08:42 UTC

head link

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

On 27/9/06 22:14, "Muli Ben-Yehuda" <muli@il.ibm.com> wrote:
>>  2. I think the buffering code is not quite there yet. What you want is
to
>> make all fd''s non-blocking, have small (e.g., 1kB) buffers,
and then only
>> add an fd to the read-set if its buffer is not full,  and only add an
fd to
>> the write-set if its buffer is not empty. The buffers can stay as ring
>> buffers, so a bunch of that code probably stays.
> 
> Ok. I think it will end up being more code than what we do right now,
> but if you prefer it this way, fine.
We''ll see. I think it''ll be more readable to have the two
issues addresses
in separate patches anyway.
>>  3. The networking options are confusing. What are the
''remote'' and
>> ''gateway'' options intended for?
> 
> --remote = activate the networking.
> --gateway = allow connections from any host, not just localhost
Isn''t --remote implied by --port? Is it useful to be able to select
networking yet not specify a port? I think --remote is a better name for
--gateway (i.e., you accept remote connections). The term
''gateway'' doesn''t
mean anything to me in this context.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Sep 2006 - [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts

[Xen-devel] Re: [PATCH resend] allow connecting to xenconsole from remote hosts