Re: [Qemu-devel] [RFC PATCH v2 10/12] mc: expose tunable parameter for checkpointing frequency

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 03/12/2014 06:49 AM, Eric Blake wrote:
> On 03/11/2014 04:15 PM, Juan Quintela wrote:
>> Eric Blake <eblake@redhat.com> wrote:
>>> On 02/18/2014 01:50 AM, mrhines@linux.vnet.ibm.com wrote:
>>>> From: "Michael R. Hines" <mrhines@us.ibm.com>
>>> We're building up a LOT of migrate- tunable commands.  Maybe it's time
>>> to think about building a more generic migrate-set-parameter, which
>>> takes both the name of the parameter to set and its value, so that a
>>> single command serves all parameters, instead of needing a proliferation
>>> of commands.  Of course, for that to be useful, we also need a way to
>>> introspect which parameters can be tuned; whereas with the current
>>> approach of one command per parameter (well, 2 for set vs. get) the
>>> introspection is based on whether the command exists.
>> I asked to have that.  My suggestion was that
>>
>> migrate_set_capability auto-throotle on
>>
>> So we could add it to new variables without extra change.
>>
>> And I agree that having a way to read them, and ask what values they
>> have is a good idea.
>>
>> Luiz, any good idea about how to do it through QMP?
> I'm trying to thing of a back-compat method, which exploits the fact
> that we now have flat unions (something we didn't have when
> migrate-set-capabilities was first added).  Maybe something like:
>
> { 'type': 'MigrationCapabilityBase',
>    'data': { 'capability': 'MigrationCapability' } }
> { 'type': 'MigrationCapabilityBool',
>    'data': { 'state': 'bool' } }
> { 'type': 'Migration CapabilityInt',
>    'data': { 'value': 'int' } }
> { 'union': 'MigrationCapabilityStatus',
>    'base': 'MigrationCapabilityBase',
>    'discriminator': 'capability',
>    'data': {
>      'xbzrle': 'MigrationCapabilityBool',
>      'auto-converge': 'MigrationCapabilityBool',
> ...
>      'mc-delay': 'MigrationCapabilityInt'
>    } }
>
> along with a tweak to query-migrate-capabilities for full back-compat:
>
> # @query-migrate-capabilities
> # @extended: #optional defaults to false; set to true to see non-boolean
> capabilities (since 2.1)
> { 'command: 'query-migrate-capabilities',
>    'data': { '*extended': 'bool' },
>    'returns': ['MigrationCapabilityStatus'] }
>
> Now, observe what happens.  If an old client calls { "execute":
> "query-migrate-capabilities" }, they get a return that lists ONLY the
> boolean members of the MigrationCapabilityStatus array (good, because if
> we returned a non-boolean, we would confuse the consumer when they are
> expecting a 'state' variable that is not present) - what's more, this
> representation is identical on the wire to the format used in earlier
> qemu.  But new clients can call { "execute":
> "query-migrate-capabilities", "arguments": { "extended": true } }, and
> get back:
>
> { "capabilities": [
>     { "capability": "xbzrle", "state": true },
>     { "capability": "auto-converge", "state": false },
> ...
>     { "capability": "mc-delay", "value": 100 }
>    ] }
>
> Also, once a new client has learned of non-boolean extended
> capabilities, they can also set them via the existing command:
> { "execute": "migrate-set-capabilities",
>    "arguments": [
>       { "capability": "xbzrle", "state", false },
>       { "capability": "mc-delay", "value", 200 }
>    ] }
>
> So, what do you think?  My slick type manipulation means that we need
> zero new commands, just a new option the the query command, and a new
> flat union type that replaces the current struct type.  The existence
> (but not the type) of non-boolean parameters is already introspectible
> to a client new enough to request an 'extended' query, and down the
> road, if we ever gain full QAPI introspection, then a client also would
> gain the ability to learn the type of any non-boolean parameter as well.
>   Stability wise, as long as we never change the type of a capability
> once first exposed, then if a client plans on using a particular
> parameter when available, it can already hard-code what type that
> parameter should have without even needing full QAPI introspection (that
> is, if libvirt is taught to manipulate mc-delay, libvirt will already
> know to expect mc-delay as an int, and not any other type, and merely
> needs to probe if qemu supports the 'mc-delay' extended capability).
> And of course, this new schema idea can retroactively cover all existing
> migration tunables, such as migrate_set_downtime, migrate_set_speed,
> migrate-set-cache-size, and so on.

I like this a lot - it's very complicated, but it is clean, I think.

Maybe you should add some "reserved" fields in there as well
to the union, in case you want to expand the number of members
of the union in the future?

- Michael