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

[Peter Xu]

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