On Mon, Dec 15, 2025 at 06:59:51PM -0300, Fabiano Rosas wrote:
> The migration parameters tls_creds, tls_authz and tls_hostname
> currently have a non-uniform handling. When used as arguments to
> migrate-set-parameters, their type is StrOrNull and when used as
> return value from query-migrate-parameters their type is a plain
> string.
>
> Not only having to convert between the types is cumbersome, but it
> also creates the issue of requiring two different QAPI types to be
> used, one for each command. MigrateSetParameters is used for
> migrate-set-parameters with the TLS arguments as StrOrNull while
> MigrationParameters is used for query-migrate-parameters with the TLS
> arguments as str.
>
> Since StrOrNull could be considered a superset of str, change the type
> of the TLS arguments in MigrationParameters to StrOrNull. Also ensure
> that QTYPE_QNULL is never used.
>
> 1) migrate-set-parameters will always write QTYPE_QSTRING to
> s->parameters, either an empty or non-empty string.
>
> 2) query-migrate-parameters will always return a QTYPE_QSTRING, either
> empty or non-empty.
>
> 3) the migrate_tls_* helpers will always return a non-empty string or
> NULL, for the internal migration code's consumption.
>
> Points (1) and (2) above help simplify the parameters validation and
> the query command handling because s->parameters is already kept in
> the format that query-migrate-parameters (and info migrate_paramters)
> expect. Point (3) is so people don't need to care about StrOrNull in
> migration code.
>
> This will allow the type duplication to be removed in the next
> patches.
>
> Note that the type of @tls_creds, @tls-hostname, @tls-authz changes
> from str to StrOrNull in introspection of the query-migrate-parameters
> command. We accept this imprecision to enable de-duplication.
>
> There's no need to free the TLS options in
> migration_instance_finalize() because they're freed by the qdev
> properties .release method.
>
> Temporary in this patch:
> migrate_params_test_apply() copies s->parameters into a temporary
> structure, so it's necessary to drop the references to the TLS options
> if they were not set by the user to avoid double-free. This is fixed
> in the next patches.
>
> Acked-by: Markus Armbruster <[email protected]>
> Signed-off-by: Fabiano Rosas <[email protected]>
[...]
> @@ -403,6 +403,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const
> QDict *qdict)
> monitor_printf(mon, "%s: %" PRIu64 " bytes\n",
> MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
> params->xbzrle_cache_size);
> + monitor_printf(mon, "%s: %" PRIu64 "\n",
> +
> MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH),
> + params->max_postcopy_bandwidth);
>
> if (params->has_block_bitmap_mapping) {
> const BitmapMigrationNodeAliasList *bmnal;
This chunk seems to be introduced by accident and removed in patch 18..
After removal, feel free to take:
Reviewed-by: Peter Xu <[email protected]>
--
Peter Xu
