Re: [RFC PATCH 00/13] migration: Unify capabilities and parameters

[Fabiano Rosas <farosas@suse.de>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Apr 14, 2025 at 02:12:35PM -0300, Fabiano Rosas wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > On Fri, Apr 11, 2025 at 04:14:30PM -0300, Fabiano Rosas wrote:
>> >> Open questions:
>> >> ---------------
>> >> 
>> >> - Deprecations/compat?
>> >> 
>> >> I think we should deprecate migrate-set/query-capabilities and everything to do
>> >> with capabilities (specifically the validation in the JSON at the end of the
>> >> stream).
>> >> 
>> >> For migrate-set/query-parameters, we could probably keep it around indefinitely,
>> >> but it'd be convenient to introduce new commands so we can give them new
>> >> semantics.
>> >> 
>> >> - How to restrict the options that should not be set when the migration is in
>> >> progress?
>> >> 
>> >> i.e.:
>> >>   all options can be set before migration (initial config)
>> >>   some options can be set during migration (runtime)
>> >> 
>> >> I thought of adding another type at the top of the hierarchy, with
>> >> just the options allowed to change at runtime, but that doesn't really
>> >> stop the others being also set at runtime. I'd need a way to have a
>> >> set of options that are rejected 'if migration_is_running()', without
>> >> adding more duplication all around.
>> >> 
>> >> - What about savevm?
>> >> 
>> >> None of this solves the issue of random caps/params being set before
>> >> calling savevm. We still need to special-case savevm and reject
>> >> everything. Unless we entirely deprecate setting initial options via
>> >> set-parameters (or set-config) and require all options to be set as
>> >> savevm (and migrate) arguments.
>> >
>> > I'd suggest we aim for a world where the commands take all options
>> > as direct args and try to remove the global state eventually.
>> >
>> 
>> Well, except the options that are adjusted during migration. But yes, I
>> agree. It all depends on how we proceed with keeping the old commands
>> around and for how long. If they're still around we can't stop people
>> from using them and later invoking "savevm" for instance.
>> 
>> > For savevm/loadvm in particular it is very much a foot-gun that
>> > 'migrate-set-*' will affect them, because savevm/loadvm aren't
>> > obviously connected to 'migrate-*' commands unless you're aware
>> > of how QEMU implements savevm internally.
>> >
>> 
>> Yes, I could perhaps reset all options once savevm is called, maybe that
>> would be acceptable, then we don't need to check and block every single
>> one. Once we add support to migration options to savevm, then they'd be
>> set in the savevm command-line from day 1 and those wouldn't be
>> reset. We could also keep HMP restricted to savevm without any migration
>> options. That's be easy to enforce. If the user wants fancy savevm, they
>> can invoke via QMP.
>
> Can we make the two approaches mutually exclusive ? Taking your
> 'migrate' command example addition:
>
>   { 'command': 'migrate',
>     'data': {'*uri': 'str',
>              '*channels': [ 'MigrationChannel' ],
>   +          '*config': 'MigrationConfig',
>              '*detach': 'bool', '*resume': 'bool' } }
>
> if 'migrate' is invoked with the '*config' data being non-nil,
> then we should ignore *all* global state previously set with
> migrate-set-XXXX, and exclusively use '*config'.
>
> That gives a clean semantic break between old and new approaches,
> without us having to worry about removing the existing commands
> quickly.
>

Good idea. I will need to do something about the -global options because
they also set the defaults for the various options. But we should be
able to decouple setting defaults from -global. Or I could just apply
-global again on top of what came in '*config'.

>
>> >> - incoming defer?
>> >> 
>> >> It seems we cannot do the final step of removing
>> >> migrate-set-capabilites before we have a form of handshake
>> >> implemented. That would take the config from qmp_migrate on source and
>> >> send it to the destination for negotiation.
>> >
>> > I'm not sure I understand why the QAPI design changes are tied
>> > to the new protocol handshake ? I guess you're wanting to avoid
>> > updating 'migrate_incoming' to accept the new parameters directly ?
>> >
>> 
>> Yes, without migrate-set-capabilities, we'd need to pass an enormous
>> command line to -incoming defer to be able to enable capabilities on the
>> destination. With the handshake, we could transfer them over the wire
>> somehow. Does that make sense?
>
> '-incoming defer' still gets paired with 'migrate-incoming' on the
> target, so no matter what, there's no reason to ever pass parameters
> on the CLI with '-incoming defer'.
>

Oops, I misread the strcmp in vl.c. I mean -incoming uri is the one
that'll need a huge cmdline.

But if we follow your suggestion above we could just tie -incoming URI
to the existing commands and make the new format require defer.

>
> With regards,
> Daniel