[Qemu-devel] [PATCH] resend: socket reconnect and virtio-rng support

[Qemu-devel] [PATCH] resend: socket reconnect and virtio-rng support

[Ian Molton]

The following patches are an update of my virtio-rng patchset.

Patches 1-3 implement needed infrastructure fpr virtio-rng to be used effectively, patch 4 implements the device.

Patch 5 is a build fix I needed in order to compile -git qemu.

Please apply :)

 qemu-sockets.c         |    9 +-
 qemu_socket.h          |    2 
 qemu-char.c            |  159 +++++++++++++++++++++++++++++---------
 qemu-char.h            |    2 
 qemu-config.c          |    3 
 vl.c                   |    4 
 hw/qdev-properties.c   |   34 ++++++++
 hw/qdev.h              |    4 
 parse_common.h         |   40 +++++++++
 qemu-option.c          |   23 -----
 Makefile.target        |    2 
 hw/pci.h               |    1 
 hw/virtio-pci.c        |   27 ++++++
 hw/virtio-rng.c        |  202 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-rng.h        |   19 ++++
 hw/virtio.h            |    2 
 rng.h                  |   18 ++++
 fpu/softfloat-native.c |    1 
 18 files changed, 489 insertions(+), 63 deletions(-)

[Qemu-devel] [PATCH 1/5] socket: Rationalise function declarations

[Ian Molton]

	This patch rationalises the declaration of inet_listen_opts such that
it matches the other inet_{listen,connect}_opts functions.

This change is needed for a patch adding socket reconection support.

Signed-off-by: Ian Molton <ian.molton@collabora.co.uk>
---
 qemu-sockets.c |    9 +++++++--
 qemu_socket.h  |    2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index d912fed..7cfb6cf 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -116,7 +116,7 @@ static void inet_print_addrinfo(const char *tag, struct addrinfo *res)
     }
 }
 
-int inet_listen_opts(QemuOpts *opts, int port_offset)
+static int do_inet_listen(QemuOpts *opts, int port_offset)
 {
     struct addrinfo ai,*res,*e;
     const char *addr;
@@ -216,6 +216,11 @@ listen:
     return slisten;
 }
 
