Re: [PATCH V2 06/10] tests/qtest: migration events

[Fabiano Rosas]

Steve Sistare <steven.sistare@oracle.com> writes:

> 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>

I'm working on top of this patch in another series and it works
fine. I have a patch adding the migration events to it (setup, active,
completed, etc).

> ---
>  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 b0c355b..2dde3af 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;

It's a bit cumbersome though to have the QTestMigrationState in
migration-test.c and having to pass it around. Could we maybe move it
inside QTestState? That way it is easily reachable from
migrate-helpers.c and we could move all the wait* functions there.