From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSPYn-0002fW-QA for qemu-devel@nongnu.org; Sun, 28 Oct 2012 05:53:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TSPYm-0004kv-Ij for qemu-devel@nongnu.org; Sun, 28 Oct 2012 05:53:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2031) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSPYm-0004kr-A1 for qemu-devel@nongnu.org; Sun, 28 Oct 2012 05:53:36 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9S9rZBT010072 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 28 Oct 2012 05:53:35 -0400 Message-ID: <508D009A.2040703@redhat.com> Date: Sun, 28 Oct 2012 11:53:30 +0200 From: Orit Wasserman MIME-Version: 1.0 References: <1350555758-29988-1-git-send-email-pbonzini@redhat.com> <1350555758-29988-9-git-send-email-pbonzini@redhat.com> In-Reply-To: <1350555758-29988-9-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 08/12] migration: xxx_close will only be called once List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, quintela@redhat.com 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 > --- > 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