[Qemu-devel] [PATCH v5 0/6] Add reconnecting to client sockets

[Qemu-devel] [PATCH v5 0/6] Add reconnecting to client sockets

[minyard]

One more time, with the rework added to qemu_chr_finish_socket_connection()
that I should have seen.  It didn't generate any errors, so there was no
point in having error returns from it.

Also added an assert to qemu_chr_socket_restart_timer().

Thanks,

-corey

[Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define

[minyard]

From: Corey Minyard <cminyard@mvista.com>

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8623c70..f9d2a02 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) {
@@ -1976,7 +1978,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;
 
@@ -2918,12 +2920,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;
@@ -2936,7 +2938,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

[Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect

[minyard]

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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 118 ++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 68 insertions(+), 50 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index f9d2a02..dcfeb73 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2891,13 +2891,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;
@@ -2905,26 +2903,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" : "");
@@ -2935,7 +2921,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",
@@ -2945,25 +2930,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;
@@ -2972,13 +2943,26 @@ 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 true;
+}
+
+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);
     }
-    return chr;
+    if (fd < 0) {
+        return false;
+    }
+
+    return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
+                                             errp);
 }
 
 /*********************************************************/
@@ -3969,23 +3953,57 @@ 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->fd = -1;
+    s->listen_fd = -1;
+    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

[Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver

[minyard]

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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index dcfeb73..a671d6b 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 =
@@ -2412,6 +2443,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);
@@ -2861,6 +2896,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) {
@@ -2892,7 +2929,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;
@@ -2913,7 +2949,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:
@@ -2924,13 +2960,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,
@@ -2946,23 +2982,21 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
     return true;
 }
 
-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);
 }
 
 /*********************************************************/
@@ -3967,7 +4001,10 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
     s->fd = -1;
     s->listen_fd = -1;
     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;
@@ -3989,7 +4026,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

[Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected

[minyard]

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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 110 +++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 72 insertions(+), 38 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index a671d6b..549ebd8 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 */
 
@@ -2727,6 +2781,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);
 }
 
@@ -2798,6 +2854,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) {
@@ -2928,43 +2995,9 @@ static void tcp_chr_close(CharDriverState *chr)
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
-static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
-                                              Error **errp)
+static void qemu_chr_finish_socket_connection(CharDriverState *chr, int fd)
 {
     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;
@@ -2978,8 +3011,6 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
         s->chan = io_channel_from_socket(s->fd);
         tcp_chr_connect(chr);
     }
-
-    return true;
 }
 
 static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
@@ -2996,7 +3027,8 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
         return false;
     }
 
-    return qemu_chr_finish_socket_connection(chr, fd, errp);
+    qemu_chr_finish_socket_connection(chr, fd);
+    return true;
 }
 
 /*********************************************************/
@@ -4019,6 +4051,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

[Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[minyard]

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 | 15 +++++++----
 qemu-char.c      | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 qemu-options.hx  | 20 ++++++++++-----
 3 files changed, 96 insertions(+), 17 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 4bfaf20..148097b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2651,14 +2651,19 @@
 # @nodelay: #optional set TCP_NODELAY socket option (default: false)
 # @telnet: #optional enable telnet protocol on server
 #          sockets (default: false)
+# @reconnect: #optional For a client socket, if a socket is disconnected,
+#          then attempt a reconnect after the given number of seconds.
+#          Setting this 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 549ebd8..aa15bd3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2501,8 +2501,21 @@ 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;
+    assert(s->connected == 0);
+    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
@@ -2784,6 +2797,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)
@@ -2964,6 +2980,10 @@ static void tcp_chr_close(CharDriverState *chr)
     TCPCharDriver *s = chr->opaque;
     int i;
 
+    if (s->reconnect_timer) {
+        g_source_remove(s->reconnect_timer);
+        s->reconnect_timer = 0;
+    }
     qapi_free_SocketAddress(s->addr);
     if (s->fd >= 0) {
         remove_fd_in_watch(chr);
@@ -3013,6 +3033,18 @@ static void qemu_chr_finish_socket_connection(CharDriverState *chr, int fd)
     }
 }
 
+static void qemu_chr_socket_connected(int fd, void *opaque)
+{
+    CharDriverState *chr = opaque;
+
+    if (fd < 0) {
+        qemu_chr_socket_restart_timer(chr);
+        return;
+    }
+
+    qemu_chr_finish_socket_connection(chr, fd);
+}
+
 static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
 {
     TCPCharDriver *s = chr->opaque;
@@ -3020,7 +3052,10 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
 
     if (s->is_listen) {
         fd = socket_listen(s->addr, errp);
-    } else  {
+    } else if (s->reconnect_time) {
+        fd = socket_connect(s->addr, errp, qemu_chr_socket_connected, chr);
+        return fd >= 0;
+    } else {
         fd = socket_connect(s->addr, errp, NULL, NULL);
     }
     if (fd < 0) {
@@ -3448,6 +3483,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");
@@ -3474,6 +3510,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) {
@@ -3873,6 +3911,9 @@ QemuOptsList qemu_chardev_opts = {
             .name = "delay",
             .type = QEMU_OPT_BOOL,
         },{
+            .name = "reconnect",
+            .type = QEMU_OPT_NUMBER,
+        },{
             .name = "telnet",
             .type = QEMU_OPT_BOOL,
         },{
@@ -4016,6 +4057,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)
 {
@@ -4026,6 +4087,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));
@@ -4058,13 +4120,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..22cf3b9 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=seconds]\n"
+    "         [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (tcp)\n"
+    "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds][,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=@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=@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=@var{seconds}]
 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

Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[Paolo Bonzini]

Il 02/10/2014 18:17, minyard@acm.org ha scritto:
> +    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);
> +    }

Can you please add a follow-up patch that only prints this message once
per reconnect?  Otherwise a server that shuts down can spam logs forever.

I'll nevertheless queue the patches for 2.2 next week.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[Corey Minyard]

On 10/03/2014 05:22 PM, Paolo Bonzini wrote:
> Il 02/10/2014 18:17, minyard@acm.org ha scritto:
>> +    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);
>> +    }
> Can you please add a follow-up patch that only prints this message once
> per reconnect?  Otherwise a server that shuts down can spam logs forever.

Yes, not a problem.  I always wonder the best way to handle something
like that, but you are probably right.

> I'll nevertheless queue the patches for 2.2 next week.

Thank you,

-corey

> Thanks,
>
> Paolo

Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[Paolo Bonzini]

Il 04/10/2014 05:24, Corey Minyard ha scritto:
>> > Can you please add a follow-up patch that only prints this message once
>> > per reconnect?  Otherwise a server that shuts down can spam logs forever.
> Yes, not a problem.  I always wonder the best way to handle something
> like that, but you are probably right.

We can add QMP events for chardev open/closed/failed to connect, similar
to what is already there for VNC.  stdout/stderr is not a management
interface, so it's better to keep it minimal.

Paolo

[Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket

[minyard]

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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index aa15bd3..62af0ef 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");
@@ -2870,15 +2875,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