diff mbox series

[V4,07/11] tests/qtest: migration events

Message ID 1693333086-392798-8-git-send-email-steven.sistare@oracle.com
State New
Headers show
Series fix migration of suspended runstate | expand

Commit Message

Steve Sistare Aug. 29, 2023, 6:18 p.m. UTC
Define a state object to capture events seen by migration tests, to allow
more events to be captured in a subsequent patch, and simplify event
checking in wait_for_migration_pass.  No functional change.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c | 24 +++++----------
 tests/qtest/migration-helpers.h |  8 +++--
 tests/qtest/migration-test.c    | 68 +++++++++++++++++++----------------------
 3 files changed, 44 insertions(+), 56 deletions(-)

Comments

Peter Xu Aug. 30, 2023, 5 p.m. UTC | #1
On Tue, Aug 29, 2023 at 11:18:02AM -0700, Steve Sistare wrote:
> Define a state object to capture events seen by migration tests, to allow
> more events to be captured in a subsequent patch, and simplify event
> checking in wait_for_migration_pass.  No functional change.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration-helpers.c | 24 +++++----------
>  tests/qtest/migration-helpers.h |  8 +++--
>  tests/qtest/migration-test.c    | 68 +++++++++++++++++++----------------------
>  3 files changed, 44 insertions(+), 56 deletions(-)
> 
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index be00c52..b541108 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -23,26 +23,16 @@
>   */
>  #define MIGRATION_STATUS_WAIT_TIMEOUT 120
>  
> -bool migrate_watch_for_stop(QTestState *who, const char *name,
> -                            QDict *event, void *opaque)
> -{
> -    bool *seen = opaque;
> -
> -    if (g_str_equal(name, "STOP")) {
> -        *seen = true;
> -        return true;
> -    }
> -
> -    return false;
> -}
> -
> -bool migrate_watch_for_resume(QTestState *who, const char *name,
> +bool migrate_watch_for_events(QTestState *who, const char *name,
>                                QDict *event, void *opaque)
>  {
> -    bool *seen = opaque;
> +    QTestMigrationState *state = opaque;
>  
> -    if (g_str_equal(name, "RESUME")) {
> -        *seen = true;
> +    if (g_str_equal(name, "STOP")) {
> +        state->stop_seen = true;
> +        return true;
> +    } else if (g_str_equal(name, "RESUME")) {
> +        state->resume_seen = true;
>          return true;
>      }
>  
> diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
> index 009e250..59fbb83 100644
> --- a/tests/qtest/migration-helpers.h
> +++ b/tests/qtest/migration-helpers.h
> @@ -15,9 +15,11 @@
>  
>  #include "libqtest.h"
>  
> -bool migrate_watch_for_stop(QTestState *who, const char *name,
> -                            QDict *event, void *opaque);
> -bool migrate_watch_for_resume(QTestState *who, const char *name,
> +typedef struct QTestMigrationState {
> +    bool stop_seen, resume_seen;
> +} QTestMigrationState;
> +
> +bool migrate_watch_for_events(QTestState *who, const char *name,
>                                QDict *event, void *opaque);
>  
>  G_GNUC_PRINTF(3, 4)
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 62d3f37..526a1b7 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -43,8 +43,8 @@
>  unsigned start_address;
>  unsigned end_address;
>  static bool uffd_feature_thread_id;
> -static bool got_src_stop;
> -static bool got_dst_resume;
> +static QTestMigrationState src_state;
> +static QTestMigrationState dst_state;
>  
>  /*
>   * An initial 3 MB offset is used as that corresponds
> @@ -188,6 +188,13 @@ static void wait_for_serial(const char *side)
>      } while (true);
>  }
>  
> +static void wait_for_stop(QTestState *who, QTestMigrationState *state)
> +{
> +    if (!state->stop_seen) {
> +        qtest_qmp_eventwait(who, "STOP");
> +    }
> +}
> +
>  /*
>   * It's tricky to use qemu's migration event capability with qtest,
>   * events suddenly appearing confuse the qmp()/hmp() responses.
> @@ -235,21 +242,19 @@ static void read_blocktime(QTestState *who)
>      qobject_unref(rsp_return);
>  }
>  
> +/*
> + * Wait for two changes in the migration pass count, but bail if we stop.
> + */
>  static void wait_for_migration_pass(QTestState *who)
>  {
> -    uint64_t initial_pass = get_migration_pass(who);
> -    uint64_t pass;
> +    uint64_t pass, prev_pass = 0, changes = 0;
>  
> -    /* Wait for the 1st sync */
> -    while (!got_src_stop && !initial_pass) {
> -        usleep(1000);
> -        initial_pass = get_migration_pass(who);
> -    }
> -
> -    do {
> +    while (changes < 2 && !src_state.stop_seen) {
>          usleep(1000);
>          pass = get_migration_pass(who);
> -    } while (pass == initial_pass && !got_src_stop);
> +        changes += (pass != prev_pass);
> +        prev_pass = pass;
> +    }

Here won't it start to wait for 2 iterations every time instead of 1?

Note that previously we only wait for 1 iteration as long as not the
initial pass.  And I think the change will double the counts for below..

            while (args->iterations > 1) {
                wait_for_migration_pass(from);
                args->iterations--;
            }

The event-related changes are all fine, but maybe leave this piece as before?

>  }
>  
>  static void check_guests_ram(QTestState *who)
> @@ -586,10 +591,7 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
>  {
>      qtest_qmp_assert_success(from, "{ 'execute': 'migrate-start-postcopy' }");
>  
> -    if (!got_src_stop) {
> -        qtest_qmp_eventwait(from, "STOP");
> -    }
> -
> +    wait_for_stop(from, &src_state);
>      qtest_qmp_eventwait(to, "RESUME");
>  }
>  
> @@ -720,8 +722,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>          }
>      }
>  
> -    got_src_stop = false;
> -    got_dst_resume = false;
> +    dst_state = (QTestMigrationState) { };
> +    src_state = (QTestMigrationState) { };
> +
>      bootpath = g_strdup_printf("%s/bootsect", tmpfs);
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          /* the assembled x86 boot sector should be exactly one sector large */
> @@ -801,8 +804,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>      if (!args->only_target) {
>          *from = qtest_init(cmd_source);
>          qtest_qmp_set_event_callback(*from,
> -                                     migrate_watch_for_stop,
> -                                     &got_src_stop);
> +                                     migrate_watch_for_events,
> +                                     &src_state);
>      }
>  
>      cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
> @@ -821,8 +824,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                   ignore_stderr);
>      *to = qtest_init(cmd_target);
>      qtest_qmp_set_event_callback(*to,
> -                                 migrate_watch_for_resume,
> -                                 &got_dst_resume);
> +                                 migrate_watch_for_events,
> +                                 &dst_state);
>  
>      /*
>       * Remove shmem file immediately to avoid memory leak in test failed case.
> @@ -1516,9 +1519,7 @@ static void test_precopy_common(MigrateCommon *args)
>           */
>          if (args->result == MIG_TEST_SUCCEED) {
>              qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
> -            if (!got_src_stop) {
> -                qtest_qmp_eventwait(from, "STOP");
> -            }
> +            wait_for_stop(from, &src_state);
>              migrate_ensure_converge(from);
>          }
>      }
> @@ -1560,9 +1561,8 @@ static void test_precopy_common(MigrateCommon *args)
>               */
>              wait_for_migration_complete(from);
>  
> -            if (!got_src_stop) {
> -                qtest_qmp_eventwait(from, "STOP");
> -            }
> +            wait_for_stop(from, &src_state);
> +
>          } else {
>              wait_for_migration_complete(from);
>              /*
> @@ -1575,7 +1575,7 @@ static void test_precopy_common(MigrateCommon *args)
>              qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
>          }
>  
> -        if (!got_dst_resume) {
> +        if (!dst_state.resume_seen) {
>              qtest_qmp_eventwait(to, "RESUME");
>          }
>  
> @@ -1696,9 +1696,7 @@ static void test_ignore_shared(void)
>  
>      migrate_wait_for_dirty_mem(from, to);
>  
> -    if (!got_src_stop) {
> -        qtest_qmp_eventwait(from, "STOP");
> -    }
> +    wait_for_stop(from, &src_state);
>  
>      qtest_qmp_eventwait(to, "RESUME");
>  
> @@ -2139,7 +2137,7 @@ static void test_migrate_auto_converge(void)
>              break;
>          }
>          usleep(20);
> -        g_assert_false(got_src_stop);
> +        g_assert_false(src_state.stop_seen);
>      } while (true);
>      /* The first percentage of throttling should be at least init_pct */
>      g_assert_cmpint(percentage, >=, init_pct);
> @@ -2481,9 +2479,7 @@ static void test_multifd_tcp_cancel(void)
>  
>      migrate_ensure_converge(from);
>  
> -    if (!got_src_stop) {
> -        qtest_qmp_eventwait(from, "STOP");
> -    }
> +    wait_for_stop(from, &src_state);
>      qtest_qmp_eventwait(to2, "RESUME");
>  
>      wait_for_serial("dest_serial");
> -- 
> 1.8.3.1
>
Steve Sistare Nov. 13, 2023, 6:33 p.m. UTC | #2
On 8/30/2023 1:00 PM, Peter Xu wrote:
> On Tue, Aug 29, 2023 at 11:18:02AM -0700, Steve Sistare wrote:
>> Define a state object to capture events seen by migration tests, to allow
>> more events to be captured in a subsequent patch, and simplify event
>> checking in wait_for_migration_pass.  No functional change.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  tests/qtest/migration-helpers.c | 24 +++++----------
>>  tests/qtest/migration-helpers.h |  8 +++--
>>  tests/qtest/migration-test.c    | 68 +++++++++++++++++++----------------------
>>  3 files changed, 44 insertions(+), 56 deletions(-)
>>
>> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> index be00c52..b541108 100644
>> --- a/tests/qtest/migration-helpers.c
>> +++ b/tests/qtest/migration-helpers.c
>> @@ -23,26 +23,16 @@
>>   */
>>  #define MIGRATION_STATUS_WAIT_TIMEOUT 120
>>  
>> -bool migrate_watch_for_stop(QTestState *who, const char *name,
>> -                            QDict *event, void *opaque)
>> -{
>> -    bool *seen = opaque;
>> -
>> -    if (g_str_equal(name, "STOP")) {
>> -        *seen = true;
>> -        return true;
>> -    }
>> -
>> -    return false;
>> -}
>> -
>> -bool migrate_watch_for_resume(QTestState *who, const char *name,
>> +bool migrate_watch_for_events(QTestState *who, const char *name,
>>                                QDict *event, void *opaque)
>>  {
>> -    bool *seen = opaque;
>> +    QTestMigrationState *state = opaque;
>>  
>> -    if (g_str_equal(name, "RESUME")) {
>> -        *seen = true;
>> +    if (g_str_equal(name, "STOP")) {
>> +        state->stop_seen = true;
>> +        return true;
>> +    } else if (g_str_equal(name, "RESUME")) {
>> +        state->resume_seen = true;
>>          return true;
>>      }
>>  
>> diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
>> index 009e250..59fbb83 100644
>> --- a/tests/qtest/migration-helpers.h
>> +++ b/tests/qtest/migration-helpers.h
>> @@ -15,9 +15,11 @@
>>  
>>  #include "libqtest.h"
>>  
>> -bool migrate_watch_for_stop(QTestState *who, const char *name,
>> -                            QDict *event, void *opaque);
>> -bool migrate_watch_for_resume(QTestState *who, const char *name,
>> +typedef struct QTestMigrationState {
>> +    bool stop_seen, resume_seen;
>> +} QTestMigrationState;
>> +
>> +bool migrate_watch_for_events(QTestState *who, const char *name,
>>                                QDict *event, void *opaque);
>>  
>>  G_GNUC_PRINTF(3, 4)
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 62d3f37..526a1b7 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -43,8 +43,8 @@
>>  unsigned start_address;
>>  unsigned end_address;
>>  static bool uffd_feature_thread_id;
>> -static bool got_src_stop;
>> -static bool got_dst_resume;
>> +static QTestMigrationState src_state;
>> +static QTestMigrationState dst_state;
>>  
>>  /*
>>   * An initial 3 MB offset is used as that corresponds
>> @@ -188,6 +188,13 @@ static void wait_for_serial(const char *side)
>>      } while (true);
>>  }
>>  
>> +static void wait_for_stop(QTestState *who, QTestMigrationState *state)
>> +{
>> +    if (!state->stop_seen) {
>> +        qtest_qmp_eventwait(who, "STOP");
>> +    }
>> +}
>> +
>>  /*
>>   * It's tricky to use qemu's migration event capability with qtest,
>>   * events suddenly appearing confuse the qmp()/hmp() responses.
>> @@ -235,21 +242,19 @@ static void read_blocktime(QTestState *who)
>>      qobject_unref(rsp_return);
>>  }
>>  
>> +/*
>> + * Wait for two changes in the migration pass count, but bail if we stop.
>> + */
>>  static void wait_for_migration_pass(QTestState *who)
>>  {
>> -    uint64_t initial_pass = get_migration_pass(who);
>> -    uint64_t pass;
>> +    uint64_t pass, prev_pass = 0, changes = 0;
>>  
>> -    /* Wait for the 1st sync */
>> -    while (!got_src_stop && !initial_pass) {
>> -        usleep(1000);
>> -        initial_pass = get_migration_pass(who);
>> -    }
>> -
>> -    do {
>> +    while (changes < 2 && !src_state.stop_seen) {
>>          usleep(1000);
>>          pass = get_migration_pass(who);
>> -    } while (pass == initial_pass && !got_src_stop);
>> +        changes += (pass != prev_pass);
>> +        prev_pass = pass;
>> +    }
> 
> Here won't it start to wait for 2 iterations every time instead of 1?
> 
> Note that previously we only wait for 1 iteration as long as not the
> initial pass.  

I don't think so.  Both the old and new code require at least a transition from
pass 0 to 1, and pass 1 to 2, to return.  With the old:
  when initial_pass becomes non-zero, done with first loop
  when pass changes again, done with 2nd loop

- Steve

> And I think the change will double the counts for below..
> 
>             while (args->iterations > 1) {
>                 wait_for_migration_pass(from);
>                 args->iterations--;
>             }
> 
> The event-related changes are all fine, but maybe leave this piece as before?
> 
>>  }
>>  
>>  static void check_guests_ram(QTestState *who)
>> @@ -586,10 +591,7 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
>>  {
>>      qtest_qmp_assert_success(from, "{ 'execute': 'migrate-start-postcopy' }");
>>  
>> -    if (!got_src_stop) {
>> -        qtest_qmp_eventwait(from, "STOP");
>> -    }
>> -
>> +    wait_for_stop(from, &src_state);
>>      qtest_qmp_eventwait(to, "RESUME");
>>  }
>>  
>> @@ -720,8 +722,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>          }
>>      }
>>  
>> -    got_src_stop = false;
>> -    got_dst_resume = false;
>> +    dst_state = (QTestMigrationState) { };
>> +    src_state = (QTestMigrationState) { };
>> +
>>      bootpath = g_strdup_printf("%s/bootsect", tmpfs);
>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>          /* the assembled x86 boot sector should be exactly one sector large */
>> @@ -801,8 +804,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>      if (!args->only_target) {
>>          *from = qtest_init(cmd_source);
>>          qtest_qmp_set_event_callback(*from,
>> -                                     migrate_watch_for_stop,
>> -                                     &got_src_stop);
>> +                                     migrate_watch_for_events,
>> +                                     &src_state);
>>      }
>>  
>>      cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
>> @@ -821,8 +824,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>                                   ignore_stderr);
>>      *to = qtest_init(cmd_target);
>>      qtest_qmp_set_event_callback(*to,
>> -                                 migrate_watch_for_resume,
>> -                                 &got_dst_resume);
>> +                                 migrate_watch_for_events,
>> +                                 &dst_state);
>>  
>>      /*
>>       * Remove shmem file immediately to avoid memory leak in test failed case.
>> @@ -1516,9 +1519,7 @@ static void test_precopy_common(MigrateCommon *args)
>>           */
>>          if (args->result == MIG_TEST_SUCCEED) {
>>              qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
>> -            if (!got_src_stop) {
>> -                qtest_qmp_eventwait(from, "STOP");
>> -            }
>> +            wait_for_stop(from, &src_state);
>>              migrate_ensure_converge(from);
>>          }
>>      }
>> @@ -1560,9 +1561,8 @@ static void test_precopy_common(MigrateCommon *args)
>>               */
>>              wait_for_migration_complete(from);
>>  
>> -            if (!got_src_stop) {
>> -                qtest_qmp_eventwait(from, "STOP");
>> -            }
>> +            wait_for_stop(from, &src_state);
>> +
>>          } else {
>>              wait_for_migration_complete(from);
>>              /*
>> @@ -1575,7 +1575,7 @@ static void test_precopy_common(MigrateCommon *args)
>>              qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
>>          }
>>  
>> -        if (!got_dst_resume) {
>> +        if (!dst_state.resume_seen) {
>>              qtest_qmp_eventwait(to, "RESUME");
>>          }
>>  
>> @@ -1696,9 +1696,7 @@ static void test_ignore_shared(void)
>>  
>>      migrate_wait_for_dirty_mem(from, to);
>>  
>> -    if (!got_src_stop) {
>> -        qtest_qmp_eventwait(from, "STOP");
>> -    }
>> +    wait_for_stop(from, &src_state);
>>  
>>      qtest_qmp_eventwait(to, "RESUME");
>>  
>> @@ -2139,7 +2137,7 @@ static void test_migrate_auto_converge(void)
>>              break;
>>          }
>>          usleep(20);
>> -        g_assert_false(got_src_stop);
>> +        g_assert_false(src_state.stop_seen);
>>      } while (true);
>>      /* The first percentage of throttling should be at least init_pct */
>>      g_assert_cmpint(percentage, >=, init_pct);
>> @@ -2481,9 +2479,7 @@ static void test_multifd_tcp_cancel(void)
>>  
>>      migrate_ensure_converge(from);
>>  
>> -    if (!got_src_stop) {
>> -        qtest_qmp_eventwait(from, "STOP");
>> -    }
>> +    wait_for_stop(from, &src_state);
>>      qtest_qmp_eventwait(to2, "RESUME");
>>  
>>      wait_for_serial("dest_serial");
>> -- 
>> 1.8.3.1
>>
>
Steve Sistare Nov. 13, 2023, 7:20 p.m. UTC | #3
On 11/13/2023 1:33 PM, Steven Sistare wrote:
> On 8/30/2023 1:00 PM, Peter Xu wrote:
>> On Tue, Aug 29, 2023 at 11:18:02AM -0700, Steve Sistare wrote:
[...]
>>> +/*
>>> + * Wait for two changes in the migration pass count, but bail if we stop.
>>> + */
>>>  static void wait_for_migration_pass(QTestState *who)
>>>  {
>>> -    uint64_t initial_pass = get_migration_pass(who);
>>> -    uint64_t pass;
>>> +    uint64_t pass, prev_pass = 0, changes = 0;
>>>  
>>> -    /* Wait for the 1st sync */
>>> -    while (!got_src_stop && !initial_pass) {
>>> -        usleep(1000);
>>> -        initial_pass = get_migration_pass(who);
>>> -    }
>>> -
>>> -    do {
>>> +    while (changes < 2 && !src_state.stop_seen) {
>>>          usleep(1000);
>>>          pass = get_migration_pass(who);
>>> -    } while (pass == initial_pass && !got_src_stop);
>>> +        changes += (pass != prev_pass);
>>> +        prev_pass = pass;
>>> +    }
>>
>> Here won't it start to wait for 2 iterations every time instead of 1?
>>
>> Note that previously we only wait for 1 iteration as long as not the
>> initial pass.  
> 
> I don't think so.  Both the old and new code require at least a transition from
> pass 0 to 1, and pass 1 to 2, to return.  With the old:
>   when initial_pass becomes non-zero, done with first loop
>   when pass changes again, done with 2nd loop

OK, you refer to count of iterations that call usleep(1000), not pass count.
Yes, the new code may call usleep(1000) twice instead of once.
args->iterations at the caller is at most 2, so this is a tiny difference.
However, I could improve it like so:

static void wait_for_migration_pass(QTestState *who)
{
    uint64_t pass = get_migration_pass(who);
    uint64_t changes = (pass > 0);
    uint64_t prev_pass;

    while (changes < 2 && !src_state.stop_seen && !src_state.suspend_seen) {
        usleep(1000);
        prev_pass = pass;
        pass = get_migration_pass(who);
        changes += (pass != prev_pass);
    }
}

- Steve

>> And I think the change will double the counts for below..
>>
>>             while (args->iterations > 1) {
>>                 wait_for_migration_pass(from);
>>                 args->iterations--;
>>             }
diff mbox series

Patch

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index be00c52..b541108 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -23,26 +23,16 @@ 
  */
 #define MIGRATION_STATUS_WAIT_TIMEOUT 120
 
-bool migrate_watch_for_stop(QTestState *who, const char *name,
-                            QDict *event, void *opaque)
-{
-    bool *seen = opaque;
-
-    if (g_str_equal(name, "STOP")) {
-        *seen = true;
-        return true;
-    }
-
-    return false;
-}
-
-bool migrate_watch_for_resume(QTestState *who, const char *name,
+bool migrate_watch_for_events(QTestState *who, const char *name,
                               QDict *event, void *opaque)
 {
-    bool *seen = opaque;
+    QTestMigrationState *state = opaque;
 
-    if (g_str_equal(name, "RESUME")) {
-        *seen = true;
+    if (g_str_equal(name, "STOP")) {
+        state->stop_seen = true;
+        return true;
+    } else if (g_str_equal(name, "RESUME")) {
+        state->resume_seen = true;
         return true;
     }
 
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 009e250..59fbb83 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -15,9 +15,11 @@ 
 
 #include "libqtest.h"
 
-bool migrate_watch_for_stop(QTestState *who, const char *name,
-                            QDict *event, void *opaque);
-bool migrate_watch_for_resume(QTestState *who, const char *name,
+typedef struct QTestMigrationState {
+    bool stop_seen, resume_seen;
+} QTestMigrationState;
+
+bool migrate_watch_for_events(QTestState *who, const char *name,
                               QDict *event, void *opaque);
 
 G_GNUC_PRINTF(3, 4)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 62d3f37..526a1b7 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -43,8 +43,8 @@ 
 unsigned start_address;
 unsigned end_address;
 static bool uffd_feature_thread_id;
-static bool got_src_stop;
-static bool got_dst_resume;
+static QTestMigrationState src_state;
+static QTestMigrationState dst_state;
 
 /*
  * An initial 3 MB offset is used as that corresponds
@@ -188,6 +188,13 @@  static void wait_for_serial(const char *side)
     } while (true);
 }
 
+static void wait_for_stop(QTestState *who, QTestMigrationState *state)
+{
+    if (!state->stop_seen) {
+        qtest_qmp_eventwait(who, "STOP");
+    }
+}
+
 /*
  * It's tricky to use qemu's migration event capability with qtest,
  * events suddenly appearing confuse the qmp()/hmp() responses.
@@ -235,21 +242,19 @@  static void read_blocktime(QTestState *who)
     qobject_unref(rsp_return);
 }
 
+/*
+ * Wait for two changes in the migration pass count, but bail if we stop.
+ */
 static void wait_for_migration_pass(QTestState *who)
 {
-    uint64_t initial_pass = get_migration_pass(who);
-    uint64_t pass;
+    uint64_t pass, prev_pass = 0, changes = 0;
 
-    /* Wait for the 1st sync */
-    while (!got_src_stop && !initial_pass) {
-        usleep(1000);
-        initial_pass = get_migration_pass(who);
-    }
-
-    do {
+    while (changes < 2 && !src_state.stop_seen) {
         usleep(1000);
         pass = get_migration_pass(who);
-    } while (pass == initial_pass && !got_src_stop);
+        changes += (pass != prev_pass);
+        prev_pass = pass;
+    }
 }
 
 static void check_guests_ram(QTestState *who)
@@ -586,10 +591,7 @@  static void migrate_postcopy_start(QTestState *from, QTestState *to)
 {
     qtest_qmp_assert_success(from, "{ 'execute': 'migrate-start-postcopy' }");
 
-    if (!got_src_stop) {
-        qtest_qmp_eventwait(from, "STOP");
-    }
-
+    wait_for_stop(from, &src_state);
     qtest_qmp_eventwait(to, "RESUME");
 }
 
@@ -720,8 +722,9 @@  static int test_migrate_start(QTestState **from, QTestState **to,
         }
     }
 
-    got_src_stop = false;
-    got_dst_resume = false;
+    dst_state = (QTestMigrationState) { };
+    src_state = (QTestMigrationState) { };
+
     bootpath = g_strdup_printf("%s/bootsect", tmpfs);
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         /* the assembled x86 boot sector should be exactly one sector large */
@@ -801,8 +804,8 @@  static int test_migrate_start(QTestState **from, QTestState **to,
     if (!args->only_target) {
         *from = qtest_init(cmd_source);
         qtest_qmp_set_event_callback(*from,
-                                     migrate_watch_for_stop,
-                                     &got_src_stop);
+                                     migrate_watch_for_events,
+                                     &src_state);
     }
 
     cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
@@ -821,8 +824,8 @@  static int test_migrate_start(QTestState **from, QTestState **to,
                                  ignore_stderr);
     *to = qtest_init(cmd_target);
     qtest_qmp_set_event_callback(*to,
-                                 migrate_watch_for_resume,
-                                 &got_dst_resume);
+                                 migrate_watch_for_events,
+                                 &dst_state);
 
     /*
      * Remove shmem file immediately to avoid memory leak in test failed case.
@@ -1516,9 +1519,7 @@  static void test_precopy_common(MigrateCommon *args)
          */
         if (args->result == MIG_TEST_SUCCEED) {
             qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
-            if (!got_src_stop) {
-                qtest_qmp_eventwait(from, "STOP");
-            }
+            wait_for_stop(from, &src_state);
             migrate_ensure_converge(from);
         }
     }
@@ -1560,9 +1561,8 @@  static void test_precopy_common(MigrateCommon *args)
              */
             wait_for_migration_complete(from);
 
-            if (!got_src_stop) {
-                qtest_qmp_eventwait(from, "STOP");
-            }
+            wait_for_stop(from, &src_state);
+
         } else {
             wait_for_migration_complete(from);
             /*
@@ -1575,7 +1575,7 @@  static void test_precopy_common(MigrateCommon *args)
             qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
         }
 
-        if (!got_dst_resume) {
+        if (!dst_state.resume_seen) {
             qtest_qmp_eventwait(to, "RESUME");
         }
 
@@ -1696,9 +1696,7 @@  static void test_ignore_shared(void)
 
     migrate_wait_for_dirty_mem(from, to);
 
-    if (!got_src_stop) {
-        qtest_qmp_eventwait(from, "STOP");
-    }
+    wait_for_stop(from, &src_state);
 
     qtest_qmp_eventwait(to, "RESUME");
 
@@ -2139,7 +2137,7 @@  static void test_migrate_auto_converge(void)
             break;
         }
         usleep(20);
-        g_assert_false(got_src_stop);
+        g_assert_false(src_state.stop_seen);
     } while (true);
     /* The first percentage of throttling should be at least init_pct */
     g_assert_cmpint(percentage, >=, init_pct);
@@ -2481,9 +2479,7 @@  static void test_multifd_tcp_cancel(void)
 
     migrate_ensure_converge(from);
 
-    if (!got_src_stop) {
-        qtest_qmp_eventwait(from, "STOP");
-    }
+    wait_for_stop(from, &src_state);
     qtest_qmp_eventwait(to2, "RESUME");
 
     wait_for_serial("dest_serial");