+int inet_listen_opts(QemuOpts *opts)
+{
+    return do_inet_listen(opts, 0);
+}
+
 int inet_connect_opts(QemuOpts *opts)
 {
     struct addrinfo ai,*res,*e;
@@ -465,7 +470,7 @@ int inet_listen(const char *str, char *ostr, int olen,
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0);
     if (inet_parse(opts, str) == 0) {
-        sock = inet_listen_opts(opts, port_offset);
+        sock = do_inet_listen(opts, port_offset);
         if (sock != -1 && ostr) {
             optstr = strchr(str, ',');
             if (qemu_opt_get_bool(opts, "ipv6", 0)) {
diff --git a/qemu_socket.h b/qemu_socket.h
index 7ee46ac..0cf96d9 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -38,7 +38,7 @@ void socket_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
 
 /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
-int inet_listen_opts(QemuOpts *opts, int port_offset);
+int inet_listen_opts(QemuOpts *opts);
 int inet_listen(const char *str, char *ostr, int olen,
                 int socktype, int port_offset);
 int inet_connect_opts(QemuOpts *opts);
-- 
1.6.6

[Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Ian Molton]

	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   |  159 +++++++++++++++++++++++++++++++++++++++++++--------------
 qemu-char.h   |    2 +
 qemu-config.c |    3 +
 vl.c          |    4 ++
 4 files changed, 129 insertions(+), 39 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 800ee6c..016afd0 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,8 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
 }
 #endif
 
+static void qemu_chr_sched_reconnect(TCPCharDriver *s);
+
 static void tcp_chr_read(void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -2030,10 +2036,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_sched_reconnect(s);
+        } else {
+            qemu_chr_event(chr, CHR_EVENT_CLOSED);
+        }
     } else if (size > 0) {
         if (s->do_telnetopt)
             tcp_chr_process_IAC_bytes(chr, s, buf, &size);
@@ -2133,11 +2145,92 @@ static void tcp_chr_close(CharDriverState *chr)
     qemu_chr_event(chr, CHR_EVENT_CLOSED);
 }
 
+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 QLIST_HEAD(reconnect_list_head, reconnect_list_entry) rcl_head;
+
+typedef struct reconnect_list_entry {
+    TCPCharDriver *s;
+    uint64_t when;
+    QLIST_ENTRY(reconnect_list_entry) entries;
+} reconnect_list_entry;
+
+static void qemu_chr_sched_reconnect(TCPCharDriver *s)
+{
+    reconnect_list_entry *new = qemu_malloc(sizeof(*new));
+    struct timeval tv;
+
+    qemu_gettimeofday(&tv);
+    new->s = s;
+    new->when = (s->reconnect + tv.tv_sec) * 1000000 + tv.tv_usec;
+    QLIST_INSERT_HEAD(&rcl_head, new, entries);
+}
+
+void qemu_chr_reconnect(void)
+{
+    struct timeval tv;
+    uint64_t now;
+    reconnect_list_entry *np;
+
+    if (!rcl_head.lh_first)
+        return;
+
+    gettimeofday(&tv, NULL);
+    now = tv.tv_sec * 1000000 + tv.tv_usec;
+
+    for (np = rcl_head.lh_first; np != NULL; np = np->entries.le_next) {
+        if (np->when <= now) {
+            if (qemu_chr_connect_socket(np->s)) {
+                qemu_chr_event(np->s->chr, CHR_EVENT_RECONNECTED);
+                QLIST_REMOVE(np, entries);
+            }
+            else {
+                np->when += np->s->reconnect * 1000000;
+            }
+        }
+    }
+}
+
 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 +2238,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 +2285,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,19 +2301,14 @@ 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;
 }
 
diff --git a/qemu-char.h b/qemu-char.h
index bcc0766..32bcfd7 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -15,6 +15,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
@@ -75,6 +76,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 c3203c8..a229350 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -144,6 +144,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 6f1e1ab..bcd7d44 100644
--- a/vl.c
+++ b/vl.c
@@ -3719,6 +3719,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.6

[Qemu-devel] [PATCH 3/5] Add SIZE type to qdev properties

[Ian Molton]

    	This patch adds a 'SIZE' type property to those available to qdevs.
    It is the analogue of the OPT_SIZE property for options.

    Signed-off-by: Ian Molton <ian.molton@collabora.co.uk>
---
 hw/qdev-properties.c |   34 ++++++++++++++++++++++++++++++++++
 hw/qdev.h            |    4 ++++
 parse_common.h       |   40 ++++++++++++++++++++++++++++++++++++++++
 qemu-option.c        |   23 +++--------------------
 4 files changed, 81 insertions(+), 20 deletions(-)
 create mode 100644 parse_common.h

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index f5ca05f..dbb1146 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1,6 +1,7 @@
 #include "sysemu.h"
 #include "net.h"
 #include "qdev.h"
+#include "parse_common.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 {
@@ -247,6 +248,39 @@ PropertyInfo qdev_prop_hex64 = {
     .print = print_hex64,
 };
 
+/* --- 64bit unsigned int 'size' type --- */
+
+static int parse_size(DeviceState *dev, Property *prop, const char *str)
+{
+    uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
+
+    if(str != NULL)
+        return parse_common_size(str, ptr);
+
+    return -1;
+}
+
+static int print_size(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+    uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
+    char suffixes[] = {'T', 'G', 'M', 'K', 'B'};
+    int i;
+    uint64_t div;
+
+    for(div = (long int)1 << 40 ;
+        !(*ptr / div) ; div >>= 10, i++);
+
+    return snprintf(dest, len, "%0.03f%c", (double)*ptr/div, suffixes[i]);
+}
+
+PropertyInfo qdev_prop_size = {
+    .name  = "size",
+    .type  = PROP_TYPE_SIZE,
+    .size  = sizeof(uint64_t),
+    .parse = parse_size,
+    .print = print_size,
+};
+
 /* --- string --- */
 
 static int parse_string(DeviceState *dev, Property *prop, const char *str)
diff --git a/hw/qdev.h b/hw/qdev.h
index 07b9603..cd718e8 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -75,6 +75,7 @@ enum PropertyType {
     PROP_TYPE_UINT32,
     PROP_TYPE_INT32,
     PROP_TYPE_UINT64,
+    PROP_TYPE_SIZE,
     PROP_TYPE_TADDR,
     PROP_TYPE_MACADDR,
     PROP_TYPE_DRIVE,
@@ -183,6 +184,7 @@ extern PropertyInfo qdev_prop_int32;
 extern PropertyInfo qdev_prop_uint64;
 extern PropertyInfo qdev_prop_hex32;
 extern PropertyInfo qdev_prop_hex64;
+extern PropertyInfo qdev_prop_size;
 extern PropertyInfo qdev_prop_string;
 extern PropertyInfo qdev_prop_chr;
 extern PropertyInfo qdev_prop_ptr;
@@ -228,6 +230,8 @@ extern PropertyInfo qdev_prop_pci_devfn;
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
 #define DEFINE_PROP_HEX64(_n, _s, _f, _d)                       \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
+#define DEFINE_PROP_SIZE(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_size, uint64_t)
 #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)                   \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, uint32_t)
 
diff --git a/parse_common.h b/parse_common.h
new file mode 100644
index 0000000..93b1fc2
--- /dev/null
+++ b/parse_common.h
@@ -0,0 +1,40 @@
+/*
+ * Common parsing routines
+ *
+ * Copyright Collabora 2009
+ *
+ * Author:
+ *  Ian Molton <ian.molton@collabora.co.uk>
+ *
+ * Derived from the parser in qemu-option.c
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+static inline int parse_common_size(const char *value, uint64_t *ptr) {
+    char *postfix;
+    double sizef;
+
+    sizef = strtod(value, &postfix);
+    switch (*postfix) {
+    case 'T':
+        sizef *= 1024;
+    case 'G':
+        sizef *= 1024;
+    case 'M':
+        sizef *= 1024;
+    case 'K':
+    case 'k':
+        sizef *= 1024;
+    case 'B':
+    case 'b':
+    case '\0':
+        *ptr = (uint64_t) sizef;
+        return 0;
+    }
+
+    return -1;
+}
+
diff --git a/qemu-option.c b/qemu-option.c
index 24392fc..91772c5 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -28,6 +28,7 @@
 
 #include "qemu-common.h"
 #include "qemu-option.h"
+#include "parse_common.h"
 
 /*
  * Extracts the name of an option from the parameter string (p points at the
@@ -203,28 +204,10 @@ static int parse_option_number(const char *name, const char *value, uint64_t *re
 
 static int parse_option_size(const char *name, const char *value, uint64_t *ret)
 {
-    char *postfix;
-    double sizef;
-
     if (value != NULL) {
-        sizef = strtod(value, &postfix);
-        switch (*postfix) {
-        case 'T':
-            sizef *= 1024;
-        case 'G':
-            sizef *= 1024;
-        case 'M':
-            sizef *= 1024;
-        case 'K':
-        case 'k':
-            sizef *= 1024;
-        case 'b':
-        case '\0':
-            *ret = (uint64_t) sizef;
-            break;
-        default:
+	if(parse_common_size(value, ret) == -1) {
             fprintf(stderr, "Option '%s' needs size as parameter\n", name);
-            fprintf(stderr, "You may use k, M, G or T suffixes for "
+            fprintf(stderr, "You may use B, K, M, G or T suffixes for bytes "
                     "kilobytes, megabytes, gigabytes and terabytes.\n");
             return -1;
         }
-- 
1.6.6

[Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Ian Molton]

	This patch adds support for virtio-rng. Data is read from a chardev and
can be either raw entropy or received via the EGD protocol.

Signed-off-by: Ian Molton <ian.molton@collabora.co.uk>
---
 Makefile.target |    2 +-
 hw/pci.h        |    1 +
 hw/virtio-pci.c |   27 +++++++
 hw/virtio-rng.c |  202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-rng.h |   19 +++++
 hw/virtio.h     |    2 +
 rng.h           |   18 +++++
 7 files changed, 270 insertions(+), 1 deletions(-)
 create mode 100644 hw/virtio-rng.c
 create mode 100644 hw/virtio-rng.h
 create mode 100644 rng.h

diff --git a/Makefile.target b/Makefile.target
index 5c0ef1f..21d28f4 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -172,7 +172,7 @@ ifdef CONFIG_SOFTMMU
 obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
-obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o virtio-rng.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 LIBS+=-lz
diff --git a/hw/pci.h b/hw/pci.h
index 8b511d2..77cb543 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -69,6 +69,7 @@
 #define PCI_DEVICE_ID_VIRTIO_BLOCK       0x1001
 #define PCI_DEVICE_ID_VIRTIO_BALLOON     0x1002
 #define PCI_DEVICE_ID_VIRTIO_CONSOLE     0x1003
+#define PCI_DEVICE_ID_VIRTIO_RNG         0x1004
 
 typedef uint64_t pcibus_t;
 #define FMT_PCIBUS                      PRIx64
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 709d13e..f057388 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -22,6 +22,7 @@
 #include "sysemu.h"
 #include "msix.h"
 #include "net.h"
+#include "rng.h"
 #include "loader.h"
 
 /* from Linux's linux/virtio_pci.h */
@@ -94,6 +95,7 @@ typedef struct {
     uint32_t nvectors;
     DriveInfo *dinfo;
     NICConf nic;
+    RNGConf rng;
     uint32_t host_features;
     /* Max. number of ports we can have for a the virtio-serial device */
     uint32_t max_virtserial_ports;
@@ -550,6 +552,21 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
     return 0;
 }
 
+static int virtio_rng_init_pci(PCIDevice *pci_dev)
+{
+    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+    VirtIODevice *vdev;
+
+    vdev = virtio_rng_init(&pci_dev->qdev, &proxy->rng);
+    virtio_init_pci(proxy, vdev,
+                    PCI_VENDOR_ID_REDHAT_QUMRANET,
+                    PCI_DEVICE_ID_VIRTIO_RNG,
+                    PCI_CLASS_OTHERS,
+                    0x00);
+
+    return 0;
+}
+
 static PCIDeviceInfo virtio_info[] = {
     {
         .qdev.name = "virtio-blk-pci",
@@ -603,6 +620,16 @@ static PCIDeviceInfo virtio_info[] = {
         },
         .qdev.reset = virtio_pci_reset,
     },{
+        .qdev.name = "virtio-rng-pci",
+        .qdev.size = sizeof(VirtIOPCIProxy),
+        .init      = virtio_rng_init_pci,
+        .exit      = virtio_exit_pci,
+        .qdev.props = (Property[]) {
+            DEFINE_RNG_PROPERTIES(VirtIOPCIProxy, rng),
+            DEFINE_PROP_END_OF_LIST(),
+        },
+        .qdev.reset = virtio_pci_reset,
+    },{
         /* end of list */
     }
 };
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
new file mode 100644
index 0000000..d8cbb74
--- /dev/null
+++ b/hw/virtio-rng.c
@@ -0,0 +1,202 @@
+/*
+ * Virtio RNG Device
+ *
+ * Copyright Collabora 2009
+ *
+ * Authors:
+ *  Ian Molton <ian.molton@collabora.co.uk>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw.h"
+#include "qemu-char.h"
+#include "virtio.h"
+#include "virtio-rng.h"
+#include "rng.h"
+#include <sys/time.h>
+
+typedef struct VirtIORng
+{
+    VirtIODevice vdev;
+    VirtQueue *vq;
+    CharDriverState *chr;
+    struct timeval last;
+    int rate;
+    int egd;
+    int entropy_remaining;
+    int pool;
+} VirtIORng;
+
+/* Maximum size of the buffer the guest expects */
+#define BUFF_MAX 64
+
+/* EGD protocol - we only use this command */
+#define EGD_READ_BLOCK 0x2
+
+#define EGD_MAX_BLOCK_SIZE 255
+#define EGD_MAX_REQUESTS 3
+#define EGD_MAX_POOL_SIZE (EGD_MAX_BLOCK_SIZE * (EGD_MAX_REQUESTS-1))
+
+static inline void req_entropy(VirtIORng *s) {
+    static const unsigned char entropy_rq[2] = { EGD_READ_BLOCK,
+                                                 EGD_MAX_BLOCK_SIZE };
+    if (s->egd) {
+        /* Let the socket buffer up the incoming data for us. Max block size
+           for EGD protocol is (stupidly) 255, so make sure we always have a
+           block pending for performance. We can have 3 outstanding buffers */
+        if (s->pool <= EGD_MAX_POOL_SIZE) {
+            s->chr->chr_write(s->chr, entropy_rq, sizeof(entropy_rq));
+            s->pool += EGD_MAX_BLOCK_SIZE;
+        }
+    } else {
+        s->pool = BUFF_MAX;
+    }
+}
+
+static int vrng_can_read(void *opaque)
+{
+    VirtIORng *s = (VirtIORng *) opaque;
+    struct timeval now, d;
+    int max_entropy;
+
+    if (!virtio_queue_ready(s->vq) ||
+        !(s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) ||
+        virtio_queue_empty(s->vq))
+        return 0;
+
+    req_entropy(s);
+
+    if (s->rate) {
+        gettimeofday(&now, NULL);
+        timersub(&now, &s->last, &d);
+        if (d.tv_sec * 1000000 + d.tv_usec > 1000000) {
+            s->entropy_remaining = s->rate;
+            s->last = now;
+        }
+        max_entropy = MIN(s->pool, s->entropy_remaining);
+    } else {
+        max_entropy = s->pool;
+    }
+
+    /* current implementations have a 64 byte buffer.
+     * We fall back to a one byte per read if there is not enough room.
+     */
+    max_entropy = MIN(max_entropy, BUFF_MAX);
+    if (max_entropy) {
+        if (virtqueue_avail_bytes(s->vq, max_entropy, 0))
+            return max_entropy;
+        if (virtqueue_avail_bytes(s->vq, 1, 0))
+            return 1;
+    }
+    return 0;
+}
+
+static void vrng_read(void *opaque, const uint8_t *buf, int size)
+{
+    VirtIORng *s = (VirtIORng *) opaque;
+    VirtQueueElement elem;
+    int offset = 0;
+
+    /* The current kernel implementation has only one outstanding input
+     * buffer of 64 bytes.
+     */
+    while (offset < size) {
+        int i = 0;
+        if (!virtqueue_pop(s->vq, &elem))
+                break;
+        while (offset < size && i < elem.in_num) {
+            int len = MIN(elem.in_sg[i].iov_len, size - offset);
+            memcpy(elem.in_sg[i].iov_base, buf + offset, len);
+            offset += len;
+            i++;
+        }
+        virtqueue_push(s->vq, &elem, size);
+    }
+
+    if (s->rate)
+        s->entropy_remaining -= size;
+    s->pool -= size;
+
+    virtio_notify(&s->vdev, s->vq);
+}
+
+static void vrng_event(void *opaque, int event)
+{
+    VirtIORng *s = opaque;
+
+    /*
+     * If our connection has been interrupted we need to kick the entropy
+     * gathering process if we are using EGD.
+     */
+
+    if(s->egd && event == CHR_EVENT_RECONNECTED)
+        s->pool = 0;
+}
+
+
+
+static void virtio_rng_handle(VirtIODevice *vdev, VirtQueue *vq)
+{
+    /* Nothing to do - we push data when its available */
+}
+
+static uint32_t virtio_rng_get_features(VirtIODevice *vdev, uint32_t features)
+{
+    return 0;
+}
+
+static void virtio_rng_save(QEMUFile *f, void *opaque)
+{
+    VirtIORng *s = opaque;
+
+    virtio_save(&s->vdev, f);
+}
+
+static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
+{
+    VirtIORng *s = opaque;
+
+    if (version_id != 1)
+        return -EINVAL;
+
+    virtio_load(&s->vdev, f);
+    return 0;
+}
+
+VirtIODevice *virtio_rng_init(DeviceState *dev, RNGConf *rngdev)
+{
+    VirtIORng *s;
+    s = (VirtIORng *)virtio_common_init("virtio-rng",
+                                            VIRTIO_ID_RNG,
+                                            0, sizeof(VirtIORng));
+
+    if (!s)
+        return NULL;
+
+    s->vdev.get_features = virtio_rng_get_features;
+
+    s->vq = virtio_add_queue(&s->vdev, 128, virtio_rng_handle);
+    s->chr = rngdev->chrdev;
+    s->rate = rngdev->rate;
+    gettimeofday(&s->last, NULL);
+
+    if(rngdev->proto && !strncmp(rngdev->proto, "egd", 3))
+        s->egd = 1;
+
+#ifdef DEBUG
+    printf("entropy being read from %s", rngdev->chrdev->label);
+    if(s->rate)
+        printf(" at %d bytes/sec max.", s->rate);
+    printf(" protocol: %s\n", s->egd?"egd":"raw");
+#endif
+
+    qemu_chr_add_handlers(s->chr, vrng_can_read, vrng_read, vrng_event, s);
+
+    register_savevm("virtio-rng", -1, 1, virtio_rng_save, virtio_rng_load, s);
+
+    return &s->vdev;
+}
+
diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h
new file mode 100644
index 0000000..bc4d2d8
--- /dev/null
+++ b/hw/virtio-rng.h
@@ -0,0 +1,19 @@
+/*
+ * Virtio RNG Support
+ *
+ * Copyright Collabora 2009
+ *
+ * Authors:
+ *  Ian Molton <ian.molton@collabora.co.uk>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#ifndef _QEMU_VIRTIO_RNG_H
+#define _QEMU_VIRTIO_RNG_H
+
+/* The ID for virtio console */
+#define VIRTIO_ID_RNG 4
+
+#endif
diff --git a/hw/virtio.h b/hw/virtio.h
index 62e882b..04a6b39 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -16,6 +16,7 @@
 
 #include "hw.h"
 #include "net.h"
+#include "rng.h"
 #include "qdev.h"
 #include "sysemu.h"
 
@@ -173,6 +174,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, DriveInfo *dinfo);
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf);
 VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports);
 VirtIODevice *virtio_balloon_init(DeviceState *dev);
