qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 1/3] Refactor inet_connect_opts function


From: Amos Kong
Subject: Re: [Qemu-devel] [PATCH v3 1/3] Refactor inet_connect_opts function
Date: Wed, 19 Sep 2012 16:33:42 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0

On 14/09/12 02:58, Orit Wasserman wrote:
From: "Michael S. Tsirkin" <address@hidden>

refactor address resolution code to fix nonblocking connect
remove getnameinfo call

Signed-off-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Amos Kong <address@hidden>
Signed-off-by: Orit Wasserman <address@hidden>

Hi Orit,

Happy new year!

---
  qemu-sockets.c |  144 +++++++++++++++++++++++++++++++------------------------
  1 files changed, 81 insertions(+), 63 deletions(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 361d890..939a453 100644

...

+
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
+{
+    struct addrinfo *res, *e;
+    int sock = -1;
+    bool block = qemu_opt_get_bool(opts, "block", 0);
+
+    res = inet_parse_connect_opts(opts, errp);
+    if (!res) {
+        return -1;
+    }
+

+    if (in_progress) {
+        *in_progress = false;
      }

trivial comment:

You moved this block to head of inet_connect_addr() in [patch 3/3],
why not do this in this patch?

I know if *in_progress becomes "true", sock is always >= 0, for loop will exits.
But moving this block to inet_connect_addr() is clearer.


      for (e = res; e != NULL; e = e->ai_next) {
-        if (getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen,
-                            uaddr,INET6_ADDRSTRLEN,uport,32,
-                            NI_NUMERICHOST | NI_NUMERICSERV) != 0) {
-            fprintf(stderr,"%s: getnameinfo: oops\n", __FUNCTION__);
-            continue;
-        }
-        sock = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
-        if (sock < 0) {
-            fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
-            inet_strfamily(e->ai_family), strerror(errno));
-            continue;
-        }
-        setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
-        if (!block) {
-            socket_set_nonblock(sock);
+        sock = inet_connect_addr(e, block, in_progress);
+        if (sock >= 0) {
+            break;
          }
-        /* connect to peer */
-        do {
-            rc = 0;
-            if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
-                rc = -socket_error();
-            }
-        } while (rc == -EINTR);
-
-  #ifdef _WIN32
-        if (!block && (rc == -EINPROGRESS || rc == -EWOULDBLOCK
-                       || rc == -WSAEALREADY)) {
-  #else
-        if (!block && (rc == -EINPROGRESS)) {
-  #endif
-            if (in_progress) {
-                *in_progress = true;
-            }
-        } else if (rc < 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;
-        }
-        freeaddrinfo(res);
-        return sock;
      }
-    error_set(errp, QERR_SOCKET_CONNECT_FAILED);
+    if (sock < 0) {
+        error_set(errp, QERR_SOCKET_CONNECT_FAILED);
+    }
      freeaddrinfo(res);
-    return -1;
+    return sock;
  }

  int inet_dgram_opts(QemuOpts *opts)


--
                        Amos.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]