| Message ID | a34bff21a0546cad0d8ea3ec370ccb38e79379e5.1316781876.git.quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/23/2011 02:50 PM, Juan Quintela wrote: > From: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> > > Although migrate_fd_put_buffer() sets MIG_STATE_ERROR if it failed, > since migrate_fd_put_notify() isn't checking error of underlying > QEMUFile, those resources are kept open. This patch checks it and > calls migrate_fd_error() in case of error. > > Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> > Signed-off-by: Juan Quintela<quintela@redhat.com> > --- > migration.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/migration.c b/migration.c > index 9498e20..0b284ff 100644 > --- a/migration.c > +++ b/migration.c > @@ -313,6 +313,9 @@ void migrate_fd_put_notify(void *opaque) > > qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > qemu_file_put_notify(s->file); > + if (qemu_file_has_error(s->file)) { > + migrate_fd_error(s); > + } > } > > ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) > @@ -329,9 +332,6 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) > > if (ret == -EAGAIN) { > qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s); > - } else if (ret< 0) { > - s->state = MIG_STATE_ERROR; > - notifier_list_notify(&migration_state_notifiers, NULL); > } > > return ret; Why not leave both (or even better, call migrate_fd_error in the else branch)? Paolo
(Adding new Yoshiaki email address) Paolo Bonzini <pbonzini@redhat.com> wrote: > On 09/23/2011 02:50 PM, Juan Quintela wrote: >> From: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> >> >> Although migrate_fd_put_buffer() sets MIG_STATE_ERROR if it failed, >> since migrate_fd_put_notify() isn't checking error of underlying >> QEMUFile, those resources are kept open. This patch checks it and >> calls migrate_fd_error() in case of error. >> >> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> >> Signed-off-by: Juan Quintela<quintela@redhat.com> >> --- >> migration.c | 6 +++--- >> 1 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/migration.c b/migration.c >> index 9498e20..0b284ff 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -313,6 +313,9 @@ void migrate_fd_put_notify(void *opaque) >> >> qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); >> qemu_file_put_notify(s->file); >> + if (qemu_file_has_error(s->file)) { >> + migrate_fd_error(s); >> + } >> } >> >> ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) >> @@ -329,9 +332,6 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) >> >> if (ret == -EAGAIN) { >> qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s); >> - } else if (ret< 0) { >> - s->state = MIG_STATE_ERROR; >> - notifier_list_notify(&migration_state_notifiers, NULL); >> } >> >> return ret; > > Why not leave both (or even better, call migrate_fd_error in the else > branch)? In the big scheme of things, it don't matter. migration_fd_put_buffer() still returns the errno. it is only called from buffered_put_buffer() and buffered_flush(), both check the error handling and setup s->has_error correctly (after this series). Yoshi, do you remember better? Later, Juan.
diff --git a/migration.c b/migration.c index 9498e20..0b284ff 100644 --- a/migration.c +++ b/migration.c @@ -313,6 +313,9 @@ void migrate_fd_put_notify(void *opaque) qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); qemu_file_put_notify(s->file); + if (qemu_file_has_error(s->file)) { + migrate_fd_error(s); + } } ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) @@ -329,9 +332,6 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) if (ret == -EAGAIN) { qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s); - } else if (ret < 0) { - s->state = MIG_STATE_ERROR; - notifier_list_notify(&migration_state_notifiers, NULL); } return ret;