From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60004)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1YH9Du-00022O-HG
	for qemu-devel@nongnu.org; Fri, 30 Jan 2015 05:54:51 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1YH9Dp-0003s1-C8
	for qemu-devel@nongnu.org; Fri, 30 Jan 2015 05:54:50 -0500
Received: from mx1.redhat.com ([209.132.183.28]:45110)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1YH9Dp-0003rt-4j
	for qemu-devel@nongnu.org; Fri, 30 Jan 2015 05:54:45 -0500
Message-ID: <54CB62E5.3000807@redhat.com>
Date: Fri, 30 Jan 2015 11:54:29 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <20150122085127.5276.53895.stgit@PASHA-ISP.def.inno>
	<20150122085323.5276.64995.stgit@PASHA-ISP.def.inno>
In-Reply-To: <20150122085323.5276.64995.stgit@PASHA-ISP.def.inno>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC PATCH v8 20/21] replay: command line options
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, alex.bennee@linaro.org, mark.burton@greensocs.com, real@ispras.ru, batuzovk@ispras.ru, maria.klimushenkova@ispras.ru, afaerber@suse.de, fred.konrad@greensocs.com



On 22/01/2015 09:53, 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.
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  cpus.c          |    3 ++-
>  qemu-options.hx |    8 ++++++--
>  vl.c            |   27 ++++++++++++++++++++++++++-
>  3 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 9c32491..7689cec 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -912,9 +912,10 @@ 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();
>  
>      while (all_cpu_threads_idle()) {
> -       /* Start accounting real time to the virtual clock if the CPUs
> +        /* Start accounting real time to the virtual clock if the CPUs
>            are idle.  */
>          qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
>          qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex);

Extraneous change.

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 10b9568..cb4b577 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3037,11 +3037,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,rrfname=<filename>]\n" \

rrfile?

>      "                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,rrfname=@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
> @@ -3063,6 +3063,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 ae3e97e..39d9024 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -475,6 +475,12 @@ static QemuOptsList qemu_icount_opts = {
>          }, {
>              .name = "align",
>              .type = QEMU_OPT_BOOL,
> +        }, {
> +            .name = "rr",
> +            .type = QEMU_OPT_STRING,
> +        }, {
> +            .name = "rrfname",
> +            .type = QEMU_OPT_STRING,
>          },
>          { /* end of list */ }
>      },
> @@ -2752,6 +2758,8 @@ int main(int argc, char **argv, char **envp)
>  {
>      int i;
>      int snapshot, linux_boot;
> +    int not_compatible_replay_param = 0;
> +    const char *icount_option = NULL;
>      const char *initrd_filename;
>      const char *kernel_filename, *kernel_cmdline;
>      const char *boot_order;
> @@ -2949,6 +2957,7 @@ int main(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_pflash:
>                  drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS);
> +                not_compatible_replay_param++;

Why not compatible?

>                  break;
>              case QEMU_OPTION_snapshot:
>                  snapshot = 1;
> @@ -3105,6 +3114,7 @@ int main(int argc, char **argv, char **envp)
>  #endif
>              case QEMU_OPTION_bt:
>                  add_device_config(DEV_BT, optarg);
> +                not_compatible_replay_param++;

Could it be enough to add a migration blocker?

>                  break;
>              case QEMU_OPTION_audio_help:
>                  AUD_help ();
> @@ -3244,6 +3254,7 @@ int main(int argc, char **argv, char **envp)
>                  if (!opts) {
>                      exit(1);
>                  }
> +                not_compatible_replay_param++;

Why not compatible?

>                  break;
>              case QEMU_OPTION_fsdev:
>                  olist = qemu_find_opts("fsdev");
> @@ -3372,6 +3383,7 @@ int main(int argc, char **argv, char **envp)
>                  if (strncmp(optarg, "mon:", 4) == 0) {
>                      default_monitor = 0;
>                  }
> +                not_compatible_replay_param++;

Add a migration blocker?

>                  break;
>              case QEMU_OPTION_debugcon:
>                  add_device_config(DEV_DEBUGCON, optarg);
> @@ -3489,6 +3501,7 @@ int main(int argc, char **argv, char **envp)
>                  if (!qemu_opts_parse(qemu_find_opts("smp-opts"), optarg, 1)) {
>                      exit(1);
>                  }
> +                not_compatible_replay_param++;

Check smp_cpus instead in replay_configure?

>                  break;
>              case QEMU_OPTION_vnc:
>  #ifdef CONFIG_VNC
> @@ -3628,6 +3641,7 @@ int main(int argc, char **argv, char **envp)
>                  if (!icount_opts) {
>                      exit(1);
>                  }
> +                replay_configure(icount_opts);

Perhaps move this outside the switch so that you have a fuller view of
the command line?

>                  break;
>              case QEMU_OPTION_incoming:
>                  incoming = optarg;
> @@ -3786,6 +3800,12 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (not_compatible_replay_param && (replay_mode != REPLAY_MODE_NONE)) {
> +        fprintf(stderr, "options -smp, -pflash, -chardev, -bt, -parallel "
> +                        "are not compatible with record/replay\n");
> +        exit(1);
> +    }

... so this (or some replacement) can be done in replay_configure.

>      if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0)) {
>          exit(1);
>      }
> @@ -4136,6 +4156,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);
> +    }

Same here.

Paolo

>      /* clean up network at qemu process termination */
>      atexit(&net_cleanup);
>  
> @@ -4172,7 +4197,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);
>      if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>                            &machine_class->block_default_type, 1) != 0) {
>