+VirtIODevice *virtio_rng_init(DeviceState *dev, RNGConf *rngdev);
 
 void virtio_net_exit(VirtIODevice *vdev);
 
diff --git a/rng.h b/rng.h
new file mode 100644
index 0000000..faf6880
--- /dev/null
+++ b/rng.h
@@ -0,0 +1,18 @@
+#ifndef QEMU_RNG_H
+#define QEMU_RNG_H
+
+#include "qemu-option.h"
+
+/* qdev rng properties */
+
+typedef struct RNGConf {
+    CharDriverState *chrdev;
+    uint64_t rate;
+    char *proto;
+} RNGConf;
+
+#define DEFINE_RNG_PROPERTIES(_state, _conf)              \
+    DEFINE_PROP_CHR("chardev",   _state, _conf.chrdev),   \
+    DEFINE_PROP_SIZE("rate",   _state, _conf.rate, 0),  \
+    DEFINE_PROP_STRING("protocol",   _state, _conf.proto)  
+#endif
-- 
1.6.6

[Qemu-devel] [PATCH 5/5] Build fix (missing header)

[Ian Molton]

	This patch addresses a build issue due to a missing include in
fpu/softfloat-native.c

Signed-off-by: Ian Molton <ian.molton@collabora.co.uk>
---
 fpu/softfloat-native.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat-native.c b/fpu/softfloat-native.c
