Message ID | ecac511f8db78f6e176faae7a7343b183c7e3305.1316782367.git.quintela@redhat.com |
---|---|
State | New |
Headers | show |
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"); >
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 --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");
Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration-tcp.c | 4 ++-- migration-unix.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)