Patchwork Socket reconnection.

login
register
mail settings
Submitter Ian Molton
Date Dec. 2, 2009, 10:40 a.m.
Message ID <4B16442E.3090706@collabora.co.uk>
Download mbox | patch
Permalink /patch/39982/
State New
Headers show

Comments

Ian Molton - Dec. 2, 2009, 10:40 a.m.
Anthony Liguori wrote:

> sleep() in qemu is very, very wrong.  It will pause the guest's
> execution and all sorts of badness can ensue.

Quite...

> The right thing to do is set a timer and not generate data while
> disconnected.

New patch attached, now with less crack...

>  I still am not confident this is really a great thing to do.

What other option is there than to drop the ability to feed entropy to
the guest when the hosts egd link drops?

btw. Does anyone know how to get t-bird to inline patches?

-Ian
Jan Kiszka - Dec. 2, 2009, 12:04 p.m.
Ian Molton wrote:
> btw. Does anyone know how to get t-bird to inline patches?

I'm using Thunderbird for single patches. For a fairly efficient
workflow, I've installed Toggle Word Wrap [1] and defined a short-cut
for it (here: Alt-W). After copy&pasting a patch into the editor window,
I simply switch off the wrapping and can then send out a properly
formatted patch. The only pitfall is to forget to switch (some visual
indication of the wrapping mode in the status bar or so would be nice).

Jan

[1] https://addons.mozilla.org/de/firefox/addon/2351
Anthony Liguori - Dec. 2, 2009, 7:56 p.m.
Ian Molton wrote:
> Anthony Liguori wrote:
>
>   
>> sleep() in qemu is very, very wrong.  It will pause the guest's
>> execution and all sorts of badness can ensue.
>>     
>
> Quite...
>
>   
>> The right thing to do is set a timer and not generate data while
>> disconnected.
>>     
>
> New patch attached, now with less crack...
>
>   
>>  I still am not confident this is really a great thing to do.
>>     
>
> What other option is there than to drop the ability to feed entropy to
> the guest when the hosts egd link drops?
>
> btw. Does anyone know how to get t-bird to inline patches?
>
> -Ian
>   

Ah, your mailer didn't attach it correctly...

Anyway, it's broken wrt CodingStyle.

Regards,

Anthony Liguori
Krumme, Chris - Dec. 2, 2009, 10:35 p.m.
Hello Ian,

Pasting chunks, then commenting:

+static int qemu_chr_sched_reconnect(TCPCharDriver *s)
+{
+    struct reconnect_list *new = qemu_malloc(sizeof(*new));
+    struct timeval tv;
+
+    if(!new)
+        return 1;


Qemu_malloc will never return 0, so sched function can return void.


+    while (this) {
+        if (this->when <= now) {
+            if(qemu_chr_connect_socket(this->s)) {
+                if(prev)
+                    prev->next = this->next;
+                else
+                    rc_list = NULL;
+                qemu_chr_event(this->s->chr, CHR_EVENT_RECONNECTED);
+                free(this);
+		if(prev)
+			this = prev;
+		else
+			this = NULL;
+            }
+            else {
+                this->when += this->s->reconnect * 1000000;
+            }
+        }
+        prev = this;
+	if(this)
+            this = this->next;

This is a mixture of tabs and spaces, for new code pick one.

The if(prev) can just be this = prev;  if prev is NULL you want NULL
anyway.

Thanks

Chris
 

> -----Original Message-----
> From: Ian Molton [mailto:ian.molton@collabora.co.uk] 
> Sent: Wednesday, December 02, 2009 4:41 AM
> To: Anthony Liguori
> Cc: Krumme, Chris; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] Socket reconnection.
> 
> Anthony Liguori wrote:
> 
> > sleep() in qemu is very, very wrong.  It will pause the guest's
> > execution and all sorts of badness can ensue.
> 
> Quite...
> 
> > The right thing to do is set a timer and not generate data while
> > disconnected.
> 
> New patch attached, now with less crack...
> 
> >  I still am not confident this is really a great thing to do.
> 
> What other option is there than to drop the ability to feed entropy to
> the guest when the hosts egd link drops?
> 
> btw. Does anyone know how to get t-bird to inline patches?
> 
> -Ian
>

Patch

From e9d4be9cd0ef9e34c65939d4604874035c45bf34 Mon Sep 17 00:00:00 2001
From: Ian Molton <ian.molton@collabora.co.uk>
Date: Tue, 1 Dec 2009 11:18:41 +0000
Subject: [PATCH 2/4] socket: Add a reconnect option.

	Add a reconnect option that allows sockets to reconnect (after a
specified delay) to the specified server. This makes the virtio-rng driver
useful in production environments where the EGD server may need to be restarted.

Signed-off-by: Ian Molton <ian.molton@collabora.co.uk>
---
 qemu-char.c   |  177 ++++++++++++++++++++++++++++++++++++++++++++-------------
 qemu-char.h   |    2 +
 qemu-config.c |    3 +
 vl.c          |    4 +
 4 files changed, 147 insertions(+), 39 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index e202585..714c119 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1870,8 +1870,12 @@  typedef struct {
     int max_size;
     int do_telnetopt;
     int do_nodelay;
+    int reconnect;
     int is_unix;
     int msgfd;
+    QemuOpts *opts;
+    CharDriverState *chr;
+    int (*setup)(QemuOpts *opts);
 } TCPCharDriver;
 
 static void tcp_chr_accept(void *opaque);
@@ -2011,6 +2015,69 @@  static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
 }
 #endif
 