index 8d64f4e..702950e 100644
--- a/fpu/softfloat-native.c
+++ b/fpu/softfloat-native.c
@@ -1,5 +1,6 @@
 /* Native implementation of soft float functions. Only a single status
    context is supported */
+#include <config-host.h>
 #include "softfloat.h"
 #include <math.h>
 #if defined(CONFIG_SOLARIS)
-- 
1.6.6

Re: [Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Anthony Liguori]

On 02/01/2010 07:34 AM, Ian Molton wrote:
> 	This patch adds support for virtio-rng. Data is read from a chardev and
> can be either raw entropy or received via the EGD protocol.
>
> Signed-off-by: Ian Molton<ian.molton@collabora.co.uk>
> ---
>   Makefile.target |    2 +-
>   hw/pci.h        |    1 +
>   hw/virtio-pci.c |   27 +++++++
>   hw/virtio-rng.c |  202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/virtio-rng.h |   19 +++++
>   hw/virtio.h     |    2 +
>   rng.h           |   18 +++++
>   7 files changed, 270 insertions(+), 1 deletions(-)
>   create mode 100644 hw/virtio-rng.c
>   create mode 100644 hw/virtio-rng.h
>   create mode 100644 rng.h
>
> diff --git a/Makefile.target b/Makefile.target
> index 5c0ef1f..21d28f4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -172,7 +172,7 @@ ifdef CONFIG_SOFTMMU
>   obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
>   # virtio has to be here due to weird dependency between PCI and virtio-net.
>   # need to fix this properly
> -obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o
> +obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o virtio-rng.o
>   obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>   obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
>   LIBS+=-lz
> diff --git a/hw/pci.h b/hw/pci.h
> index 8b511d2..77cb543 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -69,6 +69,7 @@
>   #define PCI_DEVICE_ID_VIRTIO_BLOCK       0x1001
>   #define PCI_DEVICE_ID_VIRTIO_BALLOON     0x1002
>   #define PCI_DEVICE_ID_VIRTIO_CONSOLE     0x1003
> +#define PCI_DEVICE_ID_VIRTIO_RNG         0x1004
>
>   typedef uint64_t pcibus_t;
>   #define FMT_PCIBUS                      PRIx64
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 709d13e..f057388 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -22,6 +22,7 @@
>   #include "sysemu.h"
>   #include "msix.h"
>   #include "net.h"
> +#include "rng.h"
>   #include "loader.h"
>
>   /* from Linux's linux/virtio_pci.h */
> @@ -94,6 +95,7 @@ typedef struct {
>       uint32_t nvectors;
>       DriveInfo *dinfo;
>       NICConf nic;
> +    RNGConf rng;
>       uint32_t host_features;
>       /* Max. number of ports we can have for a the virtio-serial device */
>       uint32_t max_virtserial_ports;
> @@ -550,6 +552,21 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
>       return 0;
>   }
>
> +static int virtio_rng_init_pci(PCIDevice *pci_dev)
> +{
> +    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> +    VirtIODevice *vdev;
> +
> +    vdev = virtio_rng_init(&pci_dev->qdev,&proxy->rng);
> +    virtio_init_pci(proxy, vdev,
> +                    PCI_VENDOR_ID_REDHAT_QUMRANET,
> +                    PCI_DEVICE_ID_VIRTIO_RNG,
>    

Gerd (or the right person at Red Hat) needs to Ack the assignment of 
this PCI device id.

> +                    PCI_CLASS_OTHERS,
> +                    0x00);
> +
> +    return 0;
> +}
> +
>   static PCIDeviceInfo virtio_info[] = {
>       {
>           .qdev.name = "virtio-blk-pci",
> @@ -603,6 +620,16 @@ static PCIDeviceInfo virtio_info[] = {
>           },
>           .qdev.reset = virtio_pci_reset,
>       },{
> +        .qdev.name = "virtio-rng-pci",
> +        .qdev.size = sizeof(VirtIOPCIProxy),
> +        .init      = virtio_rng_init_pci,
> +        .exit      = virtio_exit_pci,
> +        .qdev.props = (Property[]) {
> +            DEFINE_RNG_PROPERTIES(VirtIOPCIProxy, rng),
>    

I don't see any reason to use a define here.

> +            DEFINE_PROP_END_OF_LIST(),
> +        },
> +        .qdev.reset = virtio_pci_reset,
> +    },{
>           /* end of list */
>       }
>   };
> diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
> new file mode 100644
> index 0000000..d8cbb74
> --- /dev/null
> +++ b/hw/virtio-rng.c
> @@ -0,0 +1,202 @@
> +/*
> + * Virtio RNG Device
> + *
> + * Copyright Collabora 2009
> + *
> + * Authors:
> + *  Ian Molton<ian.molton@collabora.co.uk>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw.h"
> +#include "qemu-char.h"
> +#include "virtio.h"
> +#include "virtio-rng.h"
> +#include "rng.h"
> +#include<sys/time.h>
> +
> +typedef struct VirtIORng
> +{
> +    VirtIODevice vdev;
> +    VirtQueue *vq;
> +    CharDriverState *chr;
> +    struct timeval last;
> +    int rate;
> +    int egd;
> +    int entropy_remaining;
> +    int pool;
> +} VirtIORng;
> +
> +/* Maximum size of the buffer the guest expects */
> +#define BUFF_MAX 64
> +
> +/* EGD protocol - we only use this command */
> +#define EGD_READ_BLOCK 0x2
> +
> +#define EGD_MAX_BLOCK_SIZE 255
> +#define EGD_MAX_REQUESTS 3
> +#define EGD_MAX_POOL_SIZE (EGD_MAX_BLOCK_SIZE * (EGD_MAX_REQUESTS-1))
> +
> +static inline void req_entropy(VirtIORng *s) {
>    

Coding style is off here (newline between ) and { ).

> +    static const unsigned char entropy_rq[2] = { EGD_READ_BLOCK,
> +                                                 EGD_MAX_BLOCK_SIZE };
> +    if (s->egd) {
> +        /* Let the socket buffer up the incoming data for us. Max block size
> +           for EGD protocol is (stupidly) 255, so make sure we always have a
> +           block pending for performance. We can have 3 outstanding buffers */
> +        if (s->pool<= EGD_MAX_POOL_SIZE) {
> +            s->chr->chr_write(s->chr, entropy_rq, sizeof(entropy_rq));
> +            s->pool += EGD_MAX_BLOCK_SIZE;
> +        }
> +    } else {
> +        s->pool = BUFF_MAX;
> +    }
> +}
> +
> +static int vrng_can_read(void *opaque)
> +{
> +    VirtIORng *s = (VirtIORng *) opaque;
> +    struct timeval now, d;
> +    int max_entropy;
> +
> +    if (!virtio_queue_ready(s->vq) ||
> +        !(s->vdev.status&  VIRTIO_CONFIG_S_DRIVER_OK) ||
> +        virtio_queue_empty(s->vq))
> +        return 0;
> +
> +    req_entropy(s);
> +
> +    if (s->rate) {
> +        gettimeofday(&now, NULL);
> +        timersub(&now,&s->last,&d);
>    

Can't call gettimeofday directly.  You have to use qemu_gettimeofday().  
But it would be better to not rely on gettimeofday and instead make use 
of the rt_clock.
> +static void virtio_rng_save(QEMUFile *f, void *opaque)
> +{
> +    VirtIORng *s = opaque;
> +
> +    virtio_save(&s->vdev, f);
> +}
> +
> +static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +    VirtIORng *s = opaque;
> +
> +    if (version_id != 1)
> +        return -EINVAL;
> +
> +    virtio_load(&s->vdev, f);
> +    return 0;
> +}
>
>    

This doesn't look correct to me.  There is absolutely no state 
maintained by the virtio-rng backend?  I find that hard to believe.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Gerd Hoffmann]

   Hi,

