diff mbox

chardev: Add reconnecting to client sockets

Message ID 1411163937-19891-1-git-send-email-minyard@acm.org
State New
Headers show

Commit Message

Corey Minyard Sept. 19, 2014, 9:58 p.m. UTC
From: Corey Minyard <cminyard@mvista.com>

Adds a "recon" 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.

This rearranges things a bit, all socket configuration is moved to
qmp_chardev_open_socket() and that only gets called once at startup.
qemu_chr_open_socket_fd() is called to open or re-open the connection.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 qapi-schema.json |   6 +-
 qemu-char.c      | 209 +++++++++++++++++++++++++++++++++++++++++--------------
 qemu-options.hx  |  20 ++++--
 3 files changed, 174 insertions(+), 61 deletions(-)

Comments

Matthew Garrett Sept. 19, 2014, 10:04 p.m. UTC | #1
On Fri, Sep 19, 2014 at 04:58:57PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Adds a "recon" 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.

We're enthusiastic about this - we're using a virtio rng via egd, and 
restarting our entropy distributor currently kills hwrng in the guest 
because the socket has gone away.
Paolo Bonzini Sept. 19, 2014, 10:33 p.m. UTC | #2
Il 19/09/2014 23:58, minyard@acm.org ha scritto:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Adds a "recon" 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.
> 
> This rearranges things a bit, all socket configuration is moved to
> qmp_chardev_open_socket() and that only gets called once at startup.
> qemu_chr_open_socket_fd() is called to open or re-open the connection.

Please do this in a separate patch.

Also:

- qemu-char.c for various reasons is not using QEMU timers.  Please use
glib timers instead.

> +# @recon: #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)

Please specify "Since: 2.2" inside the comment, and give a more
descriptive name such as reconnect-interval to the option.

> +	s->addr = addr; /* Steal the address from the socket */
> +	sock->addr = NULL;

Please do a deep copy instead.  (The sock struct should really be
const).  A function like this will do it for you:

    *p_dest = NULL;

    qov = qmp_output_visitor_new();
    ov = qmp_output_get_visitor(qov);
    visit_type_SocketAddress(ov, &src, NULL, errp);
    obj = qmp_output_get_qobject(data->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);

    ...

    qapi_copy_SocketAddress(&s->addr, sock->addr, &error_abort);

(Something like this should really be automatically generated for all
types...).

I only skimmed the code, but nothing else jumped up.

Paolo
Eric Blake Sept. 19, 2014, 11:27 p.m. UTC | #3
On 09/19/2014 03:58 PM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Adds a "recon" 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.
> 
> This rearranges things a bit, all socket configuration is moved to
> qmp_chardev_open_socket() and that only gets called once at startup.
> qemu_chr_open_socket_fd() is called to open or re-open the connection.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  qapi-schema.json |   6 +-
>  qemu-char.c      | 209 +++++++++++++++++++++++++++++++++++++++++--------------
>  qemu-options.hx  |  20 ++++--
>  3 files changed, 174 insertions(+), 61 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 689b548..f42562f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2648,6 +2648,9 @@
>  # @nodelay: #optional set TCP_NODELAY socket option (default: false)
>  # @telnet: #optional enable telnet protocol on server
>  #          sockets (default: false)
> +# @recon: #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)

"recon" makes me think of spy novels, not reconnect.  Any reason you
can't name it something more descriptive, like "reconnect-time"?

Missing a "since 2.2" designation.
diff mbox

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index 689b548..f42562f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2648,6 +2648,9 @@ 
 # @nodelay: #optional set TCP_NODELAY socket option (default: false)
 # @telnet: #optional enable telnet protocol on server
 #          sockets (default: false)
+# @recon: #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: 1.4
 ##
@@ -2655,7 +2658,8 @@ 
                                      '*server'  : 'bool',
                                      '*wait'    : 'bool',
                                      '*nodelay' : 'bool',
-                                     '*telnet'  : 'bool' } }
+                                     '*telnet'  : 'bool',
+                                     '*recon'   : 'int' } }
 
 ##
 # @ChardevUdp:
diff --git a/qemu-char.c b/qemu-char.c
index 2a3cb9f..4c6b5e2 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2402,8 +2402,22 @@  typedef struct {
     int read_msgfds_num;
     int *write_msgfds;
     int write_msgfds_num;
+
+    QEMUTimer *recon_timer;
+    int64_t recon_time;
+
+    /* Only set if reconnect is enabled */
+    SocketAddress *addr;
 } TCPCharDriver;
 
+#define SOCKET_MAX_FILENAME_SIZE 256
+
+static void qemu_chr_socket_restart_timer(TCPCharDriver *s)
+{
+    timer_mod(s->recon_timer,
+	      (get_clock() + (s->recon_time * get_ticks_per_sec())));
+}
+
 static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque);
 
 #ifndef _WIN32
@@ -2683,6 +2697,9 @@  static void tcp_chr_disconnect(CharDriverState *chr)
     closesocket(s->fd);
     s->fd = -1;
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+    if (s->recon_timer) {
+	qemu_chr_socket_restart_timer(s);
+    }
 }
 
 static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
