Re: [RFC PATCH 00/13] migration: Unify capabilities and parameters

[Fabiano Rosas <farosas@suse.de>]

Markus Armbruster <armbru@redhat.com> writes:

> Daniel P. Berrangé <berrange@redhat.com> writes:
>
>> On Fri, Apr 11, 2025 at 04:14:30PM -0300, Fabiano Rosas wrote:
>>> Open questions:
>>> ---------------
>>> 
>>> - Deprecations/compat?
>>> 
>>> I think we should deprecate migrate-set/query-capabilities and everything to do
>>> with capabilities (specifically the validation in the JSON at the end of the
>>> stream).
>>> 
>>> For migrate-set/query-parameters, we could probably keep it around indefinitely,
>>> but it'd be convenient to introduce new commands so we can give them new
>>> semantics.
>>> 
>>> - How to restrict the options that should not be set when the migration is in
>>> progress?
>>> 
>>> i.e.:
>>>   all options can be set before migration (initial config)
>>>   some options can be set during migration (runtime)
>>> 
>>> I thought of adding another type at the top of the hierarchy, with
>>> just the options allowed to change at runtime, but that doesn't really
>>> stop the others being also set at runtime. I'd need a way to have a
>>> set of options that are rejected 'if migration_is_running()', without
>>> adding more duplication all around.
>>> 
>>> - What about savevm?
>>> 
>>> None of this solves the issue of random caps/params being set before
>>> calling savevm. We still need to special-case savevm and reject
>>> everything. Unless we entirely deprecate setting initial options via
>>> set-parameters (or set-config) and require all options to be set as
>>> savevm (and migrate) arguments.
>>
>> I'd suggest we aim for a world where the commands take all options
>> as direct args and try to remove the global state eventually.
>
> Yes.
>
> Even better: make it a job.
>

What do we gain from that in relation to being able to pass ~50
parameters to a command? I don't see it. I think that's the actual crux
here, too many options and how to get them from the QAPI into the
migration core for consumption.

The current usage of MigrationParameters as both the return type for
query-set-parameters and the global parameter store for the migration
state is really dissonant. What do the has_* fields even mean when
accessed via MigrationState::parameters? This series is not doing any
better in that regard, mind you. I'm almost tempted to ask that we
expose the QDict from the marshaling function directly to the migration
code, at least that's a data type that makes sense internally.