Re: [PATCH v2 02/29] tests/qtest: Move QTestMigrationState to libqtest

[Fabiano Rosas <farosas@suse.de>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Oct 23, 2023 at 05:35:41PM -0300, Fabiano Rosas wrote:
>> Move the QTestMigrationState into QTestState so we don't have to pass
>> it around to the wait_for_* helpers anymore. Since QTestState is
>> private to libqtest.c, move the migration state struct to libqtest.h
>> and add a getter.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  tests/qtest/libqtest.c          | 14 ++++++++++
>>  tests/qtest/libqtest.h          | 23 ++++++++++++++++
>>  tests/qtest/migration-helpers.c | 18 +++++++++++++
>>  tests/qtest/migration-helpers.h |  8 +++---
>>  tests/qtest/migration-test.c    | 47 +++++++++------------------------
>>  5 files changed, 72 insertions(+), 38 deletions(-)
>> 
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index f33a210861..f7e85486dc 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -87,6 +87,7 @@ struct QTestState
>>      GList *pending_events;
>>      QTestQMPEventCallback eventCB;
>>      void *eventData;
>> +    QTestMigrationState *migration_state;
>
> It feels wrong to have something called MigrationState in the
> general qtest code. In the end there's nothing particularly
> migration related about this though.
>
> With that in mind, we could just rename it to "QTestEventState"
> instead.
>

Ok, will do.

>>  };
>>  
>>  static GHookList abrt_hooks;
>> @@ -500,6 +501,8 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>>          s->irq_level[i] = false;
>>      }
>>  
>> +    s->migration_state = g_new0(QTestMigrationState, 1);
>> +
>>      /*
>>       * Stopping QEMU for debugging is not supported on Windows.
>>       *
>> @@ -601,6 +604,7 @@ void qtest_quit(QTestState *s)
>>      close(s->fd);
>>      close(s->qmp_fd);
>>      g_string_free(s->rx, true);
>> +    g_free(s->migration_state);
>>  
>>      for (GList *it = s->pending_events; it != NULL; it = it->next) {
>>          qobject_unref((QDict *)it->data);
>> @@ -854,6 +858,11 @@ void qtest_qmp_set_event_callback(QTestState *s,
>>      s->eventData = opaque;
>>  }
>>  
>> +void qtest_qmp_set_migration_callback(QTestState *s, QTestQMPEventCallback cb)
>> +{
>> +    qtest_qmp_set_event_callback(s, cb, s->migration_state);
>> +}
>> +
>>  QDict *qtest_qmp_event_ref(QTestState *s, const char *event)
>>  {
>>      while (s->pending_events) {
>> @@ -1906,3 +1915,8 @@ bool mkimg(const char *file, const char *fmt, unsigned size_mb)
>>  
>>      return ret && !err;
>>  }
>> +
>> +QTestMigrationState *qtest_migration_state(QTestState *s)
>> +{
>> +    return s->migration_state;
>> +}
>> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
>> index 6e3d3525bf..0421a1da24 100644
>> --- a/tests/qtest/libqtest.h
>> +++ b/tests/qtest/libqtest.h
>> @@ -23,6 +23,20 @@
>>  
>>  typedef struct QTestState QTestState;
>>  
>> +struct QTestMigrationState {
>> +    bool stop_seen;
>> +    bool resume_seen;
>> +};
>> +typedef struct QTestMigrationState QTestMigrationState;
>> +
>> +/**
>> + * qtest_migration_state:
>> + * @s: #QTestState instance to operate on.
>> + *
>> + * Returns: #QTestMigrationState instance.
>> + */
>> +QTestMigrationState *qtest_migration_state(QTestState *s);
>> +
>>  /**
>>   * qtest_initf:
>>   * @fmt: Format for creating other arguments to pass to QEMU, formatted
>> @@ -288,6 +302,15 @@ typedef bool (*QTestQMPEventCallback)(QTestState *s, const char *name,
>>  void qtest_qmp_set_event_callback(QTestState *s,
>>                                    QTestQMPEventCallback cb, void *opaque);
>>  
>> +/**
>> + * qtest_qmp_set_migration_callback:
>> + * @s: #QTestSTate instance to operate on
>> + * @cb: callback to invoke for events
>> + *
>> + * Like qtest_qmp_set_event_callback, but includes migration state events
>> + */
>> +void qtest_qmp_set_migration_callback(QTestState *s, QTestQMPEventCallback cb);
>> +
>>  /**
>>   * qtest_qmp_eventwait:
>>   * @s: #QTestState instance to operate on.
>> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> index fd3b94efa2..cffa525c81 100644
>> --- a/tests/qtest/migration-helpers.c
>> +++ b/tests/qtest/migration-helpers.c
>> @@ -92,6 +92,24 @@ void migrate_set_capability(QTestState *who, const char *capability,
>>                               capability, value);
>>  }
>>  
>> +void wait_for_stop(QTestState *who)
>> +{
>> +    QTestMigrationState *state = qtest_migration_state(who);
>> +
>> +    if (!state->stop_seen) {
>> +        qtest_qmp_eventwait(who, "STOP");
>> +    }
>> +}
>> +
>> +void wait_for_resume(QTestState *who)
>> +{
>> +    QTestMigrationState *state = qtest_migration_state(who);
>> +
>> +    if (!state->resume_seen) {
>> +        qtest_qmp_eventwait(who, "RESUME");
>> +    }
>> +}
>
> I'd be included to put them into the libqtest.c file too eg
>
>   qtest_wait_for_resume/qtst_wait_for_stop
>

Haven't I done this already? It must have gotten lost in the various
rebases.

Thanks