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

[Markus Armbruster]

"Li, Liang Z" <address@hidden> writes:

>> > +#
>> > +# Migration parameter information
>> > +#
>> > +# @compress-level: compression level
>> > +#
>> > +# @compress-threads: compression thread count # #
>> > address@hidden: decompression thread count # # Since: 2.3 ## {
>> > +'union': 'MigrationParameterStatus',
>> > +  'base': 'MigrationParameterBase',
>> > +  'discriminator': 'parameter',
>> > +  'data': { 'compress-level': 'MigrationParameterInt',
>> > +            'compress-threads': 'MigrationParameterInt',
>> > +            'decompress-threads': 'MigrationParameterInt'} } # #
>> > address@hidden # # Set the following migration parameters
>> > +(like compress-level) # # @parameters: json array of parameter
>> > +modifications to make # # Since: 2.3 ## { 'command':
>> > +'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.
>
> 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.  

I don't understand.

Schema for the three parameters we have now:

{ 'command': 'migrate-set-parameters',
  'data': { '*compression-level': 'int',
            '*compression-threads': 'int,
            '*decompression-threads': 'int' } }

Let's add a parameter with some other type:

{ 'command': 'migrate-set-parameters',
  'data': { '*compression-level': 'int',
            '*compression-threads': 'int,
            '*compression-algorithm': 'CompressionAlgorithm',
            '*decompression-threads': 'int', } }

CompressionAlgorithm could be an enum.

Why wouldn't that work?

>> > +##
>> > +# @query-migrate-parameters
>> > +#
>> > +# Returns information about the current migration parameters status #
>> > +# Returns: @MigrationParametersStatus # # Since: 2.3 ## { 'command':
>> > +'query-migrate-parameters',
>> > +  'returns': ['MigrationParameterStatus'] } ##
>> >  ##
>> >  # @MouseInfo:
>> >  #
>> 
>> Produces a list of key-value pairs.  Looks like this (stolen from the same
>> place):
>> 
>>     {
>>        "return": [
>>           {
>>              "parameter": "compress-level",
>>              "value": 1
>>           },
>>           {
>>              "parameter": "compress-threads",
>>              "value": 8
>>           },
>>           {
>>              "parameter": "decompress-threads",
>>              "value": 2
>>           }
>>        ]
>>     }
>> 
>> I'd very much prefer a simple object instead:
>> 
>>     { "return": { "compress-level": 1,
>>                   "compress-threads": 8,
>>                   "decompress-threads": 2 } }
>> 
>
> The same reason.

{ 'command': 'query-migrate-parameters',
  'returns': 'MigrationParameters' }

{ 'type': 'MigrationParameters',
  'data': { 'compression-level': 'int',
            'compression-threads': 'int,
            'decompression-threads': 'int' } }

Add one as above:

{ 'type': 'MigrationParameters',
  'data': { 'compression-level': 'int',
            'compression-threads': 'int,
            'compression-algorithm': 'CompressionAlgorithm',
            'decompression-threads': 'int', } }

Why wouldn't that work?