diff mbox

[18/29] qemu-sockets: add error propagation to inet_connect_addr

Message ID 1350653528-5834-19-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Oct. 19, 2012, 1:31 p.m. UTC
perror and fprintf can be removed because all clients can now consume
Errors properly.  However, we'll need to change the non-blocking connect
handlers to take an Error, in order to improve error handling for
migration with the TCP protocol.

This is a minor degradation in error reporting for outgoing migration.
However, until 1.2 this case just failed without even attempting to
connect, so it is still an improvement as far as overall QoI is
concerned.

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 15 ++++++---------
 1 file modificato, 6 inserzioni(+), 9 rimozioni(-)

Comments

Markus Armbruster Oct. 22, 2012, 3:53 p.m. UTC | #1
Paolo Bonzini <pbonzini@redhat.com> writes:

> perror and fprintf can be removed because all clients can now consume
> Errors properly.  However, we'll need to change the non-blocking connect
> handlers to take an Error, in order to improve error handling for
> migration with the TCP protocol.
>
> This is a minor degradation in error reporting for outgoing migration.
> However, until 1.2 this case just failed without even attempting to
> connect, so it is still an improvement as far as overall QoI is
> concerned.
>
> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-sockets.c | 15 ++++++---------
>  1 file modificato, 6 inserzioni(+), 9 rimozioni(-)
>
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index 9e3d233..88d58fe 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -216,7 +216,7 @@ typedef struct ConnectState {
>  } ConnectState;
>  
>  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
> -                             ConnectState *connect_state);
> +                             ConnectState *connect_state, Error **errp);
>  
>  static void wait_for_connect(void *opaque)
>  {
> @@ -246,7 +246,7 @@ static void wait_for_connect(void *opaque)
>      if (s->current_addr) {
>          while (s->current_addr->ai_next != NULL && s->fd < 0) {
>              s->current_addr = s->current_addr->ai_next;
> -            s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
> +            s->fd = inet_connect_addr(s->current_addr, &in_progress, s, NULL);
>              /* connect in progress */
>              if (in_progress) {
>                  return;
> @@ -263,7 +263,7 @@ static void wait_for_connect(void *opaque)
>  }
>  
>  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
> -                             ConnectState *connect_state)
> +                             ConnectState *connect_state, Error **errp)
>  {
>      int sock, rc;
>  
> @@ -271,8 +271,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>  
>      sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
>      if (sock < 0) {
> -        fprintf(stderr, "%s: socket(%s): %s\n", __func__,
> -                inet_strfamily(addr->ai_family), strerror(errno));
> +        error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
>          return -1;
>      }
>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
> @@ -293,6 +292,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>                               connect_state);
>          *in_progress = true;
>      } else if (rc < 0) {
> +        error_set_errno(errp, errno, QERR_SOCKET_CONNECT_FAILED);
>          closesocket(sock);
>          return -1;
>      }

Looks like this reports a previously unreported error condition.

If it does, it's a fix worth mentioning in commit message.

> @@ -375,7 +375,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
>          if (connect_state != NULL) {
>              connect_state->current_addr = e;
>          }
> -        sock = inet_connect_addr(e, &in_progress, connect_state);
> +        sock = inet_connect_addr(e, &in_progress, connect_state, errp);
>          if (in_progress) {
>              return sock;
>          } else if (sock >= 0) {
> @@ -386,9 +386,6 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
>              break;
>          }
>      }
> -    if (sock < 0) {
> -        error_set(errp, QERR_SOCKET_CONNECT_FAILED);
> -    }
>      g_free(connect_state);
>      freeaddrinfo(res);
>      return sock;
diff mbox

Patch

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 9e3d233..88d58fe 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -216,7 +216,7 @@  typedef struct ConnectState {
 } ConnectState;
 
 static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
-                             ConnectState *connect_state);
+                             ConnectState *connect_state, Error **errp);
 
 static void wait_for_connect(void *opaque)
 {
@@ -246,7 +246,7 @@  static void wait_for_connect(void *opaque)
     if (s->current_addr) {
         while (s->current_addr->ai_next != NULL && s->fd < 0) {
             s->current_addr = s->current_addr->ai_next;
-            s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
+            s->fd = inet_connect_addr(s->current_addr, &in_progress, s, NULL);
             /* connect in progress */
             if (in_progress) {
                 return;
@@ -263,7 +263,7 @@  static void wait_for_connect(void *opaque)
 }
 
 static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
-                             ConnectState *connect_state)
+                             ConnectState *connect_state, Error **errp)
 {
     int sock, rc;
 
@@ -271,8 +271,7 @@  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
 
     sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
     if (sock < 0) {
-        fprintf(stderr, "%s: socket(%s): %s\n", __func__,
-                inet_strfamily(addr->ai_family), strerror(errno));
+        error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
     qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
@@ -293,6 +292,7 @@  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
                              connect_state);
         *in_progress = true;
     } else if (rc < 0) {
+        error_set_errno(errp, errno, QERR_SOCKET_CONNECT_FAILED);
         closesocket(sock);
         return -1;
     }
@@ -375,7 +375,7 @@  int inet_connect_opts(QemuOpts *opts, Error **errp,
         if (connect_state != NULL) {
             connect_state->current_addr = e;
         }
-        sock = inet_connect_addr(e, &in_progress, connect_state);
+        sock = inet_connect_addr(e, &in_progress, connect_state, errp);
         if (in_progress) {
             return sock;
         } else if (sock >= 0) {
@@ -386,9 +386,6 @@  int inet_connect_opts(QemuOpts *opts, Error **errp,
             break;
         }
     }
-    if (sock < 0) {
-        error_set(errp, QERR_SOCKET_CONNECT_FAILED);
-    }
     g_free(connect_state);
     freeaddrinfo(res);
     return sock;