Re: [Qemu-devel] [PATCH 17/34] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS

[Amos Kong]

----- Original Message -----
> [cc: Amos]
> 
> Luiz Capitulino <address@hidden> writes:
> 
> > This error is currently returned by inet_connect_opts(), however
> > it causes the follow spurious message on HMP:
> >
> >     (qemu) migrate tcp:0:4444
> >     migrate: Connection can not be completed immediately

We use non-block socket for migration, connect() might return
before connection complete.

However, I agree with remove this note.


> >     (qemu)
> >
> > But migration succeeds.
>
> Broken in commit d5c5dacc.
> 
> Commit a6ba35b3 earlier in the same series might have broken other
> users
> of inet_connect() and inet_connect_opts() similarly.  Would be nice
> to
> know.  Whatever was brolen, your patch fixes it, too.


INPROGRESS 'error' will only be returned for non-block socket.
Other users(vnc, nbd) use block socket, and they pass a 'NULL'
to second argument of inet_connect(QemuOpts *opts, Error **errp),
so this Error doesn't effect them.


> > inet_connect_opts() has a 'in_progress' argument that callers can
> > use to check whether a connection is in progress. The QERR_ macro
> > is not needed anymore.
> >
> > PS: I didn't test with QMP, but I guess the migrate command will
> >     return an error response.
> 
> Plausible.
> 
> I'd squash this into PATCH 14, because the purpose of the combined
> patch
> will be obvious.  Right now, 14's isn't.
> 
> >
> > Signed-off-by: Luiz Capitulino <address@hidden>
> > ---
> >  qemu-sockets.c | 2 --
> >  qerror.c       | 4 ----
> >  qerror.h       | 3 ---
> >  3 files changed, 9 deletions(-)
> >
> > diff --git a/qemu-sockets.c b/qemu-sockets.c
> > index 82f4736..7196c5f 100644
> > --- a/qemu-sockets.c
> > +++ b/qemu-sockets.c
> > @@ -284,8 +284,6 @@ int inet_connect_opts(QemuOpts *opts, bool
> > *in_progress, Error **errp)
> >              if (in_progress) {
> >                  *in_progress = true;
> >              }
> > -
> > -            error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
> >          } else if (rc < 0) {
> >              if (NULL == e->ai_next)
> >                  fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n",
> >                  __FUNCTION__,
> > diff --git a/qerror.c b/qerror.c
> > index 691d8a8..33b8780 100644
> > --- a/qerror.c
> > +++ b/qerror.c
> > @@ -309,10 +309,6 @@ static const QErrorStringTable qerror_table[]
> > = {
> >          .desc      = "Could not start VNC server on %(target)",
> >      },
> >      {
> > -        .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
> > -        .desc      = "Connection can not be completed
> > immediately",
> > -    },
> > -    {
> >          .error_fmt = QERR_SOCKET_CONNECT_FAILED,
> >          .desc      = "Failed to connect to socket",
> >      },
> > diff --git a/qerror.h b/qerror.h
> > index de8497d..52ce58d 100644
> > --- a/qerror.h
> > +++ b/qerror.h
> > @@ -240,9 +240,6 @@ char *qerror_format(const char *fmt, QDict
> > *error);
> >  #define QERR_VNC_SERVER_FAILED \
> >      "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
> >  
> > -#define QERR_SOCKET_CONNECT_IN_PROGRESS \
> > -    "{ 'class': 'SockConnectInprogress', 'data': {} }"
> > -
> >  #define QERR_SOCKET_CONNECT_FAILED \
> >      "{ 'class': 'SockConnectFailed', 'data': {} }"
>