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

22 Jun 2023


      On Thu, Jun 22, 2023 at 12:01:55PM +0200, Juan Quintela wrote:
...
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,
PS: we may want to postpone this to be later than migration_object_init(),
when/if there's a real patch.
...
...
&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.
If we worry on breaking anyone, we can apply the keyval parsing only when
!exec.  Not sure whether it matters a huge lot..
...
Aha, that makes sense.  And will allow us to deprecate/remove the
--global migration.* stuff.
We may still need a way to set the caps/params for src qemu?..

Thanks,

-- 
Peter Xu