fridtjof/qemu-cr16
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

migration/multifd/tls: Cleanup BYE message processing on sender side

Browse source 
This patch is a trivial cleanup to the BYE messages on the multifd sender
side.  It could also be a fix, but since we do not have a solid clue,
taking this as a cleanup only.

One trivial concern is, migration_tls_channel_end() might be unsafe to be
invoked in the migration thread if migration is not successful, because
when failed / cancelled we do not know whether the multifd sender threads
can be writting to the channels, while GnuTLS library (when it's a TLS
channel) logically doesn't support concurrent writes.

When at it, cleanup on a few things.  What changed:

  - Introduce a helper to do graceful shutdowns with rich comment, hiding
    the details

  - Only send bye() iff migration succeeded, skip if it failed / cancelled

  - Detect TLS channel using channel type rather than thread created flags

  - Move the loop into the existing one that will close the channels, but
    do graceful shutdowns before channel shutdowns

  - local_err seems to have been leaked if set, fix it along the way

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20250925201601.290546-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
This commit is contained in:
Peter Xu 2025-09-25 16:16:01 -04:00
parent 2aae717122
commit 82f038d596
1 changed files with 34 additions and 31 deletions
Download patch file Download diff file Expand all files Collapse all files

65
migration/multifd.c
View file

@ -439,6 +439,39 @@ static void multifd_send_set_error(Error *err)
}
}
/*
* Gracefully shutdown IOChannels. Only needed for successful migrations on
* top of TLS channels. Otherwise it is same to qio_channel_shutdown().
*
* A successful migration also guarantees multifd sender threads are
* properly flushed and halted. It is only safe to send BYE in the
* migration thread here when we know there's no other thread writting to
* the channel, because GnuTLS doesn't support concurrent writers.
*/
static void migration_ioc_shutdown_gracefully(QIOChannel *ioc)
{
g_autoptr(Error) local_err = NULL;
if (!migration_has_failed(migrate_get_current()) &&
object_dynamic_cast((Object *)ioc, TYPE_QIO_CHANNEL_TLS)) {
/*
* 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(ioc, &local_err);
if (local_err) {
warn_report("Failed to gracefully terminate TLS connection: %s",
error_get_pretty(local_err));
}
}
qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
}
static void multifd_send_terminate_threads(void)
{
int i;
@ -460,7 +493,7 @@ static void multifd_send_terminate_threads(void)
qemu_sem_post(&p->sem);
if (p->c) {
qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
migration_ioc_shutdown_gracefully(p->c);
}
}
@ -547,36 +580,6 @@ 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++) {