Re: [Qemu-devel] [PATCH v5 2/4] qemu-socket: change inet_connect() to to support nonblock socket

[Amos Kong]

----- Original Message -----
> ----- Original Message -----
> > On 03/22/2012 05:52 AM, Amos Kong wrote:
> > > Change inet_connect(const char *str, int socktype) to
> > > inet_connect(const char *str, bool block),
> > > socktype is unused, block is used to assign if set socket
> > > to block/nonblock.



> > > Retry to connect when -EINTR/-EWOULDBLOCK is got.
> > > Connect's successful for nonblock socket when following
> > > errors are got:
> > >   -EINPROGRESS
> > >   -WSAEALREADY/-WSAEINVAL (win32)


      Retry to connect when -EINTR(win32 & posix)/-EWOULDBLOCK(only win32) is 
got.
      Connect's successful for nonblock socket when following
      errors are got:
          -EINPROGRESS
          -WSAEALREADY(win32)


Discussed with <mdroth> in IRC.
For win32, need to re-connect when EWOULDBLOCK is got.
(http://msdn.microsoft.com/en-us/library/windows/desktop/ms740668(v=vs.85).aspx)

For posix, it's safe to treat it as fail, otherwise, 
it might hang qemu on the order of seconds. (man 2 connect : EAGAIN)

For non-blocking socket, EINPROGRESS maybe got in (win32 & posix),
but WSAEALREADY can only be got after re-connecting 
(EWOULDBLOCK is got in first connecting)

Please correct me if something is wrong, thanks.

...

> > >      for (e = res; e != NULL; e = e->ai_next) {
> > > @@ -241,21 +248,44 @@ int inet_connect_opts(QemuOpts *opts)
> > >              continue;
> > >          }
> > >          setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
> > > -
> > > +        if (!block) {
> > > +            socket_set_nonblock(sock);
> > > +        }
> > >          /* connect to peer */
> > > -        if (connect(sock,e->ai_addr,e->ai_addrlen) < 0) {
> > > -            if (NULL == e->ai_next)
> > > -                fprintf(stderr, "%s: connect(%s,%s,%s,%s):
> > > %s\n",
> > > __FUNCTION__,
> > > -                        inet_strfamily(e->ai_family),
> > > -                        e->ai_canonname, uaddr, uport,
> > > strerror(errno));
> > > -            closesocket(sock);
> > > -            continue;
> > > +        do {
> > > +            err = 0;
> > > +            if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
> > > +                err = -socket_error();
> > > +            }
> > > +        } while (err == -EINTR || err == -EWOULDBLOCK);
> > > +
> > 
> > this is only relevant for non blocking socket
> 
> For blocking socket, if connect() is interrupted by a signal
> that is caught while blocked waiting to establish a connection,
> connect() shall fail and set errno to [EINTR]
> So just re-connect when EINTR/EWOULDBLOCK are set.
> 
> http://marc.info/?l=kvm&m=133238606300535&w=2
> 
> 
> > it should look something like this :
> > 
> > while ( (rc = connect(sock, e->ai_addr, e->ai_addrlen)) < 0 &&
> > !block
> > &&
> >     (socket_error() == EINTR || socket_error() == -EWOULDBLOCK))
> 
> 
>   while ( (rc = connect(sock, e->ai_addr, e->ai_addrlen)) < 0  &&
>       (socket_error() == EINTR || socket_error() == EWOULDBLOCK))
>         ;


        do {
                err = 0;
                if (rc = connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
                        err = -socket_error();
                }
#ifndef _WIN32
        } while (err == -EINTR || err == -EWOULDBLOCK);
#else
        } while (err == -EINTR);
#endif

        if (rc >= 0) {
                goto success;
        } else if (!block && err == -EINPROGRESS) {
                goto success;
#ifndef _WIN32
        } else if (!block && err == -WSAEALREADY) {
                goto success;
#endif
        }


> > > +        if (err >= 0) {
> > > +            goto success;
> > > +        } else if (!block && err == -EINPROGRESS) {
> > > +            goto success;
> > > +#ifdef _WIN32
> > > +        } else if (!block && (err == -WSAEALREADY || err ==
> > > -WSAEINVAL)) {
> > > +            goto success;
> > > +#endif
> > 
> > I prefer to check for error the code should look:
> >
> >     if (rc < 0 &&  (block ||
> > #ifdef _WIN32
> >        (socket_error() != WSAEALREADY && socket_error() !=
> >        -WSAEINVAL))
> >        {
> > #else
> >     (socket_error() != EINPROGRESS)) {
> > #endif
> >        if (NULL == e->ai_next)
> >                fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n",
> >                __func__,
> >                    inet_strfamily(e->ai_family),
> >                   e->ai_canonname, uaddr, uport, strerror(errno));
> >        closesocket(sock);
> >        continue;
> >     }
> > ...
> > 
> > Orit
> > >          }
> > > -        freeaddrinfo(res);
> > > -        return sock;
> > > +
> > > +        if (NULL == e->ai_next) {
> > > +            fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n",
> > > __func__,
> > > +                    inet_strfamily(e->ai_family),
> > > +                    e->ai_canonname, uaddr, uport,
> > > strerror(errno));
> > > +        }
> > > +        closesocket(sock);
> > >      }
> > >      freeaddrinfo(res);
> 
> 
> > > +
> > > +err:
> > > +    set_socket_error(-err);
> > >      return -1;
> 
> 
> > > +
> > > +success:
> > > +    freeaddrinfo(res);
> > > +    set_socket_error(-err);
> > > +    return sock;
> > >  }

...