On 07/17/2014 05:06 AM, Pavel Dovgalyuk wrote:
> This patch adds support for replay_info monitor command. This command
> returns the information about replay execution (replay mode and current step).
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovga...@ispras.ru>
> ---

> +++ b/qapi-schema.json
> @@ -3491,3 +3491,30 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @ReplayInfo:
> +#
> +# Information about replay process
> +#
> +# @mode: replay mode (none, play, record)
> +#
> +# @submode: play submode

What are the valid submodes?  Does the set of submodes differ according
to the main mode?  If so, is it better to use a flat union, where the
subtype is strongly typed to the correct limited possibilities according
to the mode?

> +#
> +# @step: current step of record or play
> +#
> +# Since:  2.2
> +##
> +{ 'type': 'ReplayInfo',
> +  'data': {'mode': 'str', 'submode': 'str', 'step': 'uint64'} }

Using 'str' to open-code a finite set of strings is not nice.  Please add:
 { 'enum': 'ReplayMode', 'data': [ 'none', 'play', 'record' ] }
then use 'mode': 'ReplayMode'.


> +++ b/qmp-commands.hx
> @@ -3755,3 +3755,17 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +
> +    {
> +        .name       = "replay_info",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_replay_info,
> +    },
> +
> +SQMP
> +replay_info
> +-----------
> +
> +Shows information about replay process.
> +

No example?


> +
> +const char *replay_get_mode_name(void)
> +{
> +    switch (replay_mode) {
> +    case REPLAY_NONE:
> +        return "none";
> +    case REPLAY_PLAY:
> +        return "replay";
> +    case REPLAY_SAVE:
> +        return "record";
> +    default:
> +        return "unknown";
> +    }
> +}
> +

No need to open-code this.  If you properly set up an enum type in the
.json file, then you get the conversions between enum values and strings
for free from the generated QAPI framework.

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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to