On Fri, Oct 28, 2011 at 12:58:04PM -0200, Luiz Capitulino wrote:
[...]
>
> So, 's->file' is NULL in migrate_fd_put_notify(). The interesting thing
> is that it's valid in the qemu_file_put_notify() call, which makes me
> think that either: there's a race somewhere or qemu_file_put_notify() is
> itself clearing 's->file'. In both cases the fix below could just be hiding
> the real issue, but let's get started...
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
Acked-by: Eduardo Habkost <ehabk...@redhat.com>
However, it looks like the error-check interface of QEMUFile is really
easy to misuse, and can be improved:
- Either errors are always triggered synchronously inside
qemu_file_put_notify(), or they can be triggered asynchronously
elsewhere too.
- If they are always triggered synchronously during the
qemu_file_put_notify() call, then qemu_file_put_notify() should return
error information itself instead of requiring a qemu_file_get_error()
call.
- If errors can be triggered asynchronously, then we need an error
notification mechanism that makes sure no error is ever missed,
instead of this error check on migrate_fd_put_notify().
> ---
> migration.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index bdca72e..f6e6208 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -252,7 +252,7 @@ static 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_get_error(s->file)) {
> + if (s->file && qemu_file_get_error(s->file)) {
> migrate_fd_error(s);
> }
> }
> --
> 1.7.7.1.488.ge8e1c.dirty
>
--
Eduardo