>> + PCI_VENDOR_ID_REDHAT_QUMRANET,
>> + PCI_DEVICE_ID_VIRTIO_RNG,
>
> Gerd (or the right person at Red Hat) needs to Ack the assignment of
> this PCI device id.

Using the first unused one should be ok, I'll double check that though.
pci-ids.txt must be updated too.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Anthony Liguori]

On 02/01/2010 10:03 AM, Gerd Hoffmann wrote:
>   Hi,
>
>>> + PCI_VENDOR_ID_REDHAT_QUMRANET,
>>> + PCI_DEVICE_ID_VIRTIO_RNG,
>>
>> Gerd (or the right person at Red Hat) needs to Ack the assignment of
>> this PCI device id.
>
> Using the first unused one should be ok, I'll double check that though.
> pci-ids.txt must be updated too.

Thanks.

Regards,

Anthony Liguori

> cheers,
>   Gerd
>

Re: [Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Ian Molton]

Gerd Hoffmann wrote:
>   Hi,
> 
>>> + PCI_VENDOR_ID_REDHAT_QUMRANET,
>>> + PCI_DEVICE_ID_VIRTIO_RNG,
>>
>> Gerd (or the right person at Red Hat) needs to Ack the assignment of
>> this PCI device id.
> 
> Using the first unused one should be ok, I'll double check that though.
> pci-ids.txt must be updated too.

