On Mon, Apr 14, 2025 at 02:40:25PM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <berra...@redhat.com> writes:
>
> > On Mon, Apr 14, 2025 at 02:12:35PM -0300, Fabiano Rosas wrote:
> >> Daniel P. Berrangé <berra...@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.
Yes, modern approach should require 'defer'.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
