Re: [PATCH v5 8/8] migration: Add a wrapper to cleanup migration files

[Peter Xu <peterx@redhat.com>]

On Thu, Aug 31, 2023 at 03:39:16PM -0300, Fabiano Rosas wrote:
> @@ -1166,16 +1183,9 @@ static void migrate_fd_cleanup(MigrationState *s)
>          qemu_mutex_lock_iothread();
>  
>          multifd_save_cleanup();
> -        qemu_mutex_lock(&s->qemu_file_lock);
> -        tmp = s->to_dst_file;
> -        s->to_dst_file = NULL;
> -        qemu_mutex_unlock(&s->qemu_file_lock);
> -        /*
> -         * Close the file handle without the lock to make sure the
> -         * critical section won't block for long.
> -         */
> -        migration_ioc_unregister_yank_from_file(tmp);
> -        qemu_fclose(tmp);
> +
> +        migration_ioc_unregister_yank_from_file(s->to_dst_file);

I think you suggested that we should always take the file lock when
operating on them, so this is slightly going backwards to not hold any lock
when doing it. But doing so in migrate_fd_cleanup() is probably fine (as it
serializes with bql on all the rest qmp commands, neither should migration
thread exist at this point).  Your call; it's still much cleaner.

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu