Re: [Qemu-devel] Socket reconnection.

[Anthony Liguori <anthony@codemonkey.ws>]

Ian Molton wrote:
> Fresh patch attached. Anthony, if this is ok, I can rebase this and its
> prerequisite.
>
> From 05581c5badd693b7537fe57f85a2ff5ddcb7972d 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   |  169 ++++++++++++++++++++++++++++++++++++++++++++-------------
>  qemu-char.h   |    2 +
>  qemu-config.c |    3 +
>  vl.c          |    4 ++
>  4 files changed, 139 insertions(+), 39 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index e202585..f20d697 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,61 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
>  }
>  #endif
>  
> +struct reconnect_list {
>   

CODING_STYLE is off (as I mentioned before).

> +    TCPCharDriver *s;
> +    uint64_t when;
> +    struct reconnect_list *next;
> +};
> +
> +static struct reconnect_list *rc_list;
> +
> +static void qemu_chr_sched_reconnect(TCPCharDriver *s)
> +{
> +    struct reconnect_list *new = qemu_malloc(sizeof(*new));
> +    struct timeval tv;
> +
> +    gettimeofday(&tv, NULL);
>   

This will break Win32 (use qemu_gettimeofday).

> +    new->s = s;
> +    new->when = (s->reconnect + tv.tv_sec) * 1000000 + tv.tv_usec;
> +    new->next = rc_list;
> +    rc_list = new;
>   

Don't open code a list, use one of the sys-queue types.

> +}
> +
> +static int qemu_chr_connect_socket(TCPCharDriver *s);
>   

Forward declarations usually imply some form of code motion is required.

> +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);
> +                this = prev;
> +            }
> +            else {
> +                this->when += this->s->reconnect * 1000000;
> +            }
> +        }
> +        prev = this;
> +	if (this)
> +            this = this->next;
> +    }
> +}
> +
>   

Mixing tabs and spaces.

Regards,

Anthony Liguori