Cheers Gerd :)

-Ian

Re: [Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Ian Molton]

Anthony Liguori wrote:

>> +            DEFINE_RNG_PROPERTIES(VirtIOPCIProxy, rng),
>>    
> 
> I don't see any reason to use a define here.

Consistency with the other virtio code (at the time I wrote it)

> Coding style is off here (newline between ) and { ).

Fixed.

> Can't call gettimeofday directly.  You have to use qemu_gettimeofday(). 
> But it would be better to not rely on gettimeofday and instead make use
> of the rt_clock.

Hm, this I fixed before, I'll make sure its right in the next patch.
Must have got an old revision mixed up there.

>> +static void virtio_rng_save(QEMUFile *f, void *opaque)
>> +{
>> +    VirtIORng *s = opaque;
>> +
>> +    virtio_save(&s->vdev, f);
>> +}
>> +
>> +static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
>> +{
>> +    VirtIORng *s = opaque;
>> +
>> +    if (version_id != 1)
>> +        return -EINVAL;
>> +
>> +    virtio_load(&s->vdev, f);
>> +    return 0;
>> +}
>>
>>    
> 
> This doesn't look correct to me.  There is absolutely no state
> maintained by the virtio-rng backend?  I find that hard to believe.

What state needs maintaining? when it runs out of entropy, it simply
reconnects. Unless I misunderstood what those functions are for...

