Re: [PATCH v3 32/51] tests/qtest/migration: Adapt convergence routines to config

[Peter Xu <peterx@redhat.com>]

On Mon, Dec 15, 2025 at 07:00:18PM -0300, Fabiano Rosas wrote:
> Adapt the convergence routines migrate_ensure_[non_]converge to set
> the convergence parameters in the config dict it instead of using
> migrate-set-parameters.
> 
> Some tests need to change the convergence parameters during the
> migration. The config object method is specific to configuration prior
> to starting a migration, so by design it's not suitable to effect
> migration-runtime changes. The existing routines will be kept for this
> purpose (renamed with 'ongoing' for clarity).
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration/framework.c     | 10 ++++-----
>  tests/qtest/migration/migration-qmp.c | 32 +++++++++++++++++++++++++--
>  tests/qtest/migration/migration-qmp.h |  6 +++--
>  tests/qtest/migration/misc-tests.c    |  4 ++--
>  tests/qtest/migration/precopy-tests.c | 26 +++++++++-------------
>  5 files changed, 52 insertions(+), 26 deletions(-)
> 
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index fd15bd832e..df42a8a2c6 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -583,7 +583,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>          args->postcopy_data = args->start_hook(from, to);
>      }
>  
> -    migrate_ensure_non_converge(from);
> +    migrate_ensure_non_converge(from, args->start.config);
>      migrate_prepare_for_dirty_mem(from);
>      qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
>                               "  'arguments': { "
> @@ -872,7 +872,7 @@ int test_precopy_common(MigrateCommon *args)
>      }
>  
>      if (args->live) {
> -        migrate_ensure_non_converge(from);
> +        migrate_ensure_non_converge(from, args->start.config);
>          migrate_prepare_for_dirty_mem(from);
>      } else {
>          /*
> @@ -884,7 +884,7 @@ int test_precopy_common(MigrateCommon *args)
>          if (args->result == MIG_TEST_SUCCEED) {
>              qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
>              wait_for_stop(from, &src_state);
> -            migrate_ensure_converge(from);
> +            migrate_ongoing_ensure_converge(from);
>          }
>      }
>  
> @@ -942,7 +942,7 @@ int test_precopy_common(MigrateCommon *args)
>              }
>              migrate_wait_for_dirty_mem(from, to);
>  
> -            migrate_ensure_converge(from);
> +            migrate_ongoing_ensure_converge(from);
>  
>              /*
>               * We do this first, as it has a timeout to stop us
> @@ -1047,7 +1047,7 @@ void test_file_common(MigrateCommon *args, bool stop_src)
>          data_hook = args->start_hook(from, to);
>      }
>  
> -    migrate_ensure_converge(from);
> +    migrate_ensure_converge(from, args->start.config);
>      wait_for_serial("src_serial");
>  
>      if (stop_src) {
> diff --git a/tests/qtest/migration/migration-qmp.c b/tests/qtest/migration/migration-qmp.c
> index 5c46ceb3e6..7fe47a5793 100644
> --- a/tests/qtest/migration/migration-qmp.c
> +++ b/tests/qtest/migration/migration-qmp.c
> @@ -499,20 +499,48 @@ void migrate_set_parameter_bool(QTestState *who, const char *parameter,
>      migrate_check_parameter_bool(who, parameter, value);
>  }
>  
> -void migrate_ensure_non_converge(QTestState *who)
> +void migrate_ongoing_ensure_non_converge(QTestState *who)
>  {
>      /* Can't converge with 1ms downtime + 3 mbs bandwidth limit */
>      migrate_set_parameter_int(who, "max-bandwidth", 3 * 1000 * 1000);
>      migrate_set_parameter_int(who, "downtime-limit", 1);
>  }
>  
> -void migrate_ensure_converge(QTestState *who)
> +void migrate_ongoing_ensure_converge(QTestState *who)
>  {
>      /* Should converge with 30s downtime + 1 gbs bandwidth limit */
>      migrate_set_parameter_int(who, "max-bandwidth", 1 * 1000 * 1000 * 1000);
>      migrate_set_parameter_int(who, "downtime-limit", 30 * 1000);
>  }
>  
> +void migrate_ensure_non_converge(QTestState *who, QDict *config)
> +{
> +    config = config_load(config);
> +    if (config) {
> +        /* Can't converge with 1ms downtime + 3 mbs bandwidth limit */
> +        qdict_put_int(config, "max-bandwidth", 3 * 1000 * 1000);
> +        qdict_put_int(config, "downtime-limit", 1);
> +    } else {
> +        assert(who);
> +        migrate_ongoing_ensure_non_converge(who);
> +    }
> +    config_put(config);
> +}
> +
> +void migrate_ensure_converge(QTestState *who, QDict *config)
> +{
> +    config = config_load(config);
> +    /* Should converge with 30s downtime + 1 gbs bandwidth limit */
> +    if (config) {
> +        qdict_put_int(config, "max-bandwidth", 1 * 1000 * 1000 * 1000);
> +        qdict_put_int(config, "downtime-limit", 30 * 1000);
> +    } else {
> +        assert(who);
> +        migrate_ongoing_ensure_converge(who);
> +    }
> +    config_put(config);
> +}

