Peter Xu <pet...@redhat.com> writes:
> On Fri, Jun 06, 2025 at 05:23:18PM -0300, Fabiano Rosas wrote:
>> Peter Xu <pet...@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;
>> >> +}
