Peter Xu <pet...@redhat.com> writes:
> QEMU supported graceful shutdowns for multifd channels starting from commit
> 48796f6b44 ("migration/multifd: Terminate the TLS connection"). Then error
> check was enabled for premature TLS terminations.
>
> Now if we run the preempt TLS unit test, the test would pass, but there
> will be a warning reported:
>
> qemu-system-x86_64: Cannot read from TLS channel: The TLS connection was
> non-properly terminated.
> ok 1 /x86_64/migration/postcopy/preempt/tls/psk
>
> To fix it, make the rest channels to be gracefully terminated too when it's
> a TLS channel.
>
> One note is that the qemufile helper needs to be in migration.c not
> qemu-file.c, because qemu-file.c will be linked in unit tests, which will
> not link channel.c unfortunately.
>
> Reported-by: Xiaohui Li <xiao...@redhat.com>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> migration/channel.h | 3 +++
> migration/migration.h | 2 ++
> migration/channel.c | 13 +++++++++++++
> migration/migration.c | 24 +++++++++++++++++++++++-
> 4 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/migration/channel.h b/migration/channel.h
> index 5bdb8208a7..0b25dd7c5b 100644
> --- a/migration/channel.h
> +++ b/migration/channel.h
> @@ -29,4 +29,7 @@ int migration_channel_read_peek(QIOChannel *ioc,
> const char *buf,
> const size_t buflen,
> Error **errp);
> +
> +bool migration_channel_shutdown_gracefully(QIOChannel *c, Error **errp);
> +
> #endif
> diff --git a/migration/migration.h b/migration/migration.h
> index 01329bf824..b5763af057 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -594,4 +594,6 @@ void migration_bitmap_sync_precopy(bool last_stage);
> void dirty_bitmap_mig_init(void);
> bool should_send_vmdesc(void);
>
> +bool qemu_file_shutdown_gracefully(QEMUFile *f, Error **errp);
> +
> #endif
> diff --git a/migration/channel.c b/migration/channel.c
> index a547b1fbfe..1ae839e5fe 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -145,3 +145,16 @@ int migration_channel_read_peek(QIOChannel *ioc,
>
> return 0;
> }
> +
> +/*
> + * This is only needed for a successful migration, no-op for non-TLS
> + * channels. For unexpected interruptions, use qio_channel_shutdown().
> + */
> +bool migration_channel_shutdown_gracefully(QIOChannel *c, Error **errp)
> +{
ERRP_GUARD();
due to dereferencing errp below
> + if (object_dynamic_cast((Object *)c, TYPE_QIO_CHANNEL_TLS)) {
> + qio_channel_tls_bye(QIO_CHANNEL_TLS(c), errp);
> + }
> +
> + return *errp == NULL;
> +}
> diff --git a/migration/migration.c b/migration/migration.c
> index 10c216d25d..7015c2b5e0 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -113,6 +113,27 @@ static bool close_return_path_on_source(MigrationState
> *s);
> static void migration_completion_end(MigrationState *s);
> static void migrate_hup_delete(MigrationState *s);
>
> +/*
> + * See migration_channel_shutdown_gracefully(). The "graceful" versions
> + * are only needed if migration succeeded.
> + */
> +bool qemu_file_shutdown_gracefully(QEMUFile *f, Error **errp)
> +{
> + int ret;
> +
> + if (!migration_channel_shutdown_gracefully(qemu_file_get_ioc(f), errp)) {
> + return false;
> + }
> +
> + ret = qemu_file_shutdown(f);
> + if (ret) {
> + error_setg_errno(errp, -ret, "qemu_file_shutdown() failed");
> + return false;
> + }
> +
> + return true;
> +}
> +
> static void migration_downtime_start(MigrationState *s)
> {
> trace_vmstate_downtime_checkpoint("src-downtime-start");
> @@ -2473,11 +2494,12 @@ static void
> migration_release_dst_files(MigrationState *ms)
> */
> if (ms->postcopy_qemufile_src) {
> migration_ioc_unregister_yank_from_file(ms->postcopy_qemufile_src);
> - qemu_file_shutdown(ms->postcopy_qemufile_src);
> + qemu_file_shutdown_gracefully(ms->postcopy_qemufile_src,
> &error_warn);
> qemu_fclose(ms->postcopy_qemufile_src);
> ms->postcopy_qemufile_src = NULL;
> }
>
> + qemu_file_shutdown_gracefully(file, &error_warn);
> qemu_fclose(file);
> }
