qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Yanan Wang" <wangyanan55@huawei.com>,
	"Juan Quintela" <quintela@redhat.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Peter Xu" <peterx@redhat.com>
Subject: [PULL 3/5] migration: Recover behavior of preempt channel creation for pre-7.2
Date: Wed, 12 Apr 2023 23:45:05 +0200	[thread overview]
Message-ID: <20230412214507.19816-4-quintela@redhat.com> (raw)
In-Reply-To: <20230412214507.19816-1-quintela@redhat.com>

From: Peter Xu <peterx@redhat.com>

In 8.0 devel window we reworked preempt channel creation, so that there'll
be no race condition when the migration channel and preempt channel got
established in the wrong order in commit 5655aab079.

However no one noticed that the change will also be not compatible with
older qemus, majorly 7.1/7.2 versions where preempt mode started to be
supported.

Leverage the same pre-7.2 flag introduced in the previous patch to recover
the behavior hopefully before 8.0 releases, so we don't break migration
when we migrate from 8.0 to older qemu binaries.

Fixes: 5655aab079 ("migration: Postpone postcopy preempt channel to be after main")
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.h    |  7 +++++++
 migration/migration.c    |  9 +++++++++
 migration/postcopy-ram.c | 10 ++++++++--
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index 67baba2184..310ae8901b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -384,12 +384,19 @@ struct MigrationState {
      * - postcopy preempt src QEMU instance will generate an EOS message at
      *   the end of migration to shut the preempt channel on dest side.
      *
+     * - postcopy preempt channel will be created at the setup phase on src
+         QEMU.
+     *
      * When clear:
      *
      * - postcopy preempt src QEMU instance will _not_ generate an EOS
      *   message at the end of migration, the dest qemu will shutdown the
      *   channel itself.
      *
+     * - postcopy preempt channel will be created at the switching phase
+     *   from precopy -> postcopy (to avoid race condtion of misordered
+     *   creation of channels).
+     *
      * NOTE: See message-id <ZBoShWArKDPpX/D7@work-vm> on qemu-devel
      * mailing list for more information on the possible race.  Everyone
      * should probably just keep this value untouched after set by the
diff --git a/migration/migration.c b/migration/migration.c
index 37fc4fb3e2..bda4789193 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4388,6 +4388,15 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
         }
     }
 
+    /*
+     * This needs to be done before resuming a postcopy.  Note: for newer
+     * QEMUs we will delay the channel creation until postcopy_start(), to
+     * avoid disorder of channel creations.
+     */
+    if (migrate_postcopy_preempt() && s->preempt_pre_7_2) {
+        postcopy_preempt_setup(s);
+    }
+
     if (resume) {
         /* Wakeup the main migration thread to do the recovery */
         migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_PAUSED,
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 263bab75ec..93f39f8e06 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1630,8 +1630,14 @@ int postcopy_preempt_establish_channel(MigrationState *s)
         return 0;
     }
 
-    /* Kick off async task to establish preempt channel */
-    postcopy_preempt_setup(s);
+    /*
+     * Kick off async task to establish preempt channel.  Only do so with
+     * 8.0+ machines, because 7.1/7.2 require the channel to be created in
+     * setup phase of migration (even if racy in an unreliable network).
+     */
+    if (!s->preempt_pre_7_2) {
+        postcopy_preempt_setup(s);
+    }
 
     /*
      * We need the postcopy preempt channel to be established before
-- 
2.39.2



  parent reply	other threads:[~2023-04-12 21:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-12 21:45 [PULL 0/5] Migration 20230412 patches Juan Quintela
2023-04-12 21:45 ` [PULL 1/5] io: tls: Inherit QIO_CHANNEL_FEATURE_SHUTDOWN on server side Juan Quintela
2023-04-12 21:45 ` [PULL 2/5] migration: Fix potential race on postcopy_qemufile_src Juan Quintela
2023-04-12 21:45 ` Juan Quintela [this message]
2023-04-12 21:45 ` [PULL 4/5] migration/ram.c: Fix migration with compress enabled Juan Quintela
2023-04-12 21:45 ` [PULL 5/5] migration: fix ram_state_pending_exact() Juan Quintela
2023-04-13 13:02 ` [PULL 0/5] Migration 20230412 patches Peter Maydell

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=20230412214507.19816-4-quintela@redhat.com \
    --to=quintela@redhat.com \
    --cc=berrange@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=peterx@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=wangyanan55@huawei.com \
    /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).