Re: [Qemu-devel] [PATCH v9 13/21] replay: implement replay-seek command to proceed to the desired step

[Markus Armbruster]

Pavel Dovgalyuk <address@hidden> writes:

> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
> the execution to the specified step.
> The command automatically loads nearest snapshot and replay the execution
> to find the desired step.

"step" again.

>
> 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)
> ---
>  hmp-commands.hx           |   19 +++++++++
>  hmp.h                     |    1 
>  qapi/replay.json          |   20 ++++++++++
>  replay/replay-debugging.c |   91 
> +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 6d04c02..2839acc 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       = "rewinds replay to the step specified by icount",

We use imperative mood in .help: "rewind".

"Rewind" suggests going backward.  The command can do that.  But it can
also go forward.  What about:

"replay execution to the specified intruction count"

> +        .cmd        = hmp_replay_seek,
> +    },
> +
> +STEXI
> address@hidden replay_seek @var{icount}
> address@hidden replay_seek
> +Automatically proceeds to the step specified by @var{icount}, when
> +replaying the execution. The command automatically loads nearest
> +snapshot and replay the execution to find the desired step.
> +When there is no preceding snapshot or the execution is not replayed,
> +then the command is ignored.

Uh, why isn't that an error?

> +icount for the reference may be observed with 'info replay' command.

This still uses both "step" and "instruction count".

I'm happy to help rephrasing, but first we need to nail down the failure
modes.

> +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 a63219c..7390f88 100644
> --- a/qapi/replay.json
> +++ b/qapi/replay.json
> @@ -99,3 +99,23 @@
>  #
>  ##
>  { 'command': 'replay-delete-break' }
> +
> +##
> +# @replay-seek:
> +#
> +# Automatically proceeds to the step specified by @icount when replaying
> +# the execution. The command automatically loads nearest
> +# snapshot and replay the execution to find the desired step.
> +# When there is no preceding snapshot or the execution is not replayed,
> +# then the command is ignored.
> +# icount for the reference may be obtained with @query-replay command.

My comments on hmp-commands.hx apply.

> +#
> +# @icount: destination execution step

s/execution step/instruction count/

Maybe s/destination/target/

> +#
> +# Since: 4.0
> +#
> +# Example:
> +#
> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
> +##
> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
[...]