+struct reconnect_list {
+    TCPCharDriver *s;
+    uint64_t when;
+    struct reconnect_list *next;
+};
+
+static struct reconnect_list *rc_list;
+
+static int qemu_chr_sched_reconnect(TCPCharDriver *s)
+{
+    struct reconnect_list *new = qemu_malloc(sizeof(*new));
+    struct timeval tv;
+
+    if(!new)
+        return 1;
+
+    gettimeofday(&tv, NULL);
+    new->s = s;
+    new->when = (s->reconnect + tv.tv_sec) * 1000000 + tv.tv_usec;
+    new->next = rc_list;
+    rc_list = new;
+
+    return 0;
+}
+
+static int qemu_chr_connect_socket(TCPCharDriver *s);
+
+void qemu_chr_reconnect(void)
+{
+    struct reconnect_list *this = rc_list, *prev = NULL;
+    struct timeval tv;
+    uint64_t now;
+
+    if(!this)
+        return;
+
+    gettimeofday(&tv, NULL);
+    now = tv.tv_sec * 1000000 + tv.tv_usec;
+
+    while (this) {
+        if (this->when <= now) {
+            if(qemu_chr_connect_socket(this->s)) {
+                if(prev)
+                    prev->next = this->next;
+                else
+                    rc_list = NULL;
+                qemu_chr_event(this->s->chr, CHR_EVENT_RECONNECTED);
+                free(this);
+		if(prev)
+			this = prev;
+		else
+			this = NULL;
+            }
+            else {
+                this->when += this->s->reconnect * 1000000;
+            }
+        }
+        prev = this;
+	if(this)
+            this = this->next;
+    }
+}
+
 static void tcp_chr_read(void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -2030,10 +2097,16 @@  static void tcp_chr_read(void *opaque)
         if (s->listen_fd >= 0) {
             qemu_set_fd_handler(s->listen_fd, tcp_chr_accept, NULL, chr);
         }
-        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
+        if (!s->reconnect)
+            qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
         closesocket(s->fd);
         s->fd = -1;
-        qemu_chr_event(chr, CHR_EVENT_CLOSED);
+        if (!s->reconnect) {
+            qemu_chr_event(chr, CHR_EVENT_CLOSED);
+        } else if (qemu_chr_sched_reconnect(s)) {
+            printf("Unable to queue socket for reconnection.\n");
+            qemu_chr_event(chr, CHR_EVENT_CLOSED);
+        }
     } else if (size > 0) {
         if (s->do_telnetopt)
             tcp_chr_process_IAC_bytes(chr, s, buf, &size);
@@ -2137,7 +2210,6 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
 {
     CharDriverState *chr = NULL;
     TCPCharDriver *s = NULL;
-    int fd = -1;
     int is_listen;
     int is_waitconnect;
     int do_nodelay;
@@ -2145,34 +2217,40 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     int is_telnet;
 
     is_listen      = qemu_opt_get_bool(opts, "server", 0);
+    is_unix        = qemu_opt_get(opts, "path") != NULL;
+
     is_waitconnect = qemu_opt_get_bool(opts, "wait", 1);
     is_telnet      = qemu_opt_get_bool(opts, "telnet", 0);
     do_nodelay     = !qemu_opt_get_bool(opts, "delay", 1);
-    is_unix        = qemu_opt_get(opts, "path") != NULL;
-    if (!is_listen)
+
+    if (!is_listen) {
         is_waitconnect = 0;
+    } else {
+        if (is_telnet)
+            s->do_telnetopt = 1;
+    }
+
 
-    chr = qemu_mallocz(sizeof(CharDriverState));
     s = qemu_mallocz(sizeof(TCPCharDriver));
+    chr = qemu_mallocz(sizeof(CharDriverState));
+    s->opts = opts;
+
+    if (!is_listen && !is_telnet)
+        s->reconnect = qemu_opt_get_number(opts, "reconnect", 0);
 
     if (is_unix) {
         if (is_listen) {
-            fd = unix_listen_opts(opts);
+            s->setup = unix_listen_opts;
         } else {
-            fd = unix_connect_opts(opts);
+            s->setup = unix_connect_opts;
         }
     } else {
         if (is_listen) {
-            fd = inet_listen_opts(opts, 0);
+            s->setup = inet_listen_opts;
         } else {
-            fd = inet_connect_opts(opts);
+            s->setup = inet_connect_opts;
         }
     }
-    if (fd < 0)
-        goto fail;
-
-    if (!is_waitconnect)
-        socket_set_nonblock(fd);
 
     s->connected = 0;
     s->fd = -1;
@@ -2186,19 +2264,6 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     chr->chr_close = tcp_chr_close;
     chr->get_msgfd = tcp_get_msgfd;
 
-    if (is_listen) {
-        s->listen_fd = fd;
-        qemu_set_fd_handler(s->listen_fd, tcp_chr_accept, NULL, chr);
-        if (is_telnet)
-            s->do_telnetopt = 1;
-
-    } else {
-        s->connected = 1;
-        s->fd = fd;
-        socket_set_nodelay(fd);
-        tcp_chr_connect(chr);
-    }
-
     /* for "info chardev" monitor command */
     chr->filename = qemu_malloc(256);
     if (is_unix) {
@@ -2215,22 +2280,56 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
                  qemu_opt_get_bool(opts, "server", 0) ? ",server" : "");
     }
 
-    if (is_listen && is_waitconnect) {
-        printf("QEMU waiting for connection on: %s\n",
-               chr->filename);
-        tcp_chr_accept(chr);
-        socket_set_nonblock(s->listen_fd);
-    }
-    return chr;
+    s->chr = chr;
+
+    if(qemu_chr_connect_socket(s))
+        return chr;
 
- fail:
-    if (fd >= 0)
-        closesocket(fd);
-    qemu_free(s);
     qemu_free(chr);
+    qemu_free(s);
+
     return NULL;
 }
 
