All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: peterx@redhat.com, "Juraj Marcin" <jmarcin@redhat.com>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	"Fabiano Rosas" <farosas@suse.de>
Subject: [PATCH 3/3] migration/multifd: Use the new graceful termination helper
Date: Wed, 10 Sep 2025 12:01:44 -0400	[thread overview]
Message-ID: <20250910160144.1762894-4-peterx@redhat.com> (raw)
In-Reply-To: <20250910160144.1762894-1-peterx@redhat.com>

Multifd has a separate loop to do TLS terminations gracefully.  Meanwhile,
it depends on two variables which records thread creations.

It works perfectly before, however relying on "whether some threads are
created" flag might be not as straightforward to decide a graceful
shutdown.

Since we'll need to dynamically identify TLS channels anyway with the new
helper (which is needed for main and postcopy channels), use the same
simple API for multifd channels too.  Also, we only need graceful shutdown
on success of migrations.

With that, we can remove the loop and drop migration_tls_channel_end().

The comment there is still a good explanation, move it over to the new
helper instead.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/tls.h     |  1 -
 migration/channel.c |  7 +++++++
 migration/multifd.c | 40 +++++++---------------------------------
 migration/tls.c     |  5 -----
 4 files changed, 14 insertions(+), 39 deletions(-)

diff --git a/migration/tls.h b/migration/tls.h
index 58b25e1228..75c918e156 100644
--- a/migration/tls.h
+++ b/migration/tls.h
@@ -36,7 +36,6 @@ 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);
 
diff --git a/migration/channel.c b/migration/channel.c
index 1ae839e5fe..a481b45eae 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -153,6 +153,13 @@ int migration_channel_read_peek(QIOChannel *ioc,
 bool migration_channel_shutdown_gracefully(QIOChannel *c, Error **errp)
 {
     if (object_dynamic_cast((Object *)c, 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.
+         */
         qio_channel_tls_bye(QIO_CHANNEL_TLS(c), errp);
     }
 
diff --git a/migration/multifd.c b/migration/multifd.c
index b255778855..cb0262076b 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -439,7 +439,7 @@ static void multifd_send_set_error(Error *err)
     }
 }
 
-static void multifd_send_terminate_threads(void)
+static void multifd_send_terminate_threads(bool succeeded)
 {
     int i;
 
@@ -460,6 +460,9 @@ static void multifd_send_terminate_threads(void)
 
         qemu_sem_post(&p->sem);
         if (p->c) {
+            if (succeeded) {
+                migration_channel_shutdown_gracefully(p->c, &error_warn);
+            }
             qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
         }
     }
@@ -541,50 +544,21 @@ static void multifd_send_cleanup_state(void)
 
 void multifd_send_shutdown(void)
 {
+    MigrationState *s = migrate_get_current();
     int i;
 
     if (!migrate_multifd()) {
         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();
+    multifd_send_terminate_threads(!migration_has_failed(s));
 
     for (i = 0; i < migrate_multifd_channels(); i++) {
         MultiFDSendParams *p = &multifd_send_state->params[i];
         Error *local_err = NULL;
 
         if (!multifd_send_cleanup_channel(p, &local_err)) {
-            migrate_set_error(migrate_get_current(), local_err);
+            migrate_set_error(s, local_err);
             error_free(local_err);
         }
     }
diff --git a/migration/tls.c b/migration/tls.c
index 284a6194b2..ca1595e05d 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -165,11 +165,6 @@ 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()) {
-- 
2.50.1



  parent reply	other threads:[~2025-09-10 16:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-10 16:01 [PATCH 0/3] migration/tls: Graceful shutdowns for main and postcopy channels Peter Xu
2025-09-10 16:01 ` [PATCH 1/3] migration/tls: Gracefully shutdown main and preempt channels Peter Xu
2025-09-17 20:22   ` Fabiano Rosas
2025-09-10 16:01 ` [PATCH 2/3] migration: Make migration_has_failed() work even for CANCELLING Peter Xu
2025-09-17 20:52   ` Fabiano Rosas
2025-09-17 22:00     ` Peter Xu
2025-09-18 13:43       ` Fabiano Rosas
2025-09-10 16:01 ` Peter Xu [this message]
2025-09-17 21:07   ` [PATCH 3/3] migration/multifd: Use the new graceful termination helper Fabiano Rosas
2025-09-11 13:13 ` [PATCH 0/3] migration/tls: Graceful shutdowns for main and postcopy channels Peter Xu
2025-09-17 20:56   ` Fabiano Rosas
2025-09-17 21:50     ` Peter Xu
2025-09-18 13:47       ` Fabiano Rosas
2025-09-18 16:15         ` 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=20250910160144.1762894-4-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=berrange@redhat.com \
    --cc=farosas@suse.de \
    --cc=jmarcin@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.