-Ian

Re: [Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Ian Molton]

Ian Molton wrote:

>>> +static void virtio_rng_save(QEMUFile *f, void *opaque)
>>> +{
>>> +    VirtIORng *s = opaque;
>>> +
>>> +    virtio_save(&s->vdev, f);
>>> +}
>>> +
>>> +static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
>>> +{
>>> +    VirtIORng *s = opaque;
>>> +
>>> +    if (version_id != 1)
>>> +        return -EINVAL;
>>> +
>>> +    virtio_load(&s->vdev, f);
>>> +    return 0;
>>> +}
>>>
>>>    
>> This doesn't look correct to me.  There is absolutely no state
>> maintained by the virtio-rng backend?  I find that hard to believe.
> 
> What state needs maintaining? when it runs out of entropy, it simply
> reconnects. Unless I misunderstood what those functions are for...

ping?

Re: [Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Anthony Liguori]

On 02/01/2010 07:34 AM, Ian Molton wrote:
> 	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>
>    

I went back and looked at the last series and found my feedback.  I had 
suggested that instead of automatically reconnecting, a mechanism should 
be added for a user to initiate a reconnect.

Additionally, we should emit events upon disconnect through QMP (now 
that we have that functionality).

The main reason I dislike automatic reconnecting is that there is no 
correct way to handle the period of time while the VM is disconnected.

A user might want to pause the VM, trigger a live migration to a 
non-broken system, checkpoint the VM, etc.

Auto reconnecting is implementing a policy to handle the failure within 
QEMU which is not universally the correct choice.  This isn't so bad 
except for the fact that you aren't providing the mechanisms for users 
to implement other policies which means they're stuck with this 
particular policy.

Regards,

Anthony Liguori

> ---
>   qemu-char.c   |  159 +++++++++++++++++++++++++++++++++++++++++++--------------
>   qemu-char.h   |    2 +
>   qemu-config.c |    3 +
>   vl.c          |    4 ++
>   4 files changed, 129 insertions(+), 39 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 800ee6c..016afd0 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,8 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
>   }
>   #endif
>
> +static void qemu_chr_sched_reconnect(TCPCharDriver *s);
> +
>   static void tcp_chr_read(void *opaque)
>   {
>       CharDriverState *chr = opaque;
> @@ -2030,10 +2036,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_sched_reconnect(s);
> +        } else {
> +            qemu_chr_event(chr, CHR_EVENT_CLOSED);
> +        }
>       } else if (size>  0) {
>           if (s->do_telnetopt)
>               tcp_chr_process_IAC_bytes(chr, s, buf,&size);
> @@ -2133,11 +2145,92 @@ static void tcp_chr_close(CharDriverState *chr)
>       qemu_chr_event(chr, CHR_EVENT_CLOSED);
>   }
>
> +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 QLIST_HEAD(reconnect_list_head, reconnect_list_entry) rcl_head;
> +
> +typedef struct reconnect_list_entry {
> +    TCPCharDriver *s;
> +    uint64_t when;
> +    QLIST_ENTRY(reconnect_list_entry) entries;
> +} reconnect_list_entry;
> +
> +static void qemu_chr_sched_reconnect(TCPCharDriver *s)
> +{
> +    reconnect_list_entry *new = qemu_malloc(sizeof(*new));
> +    struct timeval tv;
> +
> +    qemu_gettimeofday(&tv);
> +    new->s = s;
> +    new->when = (s->reconnect + tv.tv_sec) * 1000000 + tv.tv_usec;
> +    QLIST_INSERT_HEAD(&rcl_head, new, entries);
> +}
> +
> +void qemu_chr_reconnect(void)
> +{
> +    struct timeval tv;
> +    uint64_t now;
> +    reconnect_list_entry *np;
> +
> +    if (!rcl_head.lh_first)
> +        return;
> +
> +    gettimeofday(&tv, NULL);
> +    now = tv.tv_sec * 1000000 + tv.tv_usec;
> +
> +    for (np = rcl_head.lh_first; np != NULL; np = np->entries.le_next) {
> +        if (np->when<= now) {
> +            if (qemu_chr_connect_socket(np->s)) {
> +                qemu_chr_event(np->s->chr, CHR_EVENT_RECONNECTED);
> +                QLIST_REMOVE(np, entries);
> +            }
> +            else {
> +                np->when += np->s->reconnect * 1000000;
> +            }
> +        }
> +    }
> +}
> +
>   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 +2238,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 +2285,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,19 +2301,14 @@ 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;
>   }
>
> diff --git a/qemu-char.h b/qemu-char.h
> index bcc0766..32bcfd7 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -15,6 +15,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
> @@ -75,6 +76,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 c3203c8..a229350 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -144,6 +144,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 6f1e1ab..bcd7d44 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3719,6 +3719,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;
>    

Re: [Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Luiz Capitulino]