@@ -2881,13 +2898,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;
@@ -2895,27 +2910,15 @@  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(256);
     switch (ss.ss_family) {
 #ifndef _WIN32
     case AF_UNIX:
-        s->is_unix = 1;
-        snprintf(chr->filename, 256, "unix:%s%s",
+        snprintf(chr->filename, SOCKET_MAX_FILENAME_SIZE, "unix:%s%s",
                  ((struct sockaddr_un *)(&ss))->sun_path,
                  is_listen ? ",server" : "");
         break;
@@ -2925,35 +2928,20 @@  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, 256, "%s:%s%s%s:%s%s",
+        snprintf(chr->filename, SOCKET_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
                  is_telnet ? "telnet" : "tcp",
                  left, host, right, serv,
                  is_listen ? ",server" : "");
         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;
@@ -2962,13 +2950,54 @@  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 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, false, false, &err)) {
+	    return;
+	}
+	if (err) {
+	    error_report("%s", error_get_pretty(err));
+	    error_free(err);
+	}
+	closesocket(fd);
     }
-    return chr;
+
+    s->connected = 0;
+    qemu_chr_socket_restart_timer(s);
+}
+
+static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress *addr,
+				    bool do_nodelay,
+				    bool is_listen, bool is_telnet,
+				    Error **errp)
+{
+    TCPCharDriver *s = chr->opaque;
+    int fd;
+
+    if (s->recon_timer) {
+	fd = socket_connect(addr, errp, qemu_chr_socket_connected, chr);
+	return (fd >= 0);
+    }
+
+    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);
 }
 
 /*********************************************************/
@@ -3388,6 +3417,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 recon       = qemu_opt_get_number(opts, "recon", 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");
@@ -3414,6 +3444,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_recon = true;
+    backend->socket->recon = recon;
 
     addr = g_new0(SocketAddress, 1);
     if (path) {
@@ -3813,6 +3845,9 @@  QemuOptsList qemu_chardev_opts = {
             .name = "delay",
             .type = QEMU_OPT_BOOL,
         },{
+            .name = "recon",
+            .type = QEMU_OPT_NUMBER,
+        },{
             .name = "telnet",
             .type = QEMU_OPT_BOOL,
         },{
@@ -3956,26 +3991,94 @@  static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
 
 #endif /* WIN32 */
 
+static void socket_recon_timeout(void *opaque)
+{
+    CharDriverState *chr = opaque;
+    TCPCharDriver *s = chr->opaque;
+    Error *err;
+
+    if (chr->be_open) {
+        return;
+    }
+
+    if (!qemu_chr_open_socket_fd(chr, s->addr, s->do_nodelay, false, false,
+				 &err)) {
+	error_report("Unable to connect to char device %s\n", chr->label);
+	qemu_chr_socket_restart_timer(s);
+    }
+}
+
 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;
+    int64_t recon       = sock->has_recon   ? sock->recon   : 0;
+
+    chr = qemu_chr_alloc();
+    s = g_malloc0(sizeof(TCPCharDriver));
+
+    s->connected = 0;
+    s->fd = -1;
+    s->listen_fd = -1;
+    s->read_msgfds = 0;
+    s->read_msgfds_num = 0;
+    s->write_msgfds = 0;
+    s->write_msgfds_num = 0;
+    s->is_unix = addr->kind == SOCKET_ADDRESS_KIND_UNIX;
+
+    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(SOCKET_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;
+        }
+    } else if (recon > 0) {
+        s->recon_timer = timer_new(QEMU_CLOCK_REALTIME, SCALE_NS,
+				   socket_recon_timeout, chr);
+	s->recon_time = recon;
+	s->do_nodelay = do_nodelay;
+	s->addr = addr; /* Steal the address from the socket */
+	sock->addr = NULL;
+    }
+
+    if (!qemu_chr_open_socket_fd(chr, addr, do_nodelay, is_listen,
+				 is_telnet, errp)) {
+	if (s->recon_timer) {
+	    qemu_chr_socket_restart_timer(s);
+	} else {
+	    g_free(s);
+	    g_free(chr->filename);
+	    g_free(chr);
+	    return NULL;
+	}
     }
-    if (fd < 0) {
-        return NULL;
+
+    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 qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
-                                   is_telnet, is_waitconnect, errp);
+
+    return chr;
 }
 
 static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
diff --git a/qemu-options.hx b/qemu-options.hx
index 365b56c..619eacd 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][,recon=recon_time]\n"
+    "         [,server][,nowait][,telnet][,recon=recon_time][,mux=on|off] (tcp)\n"
+    "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,recon=recon_time][,mux=on|off] (unix)\n"
     "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
     "         [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
     "-chardev msmouse,id=id[,mux=on|off]\n"
@@ -2004,7 +2004,7 @@  Options to each backend are described below.
 A void device. This device will not emit any data, and will drop any data it
 receives. The null backend does not take any options.
 
-@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet]
+@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet] [,recon=recon_time]
 
 Create a two-way stream socket, which can be either a TCP or a unix socket. A
 unix socket will be created if @option{path} is specified. Behaviour is
@@ -2018,6 +2018,10 @@  connect to a listening socket.
 @option{telnet} specifies that traffic on the socket should interpret telnet
 escape sequences.
 
+@option{recon} 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][,recon=recon_time]
 The TCP Net Console has two modes of operation.  It can send the serial
 I/O to a location or wait for a connection from a location.  By default
 the TCP Net Console is sent to @var{host} at the @var{port}.  If you use
 the @var{server} option QEMU will wait for a client socket application
 to connect to the port before continuing, unless the @code{nowait}
 option was specified.  The @code{nodelay} option disables the Nagle buffering
-algorithm.  If @var{host} is omitted, 0.0.0.0 is assumed. Only
+algorithm.  The @code{recon} 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][,recon=recon_time]
 A unix domain socket is used instead of a tcp socket.  The option works the
 same as if you had specified @code{-serial tcp} except the unix domain socket
 @var{path} is used for connections.