Re: [RFC 4/6] migration: Deprecate -incoming <uri>

[Juan Quintela]

Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 6/22/23 10:52, Juan Quintela wrote:
>> User friendliness.
>> The problem is that if you use more than two channels with multifd, on
>> the incoming side, you need to do:
>
> You're sacrificing user-friendliness for the 99.99% that don't use
> multifd, for an error (i.e. it's not even fixing the issue) for the
> 0.01% that use multifd.  That's not user-friendly.

You are forgeting of the 0.01% that uses postocopy preempt (that is easy
just changing the default value to 2), and the 0.0000001% that uses
compression (they have exactly the same problem with compress_threads,
compress_zlib, etc).

>> - migrate_set_parameter multifd-channels 16
>> - migrate_incoming <uri>
>> 
>>> The issue is not how many features the command line has, but how
>>> they're implemented.
>> Or if they are confusing for the user?
>
> Anyone using multifd is not a typical user anyway.

>>> If they're just QMP wrappers and as such they're self-contained in
>>> softmmu/vl.c, that's fine.
>>>
>>> In fact, even for parameters, we could use keyval to parse "-incoming"
>> What is keyval?
>
> util/keyval.c and include/qemu/keyval.h.  It parses a list of
> key=value pairs into a QDict.  Once you have removed the "source" key
> from the QDict you can use a visitor to parse the rest into a
> MigrateSetParameters.  See the handling of QEMU_OPTION_audio, it could
> be something like
>
>
>             case QEMU_OPTION_incoing: {
>                 Visitor *v;
>                 MigrateSetParameters *incoming_params = NULL;
>                 QDict *dict = keyval_parse(optarg, "source", NULL,
>                 &error_fatal);
>
>                 if (incoming) {
>                     if (qdict_haskey(dict, "source")) {
>                         error_setg(&error_fatal, "Parameter 'source'
>                         is duplicate");
>                     }
>                 } else {
>                     if (!qdict_haskey(dict, "source")) {
>                         error_setg(&error_fatal, "Parameter 'source'
>                         is missing");
>                     }
>                     runstate_set(RUN_STATE_INMIGRATE);
>                     incoming = g_strdup(qdict_get_str(dict, "source"));
>                     qdict_del(dict, "source");
>                 }
>
>                 v = qobject_input_visitor_new_keyval(QOBJECT(dict));
>                 qobject_unref(dict);
>                 visit_type_MigrateSetParameters(v, NULL,
>                 &incoming_params, &error_fatal);
>                 visit_free(v);
>                 qmp_migration_set_parameters(incoming_params,
>                 &error_fatal);
>                 qapi_free_MigrateSetParameters(incoming_params);
>             }
>
>
> For example "-incoming [source=]tcp:foo,multifd-channels=16" would
> desugar to
>
>   migrate_set_parameter multifd-channels 16
>   migrate_incoming tcp:foo
>
> The only incompatibility is for people who are using "," in an URI,
> which is rare and only an issue for the "exec" protocol.

Aha, that makes sense.  And will allow us to deprecate/remove the
--global migration.* stuff.

Thanks very much.

See why this was an RFC O:-)

Later, Juan.

>
> Paolo
>
>>> and
>>> set the parameters in the same place as above.  That would remove the need
>>> for "-global migration".
>> Could you elaborate?
>
>
>
>> The other option that I can think of is changing the error messages for
>> migrate_check_parameters() and give instructions that you can't set
>> multifd channels once that you have started incoming migration.
>> Explaining there to use migrate_incoming command?
>> Later, Juan.
>>