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

[Eric Blake]

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?

> 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',

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org