Re: [PATCH V1 07/32] savevm: QMP command for cprinfo

[Steven Sistare]

On 7/30/2020 12:17 PM, Eric Blake wrote:
> On 7/30/20 10:14 AM, Steve Sistare wrote:
>> Provide the cprinfo QMP command.  This returns a string with a space-
>> separated list of modes supported by cprsave, and can be used by clients
>> as a feature test to check if the running QEMU instance supports cprsave.
> 
> When you've already got array support in the QMP language, why are you making 
> the user parse a string into an array after the fact?

Will fix as you suggest, thanks.  I had HMP on the brain - Steve

>> Syntax:
>>    {'command':'cprinfo', 'returns':'str'}
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
> 
>> +++ b/qapi/migration.json
>> @@ -1623,6 +1623,15 @@
>>     'data': { 'device-id': 'str' } }
>>     ##
>> +# @cprinfo:
>> +#
>> +# Return a space-delimited list of modes supported by the cprsave command
>> +#
>> +# Since 5.0
>> +##
>> +{ 'command': 'cprinfo', 'returns': 'str' }
> 
> Returning a 'str' is non-extensible.  The fact that you had to edit the 
> whitelist is proof that you should have done something better.  I recommend:
> 
> { 'command': 'cprinfo', 'returns': { 'modes': [ 'CprMode' ] }
> 
> using the CprMode enum I proposed earlier.
> 
>> +
>> +##
>>   # @cprsave:
>>   #
>>   # Create a checkpoint of the virtual machine device state in @file.
>> diff --git a/qapi/pragma.json b/qapi/pragma.json
>> index cffae27..43bdb39 100644
>> --- a/qapi/pragma.json
>> +++ b/qapi/pragma.json
>> @@ -5,6 +5,7 @@
>>   { 'pragma': {
>>       # Commands allowed to return a non-dictionary:
>>       'returns-whitelist': [
>> +        'cprinfo',
> 
> This should not be needed.  Design the return value correctly in the first 
> place.
> 
>>           'human-monitor-command',
>>           'qom-get',
>>           'query-migrate-cache-size',
>>
>