* [Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define
2014-09-21 23:04 [Qemu-devel] [PATCH 0/6] Add reconnect capability for client sockets minyard
@ 2014-09-21 23:04 ` minyard
2014-09-22 7:46 ` Paolo Bonzini
2014-09-21 23:04 ` [Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect minyard
` (4 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: minyard @ 2014-09-21 23:04 UTC (permalink / raw)
To: qemu-devel; +Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
From: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 2a3cb9f..39fb9e4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -84,6 +84,7 @@
#define READ_BUF_LEN 4096
#define READ_RETRIES 10
+#define CHR_MAX_FILENAME_SIZE 256
/***********************************************************/
/* character device */
@@ -989,7 +990,8 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
{
int fd_in, fd_out;
- char filename_in[256], filename_out[256];
+ char filename_in[CHR_MAX_FILENAME_SIZE];
+ char filename_out[CHR_MAX_FILENAME_SIZE];
const char *filename = opts->device;
if (filename == NULL) {
@@ -997,8 +999,8 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
return NULL;
}
- snprintf(filename_in, 256, "%s.in", filename);
- snprintf(filename_out, 256, "%s.out", filename);
+ snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
+ snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename);
TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
TFR(fd_out = qemu_open(filename_out, O_RDWR | O_BINARY));
if (fd_in < 0 || fd_out < 0) {
@@ -1968,7 +1970,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
OVERLAPPED ov;
int ret;
DWORD size;
- char openname[256];
+ char openname[CHR_MAX_FILENAME_SIZE];
s->fpipe = TRUE;
@@ -2910,12 +2912,12 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
s->write_msgfds = 0;
s->write_msgfds_num = 0;
- chr->filename = g_malloc(256);
+ chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
switch (ss.ss_family) {
#ifndef _WIN32
case AF_UNIX:
s->is_unix = 1;
- snprintf(chr->filename, 256, "unix:%s%s",
+ snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
((struct sockaddr_un *)(&ss))->sun_path,
is_listen ? ",server" : "");
break;
@@ -2928,7 +2930,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
s->do_nodelay = do_nodelay;
getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
- snprintf(chr->filename, 256, "%s:%s%s%s:%s%s",
+ snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
is_telnet ? "telnet" : "tcp",
left, host, right, serv,
is_listen ? ",server" : "");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define
2014-09-21 23:04 ` [Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define minyard
@ 2014-09-22 7:46 ` Paolo Bonzini
0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 7:46 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
Il 22/09/2014 01:04, minyard@acm.org ha scritto:
> From: Corey Minyard <cminyard@mvista.com>
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> qemu-char.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 2a3cb9f..39fb9e4 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -84,6 +84,7 @@
>
> #define READ_BUF_LEN 4096
> #define READ_RETRIES 10
> +#define CHR_MAX_FILENAME_SIZE 256
>
> /***********************************************************/
> /* character device */
> @@ -989,7 +990,8 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
> static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
> {
> int fd_in, fd_out;
> - char filename_in[256], filename_out[256];
> + char filename_in[CHR_MAX_FILENAME_SIZE];
> + char filename_out[CHR_MAX_FILENAME_SIZE];
> const char *filename = opts->device;
>
> if (filename == NULL) {
> @@ -997,8 +999,8 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
> return NULL;
> }
>
> - snprintf(filename_in, 256, "%s.in", filename);
> - snprintf(filename_out, 256, "%s.out", filename);
> + snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
> + snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename);
> TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
> TFR(fd_out = qemu_open(filename_out, O_RDWR | O_BINARY));
> if (fd_in < 0 || fd_out < 0) {
> @@ -1968,7 +1970,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
> OVERLAPPED ov;
> int ret;
> DWORD size;
> - char openname[256];
> + char openname[CHR_MAX_FILENAME_SIZE];
>
> s->fpipe = TRUE;
>
> @@ -2910,12 +2912,12 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
>
> - chr->filename = g_malloc(256);
> + chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
> switch (ss.ss_family) {
> #ifndef _WIN32
> case AF_UNIX:
> s->is_unix = 1;
> - snprintf(chr->filename, 256, "unix:%s%s",
> + snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
> ((struct sockaddr_un *)(&ss))->sun_path,
> is_listen ? ",server" : "");
> break;
> @@ -2928,7 +2930,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
> s->do_nodelay = do_nodelay;
> getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
> serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
> - snprintf(chr->filename, 256, "%s:%s%s%s:%s%s",
> + snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
> is_telnet ? "telnet" : "tcp",
> left, host, right, serv,
> is_listen ? ",server" : "");
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect
2014-09-21 23:04 [Qemu-devel] [PATCH 0/6] Add reconnect capability for client sockets minyard
2014-09-21 23:04 ` [Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define minyard
@ 2014-09-21 23:04 ` minyard
2014-09-22 8:08 ` Paolo Bonzini
2014-09-21 23:04 ` [Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver minyard
` (3 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: minyard @ 2014-09-21 23:04 UTC (permalink / raw)
To: qemu-devel; +Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
From: Corey Minyard <cminyard@mvista.com>
Move all socket configuration to qmp_chardev_open_socket().
qemu_chr_open_socket_fd() just opens the socket. This is getting ready
for the reconnect code, which will call open_sock_fd() on a reconnect
attempt.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 123 ++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 73 insertions(+), 50 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 39fb9e4..23ec641 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2883,13 +2883,11 @@ static void tcp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
- bool is_listen, bool is_telnet,
- bool is_waitconnect,
- Error **errp)
+static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
+ bool is_listen, bool is_telnet,
+ Error **errp)
{
- CharDriverState *chr = NULL;
- TCPCharDriver *s = NULL;
+ TCPCharDriver *s = chr->opaque;
char host[NI_MAXHOST], serv[NI_MAXSERV];
const char *left = "", *right = "";
struct sockaddr_storage ss;
@@ -2897,26 +2895,14 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
memset(&ss, 0, ss_len);
if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
+ closesocket(fd);
error_setg_errno(errp, errno, "getsockname");
- return NULL;
+ return false;
}
- chr = qemu_chr_alloc();
- s = g_malloc0(sizeof(TCPCharDriver));
-
- s->connected = 0;
- s->fd = -1;
- s->listen_fd = -1;
- s->read_msgfds = 0;
- s->read_msgfds_num = 0;
- s->write_msgfds = 0;
- s->write_msgfds_num = 0;
-
- chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
switch (ss.ss_family) {
#ifndef _WIN32
case AF_UNIX:
- s->is_unix = 1;
snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
((struct sockaddr_un *)(&ss))->sun_path,
is_listen ? ",server" : "");
@@ -2927,7 +2913,6 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
right = "]";
/* fall through */
case AF_INET:
- s->do_nodelay = do_nodelay;
getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
@@ -2937,25 +2922,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
break;
}
- chr->opaque = s;
- chr->chr_write = tcp_chr_write;
- chr->chr_sync_read = tcp_chr_sync_read;
- chr->chr_close = tcp_chr_close;
- chr->get_msgfds = tcp_get_msgfds;
- chr->set_msgfds = tcp_set_msgfds;
- chr->chr_add_client = tcp_chr_add_client;
- chr->chr_add_watch = tcp_chr_add_watch;
- chr->chr_update_read_handler = tcp_chr_update_read_handler;
- /* be isn't opened until we get a connection */
- chr->explicit_be_open = true;
-
if (is_listen) {
s->listen_fd = fd;
s->listen_chan = io_channel_from_socket(s->listen_fd);
- s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, tcp_chr_accept, chr);
- if (is_telnet) {
- s->do_telnetopt = 1;
- }
+ s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN,
+ tcp_chr_accept, chr);
} else {
s->connected = 1;
s->fd = fd;
@@ -2964,15 +2935,28 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
tcp_chr_connect(chr);
}
- if (is_listen && is_waitconnect) {
- fprintf(stderr, "QEMU waiting for connection on: %s\n",
- chr->filename);
- tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
- qemu_set_nonblock(s->listen_fd);
- }
return chr;
}
+static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress *addr,
+ bool is_listen, bool is_telnet,
+ Error **errp)
+{
+ int fd;
+
+ if (is_listen) {
+ fd = socket_listen(addr, errp);
+ } else {
+ fd = socket_connect(addr, errp, NULL, NULL);
+ }
+ if (fd < 0) {
+ return false;
+ }
+
+ return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
+ errp);
+}
+
/*********************************************************/
/* Ring buffer chardev */
@@ -3961,23 +3945,62 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
Error **errp)
{
+ CharDriverState *chr;
+ TCPCharDriver *s;
SocketAddress *addr = sock->addr;
bool do_nodelay = sock->has_nodelay ? sock->nodelay : false;
bool is_listen = sock->has_server ? sock->server : true;
bool is_telnet = sock->has_telnet ? sock->telnet : false;
bool is_waitconnect = sock->has_wait ? sock->wait : false;
- int fd;
+
+ chr = qemu_chr_alloc();
+ s = g_malloc0(sizeof(TCPCharDriver));
+
+ s->connected = 0;
+ s->fd = -1;
+ s->listen_fd = -1;
+ s->read_msgfds = 0;
+ s->read_msgfds_num = 0;
+ s->write_msgfds = 0;
+ s->write_msgfds_num = 0;
+ s->is_unix = addr->kind == SOCKET_ADDRESS_KIND_UNIX;
+ s->do_nodelay = do_nodelay;
+
+ chr->opaque = s;
+ chr->chr_write = tcp_chr_write;
+ chr->chr_sync_read = tcp_chr_sync_read;
+ chr->chr_close = tcp_chr_close;
+ chr->get_msgfds = tcp_get_msgfds;
+ chr->set_msgfds = tcp_set_msgfds;
+ chr->chr_add_client = tcp_chr_add_client;
+ chr->chr_add_watch = tcp_chr_add_watch;
+ chr->chr_update_read_handler = tcp_chr_update_read_handler;
+ /* be isn't opened until we get a connection */
+ chr->explicit_be_open = true;
+
+ chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
if (is_listen) {
- fd = socket_listen(addr, errp);
- } else {
- fd = socket_connect(addr, errp, NULL, NULL);
+ if (is_telnet) {
+ s->do_telnetopt = 1;
+ }
}
- if (fd < 0) {
+
+ if (!qemu_chr_open_socket_fd(chr, addr, is_listen, is_telnet, errp)) {
+ g_free(s);
+ g_free(chr->filename);
+ g_free(chr);
return NULL;
}
- return qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
- is_telnet, is_waitconnect, errp);
+
+ if (is_listen && is_waitconnect) {
+ fprintf(stderr, "QEMU waiting for connection on: %s\n",
+ chr->filename);
+ tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
+ qemu_set_nonblock(s->listen_fd);
+ }
+
+ return chr;
}
static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect
2014-09-21 23:04 ` [Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect minyard
@ 2014-09-22 8:08 ` Paolo Bonzini
0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 8:08 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
Il 22/09/2014 01:04, minyard@acm.org ha scritto:
> From: Corey Minyard <cminyard@mvista.com>
>
> Move all socket configuration to qmp_chardev_open_socket().
> qemu_chr_open_socket_fd() just opens the socket. This is getting ready
> for the reconnect code, which will call open_sock_fd() on a reconnect
> attempt.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> qemu-char.c | 123 ++++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 73 insertions(+), 50 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 39fb9e4..23ec641 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2883,13 +2883,11 @@ static void tcp_chr_close(CharDriverState *chr)
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
>
> -static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
> - bool is_listen, bool is_telnet,
> - bool is_waitconnect,
> - Error **errp)
> +static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
> + bool is_listen, bool is_telnet,
> + Error **errp)
> {
> - CharDriverState *chr = NULL;
> - TCPCharDriver *s = NULL;
> + TCPCharDriver *s = chr->opaque;
> char host[NI_MAXHOST], serv[NI_MAXSERV];
> const char *left = "", *right = "";
> struct sockaddr_storage ss;
> @@ -2897,26 +2895,14 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
>
> memset(&ss, 0, ss_len);
> if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
> + closesocket(fd);
> error_setg_errno(errp, errno, "getsockname");
> - return NULL;
> + return false;
> }
>
> - chr = qemu_chr_alloc();
> - s = g_malloc0(sizeof(TCPCharDriver));
> -
> - s->connected = 0;
> - s->fd = -1;
> - s->listen_fd = -1;
> - s->read_msgfds = 0;
> - s->read_msgfds_num = 0;
> - s->write_msgfds = 0;
> - s->write_msgfds_num = 0;
> -
> - chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
> switch (ss.ss_family) {
> #ifndef _WIN32
> case AF_UNIX:
> - s->is_unix = 1;
> snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
> ((struct sockaddr_un *)(&ss))->sun_path,
> is_listen ? ",server" : "");
> @@ -2927,7 +2913,6 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
> right = "]";
> /* fall through */
> case AF_INET:
> - s->do_nodelay = do_nodelay;
> getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
> serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
> snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
> @@ -2937,25 +2922,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
> break;
> }
>
> - chr->opaque = s;
> - chr->chr_write = tcp_chr_write;
> - chr->chr_sync_read = tcp_chr_sync_read;
> - chr->chr_close = tcp_chr_close;
> - chr->get_msgfds = tcp_get_msgfds;
> - chr->set_msgfds = tcp_set_msgfds;
> - chr->chr_add_client = tcp_chr_add_client;
> - chr->chr_add_watch = tcp_chr_add_watch;
> - chr->chr_update_read_handler = tcp_chr_update_read_handler;
> - /* be isn't opened until we get a connection */
> - chr->explicit_be_open = true;
> -
> if (is_listen) {
> s->listen_fd = fd;
> s->listen_chan = io_channel_from_socket(s->listen_fd);
> - s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, tcp_chr_accept, chr);
> - if (is_telnet) {
> - s->do_telnetopt = 1;
> - }
> + s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN,
> + tcp_chr_accept, chr);
> } else {
> s->connected = 1;
> s->fd = fd;
> @@ -2964,15 +2935,28 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
> tcp_chr_connect(chr);
> }
>
> - if (is_listen && is_waitconnect) {
> - fprintf(stderr, "QEMU waiting for connection on: %s\n",
> - chr->filename);
> - tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
> - qemu_set_nonblock(s->listen_fd);
> - }
> return chr;
> }
>
> +static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress *addr,
> + bool is_listen, bool is_telnet,
> + Error **errp)
> +{
> + int fd;
> +
> + if (is_listen) {
> + fd = socket_listen(addr, errp);
> + } else {
> + fd = socket_connect(addr, errp, NULL, NULL);
> + }
> + if (fd < 0) {
> + return false;
> + }
> +
> + return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
> + errp);
> +}
> +
> /*********************************************************/
> /* Ring buffer chardev */
>
> @@ -3961,23 +3945,62 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
> static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
> Error **errp)
> {
> + CharDriverState *chr;
> + TCPCharDriver *s;
> SocketAddress *addr = sock->addr;
> bool do_nodelay = sock->has_nodelay ? sock->nodelay : false;
> bool is_listen = sock->has_server ? sock->server : true;
> bool is_telnet = sock->has_telnet ? sock->telnet : false;
> bool is_waitconnect = sock->has_wait ? sock->wait : false;
> - int fd;
> +
> + chr = qemu_chr_alloc();
> + s = g_malloc0(sizeof(TCPCharDriver));
> +
> + s->connected = 0;
> + s->fd = -1;
> + s->listen_fd = -1;
> + s->read_msgfds = 0;
> + s->read_msgfds_num = 0;
> + s->write_msgfds = 0;
> + s->write_msgfds_num = 0;
> + s->is_unix = addr->kind == SOCKET_ADDRESS_KIND_UNIX;
> + s->do_nodelay = do_nodelay;
Initialization to zero not needed after g_malloc0.
With this change,
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> +
> + chr->opaque = s;
> + chr->chr_write = tcp_chr_write;
> + chr->chr_sync_read = tcp_chr_sync_read;
> + chr->chr_close = tcp_chr_close;
> + chr->get_msgfds = tcp_get_msgfds;
> + chr->set_msgfds = tcp_set_msgfds;
> + chr->chr_add_client = tcp_chr_add_client;
> + chr->chr_add_watch = tcp_chr_add_watch;
> + chr->chr_update_read_handler = tcp_chr_update_read_handler;
> + /* be isn't opened until we get a connection */
> + chr->explicit_be_open = true;
> +
> + chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
>
> if (is_listen) {
> - fd = socket_listen(addr, errp);
> - } else {
> - fd = socket_connect(addr, errp, NULL, NULL);
> + if (is_telnet) {
> + s->do_telnetopt = 1;
> + }
> }
> - if (fd < 0) {
> +
> + if (!qemu_chr_open_socket_fd(chr, addr, is_listen, is_telnet, errp)) {
> + g_free(s);
> + g_free(chr->filename);
> + g_free(chr);
> return NULL;
> }
> - return qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
> - is_telnet, is_waitconnect, errp);
> +
> + if (is_listen && is_waitconnect) {
> + fprintf(stderr, "QEMU waiting for connection on: %s\n",
> + chr->filename);
> + tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
> + qemu_set_nonblock(s->listen_fd);
> + }
> +
> + return chr;
> }
>
> static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver
2014-09-21 23:04 [Qemu-devel] [PATCH 0/6] Add reconnect capability for client sockets minyard
2014-09-21 23:04 ` [Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define minyard
2014-09-21 23:04 ` [Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect minyard
@ 2014-09-21 23:04 ` minyard
2014-09-22 7:46 ` Paolo Bonzini
2014-09-21 23:04 ` [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected minyard
` (2 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: minyard @ 2014-09-21 23:04 UTC (permalink / raw)
To: qemu-devel; +Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
From: Corey Minyard <cminyard@mvista.com>
This keeps them from having to be passed around and makes them
available for later functions, like printing and reconnecting.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 51 insertions(+), 14 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 23ec641..e59321d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -28,6 +28,9 @@
#include "sysemu/char.h"
#include "hw/usb.h"
#include "qmp-commands.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi-visit.h"
#include <unistd.h>
#include <fcntl.h>
@@ -87,6 +90,34 @@
#define CHR_MAX_FILENAME_SIZE 256
/***********************************************************/
+/* Socket address helpers */
+static void qapi_copy_SocketAddress(SocketAddress **p_dest,
+ SocketAddress *src)
+{
+ QmpOutputVisitor *qov;
+ QmpInputVisitor *qiv;
+ Visitor *ov, *iv;
+ QObject *obj;
+
+ *p_dest = NULL;
+
+ qov = qmp_output_visitor_new();
+ ov = qmp_output_get_visitor(qov);
+ visit_type_SocketAddress(ov, &src, NULL, &error_abort);
+ obj = qmp_output_get_qobject(qov);
+ qmp_output_visitor_cleanup(qov);
+ if (!obj) {
+ return;
+ }
+
+ qiv = qmp_input_visitor_new(obj);
+ iv = qmp_input_get_visitor(qiv);
+ visit_type_SocketAddress(iv, p_dest, NULL, &error_abort);
+ qmp_input_visitor_cleanup(qiv);
+ qobject_decref(obj);
+}
+
+/***********************************************************/
/* character device */
static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
@@ -2404,6 +2435,10 @@ typedef struct {
int read_msgfds_num;
int *write_msgfds;
int write_msgfds_num;
+
+ SocketAddress *addr;
+ bool is_listen;
+ bool is_telnet;
} TCPCharDriver;
static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque);
@@ -2853,6 +2888,8 @@ static void tcp_chr_close(CharDriverState *chr)
{
TCPCharDriver *s = chr->opaque;
int i;
+
+ qapi_free_SocketAddress(s->addr);
if (s->fd >= 0) {
remove_fd_in_watch(chr);
if (s->chan) {
@@ -2884,7 +2921,6 @@ static void tcp_chr_close(CharDriverState *chr)
}
static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
- bool is_listen, bool is_telnet,
Error **errp)
{
TCPCharDriver *s = chr->opaque;
@@ -2905,7 +2941,7 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
case AF_UNIX:
snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
((struct sockaddr_un *)(&ss))->sun_path,
- is_listen ? ",server" : "");
+ s->is_listen ? ",server" : "");
break;
#endif
case AF_INET6:
@@ -2916,13 +2952,13 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
- is_telnet ? "telnet" : "tcp",
+ s->is_telnet ? "telnet" : "tcp",
left, host, right, serv,
- is_listen ? ",server" : "");
+ s->is_listen ? ",server" : "");
break;
}
- if (is_listen) {
+ if (s->is_listen) {
s->listen_fd = fd;
s->listen_chan = io_channel_from_socket(s->listen_fd);
s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN,
@@ -2938,23 +2974,21 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
return chr;
}
-static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress *addr,
- bool is_listen, bool is_telnet,
- Error **errp)
+static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
{
+ TCPCharDriver *s = chr->opaque;
int fd;
- if (is_listen) {
- fd = socket_listen(addr, errp);
+ if (s->is_listen) {
+ fd = socket_listen(s->addr, errp);
} else {
- fd = socket_connect(addr, errp, NULL, NULL);
+ fd = socket_connect(s->addr, errp, NULL, NULL);
}
if (fd < 0) {
return false;
}
- return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
- errp);
+ return qemu_chr_finish_socket_connection(chr, fd, errp);
}
/*********************************************************/
@@ -3964,7 +3998,10 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
s->write_msgfds = 0;
s->write_msgfds_num = 0;
s->is_unix = addr->kind == SOCKET_ADDRESS_KIND_UNIX;
+ s->is_listen = is_listen;
+ s->is_telnet = is_telnet;
s->do_nodelay = do_nodelay;
+ qapi_copy_SocketAddress(&s->addr, sock->addr);
chr->opaque = s;
chr->chr_write = tcp_chr_write;
@@ -3986,7 +4023,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
}
}
- if (!qemu_chr_open_socket_fd(chr, addr, is_listen, is_telnet, errp)) {
+ if (!qemu_chr_open_socket_fd(chr, errp)) {
g_free(s);
g_free(chr->filename);
g_free(chr);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver
2014-09-21 23:04 ` [Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver minyard
@ 2014-09-22 7:46 ` Paolo Bonzini
0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 7:46 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
Il 22/09/2014 01:04, minyard@acm.org ha scritto:
> From: Corey Minyard <cminyard@mvista.com>
>
> This keeps them from having to be passed around and makes them
> available for later functions, like printing and reconnecting.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> qemu-char.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 51 insertions(+), 14 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 23ec641..e59321d 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -28,6 +28,9 @@
> #include "sysemu/char.h"
> #include "hw/usb.h"
> #include "qmp-commands.h"
> +#include "qapi/qmp-input-visitor.h"
> +#include "qapi/qmp-output-visitor.h"
> +#include "qapi-visit.h"
>
> #include <unistd.h>
> #include <fcntl.h>
> @@ -87,6 +90,34 @@
> #define CHR_MAX_FILENAME_SIZE 256
>
> /***********************************************************/
> +/* Socket address helpers */
> +static void qapi_copy_SocketAddress(SocketAddress **p_dest,
> + SocketAddress *src)
> +{
> + QmpOutputVisitor *qov;
> + QmpInputVisitor *qiv;
> + Visitor *ov, *iv;
> + QObject *obj;
> +
> + *p_dest = NULL;
> +
> + qov = qmp_output_visitor_new();
> + ov = qmp_output_get_visitor(qov);
> + visit_type_SocketAddress(ov, &src, NULL, &error_abort);
> + obj = qmp_output_get_qobject(qov);
> + qmp_output_visitor_cleanup(qov);
> + if (!obj) {
> + return;
> + }
> +
> + qiv = qmp_input_visitor_new(obj);
> + iv = qmp_input_get_visitor(qiv);
> + visit_type_SocketAddress(iv, p_dest, NULL, &error_abort);
> + qmp_input_visitor_cleanup(qiv);
> + qobject_decref(obj);
> +}
> +
> +/***********************************************************/
> /* character device */
>
> static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
> @@ -2404,6 +2435,10 @@ typedef struct {
> int read_msgfds_num;
> int *write_msgfds;
> int write_msgfds_num;
> +
> + SocketAddress *addr;
> + bool is_listen;
> + bool is_telnet;
> } TCPCharDriver;
>
> static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque);
> @@ -2853,6 +2888,8 @@ static void tcp_chr_close(CharDriverState *chr)
> {
> TCPCharDriver *s = chr->opaque;
> int i;
> +
> + qapi_free_SocketAddress(s->addr);
> if (s->fd >= 0) {
> remove_fd_in_watch(chr);
> if (s->chan) {
> @@ -2884,7 +2921,6 @@ static void tcp_chr_close(CharDriverState *chr)
> }
>
> static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
> - bool is_listen, bool is_telnet,
> Error **errp)
> {
> TCPCharDriver *s = chr->opaque;
> @@ -2905,7 +2941,7 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
> case AF_UNIX:
> snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
> ((struct sockaddr_un *)(&ss))->sun_path,
> - is_listen ? ",server" : "");
> + s->is_listen ? ",server" : "");
> break;
> #endif
> case AF_INET6:
> @@ -2916,13 +2952,13 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
> getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
> serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
> snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
> - is_telnet ? "telnet" : "tcp",
> + s->is_telnet ? "telnet" : "tcp",
> left, host, right, serv,
> - is_listen ? ",server" : "");
> + s->is_listen ? ",server" : "");
> break;
> }
>
> - if (is_listen) {
> + if (s->is_listen) {
> s->listen_fd = fd;
> s->listen_chan = io_channel_from_socket(s->listen_fd);
> s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN,
> @@ -2938,23 +2974,21 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
> return chr;
> }
>
> -static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress *addr,
> - bool is_listen, bool is_telnet,
> - Error **errp)
> +static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
> {
> + TCPCharDriver *s = chr->opaque;
> int fd;
>
> - if (is_listen) {
> - fd = socket_listen(addr, errp);
> + if (s->is_listen) {
> + fd = socket_listen(s->addr, errp);
> } else {
> - fd = socket_connect(addr, errp, NULL, NULL);
> + fd = socket_connect(s->addr, errp, NULL, NULL);
> }
> if (fd < 0) {
> return false;
> }
>
> - return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
> - errp);
> + return qemu_chr_finish_socket_connection(chr, fd, errp);
> }
>
> /*********************************************************/
> @@ -3964,7 +3998,10 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
> s->is_unix = addr->kind == SOCKET_ADDRESS_KIND_UNIX;
> + s->is_listen = is_listen;
> + s->is_telnet = is_telnet;
> s->do_nodelay = do_nodelay;
> + qapi_copy_SocketAddress(&s->addr, sock->addr);
>
> chr->opaque = s;
> chr->chr_write = tcp_chr_write;
> @@ -3986,7 +4023,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
> }
> }
>
> - if (!qemu_chr_open_socket_fd(chr, addr, is_listen, is_telnet, errp)) {
> + if (!qemu_chr_open_socket_fd(chr, errp)) {
> g_free(s);
> g_free(chr->filename);
> g_free(chr);
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
2014-09-21 23:04 [Qemu-devel] [PATCH 0/6] Add reconnect capability for client sockets minyard
` (2 preceding siblings ...)
2014-09-21 23:04 ` [Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver minyard
@ 2014-09-21 23:04 ` minyard
2014-09-22 8:06 ` Paolo Bonzini
2014-09-21 23:04 ` [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets minyard
2014-09-21 23:04 ` [Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket minyard
5 siblings, 1 reply; 25+ messages in thread
From: minyard @ 2014-09-21 23:04 UTC (permalink / raw)
To: qemu-devel; +Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
From: Corey Minyard <cminyard@mvista.com>
This way we can tell if the socket is connected or not. It also splits
the string conversions out into separate functions to make this more
convenient.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 102 ++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 69 insertions(+), 33 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index e59321d..8418e33 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -117,6 +117,60 @@ static void qapi_copy_SocketAddress(SocketAddress **p_dest,
qobject_decref(obj);
}
+static int SocketAddress_to_str(char *dest, int max_len,
+ const char *prefix, SocketAddress *addr,
+ bool is_listen, bool is_telnet)
+{
+ switch (addr->kind) {
+ case SOCKET_ADDRESS_KIND_INET:
+ return snprintf(dest, max_len, "%s%s:%s:%s%s", prefix,
+ is_telnet ? "telnet" : "tcp", addr->inet->host,
+ addr->inet->port, is_listen ? ",server" : "");
+ break;
+ case SOCKET_ADDRESS_KIND_UNIX:
+ return snprintf(dest, max_len, "%sunix:%s%s", prefix,
+ addr->q_unix->path, is_listen ? ",server" : "");
+ break;
+ case SOCKET_ADDRESS_KIND_FD:
+ return snprintf(dest, max_len, "%sfd:%s%s", prefix, addr->fd->str,
+ is_listen ? ",server" : "");
+ break;
+ default:
+ abort();
+ }
+}
+
+static int sockaddr_to_str(char *dest, int max_len,
+ struct sockaddr_storage *ss, socklen_t ss_len,
+ bool is_listen, bool is_telnet)
+{
+ char host[NI_MAXHOST], serv[NI_MAXSERV];
+ const char *left = "", *right = "";
+
+ switch (ss->ss_family) {
+#ifndef _WIN32
+ case AF_UNIX:
+ return snprintf(dest, max_len, "unix:%s%s",
+ ((struct sockaddr_un *)(ss))->sun_path,
+ is_listen ? ",server" : "");
+#endif
+ case AF_INET6:
+ left = "[";
+ right = "]";
+ /* fall through */
+ case AF_INET:
+ getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
+ serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
+ return snprintf(dest, max_len, "%s:%s%s%s:%s%s",
+ is_telnet ? "telnet" : "tcp",
+ left, host, right, serv,
+ is_listen ? ",server" : "");
+
+ default:
+ return snprintf(dest, max_len, "unknown");
+ }
+}
+
/***********************************************************/
/* character device */
@@ -2719,6 +2773,8 @@ static void tcp_chr_disconnect(CharDriverState *chr)
s->chan = NULL;
closesocket(s->fd);
s->fd = -1;
+ SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
+ "disconnected:", s->addr, s->is_listen, s->is_telnet);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
@@ -2790,6 +2846,17 @@ static void tcp_chr_connect(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr->opaque;
+ struct sockaddr_storage ss;
+ socklen_t ss_len = sizeof(ss);
+
+ memset(&ss, 0, ss_len);
+ if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) {
+ snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
+ "Error in getsockname: %s\n", strerror(errno));
+ } else {
+ sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len,
+ s->is_listen, s->is_telnet);
+ }
s->connected = 1;
if (s->chan) {
@@ -2924,39 +2991,6 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
Error **errp)
{
TCPCharDriver *s = chr->opaque;
- char host[NI_MAXHOST], serv[NI_MAXSERV];
- const char *left = "", *right = "";
- struct sockaddr_storage ss;
- socklen_t ss_len = sizeof(ss);
-
- memset(&ss, 0, ss_len);
- if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
- closesocket(fd);
- error_setg_errno(errp, errno, "getsockname");
- return false;
- }
-
- switch (ss.ss_family) {
-#ifndef _WIN32
- case AF_UNIX:
- snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
- ((struct sockaddr_un *)(&ss))->sun_path,
- s->is_listen ? ",server" : "");
- break;
-#endif
- case AF_INET6:
- left = "[";
- right = "]";
- /* fall through */
- case AF_INET:
- getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
- serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
- snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
- s->is_telnet ? "telnet" : "tcp",
- left, host, right, serv,
- s->is_listen ? ",server" : "");
- break;
- }
if (s->is_listen) {
s->listen_fd = fd;
@@ -4016,6 +4050,8 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
chr->explicit_be_open = true;
chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
+ SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, "disconnected:",
+ addr, is_listen, is_telnet);
if (is_listen) {
if (is_telnet) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
2014-09-21 23:04 ` [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected minyard
@ 2014-09-22 8:06 ` Paolo Bonzini
2014-09-22 13:15 ` Corey Minyard
0 siblings, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 8:06 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
Il 22/09/2014 01:04, minyard@acm.org ha scritto:
> From: Corey Minyard <cminyard@mvista.com>
>
> This way we can tell if the socket is connected or not. It also splits
> the string conversions out into separate functions to make this more
> convenient.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> qemu-char.c | 102 ++++++++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 69 insertions(+), 33 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index e59321d..8418e33 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -117,6 +117,60 @@ static void qapi_copy_SocketAddress(SocketAddress **p_dest,
> qobject_decref(obj);
> }
>
> +static int SocketAddress_to_str(char *dest, int max_len,
> + const char *prefix, SocketAddress *addr,
> + bool is_listen, bool is_telnet)
> +{
> + switch (addr->kind) {
> + case SOCKET_ADDRESS_KIND_INET:
> + return snprintf(dest, max_len, "%s%s:%s:%s%s", prefix,
> + is_telnet ? "telnet" : "tcp", addr->inet->host,
> + addr->inet->port, is_listen ? ",server" : "");
> + break;
> + case SOCKET_ADDRESS_KIND_UNIX:
> + return snprintf(dest, max_len, "%sunix:%s%s", prefix,
> + addr->q_unix->path, is_listen ? ",server" : "");
> + break;
> + case SOCKET_ADDRESS_KIND_FD:
> + return snprintf(dest, max_len, "%sfd:%s%s", prefix, addr->fd->str,
> + is_listen ? ",server" : "");
> + break;
> + default:
> + abort();
> + }
> +}
> +
> +static int sockaddr_to_str(char *dest, int max_len,
> + struct sockaddr_storage *ss, socklen_t ss_len,
> + bool is_listen, bool is_telnet)
> +{
> + char host[NI_MAXHOST], serv[NI_MAXSERV];
> + const char *left = "", *right = "";
> +
> + switch (ss->ss_family) {
> +#ifndef _WIN32
> + case AF_UNIX:
> + return snprintf(dest, max_len, "unix:%s%s",
> + ((struct sockaddr_un *)(ss))->sun_path,
> + is_listen ? ",server" : "");
> +#endif
> + case AF_INET6:
> + left = "[";
> + right = "]";
> + /* fall through */
> + case AF_INET:
> + getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
> + serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
> + return snprintf(dest, max_len, "%s:%s%s%s:%s%s",
> + is_telnet ? "telnet" : "tcp",
> + left, host, right, serv,
> + is_listen ? ",server" : "");
> +
> + default:
> + return snprintf(dest, max_len, "unknown");
> + }
> +}
> +
> /***********************************************************/
> /* character device */
>
> @@ -2719,6 +2773,8 @@ static void tcp_chr_disconnect(CharDriverState *chr)
> s->chan = NULL;
> closesocket(s->fd);
> s->fd = -1;
> + SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
> + "disconnected:", s->addr, s->is_listen, s->is_telnet);
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
>
> @@ -2790,6 +2846,17 @@ static void tcp_chr_connect(void *opaque)
> {
> CharDriverState *chr = opaque;
> TCPCharDriver *s = chr->opaque;
> + struct sockaddr_storage ss;
> + socklen_t ss_len = sizeof(ss);
> +
> + memset(&ss, 0, ss_len);
> + if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) {
> + snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
> + "Error in getsockname: %s\n", strerror(errno));
> + } else {
> + sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len,
> + s->is_listen, s->is_telnet);
> + }
Why move this from qemu_chr_finish_socket_connection to tcp_chr_connect?
Perhaps move this part of the patch earlier, either just before or just
after patch 2?
Except for this question, the patch looks good. Thanks,
Paolo
> s->connected = 1;
> if (s->chan) {
> @@ -2924,39 +2991,6 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
> Error **errp)
> {
> TCPCharDriver *s = chr->opaque;
> - char host[NI_MAXHOST], serv[NI_MAXSERV];
> - const char *left = "", *right = "";
> - struct sockaddr_storage ss;
> - socklen_t ss_len = sizeof(ss);
> -
> - memset(&ss, 0, ss_len);
> - if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
> - closesocket(fd);
> - error_setg_errno(errp, errno, "getsockname");
> - return false;
> - }
> -
> - switch (ss.ss_family) {
> -#ifndef _WIN32
> - case AF_UNIX:
> - snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
> - ((struct sockaddr_un *)(&ss))->sun_path,
> - s->is_listen ? ",server" : "");
> - break;
> -#endif
> - case AF_INET6:
> - left = "[";
> - right = "]";
> - /* fall through */
> - case AF_INET:
> - getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
> - serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
> - snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
> - s->is_telnet ? "telnet" : "tcp",
> - left, host, right, serv,
> - s->is_listen ? ",server" : "");
> - break;
> - }
>
> if (s->is_listen) {
> s->listen_fd = fd;
> @@ -4016,6 +4050,8 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
> chr->explicit_be_open = true;
>
> chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
> + SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, "disconnected:",
> + addr, is_listen, is_telnet);
>
> if (is_listen) {
> if (is_telnet) {
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
2014-09-22 8:06 ` Paolo Bonzini
@ 2014-09-22 13:15 ` Corey Minyard
2014-09-22 13:25 ` Paolo Bonzini
0 siblings, 1 reply; 25+ messages in thread
From: Corey Minyard @ 2014-09-22 13:15 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
On 09/22/2014 03:06 AM, Paolo Bonzini wrote:
> Il 22/09/2014 01:04, minyard@acm.org ha scritto:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> This way we can tell if the socket is connected or not. It also splits
>> the string conversions out into separate functions to make this more
>> convenient.
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>> qemu-char.c | 102 ++++++++++++++++++++++++++++++++++++++++--------------------
>> 1 file changed, 69 insertions(+), 33 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index e59321d..8418e33 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -117,6 +117,60 @@ static void qapi_copy_SocketAddress(SocketAddress **p_dest,
>> qobject_decref(obj);
>> }
>>
>> +static int SocketAddress_to_str(char *dest, int max_len,
>> + const char *prefix, SocketAddress *addr,
>> + bool is_listen, bool is_telnet)
>> +{
>> + switch (addr->kind) {
>> + case SOCKET_ADDRESS_KIND_INET:
>> + return snprintf(dest, max_len, "%s%s:%s:%s%s", prefix,
>> + is_telnet ? "telnet" : "tcp", addr->inet->host,
>> + addr->inet->port, is_listen ? ",server" : "");
>> + break;
>> + case SOCKET_ADDRESS_KIND_UNIX:
>> + return snprintf(dest, max_len, "%sunix:%s%s", prefix,
>> + addr->q_unix->path, is_listen ? ",server" : "");
>> + break;
>> + case SOCKET_ADDRESS_KIND_FD:
>> + return snprintf(dest, max_len, "%sfd:%s%s", prefix, addr->fd->str,
>> + is_listen ? ",server" : "");
>> + break;
>> + default:
>> + abort();
>> + }
>> +}
>> +
>> +static int sockaddr_to_str(char *dest, int max_len,
>> + struct sockaddr_storage *ss, socklen_t ss_len,
>> + bool is_listen, bool is_telnet)
>> +{
>> + char host[NI_MAXHOST], serv[NI_MAXSERV];
>> + const char *left = "", *right = "";
>> +
>> + switch (ss->ss_family) {
>> +#ifndef _WIN32
>> + case AF_UNIX:
>> + return snprintf(dest, max_len, "unix:%s%s",
>> + ((struct sockaddr_un *)(ss))->sun_path,
>> + is_listen ? ",server" : "");
>> +#endif
>> + case AF_INET6:
>> + left = "[";
>> + right = "]";
>> + /* fall through */
>> + case AF_INET:
>> + getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
>> + serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
>> + return snprintf(dest, max_len, "%s:%s%s%s:%s%s",
>> + is_telnet ? "telnet" : "tcp",
>> + left, host, right, serv,
>> + is_listen ? ",server" : "");
>> +
>> + default:
>> + return snprintf(dest, max_len, "unknown");
>> + }
>> +}
>> +
>> /***********************************************************/
>> /* character device */
>>
>> @@ -2719,6 +2773,8 @@ static void tcp_chr_disconnect(CharDriverState *chr)
>> s->chan = NULL;
>> closesocket(s->fd);
>> s->fd = -1;
>> + SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
>> + "disconnected:", s->addr, s->is_listen, s->is_telnet);
>> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
>> }
>>
>> @@ -2790,6 +2846,17 @@ static void tcp_chr_connect(void *opaque)
>> {
>> CharDriverState *chr = opaque;
>> TCPCharDriver *s = chr->opaque;
>> + struct sockaddr_storage ss;
>> + socklen_t ss_len = sizeof(ss);
>> +
>> + memset(&ss, 0, ss_len);
>> + if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) {
>> + snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
>> + "Error in getsockname: %s\n", strerror(errno));
>> + } else {
>> + sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len,
>> + s->is_listen, s->is_telnet);
>> + }
> Why move this from qemu_chr_finish_socket_connection to tcp_chr_connect?
> Perhaps move this part of the patch earlier, either just before or just
> after patch 2?
I had to make this change, otherwise a server socket would never show
that it reconnected. I originally hadn't moved it.
Thanks,
-corey
>
> Except for this question, the patch looks good. Thanks,
>
> Paolo
>
>> s->connected = 1;
>> if (s->chan) {
>> @@ -2924,39 +2991,6 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
>> Error **errp)
>> {
>> TCPCharDriver *s = chr->opaque;
>> - char host[NI_MAXHOST], serv[NI_MAXSERV];
>> - const char *left = "", *right = "";
>> - struct sockaddr_storage ss;
>> - socklen_t ss_len = sizeof(ss);
>> -
>> - memset(&ss, 0, ss_len);
>> - if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
>> - closesocket(fd);
>> - error_setg_errno(errp, errno, "getsockname");
>> - return false;
>> - }
>> -
>> - switch (ss.ss_family) {
>> -#ifndef _WIN32
>> - case AF_UNIX:
>> - snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
>> - ((struct sockaddr_un *)(&ss))->sun_path,
>> - s->is_listen ? ",server" : "");
>> - break;
>> -#endif
>> - case AF_INET6:
>> - left = "[";
>> - right = "]";
>> - /* fall through */
>> - case AF_INET:
>> - getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
>> - serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
>> - snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
>> - s->is_telnet ? "telnet" : "tcp",
>> - left, host, right, serv,
>> - s->is_listen ? ",server" : "");
>> - break;
>> - }
>>
>> if (s->is_listen) {
>> s->listen_fd = fd;
>> @@ -4016,6 +4050,8 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
>> chr->explicit_be_open = true;
>>
>> chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
>> + SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, "disconnected:",
>> + addr, is_listen, is_telnet);
>>
>> if (is_listen) {
>> if (is_telnet) {
>>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
2014-09-22 13:15 ` Corey Minyard
@ 2014-09-22 13:25 ` Paolo Bonzini
2014-09-22 13:30 ` Corey Minyard
0 siblings, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 13:25 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
Il 22/09/2014 15:15, Corey Minyard ha scritto:
>> > Why move this from qemu_chr_finish_socket_connection to tcp_chr_connect?
>> > Perhaps move this part of the patch earlier, either just before or just
>> > after patch 2?
> I had to make this change, otherwise a server socket would never show
> that it reconnected. I originally hadn't moved it.
Reconnected or connected? How does reconnection work with server
sockets? I guess you can then move it to patch 2.
Paolo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
2014-09-22 13:25 ` Paolo Bonzini
@ 2014-09-22 13:30 ` Corey Minyard
2014-09-22 13:39 ` Paolo Bonzini
0 siblings, 1 reply; 25+ messages in thread
From: Corey Minyard @ 2014-09-22 13:30 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
On 09/22/2014 08:25 AM, Paolo Bonzini wrote:
> Il 22/09/2014 15:15, Corey Minyard ha scritto:
>>>> Why move this from qemu_chr_finish_socket_connection to tcp_chr_connect?
>>>> Perhaps move this part of the patch earlier, either just before or just
>>>> after patch 2?
>> I had to make this change, otherwise a server socket would never show
>> that it reconnected. I originally hadn't moved it.
> Reconnected or connected? How does reconnection work with server
> sockets? I guess you can then move it to patch 2.
When a server socket disconnects, it runs through the disconnect code so it
shows "disconnected". It does not call
qemu_chr_finish_socket_connection when
it reconnects, though, so it would never show the new connection.
I can move it to patch 2, but it seems more logical to me here, since it
doesn't really
matter until this patch.
Thanks,
-corey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
2014-09-22 13:30 ` Corey Minyard
@ 2014-09-22 13:39 ` Paolo Bonzini
0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 13:39 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
Il 22/09/2014 15:30, Corey Minyard ha scritto:
>>> >> I had to make this change, otherwise a server socket would never show
>>> >> that it reconnected. I originally hadn't moved it.
>> > Reconnected or connected? How does reconnection work with server
>> > sockets? I guess you can then move it to patch 2.
> When a server socket disconnects, it runs through the disconnect code so it
> shows "disconnected". It does not call
> qemu_chr_finish_socket_connection when
> it reconnects, though, so it would never show the new connection.
Ah, I see now what you mean. Of course for a server socket it's
reconnection from a different client; it's not related to the new
functionality.
> I can move it to patch 2, but it seems more logical to me here, since it
> doesn't really
> matter until this patch.
Yes, that's fine.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets
2014-09-21 23:04 [Qemu-devel] [PATCH 0/6] Add reconnect capability for client sockets minyard
` (3 preceding siblings ...)
2014-09-21 23:04 ` [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected minyard
@ 2014-09-21 23:04 ` minyard
2014-09-22 8:02 ` Paolo Bonzini
2014-09-22 20:24 ` Eric Blake
2014-09-21 23:04 ` [Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket minyard
5 siblings, 2 replies; 25+ messages in thread
From: minyard @ 2014-09-21 23:04 UTC (permalink / raw)
To: qemu-devel; +Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
From: Corey Minyard <cminyard@mvista.com>
Adds a "reconnect" option to socket backends that gives a reconnect
timeout. This only applies to client sockets. If the other end
of a socket closes the connection, qemu will attempt to reconnect
after the given number of seconds.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qapi-schema.json | 14 +++++----
qemu-char.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
qemu-options.hx | 20 ++++++++-----
3 files changed, 106 insertions(+), 17 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 689b548..79f7a07 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2648,14 +2648,18 @@
# @nodelay: #optional set TCP_NODELAY socket option (default: false)
# @telnet: #optional enable telnet protocol on server
# sockets (default: false)
+# @reconnect: #optional If not a server socket, if the socket disconnect
+# then reconnect after the given number of seconds. Setting
+# to zero disables this function. (default: 0). Since: 2.2.
#
# Since: 1.4
##
-{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
- '*server' : 'bool',
- '*wait' : 'bool',
- '*nodelay' : 'bool',
- '*telnet' : 'bool' } }
+{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
+ '*server' : 'bool',
+ '*wait' : 'bool',
+ '*nodelay' : 'bool',
+ '*telnet' : 'bool',
+ '*reconnect' : 'int' } }
##
# @ChardevUdp:
diff --git a/qemu-char.c b/qemu-char.c
index 8418e33..eefebd4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2493,8 +2493,20 @@ typedef struct {
SocketAddress *addr;
bool is_listen;
bool is_telnet;
+
+ guint reconnect_timer;
+ int64_t reconnect_time;
} TCPCharDriver;
+static gboolean socket_reconnect_timeout(gpointer opaque);
+
+static void qemu_chr_socket_restart_timer(CharDriverState *chr)
+{
+ TCPCharDriver *s = chr->opaque;
+ s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time,
+ socket_reconnect_timeout, chr);
+}
+
static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque);
#ifndef _WIN32
@@ -2776,6 +2788,9 @@ static void tcp_chr_disconnect(CharDriverState *chr)
SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
"disconnected:", s->addr, s->is_listen, s->is_telnet);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+ if (s->reconnect_time) {
+ qemu_chr_socket_restart_timer(chr);
+ }
}
static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
@@ -2956,6 +2971,9 @@ static void tcp_chr_close(CharDriverState *chr)
TCPCharDriver *s = chr->opaque;
int i;
+ if (s->reconnect_timer) {
+ g_source_remove(s->reconnect_timer);
+ }
qapi_free_SocketAddress(s->addr);
if (s->fd >= 0) {
remove_fd_in_watch(chr);
@@ -3005,7 +3023,28 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
tcp_chr_connect(chr);
}
- return chr;
+ return true;
+}
+
+static void qemu_chr_socket_connected(int fd, void *opaque)
+{
+ CharDriverState *chr = opaque;
+ TCPCharDriver *s = chr->opaque;
+ Error *err = NULL;
+
+ if (fd >= 0) {
+ if (qemu_chr_finish_socket_connection(chr, fd, &err)) {
+ return;
+ }
+ if (err) {
+ error_report("%s", error_get_pretty(err));
+ error_free(err);
+ }
+ closesocket(fd);
+ }
+
+ s->connected = 0;
+ qemu_chr_socket_restart_timer(chr);
}
static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
@@ -3013,6 +3052,11 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
TCPCharDriver *s = chr->opaque;
int fd;
+ if (s->reconnect_time) {
+ fd = socket_connect(s->addr, errp, qemu_chr_socket_connected, chr);
+ return (fd >= 0);
+ }
+
if (s->is_listen) {
fd = socket_listen(s->addr, errp);
} else {
@@ -3442,6 +3486,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
bool is_telnet = qemu_opt_get_bool(opts, "telnet", false);
bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true);
+ int64_t reconnect = qemu_opt_get_number(opts, "reconnect", 0);
const char *path = qemu_opt_get(opts, "path");
const char *host = qemu_opt_get(opts, "host");
const char *port = qemu_opt_get(opts, "port");
@@ -3468,6 +3513,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
backend->socket->telnet = is_telnet;
backend->socket->has_wait = true;
backend->socket->wait = is_waitconnect;
+ backend->socket->has_reconnect = true;
+ backend->socket->reconnect = reconnect;
addr = g_new0(SocketAddress, 1);
if (path) {
@@ -3867,6 +3914,9 @@ QemuOptsList qemu_chardev_opts = {
.name = "delay",
.type = QEMU_OPT_BOOL,
},{
+ .name = "reconnect",
+ .type = QEMU_OPT_NUMBER,
+ },{
.name = "telnet",
.type = QEMU_OPT_BOOL,
},{
@@ -4010,6 +4060,26 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
#endif /* WIN32 */
+static gboolean socket_reconnect_timeout(gpointer opaque)
+{
+ CharDriverState *chr = opaque;
+ TCPCharDriver *s = chr->opaque;
+ Error *err;
+
+ s->reconnect_timer = 0;
+
+ if (chr->be_open) {
+ return false;
+ }
+
+ if (!qemu_chr_open_socket_fd(chr, &err)) {
+ error_report("Unable to connect to char device %s\n", chr->label);
+ qemu_chr_socket_restart_timer(chr);
+ }
+
+ return false;
+}
+
static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
Error **errp)
{
@@ -4020,6 +4090,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
bool is_listen = sock->has_server ? sock->server : true;
bool is_telnet = sock->has_telnet ? sock->telnet : false;
bool is_waitconnect = sock->has_wait ? sock->wait : false;
+ int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0;
chr = qemu_chr_alloc();
s = g_malloc0(sizeof(TCPCharDriver));
@@ -4036,6 +4107,8 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
s->is_telnet = is_telnet;
s->do_nodelay = do_nodelay;
qapi_copy_SocketAddress(&s->addr, sock->addr);
+ s->reconnect_timer = 0;
+ s->reconnect_time = 0;
chr->opaque = s;
chr->chr_write = tcp_chr_write;
@@ -4057,13 +4130,19 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
if (is_telnet) {
s->do_telnetopt = 1;
}
+ } else if (reconnect > 0) {
+ s->reconnect_time = reconnect;
}
if (!qemu_chr_open_socket_fd(chr, errp)) {
- g_free(s);
- g_free(chr->filename);
- g_free(chr);
- return NULL;
+ if (s->reconnect_time) {
+ qemu_chr_socket_restart_timer(chr);
+ } else {
+ g_free(s);
+ g_free(chr->filename);
+ g_free(chr);
+ return NULL;
+ }
}
if (is_listen && is_waitconnect) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 365b56c..0cb856f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1930,9 +1930,9 @@ ETEXI
DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
"-chardev null,id=id[,mux=on|off]\n"
- "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay]\n"
- " [,server][,nowait][,telnet][,mux=on|off] (tcp)\n"
- "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off] (unix)\n"
+ "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=reconnect_time]\n"
+ " [,server][,nowait][,telnet][,reconnect=reconnect_time][,mux=on|off] (tcp)\n"
+ "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=reconnect_time][,mux=on|off] (unix)\n"
"-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
" [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
"-chardev msmouse,id=id[,mux=on|off]\n"
@@ -2004,7 +2004,7 @@ Options to each backend are described below.
A void device. This device will not emit any data, and will drop any data it
receives. The null backend does not take any options.
-@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet]
+@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet] [,reconnect=reconnect_time]
Create a two-way stream socket, which can be either a TCP or a unix socket. A
unix socket will be created if @option{path} is specified. Behaviour is
@@ -2018,6 +2018,10 @@ connect to a listening socket.
@option{telnet} specifies that traffic on the socket should interpret telnet
escape sequences.
+@option{reconnect} sets the timeout for reconnecting on non-server sockets when
+the remote end goes away. qemu will delay this many seconds and then attempt
+to reconnect. Zero disables reconnecting, and is the default.
+
TCP and unix socket options are given below:
@table @option
@@ -2687,14 +2691,16 @@ telnet on port 5555 to access the QEMU port.
localhost 5555
@end table
-@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay]
+@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay][,reconnect=reconnect_time]
The TCP Net Console has two modes of operation. It can send the serial
I/O to a location or wait for a connection from a location. By default
the TCP Net Console is sent to @var{host} at the @var{port}. If you use
the @var{server} option QEMU will wait for a client socket application
to connect to the port before continuing, unless the @code{nowait}
option was specified. The @code{nodelay} option disables the Nagle buffering
-algorithm. If @var{host} is omitted, 0.0.0.0 is assumed. Only
+algorithm. The @code{reconnect} option only applies if @var{noserver} is
+set, if the connection goes down it will attempt to reconnect at the
+given interval. If @var{host} is omitted, 0.0.0.0 is assumed. Only
one TCP connection at a time is accepted. You can use @code{telnet} to
connect to the corresponding character device.
@table @code
@@ -2715,7 +2721,7 @@ MAGIC_SYSRQ sequence if you use a telnet that supports sending the break
sequence. Typically in unix telnet you do it with Control-] and then
type "send break" followed by pressing the enter key.
-@item unix:@var{path}[,server][,nowait]
+@item unix:@var{path}[,server][,nowait][,reconnect=reconnect_time]
A unix domain socket is used instead of a tcp socket. The option works the
same as if you had specified @code{-serial tcp} except the unix domain socket
@var{path} is used for connections.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets
2014-09-21 23:04 ` [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets minyard
@ 2014-09-22 8:02 ` Paolo Bonzini
2014-09-22 20:24 ` Eric Blake
1 sibling, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 8:02 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
Il 22/09/2014 01:04, minyard@acm.org ha scritto:
> From: Corey Minyard <cminyard@mvista.com>
>
> Adds a "reconnect" option to socket backends that gives a reconnect
> timeout. This only applies to client sockets. If the other end
> of a socket closes the connection, qemu will attempt to reconnect
> after the given number of seconds.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
Comments inline.
> ---
> qapi-schema.json | 14 +++++----
> qemu-char.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
> qemu-options.hx | 20 ++++++++-----
> 3 files changed, 106 insertions(+), 17 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 689b548..79f7a07 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2648,14 +2648,18 @@
> # @nodelay: #optional set TCP_NODELAY socket option (default: false)
> # @telnet: #optional enable telnet protocol on server
> # sockets (default: false)
> +# @reconnect: #optional If not a server socket, if the socket disconnect
> +# then reconnect after the given number of seconds. Setting
> +# to zero disables this function. (default: 0). Since: 2.2.
> #
> # Since: 1.4
> ##
> -{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
> - '*server' : 'bool',
> - '*wait' : 'bool',
> - '*nodelay' : 'bool',
> - '*telnet' : 'bool' } }
> +{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
> + '*server' : 'bool',
> + '*wait' : 'bool',
> + '*nodelay' : 'bool',
> + '*telnet' : 'bool',
> + '*reconnect' : 'int' } }
>
> ##
> # @ChardevUdp:
> diff --git a/qemu-char.c b/qemu-char.c
> index 8418e33..eefebd4 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2493,8 +2493,20 @@ typedef struct {
> SocketAddress *addr;
> bool is_listen;
> bool is_telnet;
> +
> + guint reconnect_timer;
> + int64_t reconnect_time;
> } TCPCharDriver;
>
> +static gboolean socket_reconnect_timeout(gpointer opaque);
> +
> +static void qemu_chr_socket_restart_timer(CharDriverState *chr)
> +{
> + TCPCharDriver *s = chr->opaque;
> + s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time,
> + socket_reconnect_timeout, chr);
> +}
> +
> static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque);
>
> #ifndef _WIN32
> @@ -2776,6 +2788,9 @@ static void tcp_chr_disconnect(CharDriverState *chr)
> SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
> "disconnected:", s->addr, s->is_listen, s->is_telnet);
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> + if (s->reconnect_time) {
> + qemu_chr_socket_restart_timer(chr);
> + }
> }
>
> static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
> @@ -2956,6 +2971,9 @@ static void tcp_chr_close(CharDriverState *chr)
> TCPCharDriver *s = chr->opaque;
> int i;
>
> + if (s->reconnect_timer) {
> + g_source_remove(s->reconnect_timer);
Please add "s->reconnect_timer = 0;" here for easier debugging.
> + }
> qapi_free_SocketAddress(s->addr);
> if (s->fd >= 0) {
> remove_fd_in_watch(chr);
> @@ -3005,7 +3023,28 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
> tcp_chr_connect(chr);
> }
>
> - return chr;
> + return true;
> +}
> +
> +static void qemu_chr_socket_connected(int fd, void *opaque)
> +{
> + CharDriverState *chr = opaque;
> + TCPCharDriver *s = chr->opaque;
> + Error *err = NULL;
> +
> + if (fd >= 0) {
> + if (qemu_chr_finish_socket_connection(chr, fd, &err)) {
> + return;
> + }
> + if (err) {
> + error_report("%s", error_get_pretty(err));
> + error_free(err);
> + }
> + closesocket(fd);
> + }
> +
> + s->connected = 0;
> + qemu_chr_socket_restart_timer(chr);
> }
>
> static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
> @@ -3013,6 +3052,11 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
> TCPCharDriver *s = chr->opaque;
> int fd;
>
> + if (s->reconnect_time) {
> + fd = socket_connect(s->addr, errp, qemu_chr_socket_connected, chr);
> + return (fd >= 0);
> + }
The placement of this condition looks weird...
You're missing a check somewhere that reconnect is not specified
together with listen. Please add it, and move the condition in the
"else" side of the "if" just below.
> if (s->is_listen) {
> fd = socket_listen(s->addr, errp);
> } else {
> @@ -3442,6 +3486,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
> bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
> bool is_telnet = qemu_opt_get_bool(opts, "telnet", false);
> bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true);
> + int64_t reconnect = qemu_opt_get_number(opts, "reconnect", 0);
> const char *path = qemu_opt_get(opts, "path");
> const char *host = qemu_opt_get(opts, "host");
> const char *port = qemu_opt_get(opts, "port");
> @@ -4020,6 +4090,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
> bool is_listen = sock->has_server ? sock->server : true;
> bool is_telnet = sock->has_telnet ? sock->telnet : false;
> bool is_waitconnect = sock->has_wait ? sock->wait : false;
> + int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0;
>
> chr = qemu_chr_alloc();
> s = g_malloc0(sizeof(TCPCharDriver));
> @@ -4036,6 +4107,8 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
> s->is_telnet = is_telnet;
> s->do_nodelay = do_nodelay;
> qapi_copy_SocketAddress(&s->addr, sock->addr);
> + s->reconnect_timer = 0;
> + s->reconnect_time = 0;
Not needed after g_malloc0.
>
> chr->opaque = s;
> chr->chr_write = tcp_chr_write;
> @@ -4057,13 +4130,19 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
> if (is_telnet) {
> s->do_telnetopt = 1;
> }
> + } else if (reconnect > 0) {
> + s->reconnect_time = reconnect;
> }
>
> if (!qemu_chr_open_socket_fd(chr, errp)) {
> - g_free(s);
> - g_free(chr->filename);
> - g_free(chr);
> - return NULL;
> + if (s->reconnect_time) {
> + qemu_chr_socket_restart_timer(chr);
> + } else {
> + g_free(s);
> + g_free(chr->filename);
> + g_free(chr);
> + return NULL;
> + }
> }
>
> if (is_listen && is_waitconnect) {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 365b56c..0cb856f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1930,9 +1930,9 @@ ETEXI
>
> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev null,id=id[,mux=on|off]\n"
> - "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay]\n"
> - " [,server][,nowait][,telnet][,mux=on|off] (tcp)\n"
> - "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off] (unix)\n"
> + "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=reconnect_time]\n"
s/reconnect_time/seconds/
> + " [,server][,nowait][,telnet][,reconnect=reconnect_time][,mux=on|off] (tcp)\n"
> + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=reconnect_time][,mux=on|off] (unix)\n"
s/reconnect_time/seconds/
> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
> " [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
> "-chardev msmouse,id=id[,mux=on|off]\n"
> @@ -2004,7 +2004,7 @@ Options to each backend are described below.
> A void device. This device will not emit any data, and will drop any data it
> receives. The null backend does not take any options.
>
> -@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet]
> +@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet] [,reconnect=reconnect_time]
s/reconnect_time/@var{seconds}/
> Create a two-way stream socket, which can be either a TCP or a unix socket. A
> unix socket will be created if @option{path} is specified. Behaviour is
> @@ -2018,6 +2018,10 @@ connect to a listening socket.
> @option{telnet} specifies that traffic on the socket should interpret telnet
> escape sequences.
>
> +@option{reconnect} sets the timeout for reconnecting on non-server sockets when
> +the remote end goes away. qemu will delay this many seconds and then attempt
> +to reconnect. Zero disables reconnecting, and is the default.
> +
> TCP and unix socket options are given below:
>
> @table @option
> @@ -2687,14 +2691,16 @@ telnet on port 5555 to access the QEMU port.
> localhost 5555
> @end table
>
> -@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay]
> +@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay][,reconnect=reconnect_time]
s/reconnect_time/@var{seconds}/
> The TCP Net Console has two modes of operation. It can send the serial
> I/O to a location or wait for a connection from a location. By default
> the TCP Net Console is sent to @var{host} at the @var{port}. If you use
> the @var{server} option QEMU will wait for a client socket application
> to connect to the port before continuing, unless the @code{nowait}
> option was specified. The @code{nodelay} option disables the Nagle buffering
> -algorithm. If @var{host} is omitted, 0.0.0.0 is assumed. Only
> +algorithm. The @code{reconnect} option only applies if @var{noserver} is
> +set, if the connection goes down it will attempt to reconnect at the
> +given interval. If @var{host} is omitted, 0.0.0.0 is assumed. Only
> one TCP connection at a time is accepted. You can use @code{telnet} to
> connect to the corresponding character device.
> @table @code
> @@ -2715,7 +2721,7 @@ MAGIC_SYSRQ sequence if you use a telnet that supports sending the break
> sequence. Typically in unix telnet you do it with Control-] and then
> type "send break" followed by pressing the enter key.
>
> -@item unix:@var{path}[,server][,nowait]
> +@item unix:@var{path}[,server][,nowait][,reconnect=reconnect_time]
s/reconnect_time/@var{seconds}/
Paolo
> A unix domain socket is used instead of a tcp socket. The option works the
> same as if you had specified @code{-serial tcp} except the unix domain socket
> @var{path} is used for connections.
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets
2014-09-21 23:04 ` [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets minyard
2014-09-22 8:02 ` Paolo Bonzini
@ 2014-09-22 20:24 ` Eric Blake
2014-09-22 20:36 ` Corey Minyard
1 sibling, 1 reply; 25+ messages in thread
From: Eric Blake @ 2014-09-22 20:24 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]
On 09/21/2014 05:04 PM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Adds a "reconnect" option to socket backends that gives a reconnect
> timeout. This only applies to client sockets. If the other end
> of a socket closes the connection, qemu will attempt to reconnect
> after the given number of seconds.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> qapi-schema.json | 14 +++++----
> qemu-char.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
> qemu-options.hx | 20 ++++++++-----
> 3 files changed, 106 insertions(+), 17 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 689b548..79f7a07 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2648,14 +2648,18 @@
> # @nodelay: #optional set TCP_NODELAY socket option (default: false)
> # @telnet: #optional enable telnet protocol on server
> # sockets (default: false)
> +# @reconnect: #optional If not a server socket, if the socket disconnect
Awkward. How about:
For a client socket, if a disconnect is detected,
> +# then reconnect after the given number of seconds. Setting
> +# to zero disables this function. (default: 0). Since: 2.2.
I think this is usually written "(Since 2.2)" rather than "Since: 2.2"
when it occurs in the middle of a single option.
> #
> # Since: 1.4
> ##
> -{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
> - '*server' : 'bool',
> - '*wait' : 'bool',
> - '*nodelay' : 'bool',
> - '*telnet' : 'bool' } }
> +{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
> + '*server' : 'bool',
> + '*wait' : 'bool',
> + '*nodelay' : 'bool',
> + '*telnet' : 'bool',
> + '*reconnect' : 'int' } }
Hmm, thinking aloud here. What happens if 'reconnect' is provided with a
'server':true socket? The documentation only specifies 'server':false
behavior. Should it be an error (incompatible options), or just be
silently ignored?
Going further, would it be possible to treat 'ChardevSocket' as a flat
union, where 'server' is the enum key that determines what other fields
are valid? Granted, for this to work, we'd need to teach the qapi
generator to allow a discriminator of type bool (since we can enumerate
all of its values). looking something like:
{ 'type': 'ChardevSocketBase',
'data': { 'addr': 'SocketAddress', '*nodelay': 'bool' } }
{ 'type': 'ChardevSocketServer',
'data': { '*wait': 'bool', '*telnet': 'bool' } }
{ 'type': 'ChardevSocketClient',
'data': { '*reconnect': 'int' } }
{ 'union': 'ChardevSocket', 'base': 'ChardevSocketBase',
'discriminator': 'bool',
'data': { true : 'ChardevSocketServer',
false: 'ChardevSocketClient' } }
but I don't know if it is worth the complexity for the added type safety.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets
2014-09-22 20:24 ` Eric Blake
@ 2014-09-22 20:36 ` Corey Minyard
2014-09-22 20:53 ` Eric Blake
0 siblings, 1 reply; 25+ messages in thread
From: Corey Minyard @ 2014-09-22 20:36 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
On 09/22/2014 03:24 PM, Eric Blake wrote:
> On 09/21/2014 05:04 PM, minyard@acm.org wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> Adds a "reconnect" option to socket backends that gives a reconnect
>> timeout. This only applies to client sockets. If the other end
>> of a socket closes the connection, qemu will attempt to reconnect
>> after the given number of seconds.
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>> qapi-schema.json | 14 +++++----
>> qemu-char.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>> qemu-options.hx | 20 ++++++++-----
>> 3 files changed, 106 insertions(+), 17 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 689b548..79f7a07 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2648,14 +2648,18 @@
>> # @nodelay: #optional set TCP_NODELAY socket option (default: false)
>> # @telnet: #optional enable telnet protocol on server
>> # sockets (default: false)
>> +# @reconnect: #optional If not a server socket, if the socket disconnect
> Awkward. How about:
>
> For a client socket, if a disconnect is detected,
Point taken...
>> +# then reconnect after the given number of seconds. Setting
>> +# to zero disables this function. (default: 0). Since: 2.2.
> I think this is usually written "(Since 2.2)" rather than "Since: 2.2"
> when it occurs in the middle of a single option.
Ok, I'll fix this.
>
>> #
>> # Since: 1.4
>> ##
>> -{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
>> - '*server' : 'bool',
>> - '*wait' : 'bool',
>> - '*nodelay' : 'bool',
>> - '*telnet' : 'bool' } }
>> +{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
>> + '*server' : 'bool',
>> + '*wait' : 'bool',
>> + '*nodelay' : 'bool',
>> + '*telnet' : 'bool',
>> + '*reconnect' : 'int' } }
> Hmm, thinking aloud here. What happens if 'reconnect' is provided with a
> 'server':true socket? The documentation only specifies 'server':false
> behavior. Should it be an error (incompatible options), or just be
> silently ignored?
I was going on the behavior of "telnet" and "wait", which are silently
ignored for client sockets. reconnect is silently ignored for server
sockets.
> Going further, would it be possible to treat 'ChardevSocket' as a flat
> union, where 'server' is the enum key that determines what other fields
> are valid? Granted, for this to work, we'd need to teach the qapi
> generator to allow a discriminator of type bool (since we can enumerate
> all of its values). looking something like:
>
> { 'type': 'ChardevSocketBase',
> 'data': { 'addr': 'SocketAddress', '*nodelay': 'bool' } }
> { 'type': 'ChardevSocketServer',
> 'data': { '*wait': 'bool', '*telnet': 'bool' } }
> { 'type': 'ChardevSocketClient',
> 'data': { '*reconnect': 'int' } }
> { 'union': 'ChardevSocket', 'base': 'ChardevSocketBase',
> 'discriminator': 'bool',
> 'data': { true : 'ChardevSocketServer',
> false: 'ChardevSocketClient' } }
>
> but I don't know if it is worth the complexity for the added type safety.
>
Doesn't seem terrible, but I'm not sure.
-corey
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets
2014-09-22 20:36 ` Corey Minyard
@ 2014-09-22 20:53 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2014-09-22 20:53 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]
On 09/22/2014 02:36 PM, Corey Minyard wrote:
>> Hmm, thinking aloud here. What happens if 'reconnect' is provided with a
>> 'server':true socket? The documentation only specifies 'server':false
>> behavior. Should it be an error (incompatible options), or just be
>> silently ignored?
>
> I was going on the behavior of "telnet" and "wait", which are silently
> ignored for client sockets. reconnect is silently ignored for server
> sockets.
I can live with that.
>
>> Going further, would it be possible to treat 'ChardevSocket' as a flat
>> union, where 'server' is the enum key that determines what other fields
>> are valid? Granted, for this to work, we'd need to teach the qapi
>> generator to allow a discriminator of type bool (since we can enumerate
>> all of its values). looking something like:
>>
>> { 'type': 'ChardevSocketBase',
>> 'data': { 'addr': 'SocketAddress', '*nodelay': 'bool' } }
>> { 'type': 'ChardevSocketServer',
>> 'data': { '*wait': 'bool', '*telnet': 'bool' } }
>> { 'type': 'ChardevSocketClient',
>> 'data': { '*reconnect': 'int' } }
>> { 'union': 'ChardevSocket', 'base': 'ChardevSocketBase',
>> 'discriminator': 'bool',
>> 'data': { true : 'ChardevSocketServer',
>> false: 'ChardevSocketClient' } }
Of course, this is invalid JSON. In a JSON dictionary, the left side of
any 'key':'value' pair must be a string, only the right side can be an
arbitrary JSON type. So we'd have to spell out the stringized version
'true' and 'false' as the enum keys, which is all the more special
casing to be added to the qapi generator. Lots of work for not too much
benefit.
>>
>> but I don't know if it is worth the complexity for the added type safety.
>>
>
> Doesn't seem terrible, but I'm not sure.
You're welcome to try it if you're interested, but in just typing this
email, I can already see it's not a beginner's project, so I'm not going
to insist.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket
2014-09-21 23:04 [Qemu-devel] [PATCH 0/6] Add reconnect capability for client sockets minyard
` (4 preceding siblings ...)
2014-09-21 23:04 ` [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets minyard
@ 2014-09-21 23:04 ` minyard
2014-09-22 8:02 ` Paolo Bonzini
5 siblings, 1 reply; 25+ messages in thread
From: minyard @ 2014-09-21 23:04 UTC (permalink / raw)
To: qemu-devel; +Cc: mjg59, Corey Minyard, hwd, bcketchum, mst, afaerber
From: Corey Minyard <cminyard@mvista.com>
It seems that it might be a good idea to know what is at the remote
end of a socket for tracking down issues. So add that to the
socket filename.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index eefebd4..13ef14f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -142,9 +142,11 @@ static int SocketAddress_to_str(char *dest, int max_len,
static int sockaddr_to_str(char *dest, int max_len,
struct sockaddr_storage *ss, socklen_t ss_len,
+ struct sockaddr_storage *ps, socklen_t ps_len,
bool is_listen, bool is_telnet)
{
- char host[NI_MAXHOST], serv[NI_MAXSERV];
+ char shost[NI_MAXHOST], sserv[NI_MAXSERV];
+ char phost[NI_MAXHOST], pserv[NI_MAXSERV];
const char *left = "", *right = "";
switch (ss->ss_family) {
@@ -159,12 +161,15 @@ static int sockaddr_to_str(char *dest, int max_len,
right = "]";
/* fall through */
case AF_INET:
- getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
- serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
- return snprintf(dest, max_len, "%s:%s%s%s:%s%s",
+ getnameinfo((struct sockaddr *) ss, ss_len, shost, sizeof(shost),
+ sserv, sizeof(sserv), NI_NUMERICHOST | NI_NUMERICSERV);
+ getnameinfo((struct sockaddr *) ps, ps_len, phost, sizeof(phost),
+ pserv, sizeof(pserv), NI_NUMERICHOST | NI_NUMERICSERV);
+ return snprintf(dest, max_len, "%s:%s%s%s:%s%s <-> %s%s%s:%s",
is_telnet ? "telnet" : "tcp",
- left, host, right, serv,
- is_listen ? ",server" : "");
+ left, shost, right, sserv,
+ is_listen ? ",server" : "",
+ left, phost, right, pserv);
default:
return snprintf(dest, max_len, "unknown");
@@ -2861,15 +2866,19 @@ static void tcp_chr_connect(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr->opaque;
- struct sockaddr_storage ss;
- socklen_t ss_len = sizeof(ss);
+ struct sockaddr_storage ss, ps;
+ socklen_t ss_len = sizeof(ss), ps_len = sizeof(ps);
memset(&ss, 0, ss_len);
if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) {
snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
"Error in getsockname: %s\n", strerror(errno));
+ } else if (getpeername(s->fd, (struct sockaddr *) &ps, &ps_len) != 0) {
+ snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
+ "Error in getpeername: %s\n", strerror(errno));
} else {
- sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len,
+ sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
+ &ss, ss_len, &ps, ps_len,
s->is_listen, s->is_telnet);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket
2014-09-21 23:04 ` [Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket minyard
@ 2014-09-22 8:02 ` Paolo Bonzini
0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2014-09-22 8:02 UTC (permalink / raw)
To: minyard, qemu-devel; +Cc: mjg59, mst, hwd, bcketchum, Corey Minyard, afaerber
Il 22/09/2014 01:04, minyard@acm.org ha scritto:
> From: Corey Minyard <cminyard@mvista.com>
>
> It seems that it might be a good idea to know what is at the remote
> end of a socket for tracking down issues. So add that to the
> socket filename.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> qemu-char.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index eefebd4..13ef14f 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -142,9 +142,11 @@ static int SocketAddress_to_str(char *dest, int max_len,
>
> static int sockaddr_to_str(char *dest, int max_len,
> struct sockaddr_storage *ss, socklen_t ss_len,
> + struct sockaddr_storage *ps, socklen_t ps_len,
> bool is_listen, bool is_telnet)
> {
> - char host[NI_MAXHOST], serv[NI_MAXSERV];
> + char shost[NI_MAXHOST], sserv[NI_MAXSERV];
> + char phost[NI_MAXHOST], pserv[NI_MAXSERV];
> const char *left = "", *right = "";
>
> switch (ss->ss_family) {
> @@ -159,12 +161,15 @@ static int sockaddr_to_str(char *dest, int max_len,
> right = "]";
> /* fall through */
> case AF_INET:
> - getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
> - serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
> - return snprintf(dest, max_len, "%s:%s%s%s:%s%s",
> + getnameinfo((struct sockaddr *) ss, ss_len, shost, sizeof(shost),
> + sserv, sizeof(sserv), NI_NUMERICHOST | NI_NUMERICSERV);
> + getnameinfo((struct sockaddr *) ps, ps_len, phost, sizeof(phost),
> + pserv, sizeof(pserv), NI_NUMERICHOST | NI_NUMERICSERV);
> + return snprintf(dest, max_len, "%s:%s%s%s:%s%s <-> %s%s%s:%s",
> is_telnet ? "telnet" : "tcp",
> - left, host, right, serv,
> - is_listen ? ",server" : "");
> + left, shost, right, sserv,
> + is_listen ? ",server" : "",
> + left, phost, right, pserv);
>
> default:
> return snprintf(dest, max_len, "unknown");
> @@ -2861,15 +2866,19 @@ static void tcp_chr_connect(void *opaque)
> {
> CharDriverState *chr = opaque;
> TCPCharDriver *s = chr->opaque;
> - struct sockaddr_storage ss;
> - socklen_t ss_len = sizeof(ss);
> + struct sockaddr_storage ss, ps;
> + socklen_t ss_len = sizeof(ss), ps_len = sizeof(ps);
>
> memset(&ss, 0, ss_len);
> if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) {
> snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
> "Error in getsockname: %s\n", strerror(errno));
> + } else if (getpeername(s->fd, (struct sockaddr *) &ps, &ps_len) != 0) {
> + snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
> + "Error in getpeername: %s\n", strerror(errno));
> } else {
> - sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len,
> + sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
> + &ss, ss_len, &ps, ps_len,
> s->is_listen, s->is_telnet);
> }
>
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread