diff mbox

[21/23] migration: Don't use callback on file defining it

Message ID ecac511f8db78f6e176faae7a7343b183c7e3305.1316782367.git.quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela Sept. 23, 2011, 12:57 p.m. UTC
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-tcp.c  |    4 ++--
 migration-unix.c |    6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Anthony Liguori Oct. 4, 2011, 2:32 p.m. UTC | #1
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

I don't think this is better.  It just eliminates the possibility of having 
useful trace points in get_error.

Regards,

Anthony Liguori

> ---
>   migration-tcp.c  |    4 ++--
>   migration-unix.c |    6 +++---
>   2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index f6b2288..bd3aa3a 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -58,7 +58,7 @@ static void tcp_wait_for_connect(void *opaque)
>       DPRINTF("connect completed\n");
>       do {
>           ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *)&val,&valsize);
> -    } while (ret == -1&&  (s->get_error(s)) == EINTR);
> +    } while (ret == -1&&  (socket_error()) == EINTR);
>
>       if (ret<  0) {
>           migrate_fd_error(s);
> @@ -98,7 +98,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
>       do {
>           ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
>           if (ret == -1)
> -            ret = -(s->get_error(s));
> +            ret = -(socket_error());
>
>           if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
>               qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> diff --git a/migration-unix.c b/migration-unix.c
> index d6f315a..7205b25 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -57,7 +57,7 @@ static void unix_wait_for_connect(void *opaque)
>       DPRINTF("connect completed\n");
>       do {
>           ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *)&val,&valsize);
> -    } while (ret == -1&&  (s->get_error(s)) == EINTR);
> +    } while (ret == -1&&  errno == EINTR);
>
>       if (ret<  0) {
>           migrate_fd_error(s);
> @@ -97,7 +97,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
>       do {
>           ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
>           if (ret == -1)
> -	    ret = -(s->get_error(s));
> +            ret = -errno;
>
>           if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
>   	    qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
> @@ -130,7 +130,7 @@ static void unix_accept_incoming_migration(void *opaque)
>
>       do {
>           c = qemu_accept(s, (struct sockaddr *)&addr,&addrlen);
> -    } while (c == -1&&  socket_error() == EINTR);
> +    } while (c == -1&&  errno == EINTR);
>
>       DPRINTF("accepted migration\n");
>
Juan Quintela Oct. 4, 2011, 11:50 p.m. UTC | #2
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/23/2011 07:57 AM, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>
> I don't think this is better.  It just eliminates the possibility of
> having useful trace points in get_error.

It's use is not consistent at all (same places use get_error() and the
others the native ones).

And after this change, it gets clear what get_error() does, just four
uses:

migration.c:266:    } while (ret == -1 && ((s->get_error(s)) == EINTR));
migration.c:269:        ret = -(s->get_error(s));
migration.c:341:    } while (ret == -1 && (s->get_error(s)) == EINTR);
migration.c:344:        qemu_file_set_error(s->file, s->get_error(s));

The first two are the same one (just that we don't store the value in a
variable), same for the other two.

And everything uses errno for it, except migration-tcp on windows, that
is the only valid case for windows.

So, what we really need here is a way to get error of last system call,
abstracted by OS, not by migration method, and another call that we can
remove.

After the patch, this is easy to find (before it is more complicated).

So, patch stand and will search for a way to abstract for errno on
migration code. (just using socket_error() will work.

What do you think?

Later, Juan.
diff mbox

Patch

diff --git a/migration-tcp.c b/migration-tcp.c
index f6b2288..bd3aa3a 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -58,7 +58,7 @@  static void tcp_wait_for_connect(void *opaque)
     DPRINTF("connect completed\n");
     do {
         ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
-    } while (ret == -1 && (s->get_error(s)) == EINTR);
+    } while (ret == -1 && (socket_error()) == EINTR);

     if (ret < 0) {
         migrate_fd_error(s);
@@ -98,7 +98,7 @@  int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
         if (ret == -1)
-            ret = -(s->get_error(s));
+            ret = -(socket_error());

         if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
             qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
diff --git a/migration-unix.c b/migration-unix.c
index d6f315a..7205b25 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -57,7 +57,7 @@  static void unix_wait_for_connect(void *opaque)
     DPRINTF("connect completed\n");
     do {
         ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
-    } while (ret == -1 && (s->get_error(s)) == EINTR);
+    } while (ret == -1 && errno == EINTR);

     if (ret < 0) {
         migrate_fd_error(s);
@@ -97,7 +97,7 @@  int unix_start_outgoing_migration(MigrationState *s, const char *path)
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
         if (ret == -1)
-	    ret = -(s->get_error(s));
+            ret = -errno;

         if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
 	    qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
@@ -130,7 +130,7 @@  static void unix_accept_incoming_migration(void *opaque)

     do {
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
-    } while (c == -1 && socket_error() == EINTR);
+    } while (c == -1 && errno == EINTR);

     DPRINTF("accepted migration\n");