From 48796f6b44df1dd0f78d18757889d5ac478c33e4 Mon Sep 17 00:00:00 2001 From: Fabiano Rosas Date: Wed, 5 Feb 2025 13:17:22 -0300 Subject: [PATCH] migration/multifd: Terminate the TLS connection The multifd recv side has been getting a TLS error of GNUTLS_E_PREMATURE_TERMINATION at the end of migration when the send side closes the sockets without ending the TLS session. This has been masked by the code not checking the migration error after loadvm. Start ending the TLS session at multifd_send_shutdown() so the recv side always sees a clean termination (EOF) and we can start to differentiate that from an actual premature termination that might possibly happen in the middle of the migration. There's nothing to be done if a previous migration error has already broken the connection, so add a comment explaining it and ignore any errors coming from gnutls_bye(). This doesn't break compat with older recv-side QEMUs because EOF has always caused the recv thread to exit cleanly. Reviewed-by: Peter Xu Signed-off-by: Fabiano Rosas --- migration/multifd.c | 38 +++++++++++++++++++++++++++++++++++++- migration/tls.c | 5 +++++ migration/tls.h | 2 +- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index ab73d6d984..0296758c08 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -490,6 +490,36 @@ void multifd_send_shutdown(void) return; } + for (i = 0; i < migrate_multifd_channels(); i++) { + MultiFDSendParams *p = &multifd_send_state->params[i]; + + /* thread_created implies the TLS handshake has succeeded */ + if (p->tls_thread_created && p->thread_created) { + Error *local_err = NULL; + /* + * The destination expects the TLS session to always be + * properly terminated. This helps to detect a premature + * termination in the middle of the stream. Note that + * older QEMUs always break the connection on the source + * and the destination always sees + * GNUTLS_E_PREMATURE_TERMINATION. + */ + migration_tls_channel_end(p->c, &local_err); + + /* + * The above can return an error in case the migration has + * already failed. If the migration succeeded, errors are + * not expected but there's no need to kill the source. + */ + if (local_err && !migration_has_failed(migrate_get_current())) { + warn_report( + "multifd_send_%d: Failed to terminate TLS connection: %s", + p->id, error_get_pretty(local_err)); + break; + } + } + } + multifd_send_terminate_threads(); for (i = 0; i < migrate_multifd_channels(); i++) { @@ -1141,7 +1171,13 @@ static void *multifd_recv_thread(void *opaque) ret = qio_channel_read_all_eof(p->c, (void *)p->packet, p->packet_len, &local_err); - if (ret == 0 || ret == -1) { /* 0: EOF -1: Error */ + if (!ret) { + /* EOF */ + assert(!local_err); + break; + } + + if (ret == -1) { break; } diff --git a/migration/tls.c b/migration/tls.c index fa03d9136c..5cbf952383 100644 --- a/migration/tls.c +++ b/migration/tls.c @@ -156,6 +156,11 @@ void migration_tls_channel_connect(MigrationState *s, NULL); } +void migration_tls_channel_end(QIOChannel *ioc, Error **errp) +{ + qio_channel_tls_bye(QIO_CHANNEL_TLS(ioc), errp); +} + bool migrate_channel_requires_tls_upgrade(QIOChannel *ioc) { if (!migrate_tls()) { diff --git a/migration/tls.h b/migration/tls.h index 5797d153cb..58b25e1228 100644 --- a/migration/tls.h +++ b/migration/tls.h @@ -36,7 +36,7 @@ void migration_tls_channel_connect(MigrationState *s, QIOChannel *ioc, const char *hostname, Error **errp); - +void migration_tls_channel_end(QIOChannel *ioc, Error **errp); /* Whether the QIO channel requires further TLS handshake? */ bool migrate_channel_requires_tls_upgrade(QIOChannel *ioc); -- 2.30.2