[Qemu-devel] [PATCH v2 0/6] chardev: Add reconnecting to client sockets

[Qemu-devel] [PATCH v2 0/6] chardev: Add reconnecting to client sockets

[minyard]

I believe I have fixed all the issues Paulo pointed out and added
a Reviewed-by for all the patches except patch 5.  Retested and it
looks good.  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 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

[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 39fb9e4..c39b4d4 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,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 c39b4d4..be99449 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);
 }
 
 /*********************************************************/
@@ -3959,7 +3993,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;
@@ -3981,7 +4018,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 | 102 ++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 69 insertions(+), 33 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index be99449..fc4d01f 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;
@@ -4011,6 +4045,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 | 14 +++++----
 qemu-char.c      | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 qemu-options.hx  | 20 ++++++++-----
 3 files changed, 104 insertions(+), 18 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 fc4d01f..b9f2764 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,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);
@@ -3005,7 +3024,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)
@@ -3015,7 +3055,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) {
@@ -3442,6 +3485,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 +3512,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 +3913,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 +4059,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 +4089,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));
@@ -4052,13 +4122,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

[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 b9f2764..d7cf194 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

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

[Peter Maydell]

On 22 September 2014 09:59,  <minyard@acm.org> wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Wouldn't it be better to just drop the max-size restriction
by using g_strdup_printf (and then free when done) rather than
snprintf into a fixed size buffer?

thanks
-- PMM

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

[Corey Minyard]

On 09/22/2014 12:20 PM, Peter Maydell wrote:
> On 22 September 2014 09:59,  <minyard@acm.org> wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Wouldn't it be better to just drop the max-size restriction
> by using g_strdup_printf (and then free when done) rather than
> snprintf into a fixed size buffer?
>
> thanks
> -- PMM

I'm not sure.  I was trying to minimize changes, but I did this because
the malloc and snprintf were being moved apart from each other.
This is also being used in some arrays declared on the stack, which
also might need fixing.

There may be race conditions with the filename if you free it and replace
it, but I'm not really sure.  Someone who knows better than me would
have to speak to that.

-corey

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On 09/22/2014 10:59 AM, 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      | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  qemu-options.hx  | 20 ++++++++-----
>  3 files changed, 104 insertions(+), 18 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.

Looks like I reviewed v1 after you had posted v2; my comments still
apply about this being awkward wording.


-- 
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 --]

[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 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 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

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

[Paolo Bonzini]

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>

[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>
---
 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