From: Fabiano Rosas <farosas@suse.de>
To: qemu-devel@nongnu.org
Cc: "Peter Xu" <peterx@redhat.com>,
"Maciej S . Szmigiero" <mail@maciej.szmigiero.name>,
"Daniel P . Berrangé" <berrange@redhat.com>
Subject: [RFC PATCH v3 6/8] migration/multifd: Terminate the TLS connection
Date: Fri, 7 Feb 2025 16:53:57 -0300 [thread overview]
Message-ID: <20250207195359.17443-7-farosas@suse.de> (raw)
In-Reply-To: <20250207195359.17443-1-farosas@suse.de>
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 <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
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.35.3
next prev parent reply other threads:[~2025-02-07 19:55 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-07 19:53 [RFC PATCH v3 0/8] crypto,io,migration: Add support to gnutls_bye() Fabiano Rosas
2025-02-07 19:53 ` [RFC PATCH v3 1/8] crypto: Allow gracefully ending the TLS session Fabiano Rosas
2025-02-07 19:53 ` [RFC PATCH v3 2/8] io: tls: Add qio_channel_tls_bye Fabiano Rosas
2025-02-07 19:53 ` [RFC PATCH v3 3/8] crypto: Remove qcrypto_tls_session_get_handshake_status Fabiano Rosas
2025-02-07 19:53 ` [RFC PATCH v3 4/8] io: Add flags argument to qio_channel_readv_full_all_eof Fabiano Rosas
2025-02-10 9:04 ` Daniel P. Berrangé
2025-02-07 19:53 ` [RFC PATCH v3 5/8] io: Add a read flag for relaxed EOF Fabiano Rosas
2025-02-07 19:53 ` Fabiano Rosas [this message]
2025-02-07 19:53 ` [RFC PATCH v3 7/8] migration/multifd: Add a compat property for TLS termination Fabiano Rosas
2025-02-10 17:11 ` Peter Xu
2025-02-07 19:53 ` [RFC PATCH v3 8/8] migration: Check migration error after loadvm Fabiano Rosas
2025-02-10 17:12 ` Peter Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250207195359.17443-7-farosas@suse.de \
--to=farosas@suse.de \
--cc=berrange@redhat.com \
--cc=mail@maciej.szmigiero.name \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).