On Mon, 01 Feb 2010 09:25:27 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 02/01/2010 07:34 AM, Ian Molton wrote:
> > 	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>
> >    
> 
> I went back and looked at the last series and found my feedback.  I had 
> suggested that instead of automatically reconnecting, a mechanism should 
> be added for a user to initiate a reconnect.
> 
> Additionally, we should emit events upon disconnect through QMP (now 
> that we have that functionality).

 Should we merge all disconnect events or should we keep them
separated?

 I mean, we currently have VNC_DISCONNECT and will likely have
SPICE_DISCONNECT. Maybe we could have a SOCKET_DISCONNECT and have
a 'source' member, like:

{ "event": "SOCKET_DISCONNECT", "data": { "source": "vnc" ... } }

 I can see two drawbacks:

1. We can't do this for connects, which will make this inconsistent
2. Its "data" member is dependent on the source, which makes this
   event return a set of different info (say vnc auth types vs. auth
   types)

 So, I'd keep them separated.

Re: [Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Anthony Liguori]

On 02/01/2010 10:12 AM, Luiz Capitulino wrote:
> On Mon, 01 Feb 2010 09:25:27 -0600
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>    
>> On 02/01/2010 07:34 AM, Ian Molton wrote:
>>      
>>> 	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>
>>>
>>>        
>> I went back and looked at the last series and found my feedback.  I had
>> suggested that instead of automatically reconnecting, a mechanism should
>> be added for a user to initiate a reconnect.
>>
>> Additionally, we should emit events upon disconnect through QMP (now
>> that we have that functionality).
>>      
>   Should we merge all disconnect events or should we keep them
> separated?
>
>   I mean, we currently have VNC_DISCONNECT and will likely have
> SPICE_DISCONNECT. Maybe we could have a SOCKET_DISCONNECT and have
> a 'source' member, like:
>    

Good question.

I'd suggest for now, keep them separate events.  We can always merge 
them into a single event later and deprecate the old events.  That will 
give us a better idea of what the data payload needs to be.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Ian Molton]

Anthony Liguori wrote:

> I went back and looked at the last series and found my feedback.  I had
> suggested that instead of automatically reconnecting, a mechanism should
> be added for a user to initiate a reconnect.

This sounds useful

> Additionally, we should emit events upon disconnect through QMP (now
> that we have that functionality).

This also.

> The main reason I dislike automatic reconnecting is that there is no
> correct way to handle the period of time while the VM is disconnected.

This is fine for egd protocol, the guest will just run low on entropy in
the meantime. Nothing should break.

> Auto reconnecting is implementing a policy to handle the failure within
> QEMU which is not universally the correct choice.  This isn't so bad
> except for the fact that you aren't providing the mechanisms for users
> to implement other policies which means they're stuck with this
> particular policy.

Perhaps this feature could be added if needed in future? It seems a bit
ambitious to get all this 'right' with no use cases to test against.

-Ian

Re: [Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Anthony Liguori]

On 02/01/2010 04:44 PM, Ian Molton wrote:
> Anthony Liguori wrote:
>
>    
>> I went back and looked at the last series and found my feedback.  I had
>> suggested that instead of automatically reconnecting, a mechanism should
>> be added for a user to initiate a reconnect.
>>      
> This sounds useful
>
>    
>> Additionally, we should emit events upon disconnect through QMP (now
>> that we have that functionality).
>>      
> This also.
>
>    
>> The main reason I dislike automatic reconnecting is that there is no
>> correct way to handle the period of time while the VM is disconnected.
>>      
> This is fine for egd protocol, the guest will just run low on entropy in
> the meantime. Nothing should break.
>    

Right, but you're adding a generic piece of functionality.  For 
instance, what's the result of using reconnect with virtio-console?  Is 
this something that is going to work well?

>> Auto reconnecting is implementing a policy to handle the failure within
>> QEMU which is not universally the correct choice.  This isn't so bad
>> except for the fact that you aren't providing the mechanisms for users
>> to implement other policies which means they're stuck with this
>> particular policy.
>>      
> Perhaps this feature could be added if needed in future? It seems a bit
> ambitious to get all this 'right' with no use cases to test against.
>    

I'm all for doing things incrementally but there has to be a big picture 
that the incremental bit fits into otherwise you end up with a bunch of 
random features that don't work together well.

Honestly, I'd strongly suggest splitting the reconnect logic out of the 
series when resubmitting.  I think it's just too hacky with too weak of 
a justification.  If you really want this functionality, we can discuss 
the right approach for doing it but it's gotta be done in a way that's 
not introducing a one-off case just for the random number generator.

Regards,

Anthony Liguori

> -Ian
>    

Re: [Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Ian Molton]

Anthony Liguori wrote:

> I'm all for doing things incrementally but there has to be a big picture
> that the incremental bit fits into otherwise you end up with a bunch of
> random features that don't work together well.

Well, if you just add stuff without ever changing anything that went
before, of course.

> Honestly, I'd strongly suggest splitting the reconnect logic out of the
> series when resubmitting.

IMO the RNG stuff is worthless without the reconnect logic. You cant
have a machine in a production environment that just stops getting
entropy forever when you (say) restart the EGD, perhaps during a package
update. Or when someone unplugs the entropy source temporarily or
something like that.

>  I think it's just too hacky with too weak of
> a justification.  If you really want this functionality, we can discuss
> the right approach for doing it but it's gotta be done in a way that's
> not introducing a one-off case just for the random number generator.

I dont think its a case of 'really want' as much as 'its completely
essential' :-)

I still think that unless there are any other use cases, theres not much
to discuss - The code is already generic to some degree - it notifies
users, and its got a configurable delay. What else do we need? I
implemented it generically rather than stuff it into the virtio-rng
driver *because* I didnt think a dedicated version of it was the right
way to go, but without some other use cases, I cant see what good there
is in bikeshedding over this?

-Ian