It's slightly an overkill to me to have these converge helpers to provide
two versions.  Also a bit confusing on when should we use which.

After all, parameters touched on convergence must be able to be dynamically
set..

Can we always stick with the QMP set-parameters for all these?

> +
>  void migrate_pause(QTestState *who)
>  {
>      qtest_qmp_assert_success(who, "{ 'execute': 'migrate-pause' }");
> diff --git a/tests/qtest/migration/migration-qmp.h b/tests/qtest/migration/migration-qmp.h
> index 9a36a677ba..e465c69094 100644
> --- a/tests/qtest/migration/migration-qmp.h
> +++ b/tests/qtest/migration/migration-qmp.h
> @@ -39,8 +39,10 @@ void migrate_set_parameter_strv(QTestState *who, const char *parameter,
>  void migrate_set_parameter_null(QTestState *who, const char *parameter);
>  void migrate_set_parameter_bool(QTestState *who, const char *parameter,
>                                  int value);
> -void migrate_ensure_non_converge(QTestState *who);
> -void migrate_ensure_converge(QTestState *who);
> +void migrate_ongoing_ensure_non_converge(QTestState *who);
> +void migrate_ongoing_ensure_converge(QTestState *who);
> +void migrate_ensure_non_converge(QTestState *who, QDict *config);
> +void migrate_ensure_converge(QTestState *who, QDict *config);
>  void migrate_pause(QTestState *who);
>  void migrate_continue(QTestState *who, const char *state);
>  void migrate_recover(QTestState *who, const char *uri);
> diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
> index 61bdfda857..0a737cb54f 100644
> --- a/tests/qtest/migration/misc-tests.c
> +++ b/tests/qtest/migration/misc-tests.c
> @@ -68,7 +68,7 @@ static void test_analyze_script(char *name, MigrateCommon *args)
>      file = g_strdup_printf("%s/migfile", tmpfs);
>      uri = g_strdup_printf("exec:cat > %s", file);
>  
> -    migrate_ensure_converge(from);
> +    migrate_ensure_converge(from, args->start.config);
>      migrate_qmp(from, to, uri, NULL, "{}");
>      wait_for_migration_complete(from);
>  
> @@ -102,7 +102,7 @@ static void test_ignore_shared(char *name, MigrateCommon *args)
>          return;
>      }
>  
> -    migrate_ensure_non_converge(from);
> +    migrate_ensure_non_converge(from, args->start.config);
>      migrate_prepare_for_dirty_mem(from);
>  
>      /* Wait for the first serial output from the source */
> diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
> index ab5789717f..eabbbf39c3 100644
> --- a/tests/qtest/migration/precopy-tests.c
> +++ b/tests/qtest/migration/precopy-tests.c
> @@ -374,7 +374,7 @@ static void test_auto_converge(char *name, MigrateCommon *args)
>       * Set the initial parameters so that the migration could not converge
>       * without throttling.
>       */
> -    migrate_ensure_non_converge(from);
> +    migrate_ensure_non_converge(from, args->start.config);
>  
>      /* To check remaining size after precopy */
>      migrate_set_capability(from, "pause-before-switchover", true);
> @@ -427,7 +427,7 @@ static void test_auto_converge(char *name, MigrateCommon *args)
>      g_assert_cmpint(hit, ==, 1);
>  
>      /* Now, when we tested that throttling works, let it converge */
> -    migrate_ensure_converge(from);
> +    migrate_ongoing_ensure_converge(from);
>  
>      /*
>       * Wait for pre-switchover status to check last throttle percentage
> @@ -562,7 +562,7 @@ static void test_multifd_tcp_cancel(MigrateCommon *args, bool postcopy_ram)
>          return;
>      }
>  
> -    migrate_ensure_non_converge(from);
> +    migrate_ensure_non_converge(from, args->start.config);
>      migrate_prepare_for_dirty_mem(from);
>  
>      if (postcopy_ram) {
> @@ -623,14 +623,12 @@ static void test_multifd_tcp_cancel(MigrateCommon *args, bool postcopy_ram)
>      /* Start incoming migration from the 1st socket */
>      migrate_incoming_qmp(to2, "tcp:127.0.0.1:0", NULL, "{}");
>  
> -    migrate_ensure_non_converge(from);
> +    migrate_ensure_non_converge(from, args->start.config);
>  
>      migrate_qmp(from, to2, NULL, NULL, "{}");
>  
>      migrate_wait_for_dirty_mem(from, to2);
> -
> -    migrate_ensure_converge(from);
> -
> +    migrate_ongoing_ensure_converge(from);
>      wait_for_stop(from, get_src());
>      qtest_qmp_eventwait(to2, "RESUME");
>  
> @@ -659,7 +657,7 @@ static void test_cancel_src_after_failed(QTestState *from, QTestState *to,
>       */
>  
>      wait_for_serial("src_serial");
> -    migrate_ensure_converge(from);
> +    migrate_ensure_converge(from, args->config);
>  
>      migrate_qmp(from, to, uri, NULL, "{}");
>  
> @@ -684,7 +682,7 @@ static void test_cancel_src_after_cancelled(QTestState *from, QTestState *to,
>      migrate_incoming_qmp(to, uri, NULL, "{ 'exit-on-error': false }");
>  
>      wait_for_serial("src_serial");
> -    migrate_ensure_converge(from);
> +    migrate_ensure_converge(from, args->config);
>  
>      migrate_qmp(from, to, uri, NULL, "{}");
>  
> @@ -709,7 +707,7 @@ static void test_cancel_src_after_complete(QTestState *from, QTestState *to,
>      migrate_incoming_qmp(to, uri, NULL, "{ 'exit-on-error': false }");
>  
>      wait_for_serial("src_serial");
> -    migrate_ensure_converge(from);
> +    migrate_ensure_converge(from, args->config);
>  
>      migrate_qmp(from, to, uri, NULL, "{}");
>  
> @@ -739,7 +737,7 @@ static void test_cancel_src_after_none(QTestState *from, QTestState *to,
>  
>      migrate_incoming_qmp(to, uri, NULL, "{ 'exit-on-error': false }");
>  
> -    migrate_ensure_converge(from);
> +    migrate_ensure_converge(from, args->config);
>      migrate_qmp(from, to, uri, NULL, "{}");
>  
>      wait_for_migration_complete(from);
> @@ -759,7 +757,7 @@ static void test_cancel_src_pre_switchover(QTestState *from, QTestState *to,
>      migrate_incoming_qmp(to, uri, NULL, "{ 'exit-on-error': false }");
>  
>      wait_for_serial("src_serial");
> -    migrate_ensure_converge(from);
> +    migrate_ensure_converge(from, args->config);
>  
>      migrate_qmp(from, to, uri, NULL, "{}");
>  
> @@ -1066,9 +1064,6 @@ static void migrate_dirty_limit_wait_showup(QTestState *from,
>      migrate_set_parameter_int(from, "x-vcpu-dirty-limit-period", period);
>      migrate_set_parameter_int(from, "vcpu-dirty-limit", value);
>  
> -    /* Make sure migrate can't converge */
> -    migrate_ensure_non_converge(from);
> -
>      /* To check limit rate after precopy */
>      migrate_set_capability(from, "pause-before-switchover", true);
>  
> @@ -1128,6 +1123,7 @@ static void test_dirty_limit(char *name, MigrateCommon *args)
>      }
>  
>      /* Prepare for dirty limit migration and wait src vm show up */
> +    migrate_ensure_non_converge(from, args->start.config);
>      migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
>  
>      /* Start migrate */
> -- 
> 2.51.0
> 

-- 
Peter Xu