[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to
From: |
Amos Kong |
Subject: |
Re: [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to support nonblock socket |
Date: |
Wed, 21 Mar 2012 23:13:12 -0400 (EDT) |
----- Original Message -----
> On Mon, Mar 19, 2012 at 10:11:54PM +0800, Amos Kong wrote:
> > Change inet_connect(const char *str, int socktype) to
> > inet_connect(const char *str, bool block, int *sock_err),
> > socktype is unused, block is used to assign if set socket
> > to block/nonblock, sock_err is used to restore socket error.
> >
> > Connect's successful for nonblock socket when following errors are
> > returned:
> > -EINPROGRESS or -EWOULDBLOCK
> > -WSAEALREADY or -WSAEINVAL (win32)
> >
> > Also change the wrap function inet_connect_opts(QemuOpts *opts)
> > to inet_connect_opts(QemuOpts *opts, int *sock_err).
> >
> > Add a bool entry(block) for dummy_opts to tag block type.
> > Change nbd, vnc to use new interface.
> >
> > Signed-off-by: Amos Kong <address@hidden>
>
> Looks good, and non-blocking support for qemu-socket interfaces is
> pretty
> useful in and of itself. One general suggestion I would make is to
> use Error to
> pass back errors to the callers rather than an int*.
I have implemented it, thanks
> Some additional comments below.
>
> > ---
> > nbd.c | 2 +-
> > qemu-char.c | 2 +-
> > qemu-sockets.c | 73
> > ++++++++++++++++++++++++++++++++++++++++++++------------
> > qemu_socket.h | 4 ++-
> > ui/vnc.c | 2 +-
> > 5 files changed, 62 insertions(+), 21 deletions(-)
> >
> > diff --git a/nbd.c b/nbd.c
> > index 567e94e..ad4de06 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, NULL);
> > }
> >
> > int tcp_socket_incoming(const char *address, uint16_t port)
> > diff --git a/qemu-char.c b/qemu-char.c
> > index bb9e3f5..d3543ea 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -2443,7 +2443,7 @@ static CharDriverState
> > *qemu_chr_open_socket(QemuOpts *opts)
> > if (is_listen) {
> > fd = inet_listen_opts(opts, 0);
> > } else {
> > - fd = inet_connect_opts(opts);
> > + fd = inet_connect_opts(opts, NULL);
> > }
> > }
> > if (fd < 0) {
> > diff --git a/qemu-sockets.c b/qemu-sockets.c
> > index 6bcb8e3..8ed45f8 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 */ }
> > },
> > @@ -194,14 +197,15 @@ listen:
> > return slisten;
> > }
> >
> > -int inet_connect_opts(QemuOpts *opts)
> > +int inet_connect_opts(QemuOpts *opts, int *sock_err)
> > {
> > struct addrinfo ai,*res,*e;
> > const char *addr;
> > 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;
> > }
> >
> > 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;
> > }
> >
> > for (e = res; e != NULL; e = e->ai_next) {
> > @@ -241,21 +248,52 @@ 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();
> > + if (block) {
> > + break;
Above code is used to keep behavior same as original, return this error(connect
will be failed)
analysis about process EINTR :
http://www.madore.org/~david/computers/connect-intr.html
""" If connect() is interrupted by a signal that is caught while blocked
waiting to establish a connection, connect() shall fail and set connect() to
[EINTR], but the connection request shall not be aborted, and the connection
shall be established asynchronously."""
When EINTR is got, what will happen if re connect()?
1. block until connected
2. return -1, set errno to EALREADY
However re-connect is not wrong.
-----
/* connect to peer */
do {
err = 0;
if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
err = -socket_error();
}
} while (err == -EINTR || err == -EWOULDBLOCK);
if (err >= 0) {
goto success;
} else if (!block && err == -EINPROGRESS) {
goto success;
> > + }
>
> Can still get an EINTR even in blocking mode,
EINTR can be got in blocking socket. not sure if it can _Only_ be got in
blocking socket ?
http://pubs.opengroup.org/onlinepubs/009695399/functions/connect.html
""" If the initiating socket is connection-mode, then connect() shall attempt
to establish a connection to the address specified by the address argument. If
the connection cannot be established immediately and O_NONBLOCK is not set for
the file descriptor for the socket, connect() shall block for up to an
unspecified timeout interval until the connection is established. If the
timeout interval expires before the connection is established, connect() shall
fail and the connection attempt shall be aborted. 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], but the connection request shall
not be aborted, and the connection shall be established asynchronously.
If the connection cannot be established immediately and O_NONBLOCK is set for
the file descriptor for the socket, connect() shall fail and set errno to
[EINPROGRESS], but the connection request shall not be aborted, and the
connection shall be established asynchronously. Subsequent calls to connect()
for the same socket, before the connection is established, shall fail and set
errno to [EALREADY]. """
> is breaking in that case intentional?
>
> > + }
> > + } while (err == -EINTR);
>
> > +
> > + if (err >= 0) {
> > + goto success;
> > + } else if (!block && (err == -EINPROGRESS || err ==
> > -EWOULDBLOCK)) {
> > + goto success;
>
> EWOULDBLOCK should actually be an error:
Yes, need to reconnect.
> EWOULDBLOCK == EAGAIN:
right. macro EWOULDBLOCK is another name for EAGAIN
http://www.gnu.org/software/libc/manual/html_node/Error-Codes.html
> No more free local ports or insufficient entries in the
> routing
> cache. For AF_INET see the description
> of
> /proc/sys/net/ipv4/ip_local_port_range ip(7) for information on
> how
> to increase the number of local ports.
>
> There's nothing useful they can do with the socket fd you pass back
> except try
> to call connect() on it again.
- [Qemu-devel] [PATCH v4 0/2] support to migrate with IPv6 address, Amos Kong, 2012/03/19
- [Qemu-devel] [PATCH v4 2/2] use inet_listen()/inet_connect() to support ipv6 migration, Amos Kong, 2012/03/19
- [Qemu-devel] [PATCH v4 1/2] qemu-socket: change inet_connect() to to support nonblock socket, Amos Kong, 2012/03/19
- [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, 2012/03/27
- 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