From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MDhDB-0004su-Ut for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:56:37 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MDhD7-0004qH-Ei for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:56:37 -0400 Received: from [199.232.76.173] (port=34622 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MDhD7-0004qD-68 for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:56:33 -0400 Received: from mx2.redhat.com ([66.187.237.31]:50465) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MDhD6-0008B6-Ik for qemu-devel@nongnu.org; Mon, 08 Jun 2009 11:56:32 -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 n58FuV66016848 for ; Mon, 8 Jun 2009 11:56:31 -0400 Message-ID: <4A2D34A7.1080208@redhat.com> Date: Mon, 08 Jun 2009 18:56:23 +0300 From: Uri Lublin MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] migrate_fd_close: delete associated io-handler before closing the fd References: <1244460481-13575-1-git-send-email-uril@redhat.com> <1244476182.3632.40.camel@blaa> In-Reply-To: <1244476182.3632.40.camel@blaa> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark McLoughlin Cc: qemu-devel@nongnu.org On 06/08/2009 06:49 PM, Mark McLoughlin wrote: > 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. Do you mean in addition to the log-message (copy part of the log message as a comment in the code) ? > > Reviewed-by: Mark McLoughlin Thanks for the review, Uri.