Re: [PATCH 19/21] migration: Allow migrate commands to provide the migration config

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> On Fri, Jun 06, 2025 at 05:23:18PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > On Mon, Jun 02, 2025 at 10:38:08PM -0300, Fabiano Rosas wrote:
>> >> Allow the migrate and migrate_incoming commands to pass the migration
>> >> configuration options all at once, dispensing the use of
>> >> migrate-set-parameters and migrate-set-capabilities.
>> >> 
>> >> The motivation of this is to simplify the interface with the
>> >> management layer and avoid the usage of several command invocations to
>> >> configure a migration. It also avoids stale parameters from a previous
>> >> migration to influence the current migration.
>> >> 
>> >> The options that are changed during the migration can still be set
>> >> with the existing commands.
>> >> 
>> >> The order of precedence is:
>> >> 
>> >> 'config' argument > -global cmdline > defaults (migration_properties)
>> >
>> > Could we still keep the QMP migrate-set-parameters values?
>> >
>> >   'config' argument > QMP setups using migrate-set-parameters >
>> >     -global cmdline > defaults (migration_properties)
>> >
>> 
>> That's the case. I failed to mention it in the commit message. IOW it
>> behaves just like today, but the new 'config' way takes precedence over
>> all.
>
> Referring to below chunk of code:
>
> [...]
>
>> >> +bool migrate_params_override(MigrationState *s, MigrationParameters *new,
>> >> +                             Error **errp)
>> >> +{
>> >> +    ERRP_GUARD();
>> >> +
>> >> +    assert(bql_locked());
>> >> +
>> >> +    /* reset to default parameters */
>> >> +    migrate_params_apply(&s->defaults);
>
> IIUC here it'll reset all global parameters using the initial defaults
> first, then apply the "config" specified in "migrate" QMP command?
>

Yes, this is so any previously set parameter via migrate-set-parameter
gets erased. I think what we want (but feel free to disagree) is to have
the migrate-set-parameter _eventually_ only handle parameters that need
to be modifed during migration runtime. Anything else can be done via
passing config to qmp_migrate.

For -global, I don't have a preference. Having -global take precedence
over all would require a way to know which options were present in the
command-line and which are just the defaults seet in
migration_properties. I currently don't know how to do that. If it is at
all possible (within reason) we could make the change, no worries.

> I think there're actually two separate questions to be asked, to make it
> clearer, they are:

Here it got ambiguous when you say "global", I've been using -global to
refer to the cmdline -global migration.foo, but others have used global
to mean s->parameters (which has an extended lifetime). Could you
clarify?

>
>   (1) Whether we should allow QMP "migrate" 'config' parameter to overwrite
>       global setup?
>
>   (2) Whether we should allow previous QMP global setup to be used even if
>       QMP "migrate" provided 'config' parameter?
>
> So IIUC the patch does (1) YES (2) NO, while what I think might be more
> intuitive is (1) NO (2) YES.
>
>> >> +
>> >> +    /* overwrite with the new ones */
>> >> +    qmp_migrate_set_parameters(new, errp);
>> >> +    if (*errp) {
>> >> +        return false;
>> >> +    }
>> >> +
>> >> +    return true;
>> >> +}