Re: [Qemu-devel] [PATCH v6 5/5] use inet_listen()/inet_connect() to support ipv6 migration

[Amos Kong]

On 18/04/12 10:17, Michael Roth wrote:
> On Tue, Apr 17, 2012 at 10:54:39PM +0800, Amos Kong wrote:
> > Use help functions in qemu-socket.c for tcp migration,
> > which already support ipv6 addresses.
> >
> > Currently errp will be set to UNDEFINED_ERROR when migration fails,
> > qemu would output "migration failed: ...", and current user can
> > see a message("An undefined error has occurred") in monitor.
> >
> > This patch changed tcp_start_outgoing_migration()/inet_connect()
> > /inet_connect_opts(), socket error would be passed back,
> > then current user can see a meaningful err message in monitor.
> >
> > Qemu will exit if listening fails, so output socket error
> > to qemu stderr.
> >
> > For IPv6 brackets must be mandatory if you require a port.
> > Referencing to RFC5952, the recommended format is:
> >    [2312::8274]:5200
> >
> > test status: Successed
> > listen side: qemu-kvm .... -incoming tcp:[2312::8274]:5200
> > client side: qemu-kvm ...
> >               (qemu) migrate -d tcp:[2312::8274]:5200
> >
> > Signed-off-by: Amos Kong<address@hidden>
> > ---
> >   migration-tcp.c |   74 +++++++++++++------------------------------------------
> >   migration.c     |   14 ++++++----
> >   migration.h     |    7 +++--
> >   vl.c            |    6 ++++
> >   4 files changed, 35 insertions(+), 66 deletions(-)
> >
> > diff --git a/migration-tcp.c b/migration-tcp.c
> > index 35a5781..1ecfd1e 100644
> > --- a/migration-tcp.c
> > +++ b/migration-tcp.c
> > @@ -79,45 +79,29 @@ static void tcp_wait_for_connect(void *opaque)
> >       }
> >   }
> >
> > -int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
> > +int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
> > +                                 Error **errp)
> >   {
> > -    struct sockaddr_in addr;
> > -    int ret;
> > -
> > -    ret = parse_host_port(&addr, host_port);
> > -    if (ret<  0) {
> > -        return ret;
> > -    }
> > -
> >       s->get_error = socket_errno;
> >       s->write = socket_write;
> >       s->close = tcp_close;
> >
> > -    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> > -    if (s->fd == -1) {
> > -        DPRINTF("Unable to open socket");
> > -        return -socket_error();
> > -    }
> > -
> > -    socket_set_nonblock(s->fd);
> > -
> > -    do {
> > -        ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> > -        if (ret == -1) {
> > -            ret = -socket_error();
> > -        }
> > -        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
> > -            qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> > -            return 0;
> > -        }
> > -    } while (ret == -EINTR);
> > +    s->fd = inet_connect(host_port, false, errp);
> >
> > -    if (ret<  0) {
> > +    if (error_is_type(*errp, QERR_SOCKET_CONNECT_IN_PROGRESS)) {
> > +        DPRINTF("connect in progress");
> > +        qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);


In this condition(most of time), and message will be outputted to 
monitor, is this ok?
 "migrate: Connection cannot be completed immediately"

> > +    } else if (error_is_type(*errp, QERR_SOCKET_CREATE_FAILED)) {
> > +        DPRINTF("connect failed\n");
> > +        return -1;
>
> Shouldn't this be handled the same way as CONNECT_FAILED?

migrate_fd_error() is used to set migration error state,
we only set the state when connect fails.
If the socket is not create successfully (create fails
/parse issue/etc), we will not set the state, user can
retry to migrate.

This behavior is same as original.

> > +    } else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
> >           DPRINTF("connect failed\n");
> >           migrate_fd_error(s);

             ^^^^^

> > -        return ret;
> > +        return -1;
> > +    } else {
> > +        migrate_fd_connect(s);
> >       }
> > -    migrate_fd_connect(s);

...

--
                        Amos.