Message ID | 1350555758-29988-7-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 10/18/2012 12:22 PM, Paolo Bonzini wrote: > migrate_fd_cleanup will usually close the file descriptor via > buffered_file_close's call to migrate_fd_close. However, in the case > of s->file == NULL it is "inlining" migrate_fd_close (almost: there is a > direct close() instead of using s->close(s)). To fix the inconsistency > and clean up the code, allow multiple calls to migrate_fd_close and use > the function in migrate_fd_cleanup. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > migration.c | 20 ++++++++------------ > 1 file modificato, 8 inserzioni(+), 12 rimozioni(-) > > diff --git a/migration.c b/migration.c > index 62e0304..c1807e0 100644 > --- a/migration.c > +++ b/migration.c > @@ -246,21 +246,12 @@ static int migrate_fd_cleanup(MigrationState *s) > { > int ret = 0; > > - if (s->fd != -1) { > - qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > - } > - > if (s->file) { > DPRINTF("closing file\n"); > ret = qemu_fclose(s->file); > - s->file = NULL; why remove this line? > - } > - > - if (s->fd != -1) { > - close(s->fd); > - s->fd = -1; > } > > + migrate_fd_close(s); > return ret; > } > > @@ -396,8 +387,13 @@ int migrate_fd_wait_for_unfreeze(MigrationState *s) > > int migrate_fd_close(MigrationState *s) > { > - qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > - return s->close(s); > + int rc = 0; > + if (s->fd != -1) { > + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > + rc = s->close(s); > + s->fd = -1; > + } > + return rc; > } > > void add_migration_state_change_notifier(Notifier *notify) >
Il 28/10/2012 10:47, Orit Wasserman ha scritto: >> > if (s->file) { >> > DPRINTF("closing file\n"); >> > ret = qemu_fclose(s->file); >> > - s->file = NULL; > why remove this line? Good catch. Paolo
diff --git a/migration.c b/migration.c index 62e0304..c1807e0 100644 --- a/migration.c +++ b/migration.c @@ -246,21 +246,12 @@ static int migrate_fd_cleanup(MigrationState *s) { int ret = 0; - if (s->fd != -1) { - qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); - } - if (s->file) { DPRINTF("closing file\n"); ret = qemu_fclose(s->file); - s->file = NULL; - } - - if (s->fd != -1) { - close(s->fd); - s->fd = -1; } + migrate_fd_close(s); return ret; } @@ -396,8 +387,13 @@ int migrate_fd_wait_for_unfreeze(MigrationState *s) int migrate_fd_close(MigrationState *s) { - qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); - return s->close(s); + int rc = 0; + if (s->fd != -1) { + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); + rc = s->close(s); + s->fd = -1; + } + return rc; } void add_migration_state_change_notifier(Notifier *notify)
migrate_fd_cleanup will usually close the file descriptor via buffered_file_close's call to migrate_fd_close. However, in the case of s->file == NULL it is "inlining" migrate_fd_close (almost: there is a direct close() instead of using s->close(s)). To fix the inconsistency and clean up the code, allow multiple calls to migrate_fd_close and use the function in migrate_fd_cleanup. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- migration.c | 20 ++++++++------------ 1 file modificato, 8 inserzioni(+), 12 rimozioni(-)