Re: [PATCH v3 09/10] migration: Be consistent about shutdown of source shared files

[Peter Xu <peterx@redhat.com>]

On Fri, Aug 11, 2023 at 12:08:35PM -0300, Fabiano Rosas wrote:
> When doing cleanup, we currently close() some of the shared migration
> files and shutdown() + close() others. Be consistent by always calling
> shutdown() before close().
> 
> Do this only for the source files for now because the source runs
> multiple threads which could cause races between the two calls. Having
> them together allows us to move them to a centralized place under the
> protection of a lock the next patch.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Logically I think we should only need shutdown() when we don't want to
close immediately, or can't for some reason..  Maybe instead of adding
shutdown()s, we can remove some?

> ---
>  migration/migration.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 7fec57ad7f..4df5ca25c1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1175,6 +1175,7 @@ static void migrate_fd_cleanup(MigrationState *s)
>           * critical section won't block for long.
>           */
>          migration_ioc_unregister_yank_from_file(tmp);
> +        qemu_file_shutdown(tmp);
>          qemu_fclose(tmp);
>      }
>  
> @@ -1844,6 +1845,7 @@ static void migration_release_dst_files(MigrationState *ms)
>          ms->postcopy_qemufile_src = NULL;
>      }
>  
> +    qemu_file_shutdown(file);
>      qemu_fclose(file);
>  }
>  
> -- 
> 2.35.3
> 

-- 
Peter Xu