Fabiano Rosas <faro...@suse.de> writes: > Markus Armbruster <arm...@redhat.com> writes: > >> 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. >> >> Yes. >> >> Even better: make it a job. > > What do we gain from that in relation to being able to pass ~50 > parameters to a command? I don't see it. I think that's the actual crux > here, too many options and how to get them from the QAPI into the > migration core for consumption.
Two separate interface design problems: 1. Query and manipulate complex configuration 2. Control and monitor long-running jobs Let's start with 1. Migration is not the only instance of complex configuration. Block devices are arguably even more complex. A difference: we can have many block devices, but just one migration. Here's the usual way to configure things: commands creating a thing take the thing's configuration as arguments, other commands query and manipulate a thing's configuration. Works fine both for simple and complex configuration. Block devices work this way. Migration doesn't. Instead, we have global configuration state, and commands to query and manipulate it. Also works. Instead of passing the entire configuration to the "create" command, you can pass it piecemeal to "manipulate global configuration" commands. You pay for the convenience by having to reset the entire global configuration to a known state in certain situations. Since use of the global configuration state is implicit, it can be surprising, as Daniel pointed out for savevm. Overall, this isn't simpler or more convenient than the usual way, just different. Both ways use QAPI objects to hold configuration. Query commands can return such an object. Commands that deal with partial configuration can only take the same object when its members are optional. In places where configuration is complete (internal configuration state, the query command that returns it), optional makes no sense, and is annoying. We either accept that, or duplicate the configuration object. Duplicating violates DRY. It does result in better command documentation, though. On to 2. Migration is not the only kind of long-running job. There's also block-commit, block-stream, drive-mirror, drive-backup, blockdev-create, x-blockdev-amend, snapshot-load, snapshot-save, snapshot-delete. These all create a "job" object that represents the long-running job. We have commands and events to control and monitor such jobs: job-pause, job-resume, job-cancel, job-complete, job-dismiss, job-finalize, query-jobs, JOB_STATUS_CHANGE. Migration does its own thing instead. > The current usage of MigrationParameters as both the return type for > query-set-parameters and the global parameter store for the migration > state is really dissonant. What do the has_* fields even mean when > accessed via MigrationState::parameters? Yes, it's ugly, and the alternative is differently ugly, as discussed above. > This series is not doing any > better in that regard, mind you. I'm almost tempted to ask that we > expose the QDict from the marshaling function directly to the migration > code, at least that's a data type that makes sense internally. Based on experience in the block layer, I predict you'd regret this :) Use of native C data types is just so much easier on the eyes than messing with QObjects. Moreover, the compiler can't help you as much with QObjects.
