[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 2/4] qemu-socket: change inet_connect() to to
From: |
Amos Kong |
Subject: |
Re: [Qemu-devel] [PATCH v5 2/4] qemu-socket: change inet_connect() to to support nonblock socket |
Date: |
Tue, 27 Mar 2012 12:14:09 -0400 (EDT) |
----- 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)
> >
> > Add a bool entry(block) for dummy_opts to tag block type.
> >
> > Use set_socket_error() to set real errno, set errno to
> > EINVAL for parse error.
> >
> > Change nbd, vnc to use new interface.
> >
> > Signed-off-by: Amos Kong <address@hidden>
> > ---
> > nbd.c | 2 +-
> > qemu-sockets.c | 66
> > +++++++++++++++++++++++++++++++++++++++++++-------------
> > qemu_socket.h | 2 +-
> > ui/vnc.c | 2 +-
> > 4 files changed, 54 insertions(+), 18 deletions(-)
> >
> > diff --git a/nbd.c b/nbd.c
> > index 567e94e..3618344 100644
> > --- a/nbd.c
> > +++ b/nbd.c
> > @@ -146,7 +146,7 @@ int tcp_socket_outgoing(const char *address,
> > uint16_t port)
> >
> > int tcp_socket_outgoing_spec(const char *address_and_port)
> > {
> > - return inet_connect(address_and_port, SOCK_STREAM);
> > + return inet_connect(address_and_port, true);
> > }
> >
> > int tcp_socket_incoming(const char *address, uint16_t port)
> > diff --git a/qemu-sockets.c b/qemu-sockets.c
> > index 6bcb8e3..908479e 100644
> > --- a/qemu-sockets.c
> > +++ b/qemu-sockets.c
> > @@ -51,6 +51,9 @@ static QemuOptsList dummy_opts = {
> > },{
> > .name = "ipv6",
> > .type = QEMU_OPT_BOOL,
> > + },{
> > + .name = "block",
> > + .type = QEMU_OPT_BOOL,
> > },
> > { /* end if list */ }
> > },
> > @@ -201,7 +204,8 @@ int inet_connect_opts(QemuOpts *opts)
> > const char *port;
> > char uaddr[INET6_ADDRSTRLEN+1];
> > char uport[33];
> > - int sock,rc;
> > + int sock, rc, err;
> > + bool block;
> >
> > memset(&ai,0, sizeof(ai));
> > ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
> > @@ -210,9 +214,11 @@ int inet_connect_opts(QemuOpts *opts)
> >
> > addr = qemu_opt_get(opts, "host");
> > port = qemu_opt_get(opts, "port");
> > + block = qemu_opt_get_bool(opts, "block", 0);
> > if (addr == NULL || port == NULL) {
> > fprintf(stderr, "inet_connect: host and/or port not
> > specified\n");
> > - return -1;
> > + err = -EINVAL;
> > + goto err;
>
> I would call set_socket_error with -EINVAL and than return -1;
Hi Orit,
This is what I did.
restore "-EINVAL" to 'err', set_socket_error(-err), then return -1
> > }
> >
> > if (qemu_opt_get_bool(opts, "ipv4", 0))
> > @@ -224,7 +230,8 @@ int inet_connect_opts(QemuOpts *opts)
> > if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) {
> > fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
> > gai_strerror(rc));
> > - return -1;
> > + err = -EINVAL;
> > + goto err;
>
> I would call set_socket_error with EINVAL and than return -1;
>
> > }
> >
> > 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))
;
> > + 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;
> > }
> >
> > int inet_dgram_opts(QemuOpts *opts)
> > @@ -449,14 +479,20 @@ int inet_listen(const char *str, char *ostr,
> > int olen,
> > return sock;
> > }
> >
> > -int inet_connect(const char *str, int socktype)
> > +int inet_connect(const char *str, bool block)
> > {
> > QemuOpts *opts;
> > int sock = -1;
> >
> > opts = qemu_opts_create(&dummy_opts, NULL, 0);
> > - if (inet_parse(opts, str) == 0)
> > + if (inet_parse(opts, str) == 0) {
> > + if (block) {
> > + qemu_opt_set(opts, "block", "on");
> > + }
> > sock = inet_connect_opts(opts);
> > + } else {
> > + set_socket_error(EINVAL);
> > + }
> > qemu_opts_del(opts);
> > return sock;
> > }
> > diff --git a/qemu_socket.h b/qemu_socket.h
> > index a4c5170..f86cd3f 100644
> > --- a/qemu_socket.h
> > +++ b/qemu_socket.h
> > @@ -47,7 +47,7 @@ int inet_listen_opts(QemuOpts *opts, int
> > port_offset);
> > int inet_listen(const char *str, char *ostr, int olen,
> > int socktype, int port_offset);
> > int inet_connect_opts(QemuOpts *opts);
> > -int inet_connect(const char *str, int socktype);
> > +int inet_connect(const char *str, bool block);
> > int inet_dgram_opts(QemuOpts *opts);
> > const char *inet_strfamily(int family);
> >
> > diff --git a/ui/vnc.c b/ui/vnc.c
> > index deb9ecd..4a96153 100644
> > --- a/ui/vnc.c
> > +++ b/ui/vnc.c
> > @@ -3068,7 +3068,7 @@ int vnc_display_open(DisplayState *ds, const
> > char *display)
> > if (strncmp(display, "unix:", 5) == 0)
> > vs->lsock = unix_connect(display+5);
> > else
> > - vs->lsock = inet_connect(display, SOCK_STREAM);
> > + vs->lsock = inet_connect(display, true);
> > if (-1 == vs->lsock) {
> > g_free(vs->display);
> > vs->display = NULL;
> >
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to address@hidden
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
- [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to support nonblock socket, (continued)
- [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to support nonblock socket, Amos Kong, 2012/03/19
- Re: [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to support nonblock socket, Orit Wasserman, 2012/03/20
- Re: [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to support nonblock socket, Michael Roth, 2012/03/21
- Re: [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to support nonblock socket, Amos Kong, 2012/03/21
- [Qemu-devel] [PATCH v5 0/2] support to migrate with IPv6 address, Amos Kong, 2012/03/21
- [Qemu-devel] [PATCH v5 1/4] sockets: introduce set_socket_error(), Amos Kong, 2012/03/21
- Re: [Qemu-devel] [PATCH v5 1/4] sockets: introduce set_socket_error(), Orit Wasserman, 2012/03/27
- [Qemu-devel] [PATCH v6 1/4] sockets: introduce set_socket_error(), Amos Kong, 2012/03/27
- [Qemu-devel] [PATCH v5 2/4] qemu-socket: change inet_connect() to to support nonblock socket, Amos Kong, 2012/03/21
- Re: [Qemu-devel] [PATCH v5 2/4] qemu-socket: change inet_connect() to to support nonblock socket, Orit Wasserman, 2012/03/27
- Re: [Qemu-devel] [PATCH v5 2/4] qemu-socket: change inet_connect() to to support nonblock socket,
Amos Kong <=
- Re: [Qemu-devel] [PATCH v5 2/4] qemu-socket: change inet_connect() to to support nonblock socket, Amos Kong, 2012/03/27
- [Qemu-devel] [PATCH v5 3/4] sockets: pass back errors in inet_listen(), Amos Kong, 2012/03/21
- Re: [Qemu-devel] [PATCH v5 3/4] sockets: pass back errors in inet_listen(), Michael Roth, 2012/03/27
- [Qemu-devel] [PATCH v5 4/4] use inet_listen()/inet_connect() to support ipv6 migration, Amos Kong, 2012/03/21