Re: [Qemu-devel] [PATCH] migrate_fd_close: delete associated io-handler before closing the fd

[Uri Lublin <uril@redhat.com>]

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<uril@redhat.com>
>> ---
>>   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<markmc@redhat.com>

Thanks for the review,
     Uri.