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

[Uri Lublin]

On 06/08/2009 07:38 PM, Mark McLoughlin wrote:
> Would the below patch working equally well? But then again, we should
> really remove the I/O handler before closing the fd.

I think it should work too.
My patch does remove the I/O handler before closing the fd.

> (The close(s->fd) in migration_fd_cleanup() looks like it can never
> happen - perhaps we should remove it)

Perhaps we should. I think it's like a "plan b" in case qemu_fclose did not 
actually closed the file descriptor (which, as you mentioned, currently, can 
never happen).

> diff --git a/migration.c b/migration.c
> index 401383c..078967f 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -154,13 +154,21 @@ void migrate_fd_error(FdMigrationState *s)
>
>   void migrate_fd_cleanup(FdMigrationState *s)
>   {
> -    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);

Would it be safer to keep the I/O handler deletion before the call to 
qemu_fclose (in addition to adding the one below), or it does not matter.


> +    int fd;
> +
> +    /* qemu_fclose() can cause I/O to be flushed (see buffered_close())
> +     * which, in turn, can cause an I/O handler to be registered. We
> +     * need to delay removing the I/O handler until after qemu_fclose().
> +     */
> +    fd = s->fd;
>
>       if (s->file) {
>           dprintf("closing file\n");
>           qemu_fclose(s->file);
>       }
>
> +    qemu_set_fd_handler2(fd, NULL, NULL, NULL, NULL);
> +
>       if (s->fd != -1)
>           close(s->fd);

Regards,
    Uri.