diff mbox

[08/12] migration: xxx_close will only be called once

Message ID 1350555758-29988-9-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Oct. 18, 2012, 10:22 a.m. UTC 
No need to test s->fd again, it is tested in the caller.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration-exec.c | 14 ++++++--------
 migration-fd.c   | 33 +++++++++++++++------------------
 migration-tcp.c  |  7 ++-----
 migration-unix.c |  7 ++-----
 4 file modificati, 25 inserzioni(+), 36 rimozioni(-)

Comments

Orit Wasserman Oct. 28, 2012, 9:53 a.m. UTC | #1 
On 10/18/2012 12:22 PM, Paolo Bonzini wrote:
> No need to test s->fd again, it is tested in the caller.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  migration-exec.c | 14 ++++++--------
>  migration-fd.c   | 33 +++++++++++++++------------------
>  migration-tcp.c  |  7 ++-----
>  migration-unix.c |  7 ++-----
>  4 file modificati, 25 inserzioni(+), 36 rimozioni(-)
> 
> diff --git a/migration-exec.c b/migration-exec.c
> index 0964dbb..1c562ab 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -48,14 +48,12 @@ static int exec_close(MigrationState *s)
>  {
>      int ret = 0;
>      DPRINTF("exec_close\n");
> -    if (s->opaque) {
> -        ret = qemu_fclose(s->opaque);
> -        s->opaque = NULL;
> -        s->fd = -1;
> -        if (ret >= 0 && !(WIFEXITED(ret) && WEXITSTATUS(ret) == 0)) {
> -            /* close succeeded, but non-zero exit code: */
> -            ret = -EIO; /* fake errno value */
> -        }
> +    ret = qemu_fclose(s->opaque);
> +    s->opaque = NULL;
> +    s->fd = -1;
> +    if (ret >= 0 && !(WIFEXITED(ret) && WEXITSTATUS(ret) == 0)) {
> +        /* close succeeded, but non-zero exit code: */
> +        ret = -EIO; /* fake errno value */
>      }
>      return ret;
>  }
> diff --git a/migration-fd.c b/migration-fd.c
> index bb91e0a..67ee3d1 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -48,29 +48,26 @@ static int fd_close(MigrationState *s)
>      int ret;
>  
>      DPRINTF("fd_close\n");
> -    if (s->fd != -1) {
> -        ret = fstat(s->fd, &st);
> -        if (ret == 0 && S_ISREG(st.st_mode)) {
> -            /*
> -             * If the file handle is a regular file make sure the
> -             * data is flushed to disk before signaling success.
> -             */
> -            ret = fsync(s->fd);
> -            if (ret != 0) {
> -                ret = -errno;
> -                perror("migration-fd: fsync");
> -                return ret;
> -            }
> -        }
> -        ret = close(s->fd);
> -        s->fd = -1;
> +    ret = fstat(s->fd, &st);
> +    if (ret == 0 && S_ISREG(st.st_mode)) {
> +        /*
> +         * If the file handle is a regular file make sure the
> +         * data is flushed to disk before signaling success.
> +         */
> +        ret = fsync(s->fd);
>          if (ret != 0) {
>              ret = -errno;
> -            perror("migration-fd: close");
> +            perror("migration-fd: fsync");
>              return ret;
>          }
>      }
> -    return 0;
> +    ret = close(s->fd);
> +    s->fd = -1;
> +    if (ret != 0) {
> +        ret = -errno;
> +        perror("migration-fd: close");
> +    }
> +    return ret;
>  }
>  
>  int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
> diff --git a/migration-tcp.c b/migration-tcp.c
> index e797d23..07715de 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -44,11 +44,8 @@ static int tcp_close(MigrationState *s)
>  {
>      int r = 0;
>      DPRINTF("tcp_close\n");
> -    if (s->fd != -1) {
> -        if (closesocket(s->fd) < 0) {
> -            r = -errno;
> -        }
> -        s->fd = -1;
> +    if (closesocket(s->fd) < 0) {
> +        r = -socket_error();
>      }
>      return r;
>  }
> diff --git a/migration-unix.c b/migration-unix.c
> index a407af2..def1969 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -44,11 +44,8 @@ static int unix_close(MigrationState *s)
>  {
>      int r = 0;
>      DPRINTF("unix_close\n");
> -    if (s->fd != -1) {
> -        if (close(s->fd) < 0) {
> -            r = -errno;
> -        }
> -        s->fd = -1;
> +    if (close(s->fd) < 0) {
> +        r = -errno;
>      }
>      return r;
>  }
> 
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Juan Quintela Oct. 30, 2012, 12:35 p.m. UTC | #2 
Paolo Bonzini <pbonzini@redhat.com> wrote:
> No need to test s->fd again, it is tested in the caller.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>
diff mbox

Patch

diff --git a/migration-exec.c b/migration-exec.c
index 0964dbb..1c562ab 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -48,14 +48,12 @@  static int exec_close(MigrationState *s)
 {
     int ret = 0;
     DPRINTF("exec_close\n");
-    if (s->opaque) {
-        ret = qemu_fclose(s->opaque);
-        s->opaque = NULL;
-        s->fd = -1;
-        if (ret >= 0 && !(WIFEXITED(ret) && WEXITSTATUS(ret) == 0)) {
-            /* close succeeded, but non-zero exit code: */
-            ret = -EIO; /* fake errno value */
-        }
+    ret = qemu_fclose(s->opaque);
+    s->opaque = NULL;
+    s->fd = -1;
+    if (ret >= 0 && !(WIFEXITED(ret) && WEXITSTATUS(ret) == 0)) {
+        /* close succeeded, but non-zero exit code: */
+        ret = -EIO; /* fake errno value */
     }
     return ret;
 }
diff --git a/migration-fd.c b/migration-fd.c
index bb91e0a..67ee3d1 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -48,29 +48,26 @@  static int fd_close(MigrationState *s)
     int ret;
 
     DPRINTF("fd_close\n");
-    if (s->fd != -1) {
-        ret = fstat(s->fd, &st);
-        if (ret == 0 && S_ISREG(st.st_mode)) {
-            /*
-             * If the file handle is a regular file make sure the
-             * data is flushed to disk before signaling success.
-             */
-            ret = fsync(s->fd);
-            if (ret != 0) {
-                ret = -errno;
-                perror("migration-fd: fsync");
-                return ret;
-            }
-        }
-        ret = close(s->fd);
-        s->fd = -1;
+    ret = fstat(s->fd, &st);
+    if (ret == 0 && S_ISREG(st.st_mode)) {
+        /*
+         * If the file handle is a regular file make sure the
+         * data is flushed to disk before signaling success.
+         */
+        ret = fsync(s->fd);
         if (ret != 0) {
             ret = -errno;
-            perror("migration-fd: close");
+            perror("migration-fd: fsync");
             return ret;
         }
     }
-    return 0;
+    ret = close(s->fd);
+    s->fd = -1;
+    if (ret != 0) {
+        ret = -errno;
+        perror("migration-fd: close");
+    }
+    return ret;
 }
 
 int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
diff --git a/migration-tcp.c b/migration-tcp.c
index e797d23..07715de 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -44,11 +44,8 @@  static int tcp_close(MigrationState *s)
 {
     int r = 0;
     DPRINTF("tcp_close\n");
-    if (s->fd != -1) {
-        if (closesocket(s->fd) < 0) {
-            r = -errno;
-        }
-        s->fd = -1;
+    if (closesocket(s->fd) < 0) {
+        r = -socket_error();
     }
     return r;
 }
diff --git a/migration-unix.c b/migration-unix.c
index a407af2..def1969 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -44,11 +44,8 @@  static int unix_close(MigrationState *s)
 {
     int r = 0;
     DPRINTF("unix_close\n");
-    if (s->fd != -1) {
-        if (close(s->fd) < 0) {
-            r = -errno;
-        }
-        s->fd = -1;
+    if (close(s->fd) < 0) {
+        r = -errno;
     }
     return r;
 }