Re: [Qemu-devel] [PATCH v10 13/24] replay: implement replay-seek command

[Markus Armbruster]

Pavel Dovgalyuk <address@hidden> writes:

> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
> the execution to the specified instruction count.
> The command automatically loads nearest snapshot and replays the execution
> to find the desired instruction count.
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
>
> --
>
> v2:
>  - renamed replay_seek qmp command into replay-seek
>    (suggested by Eric Blake)
> v7:
>  - small fixes related to Markus Armbruster's review
> v9:
>  - changed 'step' parameter name to 'icount'
>  - moved json stuff to replay.json and updated the description
>    (suggested by Markus Armbruster)
> v10:
>  - updated the descriptions
> ---
>  hmp-commands.hx           |   19 +++++++++
>  hmp.h                     |    1 
>  qapi/replay.json          |   20 ++++++++++
>  replay/replay-debugging.c |   92 
> +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 132 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d529f24..57ec43c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1924,6 +1924,25 @@ The command is ignored when there are no replay 
> breakpoints.
>  ETEXI
>  
>      {
> +        .name       = "replay_seek",
> +        .args_type  = "icount:i",
> +        .params     = "icount",
> +        .help       = "replay execution to the specified instruction count",
> +        .cmd        = hmp_replay_seek,
> +    },
> +
> +STEXI
> address@hidden replay_seek @var{icount}
> address@hidden replay_seek
> +Automatically proceeds to the instruction count @var{icount}, when
> +replaying the execution. The command automatically loads nearest
> +snapshot and replays the execution to find the desired instruction.
> +When there is no preceding snapshot or the execution is not replayed,
> +then the command is ignored and error message is displayed.

Sounds like roundabout way to say "then the command fails".  If that's
correct, then let's phrase it that way.

> +icount for the reference may be observed with 'info replay' command.
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.h b/hmp.h
> index c9b9b4f..d6e1d7e 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -151,5 +151,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
>  void hmp_info_replay(Monitor *mon, const QDict *qdict);
>  void hmp_replay_break(Monitor *mon, const QDict *qdict);
>  void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
> +void hmp_replay_seek(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi/replay.json b/qapi/replay.json
> index cb94991..0b5afce 100644
> --- a/qapi/replay.json
> +++ b/qapi/replay.json
> @@ -99,3 +99,23 @@
>  #
>  ##
>  { 'command': 'replay-delete-break' }
> +
> +##
> +# @replay-seek:
> +#
> +# Automatically proceeds to the instruction count @icount, when
> +# replaying the execution. The command automatically loads nearest
> +# snapshot and replays the execution to find the desired instruction.
> +# When there is no preceding snapshot or the execution is not replayed,
> +# then the command is ignored and error is returned.
> +# icount for the reference may be obtained with @query-replay command.

My comment on hmp-commands.hx applies.

> +#
> +# @icount: target instruction count
> +#
> +# Since: 4.0
> +#
> +# Example:
> +#
> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
> +##
> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }

[...]

QAPI schema part
Acked-by: Markus Armbruster <address@hidden>

Please consider my comment suggestions regardless.