Re: [Qemu-devel] [PATCH 15/35] migration: don't rely on any QERR_SOCKET_*

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 14 Aug 2012 13:35:59 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Juan Quintela <quintela@redhat.com> writes:
> 
> > Markus Armbruster <armbru@redhat.com> wrote:
> >> Luiz Capitulino <lcapitulino@redhat.com> writes:
> >>
> >>> Use the in_progress argument for QERR_SOCKET_CONNECT_IN_PROGRESS. The
> >>> other errors are handled the same by checking if the error is set and
> >>> then calling migrate_fd_error() if it's.
> >>>
> >>> It's also necessary to change inet_connect_opts() not to set
> >>> QERR_SOCKET_CONNECT_IN_PROGRESS. This error is only used by
> >>> tcp_start_outgoing_migration() and not changing it along with the
> >>> usage of in_progress would break migration.
> >>>
> >>> Furthermore this commit fixes a bug. Today, there's a spurious error
> >>> report when migration succeeds:
> >>>
> >>> (qemu) migrate tcp:0:4444
> >>> migrate: Connection can not be completed immediately
> >>> (qemu)
> >>>
> >>> After this commit no spurious error is reported anymore.
> >>>
> >>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> >>
> >> I'd prefer
> >>
> >>        s->fd = inet_connect(host_port, false, &in_progress, errp);
> >>        if (error_is_set(errp)) {
> >>            migrate_fd_error(s);
> >>            return -1;
> >>        }
> >>        if (in_progress) {
> >>            DPRINTF("connect in progress\n");
> >>            qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> >>        } else {
> >>            migrate_fd_connect(s);
> >>        }
> >>        return 0;
> >>
> >> because it separates abnormal and normal code paths more clearly.
> >>
> >> Matter of taste.
> >
> >
> > The 1st migrate_fd_error() is not needed (it was already wrong).
> >
> > migrate_fd_* functions are supposed to be called only after
> > migrate_fd_connect() has been called.
> 
> It's been committed.  Could you post a patch for it?

Juan, are you going to do this or do you want me to do it?