Re: [Qemu-devel] [PATCH v7 18/19] replay: init rtc after enabling the replay

[Artem Pisarenko]

I guess 'configure_rtc' function is not the only one which call should be
postponed. Since record/replay uses its 'hooks' at very low levels of qemu
core, any option processing is subject to potential indirect dependence on
effects of 'replay_configure' call. The most apparent example would be
'qemu_clock_get_*(QEMU_CLOCK_HOST)' calls, which are used widely.
Furthermore, right now I submitting a patch which adds one such call in
place before any option parsed yet, so it will also need to be moved to
follow this patch. Even if such calls aren't involved in current version,
no guarantees that they will not appear in future (like I already doing
it), because there are nothing which prevents developers from doing so.

One way, how this could be enforced, is to add something like
'REPLAY_MODE_INVALID' to 'ReplayMode' enum, replace initialization of
global 'replay_mode' variable with this value and add corresponding
checks/assertions to 'REPLAY_CLOCK' macro, etc.

But it would be more reliable to move 'icount_opts' processing out of
common processing loop (marked as 'second pass of option parsing') and
place it to new pass, located before current 'second' one (like it already
done for '-nouserconfig' option), and also move 'replace_configure' right
after it. Probably it will also require to tune 'icount_opts' processing
and record/replay initialization. Since record/replay feature is very
specific, I consider it's worth to make such exception for it.

ср, 10 окт. 2018 г. в 19:32, Pavel Dovgalyuk <address@hidden>:

> This patch postpones the call of 'configure_rtc' function. This call
> uses host clock to configure the rtc, but host clock access should be
> recorded when using icount record/replay mode. Therefore now rtc
> is configured after switching record/replay mode on.
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> ---
>  vl.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index a3e2e47..f910072 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2932,6 +2932,7 @@ int main(int argc, char **argv, char **envp)
>      DisplayState *ds;
>      QemuOpts *opts, *machine_opts;
>      QemuOpts *icount_opts = NULL, *accel_opts = NULL;
> +    QemuOpts *rtc_opts = NULL;
>      QemuOptsList *olist;
>      int optind;
>      const char *optarg;
> @@ -3738,12 +3739,11 @@ int main(int argc, char **argv, char **envp)
>                  warn_report("This option is ignored and will be removed
> soon");
>                  break;
>              case QEMU_OPTION_rtc:
> -                opts = qemu_opts_parse_noisily(qemu_find_opts("rtc"),
> optarg,
> -                                               false);
> -                if (!opts) {
> +                rtc_opts = qemu_opts_parse_noisily(qemu_find_opts("rtc"),
> +                                                   optarg, false);
> +                if (!rtc_opts) {
>                      exit(1);
>                  }
> -                configure_rtc(opts);
>                  break;
>              case QEMU_OPTION_tb_size:
>  #ifndef CONFIG_TCG
> @@ -3954,6 +3954,9 @@ int main(int argc, char **argv, char **envp)
>      loc_set_none();
>
>      replay_configure(icount_opts);
> +    if (rtc_opts) {
> +        configure_rtc(rtc_opts);
> +    }
>
>      if (incoming && !preconfig_exit_requested) {
>          error_report("'preconfig' and 'incoming' options are "
>
> --

С уважением,
  Артем Писаренко