Re: [Qemu-devel] [RFC PATCH v9 22/23] replay: command line options

[Paolo Bonzini <pbonzini@redhat.com>]

On 18/02/2015 12:57, Pavel Dovgalyuk wrote:
> This patch introduces command line options for enabling recording or replaying
> virtual machine behavior. "-record" option starts recording of the execution
> and saves it into the log, specified with "fname" parameter. "-replay" option
> is intended for replaying previously saved log.

Inaccurate commit message...

> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  cpus.c          |    1 +
>  qemu-options.hx |    8 ++++++--
>  vl.c            |   16 +++++++++++++++-
>  3 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 5b2f896..e289a9d 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -895,6 +895,7 @@ static void qemu_wait_io_event_common(CPUState *cpu)
>  static void qemu_tcg_wait_io_event(void)
>  {
>      CPUState *cpu;
> +    GMainContext *context = g_main_context_default();

... and unnecessary change (I think---shouldn't this give a warning?).
>  
>      while (all_cpu_threads_idle()) {
>         /* Start accounting real time to the virtual clock if the CPUs
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 85ca3ad..c62661a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3038,11 +3038,11 @@ re-inject them.
>  ETEXI
>  
>  DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
> -    "-icount [shift=N|auto][,align=on|off]\n" \
> +    "-icount [shift=N|auto][,align=on|off][,rr=record|replay,rrfile=<filename>]\n" \
>      "                enable virtual instruction counter with 2^N clock ticks per\n" \
>      "                instruction and enable aligning the host and virtual clocks\n", QEMU_ARCH_ALL)
>  STEXI
> -@item -icount [shift=@var{N}|auto]
> +@item -icount [shift=@var{N}|auto][,rr=record|replay,rrfile=@var{filename}]
>  @findex -icount
>  Enable virtual instruction counter.  The virtual cpu will execute one
>  instruction every 2^@var{N} ns of virtual time.  If @code{auto} is specified
> @@ -3064,6 +3064,10 @@ Currently this option does not work when @option{shift} is @code{auto}.
>  Note: The sync algorithm will work for those shift values for which
>  the guest clock runs ahead of the host clock. Typically this happens
>  when the shift value is high (how high depends on the host machine).
> +
> +When @option{rr} option is specified deterministic record/replay is enabled.
> +Replay log is written into @var{filename} file in record mode and
> +read from this file in replay mode.
>  ETEXI
>  
>  DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
> diff --git a/vl.c b/vl.c
> index a5555fb..f12d3db 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -472,6 +472,12 @@ static QemuOptsList qemu_icount_opts = {
>          }, {
>              .name = "align",
>              .type = QEMU_OPT_BOOL,
> +        }, {
> +            .name = "rr",
> +            .type = QEMU_OPT_STRING,
> +        }, {
> +            .name = "rrfile",
> +            .type = QEMU_OPT_STRING,
>          },
>          { /* end of list */ }
>      },
> @@ -2751,6 +2757,7 @@ int main(int argc, char **argv, char **envp)
>  {
>      int i;
>      int snapshot, linux_boot;
> +    const char *icount_option = NULL;
>      const char *initrd_filename;
>      const char *kernel_filename, *kernel_cmdline;
>      const char *boot_order;
> @@ -3770,6 +3777,8 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>  
> +    replay_configure(icount_opts);

Is it possible to call this together with configure_icount (or even from
configure_icount)?

>      opts = qemu_get_machine_opts();
>      optarg = qemu_opt_get(opts, "type");
>      if (optarg) {
> @@ -4137,6 +4146,11 @@ int main(int argc, char **argv, char **envp)
>          qemu_opts_del(icount_opts);
>      }
>  
> +    if (replay_mode != REPLAY_MODE_NONE && !use_icount) {
> +        fprintf(stderr, "Please enable icount to use record/replay\n");
> +        exit(1);
> +    }

Can be checked in configure_icount and/or replay_configure.

>      /* clean up network at qemu process termination */
>      atexit(&net_cleanup);
>  
> @@ -4173,7 +4187,7 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      /* open the virtual block devices */
> -    if (snapshot)
> +    if (snapshot || replay_mode != REPLAY_MODE_NONE)
>          qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);

Please add braces around "if" body, since you are at it.

>      if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>                            &machine_class->block_default_type, 1) != 0) {
> 

Otherwise looks good.

Paolo