From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MDh6f-0002JE-Mo for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:49:53 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MDh6a-0002IT-Nk for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:49:53 -0400 Received: from [199.232.76.173] (port=57158 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MDh6a-0002IO-GL for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:49:48 -0400 Received: from mx2.redhat.com ([66.187.237.31]:34750) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MDh6Z-00072i-V7 for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:49:48 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n58Fnkbd015104 for ; Mon, 8 Jun 2009 11:49:46 -0400 Subject: Re: [Qemu-devel] [PATCH] migrate_fd_close: delete associated io-handler before closing the fd From: Mark McLoughlin In-Reply-To: <1244460481-13575-1-git-send-email-uril@redhat.com> References: <1244460481-13575-1-git-send-email-uril@redhat.com> Content-Type: text/plain Date: Mon, 08 Jun 2009 16:49:42 +0100 Message-Id: <1244476182.3632.40.camel@blaa> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Reply-To: Mark McLoughlin List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Uri Lublin Cc: qemu-devel@nongnu.org On Mon, 2009-06-08 at 14:28 +0300, Uri Lublin wrote: > It may happen that the io-handler is still registered. That causes > select() to return with EBADF, not calling handlers for other fds. > > The io-handler would be registered when (on the source) the whole state > was written but not yet flushed. For example when using QEMUFileBuffered, > (tcp-migration) there may be data left in a buffer waiting to be transferred. > In such a case buffered_close() calls buffered_flush() which calls > migrate_fd_put_buffer, which may, upon EAGAIN, register migrate_fd_put_notify > as a handler. > > Signed-off-by: Uri Lublin > --- > migration.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/migration.c b/migration.c > index 401383c..57f2a52 100644 > --- a/migration.c > +++ b/migration.c > @@ -301,5 +301,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque) > int migrate_fd_close(void *opaque) > { > FdMigrationState *s = opaque; > + > + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); Looks good, but perhaps a comment explaining how the I/O handler could possibly be registered here would be useful - at first glance, it seemed to me that the I/O handler should always be de-registered in migrate_fd_cleanup() before getting here. The key to understanding the problem is that qemu_fclose() on a buffered file can cause I/O to be flushed. Reviewed-by: Mark McLoughlin Cheers, Mark.