Re: [Qemu-devel] [PATCH] chardev: Add reconnecting to client sockets

[Paolo Bonzini <pbonzini@redhat.com>]

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