Re: [Qemu-devel] [v5 12/12] migration: Add commands to set and query parameter

[Eric Blake]

On 03/18/2015 08:30 PM, Li, Liang Z wrote:

>>> +'migrate-set-parameters',
>>> +  'data': { 'parameters': ['MigrationParameterStatus'] } }
>>
>> The command takes a list of key-value pairs.  Looks like this (example stolen
>> from your patch to qmp-commands.hx):
>>
>>     { "execute": "migrate-set-parameters",
>>       "arguments": { "parameters":
>>                      [ { "parameter": "compress-level", "value": 1 } ] } }
>>
>> Awkward.  I'd very much prefer
>>
>>     { "execute": "migrate-set-parameters",
>>       "arguments": { "compress-level", 1 } }
>>
>> I.e. the command simply takes the parameters as optional arguments.
>> Simpler, and a natural use of the schema language.

Indeed, if we are going to add a new command, then we don't need to
worry about making it super-generic, and can avoid the nesting of
complex types.

> 
> Yes, it seems complicated.  Eric suggested to use this type of interface, 
> because it can 
> support different type of parameters if some new parameters will be added.  

When I originally suggested complex nested types, I was suggesting that
we _reuse_ the existing migrate-set-capabilities (by making it set both
boolean and integer capabilities in one go).  If we do that, then we
need the struct complexity.  But since we are proposing a new command
rather than shoe-horning into an existing command, we might as well do
it cleanly to begin with.  So I like Markus' suggestion that we
eliminate some of the complexity and just go with specifying parameters
directly.


>> I'd very much prefer a simple object instead:
>>
>>     { "return": { "compress-level": 1,
>>                   "compress-threads": 8,
>>                   "decompress-threads": 2 } }
>>
> 
> The same reason.

I like the idea as well.  Sorry for misleading you into a more complex
initial implementation when a simpler will do.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature