From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBBvH-00048l-E3 for qemu-devel@nongnu.org; Tue, 04 Oct 2011 16:49:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RBBvF-0004lT-Tt for qemu-devel@nongnu.org; Tue, 04 Oct 2011 16:49:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36637) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBBvF-0004lO-G0 for qemu-devel@nongnu.org; Tue, 04 Oct 2011 16:49:05 -0400 From: Juan Quintela In-Reply-To: <4E7C8D77.8070301@redhat.com> (Paolo Bonzini's message of "Fri, 23 Sep 2011 15:45:27 +0200") References: <4E7C8D77.8070301@redhat.com> Date: Tue, 04 Oct 2011 22:48:56 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 05/11] migration: add error handling to migrate_fd_put_notify(). Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Yoshiaki Tamura , qemu-devel@nongnu.org (Adding new Yoshiaki email address) Paolo Bonzini wrote: > On 09/23/2011 02:50 PM, Juan Quintela wrote: >> From: Yoshiaki Tamura >> >> 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 >> Signed-off-by: Juan Quintela >> --- >> 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.