Peter Xu <[email protected]> writes:
> On Mon, Dec 15, 2025 at 07:00:16PM -0300, Fabiano Rosas wrote:
>> The tests are being refactored to pass migration options to QEMU using
>> the new API of passing a JSON object as argument the migration
>> commands instead of using several calls to the
>> migrate_set_capabilities|parameters commands.
>>
>> Since multiple tests share common infrastructure (framework.c,
>> migration-utils.c, migration-qmp.c), it's cumbersome to convert tests
>> in small chunks, which would require changes to every common function
>> to accept both the new and old ways.
>>
>> After some tinkering, an easier way to do this transition is to allow
>> the tests to set a key in the config dict itself telling whether the
>> config is supported. With this, the common functions can be fully
>> altered to support the config object, as long as they check this
>> temporary key and do the right thing.
>>
>> QEMU doesn't know about this hack, so some code is needed to hide it
>> when issuing QMP commands with the config object.
>>
>> This will all be removed once tests are fully converted.
>>
>> Signed-off-by: Fabiano Rosas <[email protected]>
>> ---
>> tests/qtest/migration/migration-qmp.h | 1 -
>> tests/qtest/migration/migration-util.c | 1 +
>> tests/qtest/migration/migration-util.h | 34 ++++++++++++++++++++++++++
>> 3 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qtest/migration/migration-qmp.h
>> b/tests/qtest/migration/migration-qmp.h
>> index 940ffd5950..9a36a677ba 100644
>> --- a/tests/qtest/migration/migration-qmp.h
>> +++ b/tests/qtest/migration/migration-qmp.h
>> @@ -47,5 +47,4 @@ void migrate_recover(QTestState *who, const char *uri);
>> void migrate_cancel(QTestState *who);
>> void migrate_postcopy_start(QTestState *from, QTestState *to,
>> QTestMigrationState *src_state);
>> -
>> #endif /* MIGRATION_QMP_H */
>> diff --git a/tests/qtest/migration/migration-util.c
>> b/tests/qtest/migration/migration-util.c
>> index 416dd10ef8..e702f00896 100644
>> --- a/tests/qtest/migration/migration-util.c
>> +++ b/tests/qtest/migration/migration-util.c
>> @@ -255,6 +255,7 @@ static void migration_test_wrapper(const void *data)
>>
>> test->data = g_new0(MigrateCommon, 1);
>> test->data->start.config = qdict_new();
>> + qdict_put_bool(test->data->start.config, "use-config", false);
>>
>> g_test_message("Running /%s%s", qtest_get_arch(), test->name);
>> test->func(test->name, test->data);
>> diff --git a/tests/qtest/migration/migration-util.h
>> b/tests/qtest/migration/migration-util.h
>> index e73d69bab0..3c3b5a8777 100644
>> --- a/tests/qtest/migration/migration-util.h
>> +++ b/tests/qtest/migration/migration-util.h
>> @@ -60,4 +60,38 @@ void migration_test_add_suffix(const char *path, const
>> char *suffix,
>> char *migrate_get_connect_uri(QTestState *who);
>> void migrate_set_ports(QTestState *to, QList *channel_list);
>>
>> +/*
>> + * Scaffolding to allow the framework _common functions and _qmp
>> + * functions to use the config object while some tests are still using
>> + * migrate_set_*. Tests that have been converted will set use-config =
>> + * true on the config dict.
>> + */
>> +static bool has_key;
>> +static bool use_config;
>
> Looks like this is temp measure, so no strong opinions.. said that, it
> looks tricky to have the two globals shared between all the tests, and
> having magic keys in the qdict.
>
It is tricky, but it works. The other options all require "passing
something" in, which ends up touching good code and causing a mess with
rebases and the overall clarity of the patches. But let me read about
your suggestions below...
> Can we pass in MigrateStart* for config_load() and config_put()? Then at
> least we can change globals into per-test flags of MigrateStart.
>
> Btw, AFAIU the two helpers should always used in a pair but load() and
> put() do not look like a pair..
>
My mind went to vcpu_load/vcpu_put from kvm code. =D
> If we can have args->use_config as a bool, having tests opt-in config
> setups by setting it, then I wonder if we can do that like:
>
The migrate_qmp commands don't take args. So I'd have to alter their
signature just for this temporary state. That's why I put the flag in
the dict itself.
> if (args->use_config) {
> // do whatever with args->config...
> } else {
> // covered by other migrate-set-parameters QMP commands..
> }
>
> Do we really need config_put()? I'll keep reading, but please evaluate..
>
Because of the migrate_incoming_qmp and -incoming calls, we need to take
the key out of the dict to hide it. Then put it back so the rest of the
code, e.g. migrate_qmp can use it.
>> +static inline QDict *config_load(QDict *config)
>> +{
>> + if (!config) {
>> + return NULL;
>> + }
>> +
>> + has_key = qdict_haskey(config, "use-config");
>> + if (has_key) {
>> + use_config = qdict_get_try_bool(config, "use-config", false);
>> + qdict_del(config, "use-config");
>> + }
>> +
>> + if (use_config) {
>> + return config;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static inline void config_put(QDict *config)
>> +{
>> + if (config && has_key) {
>> + qdict_put_bool(config, "use-config", use_config);
>> + }
>> +}
>> +
>> #endif /* MIGRATION_UTIL_H */
>> --
>> 2.51.0
>>
