qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Wei Yang <richardw.yang@linux.intel.com>
To: quintela@redhat.com, dgilbert@redhat.com, peterx@redhat.com
Cc: qemu-devel@nongnu.org, Wei Yang <richardw.yang@linux.intel.com>
Subject: [PATCH v2 2/2] migration/postcopy: check PostcopyState before setting to POSTCOPY_INCOMING_RUNNING
Date: Thu, 10 Oct 2019 09:13:16 +0800	[thread overview]
Message-ID: <20191010011316.31363-3-richardw.yang@linux.intel.com> (raw)
In-Reply-To: <20191010011316.31363-1-richardw.yang@linux.intel.com>

Currently, we set PostcopyState blindly to RUNNING, even we found the
previous state is not LISTENING. This will lead to a corner case.

First let's look at the code flow:

qemu_loadvm_state_main()
    ret = loadvm_process_command()
        loadvm_postcopy_handle_run()
            return -1;
    if (ret < 0) {
        if (postcopy_state_get() == POSTCOPY_INCOMING_RUNNING)
            ...
    }

>From above snippet, the corner case is loadvm_postcopy_handle_run()
always sets state to RUNNING. And then it checks the previous state. If
the previous state is not LISTENING, it will return -1. But at this
moment, PostcopyState is already been set to RUNNING.

Then ret is checked in qemu_loadvm_state_main(), when it is -1
PostcopyState is checked. Current logic would pause postcopy and retry
if PostcopyState is RUNNING. This is not what we expect, because
postcopy is not active yet.

This patch makes sure state is set to RUNNING only previous state is
LISTENING by checking the state first.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Suggested by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 78c2965ca4..b9f30a7090 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1934,7 +1934,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
 /* After all discards we can start running and asking for pages */
 static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
 {
-    PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
+    PostcopyState ps = postcopy_state_get();
 
     trace_loadvm_postcopy_handle_run();
     if (ps != POSTCOPY_INCOMING_LISTENING) {
@@ -1942,6 +1942,7 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
         return -1;
     }
 
+    postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
     mis->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, mis);
     qemu_bh_schedule(mis->bh);
 
-- 
2.17.1



  parent reply	other threads:[~2019-10-10  1:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10  1:13 [PATCH v2 0/2] migration/postcopy: cleanup related to postcopy Wei Yang
2019-10-10  1:13 ` [PATCH v2 1/2] migration/postcopy: rename postcopy_ram_enable_notify to postcopy_ram_incoming_setup Wei Yang
2019-10-10  1:13 ` Wei Yang [this message]
2019-10-11 10:12   ` [PATCH v2 2/2] migration/postcopy: check PostcopyState before setting to POSTCOPY_INCOMING_RUNNING Dr. David Alan Gilbert
2019-10-10  2:00 ` [PATCH v2 0/2] migration/postcopy: cleanup related to postcopy Peter Xu
2019-10-11 14:00 ` Dr. David Alan Gilbert

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=20191010011316.31363-3-richardw.yang@linux.intel.com \
    --to=richardw.yang@linux.intel.com \
    --cc=dgilbert@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.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).