+
+static int qemu_chr_connect_socket(TCPCharDriver *s)
+{
+    QemuOpts *opts = s->opts;
+    int is_listen;
+    int fd;
+    int is_waitconnect;
+    int do_nodelay;
+
+    is_waitconnect = qemu_opt_get_bool(opts, "wait", 1);
+    is_listen      = qemu_opt_get_bool(opts, "server", 0);
+    do_nodelay     = !qemu_opt_get_bool(opts, "delay", 1);
+
+
+    fd = s->setup(s->opts);
+    if (fd < 0)
+        return 0;
+
+    if (!is_waitconnect)
+        socket_set_nonblock(fd);
+
+    if (is_listen) {
+        s->listen_fd = fd;
+        qemu_set_fd_handler(s->listen_fd, tcp_chr_accept, NULL, s->chr);
+        if (is_waitconnect) {
+            printf("QEMU waiting for connection on: %s\n",
+                   s->chr->filename);
+            tcp_chr_accept(s->chr);
+            socket_set_nonblock(s->listen_fd);
+        }
+    } else {
+        s->fd = fd;
+        socket_set_nodelay(fd);
+        tcp_chr_connect(s->chr);
+    }
+
+    return 1;
+}
+
 static QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
 {
     char host[65], port[33], width[8], height[8];
diff --git a/qemu-char.h b/qemu-char.h
index 9957db1..dc954e2 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -14,6 +14,7 @@ 
 #define CHR_EVENT_MUX_IN  3 /* mux-focus was set to this terminal */
 #define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
 #define CHR_EVENT_CLOSED  5 /* connection closed */
+#define CHR_EVENT_RECONNECTED  6 /* reconnect event */
 
 
 #define CHR_IOCTL_SERIAL_SET_PARAMS   1
@@ -73,6 +74,7 @@  CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
                                     void (*init)(struct CharDriverState *s));
 CharDriverState *qemu_chr_open(const char *label, const char *filename, void (*init)(struct CharDriverState *s));
 void qemu_chr_close(CharDriverState *chr);
+void qemu_chr_reconnect(void);
 void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
 void qemu_chr_send_event(CharDriverState *s, int event);
diff --git a/qemu-config.c b/qemu-config.c
index 590fc05..ff8b06e 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -140,6 +140,9 @@  QemuOptsList qemu_chardev_opts = {
         },{
             .name = "signal",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "reconnect",
+            .type = QEMU_OPT_NUMBER,
         },
         { /* end if list */ }
     },
diff --git a/vl.c b/vl.c
index 44763af..5876c3e 100644
--- a/vl.c
+++ b/vl.c
@@ -3795,6 +3795,10 @@  void main_loop_wait(int timeout)
 
     host_main_loop_wait(&timeout);
 
+    /* Reconnect any disconnected sockets, if necessary */
+
+    qemu_chr_reconnect();
+
     /* poll any events */
     /* XXX: separate device handlers from system ones */
     nfds = -1;
-- 
1.6.5