On 06/05/2015 03:50 AM, Dr. David Alan Gilbert wrote: > Hi, > Is there any way that we could make it easier to add new migration > parameters? The current way is complicated and error prone; > as far as I can tell, to add a new parameter we need to: > > 1) qapi-schema.json > a) Add to 'MigrationParameter' enum, include comment > b) Add to migrate-set-parameters > c) Add to MigrationParameters
Perhaps putting a "see XXX for use" could reduce documentation
duplication, but it doesn't help the need to add to the enum and to
multiple clients.
> 2) Define the 'default' macro at the top of migration.c
> 3) Add the initialisation to migrate_get_current to set the default
If we get to the point where qapi can define default values for
variables, then the defaulting moves out of migration.c into the .json file.
> 4) qmp_migrate_set_parameters:
> a) Add the 'has' and value arguments to qmp_migrate_set_parameters
> *** Make really sure this matches the order in migrate-set-parameters!
Also, moving defaults into qapi will eliminate the need for the has_
counterpart on optional variables (the C code will be passed the
defaulted value, if the user omitted the variable at the QMP layer).
> b) Add a bounds check on the value
Once we have the qapi syntax for defaults, it would not be that much
more work to move bounds checking into qapi. For example:
'data': { 'value': { 'type': 'uint', 'default': 1, 'max': 10 } }
would be a reasonable way to document an option that can range from 0 to
10 but defaults to 1.
> c) Set the value in the array if the has_ is true
> 5) Fixup migrate_init to preserve the parameter around the init
> 6) Add a bool and case entry to hmp_migrate_set_parameter and
> pass to qmp_migrate_set_parameters
> *** Make sure you get the order to qmp_migrate_set_parameters right
Is there a way to pass a QDict instead of individual parameters to make
this part easier? Back when we started adding blockdev-add, a lot of
the magic was related to adding code for passing dictionaries around
(keeping things in name/value pairs through more of the call stack)
rather than adding parameters right and left at all points.
> 7) Fixup hmp_info_migrate_parameters
>
> oh, and don't forget to:
> 8) add the entries to qmp_query_migrate_parameters
>
> (I forgot).
Yeah, that's a lot to do. I'm not sure if there is anything else that
can be done to make it more automatic in some of those places, but even
having a list of things to touch helps future additions. Maybe worth
something in docs/?
>
>
> The three separate changes needed in the qapi-schema.json seem odd,
> and the 'has'/value pairs on qmp_migrate_set_parameters is just a nightmare
> because there's nothing to check the ordering, and it's just getting
> a silly number of arguments to the function now (I've got 10
> parameters in one of my dev worlds, so that function has 21 arguments).
>
> In my ideal world there would be:
> a) One thing to add to qapi-schema.json
> b) qmp_migrate_set_parameters would take an array pointer indexed
> by the enum
> c) A way to define the bounds so that we didn't have to manually
> add the bound checking.
> d) Something where I defined the default value
Not sure I can simplify a) or b); but c) and d) seem doable at the qapi
level.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
