[Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Paolo Bonzini]

These are some comments I have about the record/replay code.  I can
integrate these in your patches myself, but I need an ack/tested-by and
in some case more answers...  Please take a look.

Paolo Bonzini (4):
  replay: generalize ptimer event to bottom halves
  more replay fixes
  why is runstate_is_running needed?
  events doubts

 Makefile.objs                       |  2 ++
 Makefile.target                     |  1 -
 cpu-exec.c                          |  2 +-
 cpus.c                              |  2 +-
 exec.c                              |  2 +-
 hw/bt/hci.c                         |  4 ++--
 hw/core/ptimer.c                    |  8 ++------
 include/qapi/qmp/qerror.h           |  2 +-
 {replay => include/sysemu}/replay.h |  4 ++--
 qapi/common.json                    |  6 +-----
 qemu-timer.c                        | 23 +++++++++--------------
 replay/Makefile.objs                | 11 +++++------
 replay/replay-events.c              | 24 +++++++++++++++---------
 replay/replay-input.c               |  2 +-
 replay/replay-internal.c            |  4 ++--
 replay/replay-internal.h            |  2 +-
 replay/replay-time.c                |  2 +-
 replay/replay.c                     |  2 +-
 stubs/Makefile.objs                 |  1 +
 {replay => stubs}/replay-user.c     |  6 +-----
 stubs/replay.c                      | 10 +++++-----
 ui/input.c                          |  2 +-
 vl.c                                |  6 +++---
 23 files changed, 59 insertions(+), 69 deletions(-)
 rename {replay => include/sysemu}/replay.h (97%)
 mode change 100755 => 100644 replay/Makefile.objs
 mode change 100755 => 100644 replay/replay-events.c
 mode change 100755 => 100644 replay/replay-input.c
 mode change 100755 => 100644 replay/replay-internal.c
 mode change 100755 => 100644 replay/replay-internal.h
 mode change 100755 => 100644 replay/replay-time.c
 mode change 100755 => 100644 replay/replay.c
 rename {replay => stubs}/replay-user.c (90%)

-- 
2.5.0

[Qemu-devel] [PATCH 1/4] replay: generalize ptimer event to bottom halves

[Paolo Bonzini]

Make the code a bit more type safe and follow the same scheme as
replay_input_event and replay_input_sync_event.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/ptimer.c         |  6 +-----
 replay/replay-events.c   | 15 ++++++++++-----
 replay/replay-internal.h |  2 +-
 replay/replay.h          |  4 ++--
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index c56078d..86d544f 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -28,11 +28,7 @@ struct ptimer_state
 static void ptimer_trigger(ptimer_state *s)
 {
     if (s->bh) {
-        if (replay_mode != REPLAY_MODE_NONE) {
-            replay_add_ptimer_event(s->bh, replay_get_current_step());
-        } else {
-            qemu_bh_schedule(s->bh);
-        }
+        replay_bh_schedule_event(s->bh);
     }
 }
 
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 23f3b12..06dd4ca 100755
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -37,7 +37,7 @@ static bool events_enabled;
 static void replay_run_event(Event *event)
 {
     switch (event->event_kind) {
-    case REPLAY_ASYNC_EVENT_PTIMER:
+    case REPLAY_ASYNC_EVENT_BH:
         aio_bh_call(event->opaque);
         break;
     case REPLAY_ASYNC_EVENT_INPUT:
@@ -129,9 +129,14 @@ static void replay_add_event(ReplayAsyncEventKind event_kind,
     replay_mutex_unlock();
 }
 
-void replay_add_ptimer_event(void *bh, uint64_t id)
+void replay_bh_schedule_event(QEMUBH *bh)
 {
-    replay_add_event(REPLAY_ASYNC_EVENT_PTIMER, bh, NULL, id);
+    if (replay_mode != REPLAY_MODE_NONE) {
+        uint64_t id = replay_get_current_step();
+        replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
+    } else {
+        qemu_bh_schedule(bh);
+    }
 }
 
 void replay_add_input_event(struct InputEvent *event)
@@ -154,7 +159,7 @@ static void replay_save_event(Event *event, int checkpoint)
 
         /* save event-specific data */
         switch (event->event_kind) {
-        case REPLAY_ASYNC_EVENT_PTIMER:
+        case REPLAY_ASYNC_EVENT_BH:
             replay_put_qword(event->id);
             break;
         case REPLAY_ASYNC_EVENT_INPUT:
@@ -200,7 +205,7 @@ static Event *replay_read_event(int checkpoint)
 
     /* Events that has not to be in the queue */
     switch (read_event_kind) {
-    case REPLAY_ASYNC_EVENT_PTIMER:
+    case REPLAY_ASYNC_EVENT_BH:
         if (read_id == -1) {
             read_id = replay_get_qword();
         }
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 04d2e1b..77e0d29 100755
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -41,7 +41,7 @@ enum ReplayEvents {
 /* Asynchronous events IDs */
 
 enum ReplayAsyncEventKind {
-    REPLAY_ASYNC_EVENT_PTIMER,
+    REPLAY_ASYNC_EVENT_BH,
     REPLAY_ASYNC_EVENT_INPUT,
     REPLAY_ASYNC_EVENT_INPUT_SYNC,
     REPLAY_ASYNC_COUNT
diff --git a/replay/replay.h b/replay/replay.h
index cbb4e11..abb4688 100755
--- a/replay/replay.h
+++ b/replay/replay.h
@@ -110,8 +110,8 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint);
 void replay_disable_events(void);
 /*! Returns true when saving events is enabled */
 bool replay_events_enabled(void);
-/*! Adds ptimer event to the queue */
-void replay_add_ptimer_event(void *bh, uint64_t id);
+/*! Adds bottom half event to the queue */
+void replay_bh_schedule_event(QEMUBH *bh);
 /*! Adds input event to the queue */
 void replay_input_event(QemuConsole *src, InputEvent *evt);
 /*! Adds input sync event to the queue */
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/4] replay: generalize ptimer event to bottom halves

[Pavel Dovgaluk]

This one is ok.

Pavel Dovgalyuk

> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Tuesday, October 06, 2015 11:01 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru
> Subject: [PATCH 1/4] replay: generalize ptimer event to bottom halves
> 
> Make the code a bit more type safe and follow the same scheme as
> replay_input_event and replay_input_sync_event.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/ptimer.c         |  6 +-----
>  replay/replay-events.c   | 15 ++++++++++-----
>  replay/replay-internal.h |  2 +-
>  replay/replay.h          |  4 ++--
>  4 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
> index c56078d..86d544f 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -28,11 +28,7 @@ struct ptimer_state
>  static void ptimer_trigger(ptimer_state *s)
>  {
>      if (s->bh) {
> -        if (replay_mode != REPLAY_MODE_NONE) {
> -            replay_add_ptimer_event(s->bh, replay_get_current_step());
> -        } else {
> -            qemu_bh_schedule(s->bh);
> -        }
> +        replay_bh_schedule_event(s->bh);
>      }
>  }
> 
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 23f3b12..06dd4ca 100755
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -37,7 +37,7 @@ static bool events_enabled;
>  static void replay_run_event(Event *event)
>  {
>      switch (event->event_kind) {
> -    case REPLAY_ASYNC_EVENT_PTIMER:
> +    case REPLAY_ASYNC_EVENT_BH:
>          aio_bh_call(event->opaque);
>          break;
>      case REPLAY_ASYNC_EVENT_INPUT:
> @@ -129,9 +129,14 @@ static void replay_add_event(ReplayAsyncEventKind event_kind,
>      replay_mutex_unlock();
>  }
> 
> -void replay_add_ptimer_event(void *bh, uint64_t id)
> +void replay_bh_schedule_event(QEMUBH *bh)
>  {
> -    replay_add_event(REPLAY_ASYNC_EVENT_PTIMER, bh, NULL, id);
> +    if (replay_mode != REPLAY_MODE_NONE) {
> +        uint64_t id = replay_get_current_step();
> +        replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
> +    } else {
> +        qemu_bh_schedule(bh);
> +    }
>  }
> 
>  void replay_add_input_event(struct InputEvent *event)
> @@ -154,7 +159,7 @@ static void replay_save_event(Event *event, int checkpoint)
> 
>          /* save event-specific data */
>          switch (event->event_kind) {
> -        case REPLAY_ASYNC_EVENT_PTIMER:
> +        case REPLAY_ASYNC_EVENT_BH:
>              replay_put_qword(event->id);
>              break;
>          case REPLAY_ASYNC_EVENT_INPUT:
> @@ -200,7 +205,7 @@ static Event *replay_read_event(int checkpoint)
> 
>      /* Events that has not to be in the queue */
>      switch (read_event_kind) {
> -    case REPLAY_ASYNC_EVENT_PTIMER:
> +    case REPLAY_ASYNC_EVENT_BH:
>          if (read_id == -1) {
>              read_id = replay_get_qword();
>          }
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 04d2e1b..77e0d29 100755
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -41,7 +41,7 @@ enum ReplayEvents {
>  /* Asynchronous events IDs */
> 
>  enum ReplayAsyncEventKind {
> -    REPLAY_ASYNC_EVENT_PTIMER,
> +    REPLAY_ASYNC_EVENT_BH,
>      REPLAY_ASYNC_EVENT_INPUT,
>      REPLAY_ASYNC_EVENT_INPUT_SYNC,
>      REPLAY_ASYNC_COUNT
> diff --git a/replay/replay.h b/replay/replay.h
> index cbb4e11..abb4688 100755
> --- a/replay/replay.h
> +++ b/replay/replay.h
> @@ -110,8 +110,8 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint);
>  void replay_disable_events(void);
>  /*! Returns true when saving events is enabled */
>  bool replay_events_enabled(void);
> -/*! Adds ptimer event to the queue */
> -void replay_add_ptimer_event(void *bh, uint64_t id);
> +/*! Adds bottom half event to the queue */
> +void replay_bh_schedule_event(QEMUBH *bh);
>  /*! Adds input event to the queue */
>  void replay_input_event(QemuConsole *src, InputEvent *evt);
>  /*! Adds input sync event to the queue */
> --
> 2.5.0
> 

[Qemu-devel] [PATCH 2/4] more replay fixes

[Paolo Bonzini]

1) Compile files once

2) Move include file from replay/replay.h to include/sysemu/replay.h.

3) Fix Error usage

4) cleanup timerlistgroup_deadline_ns a bit and allow clock jump
notifiers to run

5) move replay-user.c to stubs/
---
 Makefile.objs                       |  2 ++
 Makefile.target                     |  1 -
 cpu-exec.c                          |  2 +-
 cpus.c                              |  2 +-
 exec.c                              |  2 +-
 hw/bt/hci.c                         |  4 ++--
 hw/core/ptimer.c                    |  2 +-
 include/qapi/qmp/qerror.h           |  2 +-
 {replay => include/sysemu}/replay.h |  0
 qapi/common.json                    |  6 +-----
 qemu-timer.c                        | 14 ++++++--------
 replay/Makefile.objs                | 11 +++++------
 replay/replay-events.c              |  2 +-
 replay/replay-input.c               |  2 +-
 replay/replay-internal.c            |  4 ++--
 replay/replay-internal.h            |  0
 replay/replay-time.c                |  2 +-
 replay/replay.c                     |  2 +-
 stubs/Makefile.objs                 |  1 +
 {replay => stubs}/replay-user.c     |  6 +-----
 stubs/replay.c                      |  9 +++++++--
 ui/input.c                          |  2 +-
 vl.c                                |  6 +++---
 23 files changed, 40 insertions(+), 44 deletions(-)
 rename {replay => include/sysemu}/replay.h (100%)
 mode change 100755 => 100644 replay/Makefile.objs
 mode change 100755 => 100644 replay/replay-events.c
 mode change 100755 => 100644 replay/replay-input.c
 mode change 100755 => 100644 replay/replay-internal.c
 mode change 100755 => 100644 replay/replay-internal.h
 mode change 100755 => 100644 replay/replay-time.c
 mode change 100755 => 100644 replay/replay.c
 rename {replay => stubs}/replay-user.c (90%)

diff --git a/Makefile.objs b/Makefile.objs
index bc43e5c..ba4b45e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -58,6 +58,8 @@ common-obj-y += audio/
 common-obj-y += hw/
 common-obj-y += accel.o
 
+common-obj-y += replay/
+
 common-obj-y += ui/
 common-obj-y += bt-host.o bt-vhci.o
 bt-host.o-cflags := $(BLUEZ_CFLAGS)
diff --git a/Makefile.target b/Makefile.target
index ca8f351..962d004 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -88,7 +88,6 @@ obj-y = exec.o translate-all.o cpu-exec.o
 obj-y += translate-common.o
 obj-y += cpu-exec-common.o
 obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
-obj-y += replay/
 obj-$(CONFIG_TCG_INTERPRETER) += tci.o
 obj-y += tcg/tcg-common.o
 obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
diff --git a/cpu-exec.c b/cpu-exec.c
index 2b83e18..0850f8c 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -30,7 +30,7 @@
 #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
 #include "hw/i386/apic.h"
 #endif
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 
 /* -icount align implementation. */
 
diff --git a/cpus.c b/cpus.c
index 5130806..7e846e3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -42,7 +42,7 @@
 #include "qemu/seqlock.h"
 #include "qapi-event.h"
 #include "hw/nmi.h"
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 
 #ifndef _WIN32
 #include "qemu/compatfd.h"
diff --git a/exec.c b/exec.c
index dba9258..38f968a 100644
--- a/exec.c
+++ b/exec.c
@@ -50,7 +50,7 @@
 #include "qemu/rcu_queue.h"
 #include "qemu/main-loop.h"
 #include "translate-all.h"
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
diff --git a/hw/bt/hci.c b/hw/bt/hci.c
index 93dd1dc..2151d01 100644
--- a/hw/bt/hci.c
+++ b/hw/bt/hci.c
@@ -24,7 +24,7 @@
 #include "sysemu/bt.h"
 #include "hw/bt.h"
 #include "qapi/qmp/qerror.h"
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 
 struct bt_hci_s {
     uint8_t *(*evt_packet)(void *opaque);
@@ -2193,7 +2193,7 @@ struct HCIInfo *bt_new_hci(struct bt_scatternet_s *net)
 
     s->device.handle_destroy = bt_hci_destroy;
 
-    error_set(&s->replay_blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, "bt hci");
+    error_setg(&s->replay_blocker, QERR_REPLAY_NOT_SUPPORTED, "-bt hci");
     replay_add_blocker(s->replay_blocker);
 
     return &s->info;
diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 86d544f..edf077c 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -9,7 +9,7 @@
 #include "qemu/timer.h"
 #include "hw/ptimer.h"
 #include "qemu/host-utils.h"
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 
 struct ptimer_state
 {
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 0781a7f..f601499 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -107,6 +107,6 @@
     "this feature or command is not currently supported"
 
 #define QERR_REPLAY_NOT_SUPPORTED \
-    ERROR_CLASS_GENERIC_ERROR, "Record/replay feature is not supported for '%s'"
+    "Record/replay feature is not supported for '%s'"
 
 #endif /* QERROR_H */
diff --git a/replay/replay.h b/include/sysemu/replay.h
similarity index 100%
rename from replay/replay.h
rename to include/sysemu/replay.h
diff --git a/qapi/common.json b/qapi/common.json
index d80e3d4..bad56bf 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -22,15 +22,11 @@
 # @KVMMissingCap: the requested operation can't be fulfilled because a
 #                 required KVM capability is missing
 #
-# @ReplayNotSupported: the requested feature is not supported with
-#                      record/replay mode enabled
-#
 # Since: 1.2
 ##
 { 'enum': 'ErrorClass',
   'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
-            'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
-            'ReplayNotSupported' ] }
+            'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
 
 ##
 # @VersionTriple
diff --git a/qemu-timer.c b/qemu-timer.c
index e7a5c96..3c6e4c3 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -24,7 +24,7 @@
 
 #include "qemu/main-loop.h"
 #include "qemu/timer.h"
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 #include "sysemu/sysemu.h"
 
 #ifdef CONFIG_POSIX
@@ -572,15 +572,14 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
     QEMUClockType type;
     bool play = replay_mode == REPLAY_MODE_PLAY;
     for (type = 0; type < QEMU_CLOCK_MAX; type++) {
-        if (qemu_clock_use_for_deadline(tlg->tl[type]->clock->type)) {
-            if (!play || tlg->tl[type]->clock->type == QEMU_CLOCK_REALTIME) {
+        if (qemu_clock_use_for_deadline(type)) {
+            if (!play || type == QEMU_CLOCK_REALTIME) {
                 deadline = qemu_soonest_timeout(deadline,
-                                                timerlist_deadline_ns(
-                                                    tlg->tl[type]));
+                                                timerlist_deadline_ns(tlg->tl[type]));
             } else {
                 /* Read clock from the replay file and
                    do not calculate the deadline, based on virtual clock. */
-                qemu_clock_get_ns(tlg->tl[type]->clock->type);
+                qemu_clock_get_ns(type);
             }
         }
     }
@@ -606,8 +605,7 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
         now = REPLAY_CLOCK(REPLAY_CLOCK_HOST, get_clock_realtime());
         last = clock->last;
         clock->last = now;
-        if ((now < last || now > (last + get_max_clock_jump()))
-            && replay_mode == REPLAY_MODE_NONE) {
+        if (now < last || now > (last + get_max_clock_jump())) {
             notifier_list_notify(&clock->reset_notifiers, &now);
         }
         return now;
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
old mode 100755
new mode 100644
index 1267969..232193a
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -1,6 +1,5 @@
-obj-$(CONFIG_SOFTMMU) += replay.o
-obj-$(CONFIG_SOFTMMU) += replay-internal.o
-obj-$(CONFIG_SOFTMMU) += replay-events.o
-obj-$(CONFIG_SOFTMMU) += replay-time.o
-obj-$(CONFIG_SOFTMMU) += replay-input.o
-obj-$(CONFIG_USER_ONLY) += replay-user.o
+common-obj-y += replay.o
+common-obj-y += replay-internal.o
+common-obj-y += replay-events.o
+common-obj-y += replay-time.o
+common-obj-y += replay-input.o
diff --git a/replay/replay-events.c b/replay/replay-events.c
old mode 100755
new mode 100644
index 06dd4ca..402f644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -11,7 +11,7 @@
 
 #include "qemu-common.h"
 #include "qemu/error-report.h"
-#include "replay.h"
+#include "sysemu/replay.h"
 #include "replay-internal.h"
 #include "block/aio.h"
 #include "ui/input.h"
diff --git a/replay/replay-input.c b/replay/replay-input.c
old mode 100755
new mode 100644
index 9c3b45b..de628ea
--- a/replay/replay-input.c
+++ b/replay/replay-input.c
@@ -10,7 +10,7 @@
  */
 
 #include "qemu-common.h"
-#include "replay.h"
+#include "sysemu/replay.h"
 #include "replay-internal.h"
 #include "qemu/notify.h"
 #include "ui/input.h"
diff --git a/replay/replay-internal.c b/replay/replay-internal.c
old mode 100755
new mode 100644
index 69fe49f..35cff44
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -10,7 +10,7 @@
  */
 
 #include "qemu-common.h"
-#include "replay.h"
+#include "sysemu/replay.h"
 #include "replay-internal.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
@@ -196,7 +196,7 @@ void replay_save_instructions(void)
     if (replay_file && replay_mode == REPLAY_MODE_RECORD) {
         replay_mutex_lock();
         int diff = (int)(replay_get_current_step() - replay_state.current_step);
-        if (first_cpu != NULL && diff > 0) {
+        if (diff > 0) {
             replay_put_event(EVENT_INSTRUCTION);
             replay_put_dword(diff);
             replay_state.current_step += diff;
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
old mode 100755
new mode 100644
diff --git a/replay/replay-time.c b/replay/replay-time.c
old mode 100755
new mode 100644
index f292ab6..6d06951
--- a/replay/replay-time.c
+++ b/replay/replay-time.c
@@ -10,7 +10,7 @@
  */
 
 #include "qemu-common.h"
-#include "replay.h"
+#include "sysemu/replay.h"
 #include "replay-internal.h"
 #include "qemu/error-report.h"
 
diff --git a/replay/replay.c b/replay/replay.c
old mode 100755
new mode 100644
index dfa3d6f..0d33e82
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -10,7 +10,7 @@
  */
 
 #include "qemu-common.h"
-#include "replay.h"
+#include "sysemu/replay.h"
 #include "replay-internal.h"
 #include "qemu/timer.h"
 #include "qemu/main-loop.h"
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index c2f9e51..58de861 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -26,6 +26,7 @@ stub-obj-y += notify-event.o
 stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
 stub-obj-y += qtest.o
 stub-obj-y += replay.o
+stub-obj-y += replay-user.o
 stub-obj-y += reset.o
 stub-obj-y += runstate-check.o
 stub-obj-y += set-fd-handler.o
diff --git a/replay/replay-user.c b/stubs/replay-user.c
similarity index 90%
rename from replay/replay-user.c
rename to stubs/replay-user.c
index eeaa41d..ab8cff7 100755
--- a/replay/replay-user.c
+++ b/stubs/replay-user.c
@@ -9,7 +9,7 @@
  *
  */
 
-#include "replay.h"
+#include "sysemu/replay.h"
 
 bool replay_exception(void)
 {
@@ -30,7 +30,3 @@ bool replay_has_interrupt(void)
 {
     return true;
 }
-
-void replay_finish(void)
-{
-}
diff --git a/stubs/replay.c b/stubs/replay.c
index f7f74c9..71fa7d5 100755
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -1,4 +1,4 @@
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 #include <stdlib.h>
 #include "sysemu/sysemu.h"
 
@@ -19,14 +19,19 @@ int64_t replay_read_clock(unsigned int kind)
 bool replay_checkpoint(ReplayCheckpoint checkpoint)
 {
     return 0;
+    return true;
 }
 
 int runstate_is_running(void)
 {
-    return 0;
+    abort();
 }
 
 bool replay_events_enabled(void)
 {
     return false;
 }
+
+void replay_finish(void)
+{
+}
diff --git a/ui/input.c b/ui/input.c
index 9939722..4be7e3c 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -6,7 +6,7 @@
 #include "trace.h"
 #include "ui/input.h"
 #include "ui/console.h"
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 
 struct QemuInputHandlerState {
     DeviceState       *dev;
diff --git a/vl.c b/vl.c
index 2e5c208..c12fc19 100644
--- a/vl.c
+++ b/vl.c
@@ -122,7 +122,7 @@ int main(int argc, char **argv)
 #include "qapi-event.h"
 #include "exec/semihost.h"
 #include "crypto/init.h"
-#include "replay/replay.h"
+#include "sysemu/replay.h"
 #include "qapi/qmp/qerror.h"
 
 #define MAX_VIRTIO_CONSOLES 1
@@ -851,7 +851,7 @@ static void configure_rtc(QemuOpts *opts)
         } else if (!strcmp(value, "localtime")) {
             Error *blocker = NULL;
             rtc_utc = 0;
-            error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED,
+            error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
                       "-rtc base=localtime");
             replay_add_blocker(blocker);
         } else {
@@ -1258,7 +1258,7 @@ static void smp_parse(QemuOpts *opts)
 
     if (smp_cpus > 1 || smp_cores > 1 || smp_threads > 1) {
         Error *blocker = NULL;
-        error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, "smp");
+        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
         replay_add_blocker(blocker);
     }
 }
-- 
2.5.0

Re: [Qemu-devel] [PATCH 2/4] more replay fixes

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3221 bytes --]

On 10/06/2015 02:00 PM, Paolo Bonzini wrote:
> 1) Compile files once
> 
> 2) Move include file from replay/replay.h to include/sysemu/replay.h.
> 
> 3) Fix Error usage
> 
> 4) cleanup timerlistgroup_deadline_ns a bit and allow clock jump
> notifiers to run
> 
> 5) move replay-user.c to stubs/
> ---

> +++ b/include/qapi/qmp/qerror.h
> @@ -107,6 +107,6 @@
>      "this feature or command is not currently supported"
>  
>  #define QERR_REPLAY_NOT_SUPPORTED \
> -    ERROR_CLASS_GENERIC_ERROR, "Record/replay feature is not supported for '%s'"
> +    "Record/replay feature is not supported for '%s'"

We should not be adding new #defines to this file.  Instead, inline the
message into the callers that do error_setg() (I see hw/bt/hci.c as the
first such caller).

> +++ b/qapi/common.json
> @@ -22,15 +22,11 @@
>  # @KVMMissingCap: the requested operation can't be fulfilled because a
>  #                 required KVM capability is missing
>  #
> -# @ReplayNotSupported: the requested feature is not supported with
> -#                      record/replay mode enabled
> -#
>  # Since: 1.2
>  ##
>  { 'enum': 'ErrorClass',
>    'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
> -            'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
> -            'ReplayNotSupported' ] }
> +            'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }

Thank you for this. We definitely do not want to be adding new error
classes without a very strong reason, and even if such classes are
added, they must properly be documented as 'Since 2.5'.

> +++ b/vl.c
> @@ -122,7 +122,7 @@ int main(int argc, char **argv)
>  #include "qapi-event.h"
>  #include "exec/semihost.h"
>  #include "crypto/init.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
>  #include "qapi/qmp/qerror.h"
>  
>  #define MAX_VIRTIO_CONSOLES 1
> @@ -851,7 +851,7 @@ static void configure_rtc(QemuOpts *opts)
>          } else if (!strcmp(value, "localtime")) {
>              Error *blocker = NULL;
>              rtc_utc = 0;
> -            error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED,
> +            error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
>                        "-rtc base=localtime");
>              replay_add_blocker(blocker);
>          } else {
> @@ -1258,7 +1258,7 @@ static void smp_parse(QemuOpts *opts)
>  
>      if (smp_cpus > 1 || smp_cores > 1 || smp_threads > 1) {
>          Error *blocker = NULL;
> -        error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, "smp");
> +        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
>          replay_add_blocker(blocker);

Okay, I see that there is more than one location with the same failure,
which is where using the #define sort of makes it nicer to guarantee a
consistent message.  But in general, use of error_setg() with a macro
that passes a %s to printf at a distance is ugly, and should be avoided
compared to just inlining the error message directly or writing a helper
method that can properly set a consistent message.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/4] more replay fixes

[Pavel Dovgaluk]

This one is ok too.

Pavel Dovgalyuk


> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Tuesday, October 06, 2015 11:01 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru
> Subject: [PATCH 2/4] more replay fixes
> 
> 1) Compile files once
> 
> 2) Move include file from replay/replay.h to include/sysemu/replay.h.
> 
> 3) Fix Error usage
> 
> 4) cleanup timerlistgroup_deadline_ns a bit and allow clock jump
> notifiers to run
> 
> 5) move replay-user.c to stubs/
> ---
>  Makefile.objs                       |  2 ++
>  Makefile.target                     |  1 -
>  cpu-exec.c                          |  2 +-
>  cpus.c                              |  2 +-
>  exec.c                              |  2 +-
>  hw/bt/hci.c                         |  4 ++--
>  hw/core/ptimer.c                    |  2 +-
>  include/qapi/qmp/qerror.h           |  2 +-
>  {replay => include/sysemu}/replay.h |  0
>  qapi/common.json                    |  6 +-----
>  qemu-timer.c                        | 14 ++++++--------
>  replay/Makefile.objs                | 11 +++++------
>  replay/replay-events.c              |  2 +-
>  replay/replay-input.c               |  2 +-
>  replay/replay-internal.c            |  4 ++--
>  replay/replay-internal.h            |  0
>  replay/replay-time.c                |  2 +-
>  replay/replay.c                     |  2 +-
>  stubs/Makefile.objs                 |  1 +
>  {replay => stubs}/replay-user.c     |  6 +-----
>  stubs/replay.c                      |  9 +++++++--
>  ui/input.c                          |  2 +-
>  vl.c                                |  6 +++---
>  23 files changed, 40 insertions(+), 44 deletions(-)
>  rename {replay => include/sysemu}/replay.h (100%)
>  mode change 100755 => 100644 replay/Makefile.objs
>  mode change 100755 => 100644 replay/replay-events.c
>  mode change 100755 => 100644 replay/replay-input.c
>  mode change 100755 => 100644 replay/replay-internal.c
>  mode change 100755 => 100644 replay/replay-internal.h
>  mode change 100755 => 100644 replay/replay-time.c
>  mode change 100755 => 100644 replay/replay.c
>  rename {replay => stubs}/replay-user.c (90%)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index bc43e5c..ba4b45e 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -58,6 +58,8 @@ common-obj-y += audio/
>  common-obj-y += hw/
>  common-obj-y += accel.o
> 
> +common-obj-y += replay/
> +
>  common-obj-y += ui/
>  common-obj-y += bt-host.o bt-vhci.o
>  bt-host.o-cflags := $(BLUEZ_CFLAGS)
> diff --git a/Makefile.target b/Makefile.target
> index ca8f351..962d004 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -88,7 +88,6 @@ obj-y = exec.o translate-all.o cpu-exec.o
>  obj-y += translate-common.o
>  obj-y += cpu-exec-common.o
>  obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
> -obj-y += replay/
>  obj-$(CONFIG_TCG_INTERPRETER) += tci.o
>  obj-y += tcg/tcg-common.o
>  obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 2b83e18..0850f8c 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -30,7 +30,7 @@
>  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
>  #include "hw/i386/apic.h"
>  #endif
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
> 
>  /* -icount align implementation. */
> 
> diff --git a/cpus.c b/cpus.c
> index 5130806..7e846e3 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -42,7 +42,7 @@
>  #include "qemu/seqlock.h"
>  #include "qapi-event.h"
>  #include "hw/nmi.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
> 
>  #ifndef _WIN32
>  #include "qemu/compatfd.h"
> diff --git a/exec.c b/exec.c
> index dba9258..38f968a 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -50,7 +50,7 @@
>  #include "qemu/rcu_queue.h"
>  #include "qemu/main-loop.h"
>  #include "translate-all.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
> 
>  #include "exec/memory-internal.h"
>  #include "exec/ram_addr.h"
> diff --git a/hw/bt/hci.c b/hw/bt/hci.c
> index 93dd1dc..2151d01 100644
> --- a/hw/bt/hci.c
> +++ b/hw/bt/hci.c
> @@ -24,7 +24,7 @@
>  #include "sysemu/bt.h"
>  #include "hw/bt.h"
>  #include "qapi/qmp/qerror.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
> 
>  struct bt_hci_s {
>      uint8_t *(*evt_packet)(void *opaque);
> @@ -2193,7 +2193,7 @@ struct HCIInfo *bt_new_hci(struct bt_scatternet_s *net)
> 
>      s->device.handle_destroy = bt_hci_destroy;
> 
> -    error_set(&s->replay_blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, "bt hci");
> +    error_setg(&s->replay_blocker, QERR_REPLAY_NOT_SUPPORTED, "-bt hci");
>      replay_add_blocker(s->replay_blocker);
> 
>      return &s->info;
> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
> index 86d544f..edf077c 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -9,7 +9,7 @@
>  #include "qemu/timer.h"
>  #include "hw/ptimer.h"
>  #include "qemu/host-utils.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
> 
>  struct ptimer_state
>  {
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 0781a7f..f601499 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -107,6 +107,6 @@
>      "this feature or command is not currently supported"
> 
>  #define QERR_REPLAY_NOT_SUPPORTED \
> -    ERROR_CLASS_GENERIC_ERROR, "Record/replay feature is not supported for '%s'"
> +    "Record/replay feature is not supported for '%s'"
> 
>  #endif /* QERROR_H */
> diff --git a/replay/replay.h b/include/sysemu/replay.h
> similarity index 100%
> rename from replay/replay.h
> rename to include/sysemu/replay.h
> diff --git a/qapi/common.json b/qapi/common.json
> index d80e3d4..bad56bf 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -22,15 +22,11 @@
>  # @KVMMissingCap: the requested operation can't be fulfilled because a
>  #                 required KVM capability is missing
>  #
> -# @ReplayNotSupported: the requested feature is not supported with
> -#                      record/replay mode enabled
> -#
>  # Since: 1.2
>  ##
>  { 'enum': 'ErrorClass',
>    'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
> -            'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
> -            'ReplayNotSupported' ] }
> +            'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
> 
>  ##
>  # @VersionTriple
> diff --git a/qemu-timer.c b/qemu-timer.c
> index e7a5c96..3c6e4c3 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -24,7 +24,7 @@
> 
>  #include "qemu/main-loop.h"
>  #include "qemu/timer.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
>  #include "sysemu/sysemu.h"
> 
>  #ifdef CONFIG_POSIX
> @@ -572,15 +572,14 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
>      QEMUClockType type;
>      bool play = replay_mode == REPLAY_MODE_PLAY;
>      for (type = 0; type < QEMU_CLOCK_MAX; type++) {
> -        if (qemu_clock_use_for_deadline(tlg->tl[type]->clock->type)) {
> -            if (!play || tlg->tl[type]->clock->type == QEMU_CLOCK_REALTIME) {
> +        if (qemu_clock_use_for_deadline(type)) {
> +            if (!play || type == QEMU_CLOCK_REALTIME) {
>                  deadline = qemu_soonest_timeout(deadline,
> -                                                timerlist_deadline_ns(
> -                                                    tlg->tl[type]));
> +                                                timerlist_deadline_ns(tlg->tl[type]));
>              } else {
>                  /* Read clock from the replay file and
>                     do not calculate the deadline, based on virtual clock. */
> -                qemu_clock_get_ns(tlg->tl[type]->clock->type);
> +                qemu_clock_get_ns(type);
>              }
>          }
>      }
> @@ -606,8 +605,7 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
>          now = REPLAY_CLOCK(REPLAY_CLOCK_HOST, get_clock_realtime());
>          last = clock->last;
>          clock->last = now;
> -        if ((now < last || now > (last + get_max_clock_jump()))
> -            && replay_mode == REPLAY_MODE_NONE) {
> +        if (now < last || now > (last + get_max_clock_jump())) {
>              notifier_list_notify(&clock->reset_notifiers, &now);
>          }
>          return now;
> diff --git a/replay/Makefile.objs b/replay/Makefile.objs
> old mode 100755
> new mode 100644
> index 1267969..232193a
> --- a/replay/Makefile.objs
> +++ b/replay/Makefile.objs
> @@ -1,6 +1,5 @@
> -obj-$(CONFIG_SOFTMMU) += replay.o
> -obj-$(CONFIG_SOFTMMU) += replay-internal.o
> -obj-$(CONFIG_SOFTMMU) += replay-events.o
> -obj-$(CONFIG_SOFTMMU) += replay-time.o
> -obj-$(CONFIG_SOFTMMU) += replay-input.o
> -obj-$(CONFIG_USER_ONLY) += replay-user.o
> +common-obj-y += replay.o
> +common-obj-y += replay-internal.o
> +common-obj-y += replay-events.o
> +common-obj-y += replay-time.o
> +common-obj-y += replay-input.o
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> old mode 100755
> new mode 100644
> index 06dd4ca..402f644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -11,7 +11,7 @@
> 
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
> -#include "replay.h"
> +#include "sysemu/replay.h"
>  #include "replay-internal.h"
>  #include "block/aio.h"
>  #include "ui/input.h"
> diff --git a/replay/replay-input.c b/replay/replay-input.c
> old mode 100755
> new mode 100644
> index 9c3b45b..de628ea
> --- a/replay/replay-input.c
> +++ b/replay/replay-input.c
> @@ -10,7 +10,7 @@
>   */
> 
>  #include "qemu-common.h"
> -#include "replay.h"
> +#include "sysemu/replay.h"
>  #include "replay-internal.h"
>  #include "qemu/notify.h"
>  #include "ui/input.h"
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> old mode 100755
> new mode 100644
> index 69fe49f..35cff44
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -10,7 +10,7 @@
>   */
> 
>  #include "qemu-common.h"
> -#include "replay.h"
> +#include "sysemu/replay.h"
>  #include "replay-internal.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
> @@ -196,7 +196,7 @@ void replay_save_instructions(void)
>      if (replay_file && replay_mode == REPLAY_MODE_RECORD) {
>          replay_mutex_lock();
>          int diff = (int)(replay_get_current_step() - replay_state.current_step);
> -        if (first_cpu != NULL && diff > 0) {
> +        if (diff > 0) {
>              replay_put_event(EVENT_INSTRUCTION);
>              replay_put_dword(diff);
>              replay_state.current_step += diff;
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> old mode 100755
> new mode 100644
> diff --git a/replay/replay-time.c b/replay/replay-time.c
> old mode 100755
> new mode 100644
> index f292ab6..6d06951
> --- a/replay/replay-time.c
> +++ b/replay/replay-time.c
> @@ -10,7 +10,7 @@
>   */
> 
>  #include "qemu-common.h"
> -#include "replay.h"
> +#include "sysemu/replay.h"
>  #include "replay-internal.h"
>  #include "qemu/error-report.h"
> 
> diff --git a/replay/replay.c b/replay/replay.c
> old mode 100755
> new mode 100644
> index dfa3d6f..0d33e82
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -10,7 +10,7 @@
>   */
> 
>  #include "qemu-common.h"
> -#include "replay.h"
> +#include "sysemu/replay.h"
>  #include "replay-internal.h"
>  #include "qemu/timer.h"
>  #include "qemu/main-loop.h"
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index c2f9e51..58de861 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -26,6 +26,7 @@ stub-obj-y += notify-event.o
>  stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
>  stub-obj-y += qtest.o
>  stub-obj-y += replay.o
> +stub-obj-y += replay-user.o
>  stub-obj-y += reset.o
>  stub-obj-y += runstate-check.o
>  stub-obj-y += set-fd-handler.o
> diff --git a/replay/replay-user.c b/stubs/replay-user.c
> similarity index 90%
> rename from replay/replay-user.c
> rename to stubs/replay-user.c
> index eeaa41d..ab8cff7 100755
> --- a/replay/replay-user.c
> +++ b/stubs/replay-user.c
> @@ -9,7 +9,7 @@
>   *
>   */
> 
> -#include "replay.h"
> +#include "sysemu/replay.h"
> 
>  bool replay_exception(void)
>  {
> @@ -30,7 +30,3 @@ bool replay_has_interrupt(void)
>  {
>      return true;
>  }
> -
> -void replay_finish(void)
> -{
> -}
> diff --git a/stubs/replay.c b/stubs/replay.c
> index f7f74c9..71fa7d5 100755
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -1,4 +1,4 @@
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
>  #include <stdlib.h>
>  #include "sysemu/sysemu.h"
> 
> @@ -19,14 +19,19 @@ int64_t replay_read_clock(unsigned int kind)
>  bool replay_checkpoint(ReplayCheckpoint checkpoint)
>  {
>      return 0;
> +    return true;
>  }
> 
>  int runstate_is_running(void)
>  {
> -    return 0;
> +    abort();
>  }
> 
>  bool replay_events_enabled(void)
>  {
>      return false;
>  }
> +
> +void replay_finish(void)
> +{
> +}
> diff --git a/ui/input.c b/ui/input.c
> index 9939722..4be7e3c 100644
> --- a/ui/input.c
> +++ b/ui/input.c
> @@ -6,7 +6,7 @@
>  #include "trace.h"
>  #include "ui/input.h"
>  #include "ui/console.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
> 
>  struct QemuInputHandlerState {
>      DeviceState       *dev;
> diff --git a/vl.c b/vl.c
> index 2e5c208..c12fc19 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -122,7 +122,7 @@ int main(int argc, char **argv)
>  #include "qapi-event.h"
>  #include "exec/semihost.h"
>  #include "crypto/init.h"
> -#include "replay/replay.h"
> +#include "sysemu/replay.h"
>  #include "qapi/qmp/qerror.h"
> 
>  #define MAX_VIRTIO_CONSOLES 1
> @@ -851,7 +851,7 @@ static void configure_rtc(QemuOpts *opts)
>          } else if (!strcmp(value, "localtime")) {
>              Error *blocker = NULL;
>              rtc_utc = 0;
> -            error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED,
> +            error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
>                        "-rtc base=localtime");
>              replay_add_blocker(blocker);
>          } else {
> @@ -1258,7 +1258,7 @@ static void smp_parse(QemuOpts *opts)
> 
>      if (smp_cpus > 1 || smp_cores > 1 || smp_threads > 1) {
>          Error *blocker = NULL;
> -        error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, "smp");
> +        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
>          replay_add_blocker(blocker);
>      }
>  }
> --
> 2.5.0
> 

[Qemu-devel] [PATCH 3/4] why is runstate_is_running needed?

[Paolo Bonzini]

It doesn't seem correct to call it for all checkpoints, but why
is it right for timerlist_run_timers?
---
 qemu-timer.c   | 9 +++------
 stubs/replay.c | 5 -----
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 3c6e4c3..f16e422 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -488,20 +488,17 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
         break;
     default:
     case QEMU_CLOCK_VIRTUAL:
-        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
-            || !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
+        if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
             goto out;
         }
         break;
     case QEMU_CLOCK_HOST:
-        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
-            || !replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
+        if (!replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
             goto out;
         }
         break;
     case QEMU_CLOCK_VIRTUAL_RT:
-        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
-            || !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
+        if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
             goto out;
         }
         break;
diff --git a/stubs/replay.c b/stubs/replay.c
index 71fa7d5..42d01b5 100755
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -22,11 +22,6 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
     return true;
 }
 
-int runstate_is_running(void)
-{
-    abort();
-}
-
 bool replay_events_enabled(void)
 {
     return false;
-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/4] why is runstate_is_running needed?

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Tuesday, October 06, 2015 11:01 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru
> Subject: [PATCH 3/4] why is runstate_is_running needed?
> 
> It doesn't seem correct to call it for all checkpoints, but why
> is it right for timerlist_run_timers?

Because replaying shouldn't proceed when machine is stopped.
These checks could be also useful for creating snapshots in record mode,
but I don't remember exact reasons of adding them. I'll check your changes 
for the current version.

> ---
>  qemu-timer.c   | 9 +++------
>  stubs/replay.c | 5 -----
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 3c6e4c3..f16e422 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -488,20 +488,17 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
>          break;
>      default:
>      case QEMU_CLOCK_VIRTUAL:
> -        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
> -            || !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
> +        if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
>              goto out;
>          }
>          break;
>      case QEMU_CLOCK_HOST:
> -        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
> -            || !replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
> +        if (!replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
>              goto out;
>          }
>          break;
>      case QEMU_CLOCK_VIRTUAL_RT:
> -        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
> -            || !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
> +        if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
>              goto out;
>          }
>          break;
> diff --git a/stubs/replay.c b/stubs/replay.c
> index 71fa7d5..42d01b5 100755
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -22,11 +22,6 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
>      return true;
>  }
> 
> -int runstate_is_running(void)
> -{
> -    abort();
> -}
> -
>  bool replay_events_enabled(void)
>  {
>      return false;
> --
> 2.5.0
> 


Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH 3/4] why is runstate_is_running needed?

[Paolo Bonzini]

On 07/10/2015 10:14, Pavel Dovgaluk wrote:
> > It doesn't seem correct to call it for all checkpoints, but why
> > is it right for timerlist_run_timers?
> 
> Because replaying shouldn't proceed when machine is stopped.

Right, but VIRTUAL and VIRTUAL_RT do not proceed when the machine is
stopped.  For HOST it makes sense, but then the same should happen
independent of the replay mode (using qemu_clock_enable).

Otherwise, if a checkpoint for the wrong clock sneaks in at the wrong
point while the VM is running, I wonder if replay could grind to a halt.

Paolo

> These checks could be also useful for creating snapshots in record mode,
> but I don't remember exact reasons of adding them. I'll check your changes 
> for the current version.

Re: [Qemu-devel] [PATCH 3/4] why is runstate_is_running needed?

[Pavel Dovgaluk]

I checked this patch.
Let's leave it without runstate_is_running() call.
If it will be needed later, we'll find it out.

Pavel Dovgalyuk


> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Tuesday, October 06, 2015 11:01 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru
> Subject: [PATCH 3/4] why is runstate_is_running needed?
> 
> It doesn't seem correct to call it for all checkpoints, but why
> is it right for timerlist_run_timers?
> ---
>  qemu-timer.c   | 9 +++------
>  stubs/replay.c | 5 -----
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 3c6e4c3..f16e422 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -488,20 +488,17 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
>          break;
>      default:
>      case QEMU_CLOCK_VIRTUAL:
> -        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
> -            || !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
> +        if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
>              goto out;
>          }
>          break;
>      case QEMU_CLOCK_HOST:
> -        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
> -            || !replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
> +        if (!replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
>              goto out;
>          }
>          break;
>      case QEMU_CLOCK_VIRTUAL_RT:
> -        if ((replay_mode != REPLAY_MODE_NONE && !runstate_is_running())
> -            || !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
> +        if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
>              goto out;
>          }
>          break;
> diff --git a/stubs/replay.c b/stubs/replay.c
> index 71fa7d5..42d01b5 100755
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -22,11 +22,6 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
>      return true;
>  }
> 
> -int runstate_is_running(void)
> -{
> -    abort();
> -}
> -
>  bool replay_events_enabled(void)
>  {
>      return false;
> --
> 2.5.0
> 

[Qemu-devel] [PATCH 4/4] events doubts

[Paolo Bonzini]

It is not clear what separates REPLAY_ASYNC_EVENT_BH from other async
events.  It seems to be an ordering issue, but then why do input events
not have to be looked up in the queue?  It would be much simpler if they
are all handled the same way.
---
 replay/replay-events.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/replay/replay-events.c b/replay/replay-events.c
index 402f644..d6c61f6 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -203,13 +203,15 @@ static Event *replay_read_event(int checkpoint)
         return NULL;
     }
 
-    /* Events that has not to be in the queue */
+    /* Read event-specific data */
     switch (read_event_kind) {
     case REPLAY_ASYNC_EVENT_BH:
         if (read_id == -1) {
             read_id = replay_get_qword();
         }
         break;
+
+    /* Events that do not have to be in the queue - ### WHY? */
     case REPLAY_ASYNC_EVENT_INPUT:
         event = g_malloc0(sizeof(Event));
         event->event_kind = read_event_kind;
@@ -220,6 +222,7 @@ static Event *replay_read_event(int checkpoint)
         event->event_kind = read_event_kind;
         event->opaque = 0;
         return event;
+
     default:
         error_report("Unknown ID %d of replay event", read_event_kind);
         exit(1);
@@ -239,8 +242,6 @@ static Event *replay_read_event(int checkpoint)
         return NULL;
     }
 
-    /* Read event-specific data */
-
     return event;
 }
 
-- 
2.5.0

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> 
> It is not clear what separates REPLAY_ASYNC_EVENT_BH from other async
> events.  It seems to be an ordering issue, but then why do input events
> not have to be looked up in the queue?  It would be much simpler if they
> are all handled the same way.

There are two kinds of events:
- read from the log and injected immediately (user input, network input)
- read from the log and wait for corresponding event in the queue (BH)

We cannot inject BH event immediately because we do not have any information
about callback and to preserve consistency - BH cannot be processed before
it is scheduled by qemu core.

Pavel Dovgalyuk

> ---
>  replay/replay-events.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 402f644..d6c61f6 100644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -203,13 +203,15 @@ static Event *replay_read_event(int checkpoint)
>          return NULL;
>      }
> 
> -    /* Events that has not to be in the queue */
> +    /* Read event-specific data */
>      switch (read_event_kind) {
>      case REPLAY_ASYNC_EVENT_BH:
>          if (read_id == -1) {
>              read_id = replay_get_qword();
>          }
>          break;
> +
> +    /* Events that do not have to be in the queue - ### WHY? */
>      case REPLAY_ASYNC_EVENT_INPUT:
>          event = g_malloc0(sizeof(Event));
>          event->event_kind = read_event_kind;
> @@ -220,6 +222,7 @@ static Event *replay_read_event(int checkpoint)
>          event->event_kind = read_event_kind;
>          event->opaque = 0;
>          return event;
> +
>      default:
>          error_report("Unknown ID %d of replay event", read_event_kind);
>          exit(1);
> @@ -239,8 +242,6 @@ static Event *replay_read_event(int checkpoint)
>          return NULL;
>      }
> 
> -    /* Read event-specific data */
> -
>      return event;
>  }
> 
> --
> 2.5.0

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Paolo Bonzini]

On 07/10/2015 10:21, Pavel Dovgaluk wrote:
> There are two kinds of events:
> - read from the log and injected immediately (user input, network input)
> - read from the log and wait for corresponding event in the queue (BH)
> 
> We cannot inject BH event immediately because we do not have any information
> about callback

Actually we do (indirectly, through aio_bh_call).  But that may not be
the central issue, because...

> and to preserve consistency - BH cannot be processed before
> it is scheduled by qemu core.

... you are processing them differently anyway between record mode
(where the BH is scheduled by the core) and replay (where the BH is
called directly).

In fact, I don't understand what introduces the difference between
record and replay that requires special handling of ptimers' bottom
halves.  In both cases, the ptimer triggers at the desired time (based
on checkpoints) and then the bottom half is called as soon as possible.
 Why is a separate async event necessary?

Because we only care about bottom halves from ptimers, their order
should be the same for both record and replay.

If bottom halves async events could be removed, that would simplify a
lot the code, and it would make it a lot easier to understand for me.

Paolo

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> On 07/10/2015 10:21, Pavel Dovgaluk wrote:
> > There are two kinds of events:
> > - read from the log and injected immediately (user input, network input)
> > - read from the log and wait for corresponding event in the queue (BH)
> >
> > We cannot inject BH event immediately because we do not have any information
> > about callback
> 
> Actually we do (indirectly, through aio_bh_call).  But that may not be
> the central issue, because...
> 
> > and to preserve consistency - BH cannot be processed before
> > it is scheduled by qemu core.
> 
> ... you are processing them differently anyway between record mode
> (where the BH is scheduled by the core) and replay (where the BH is
> called directly).

In record it also called through replay. It is scheduled into the replay
events queue and called at checkpoint, where queue is flushed.

> In fact, I don't understand what introduces the difference between
> record and replay that requires special handling of ptimers' bottom
> halves.  In both cases, the ptimer triggers at the desired time (based
> on checkpoints) and then the bottom half is called as soon as possible.
>  Why is a separate async event necessary?

We want to preserve order of all events that affect virtual machine behavior,
not only instructions execution. These events include processing of
interrupts, exceptions, and bottom halves.
That is why bottom halves are bind to checkpoints and recorded into the log.

> Because we only care about bottom halves from ptimers, their order
> should be the same for both record and replay.
> 
> If bottom halves async events could be removed, that would simplify a
> lot the code, and it would make it a lot easier to understand for me.

I added ptimer handling because replay didn't work when I removed BH queuing.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Paolo Bonzini]

On 07/10/2015 11:50, Pavel Dovgaluk wrote:
>> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
>> On 07/10/2015 10:21, Pavel Dovgaluk wrote:
>>> There are two kinds of events:
>>> - read from the log and injected immediately (user input, network input)
>>> - read from the log and wait for corresponding event in the queue (BH)
>>>
>>> We cannot inject BH event immediately because we do not have any information
>>> about callback
>>
>> Actually we do (indirectly, through aio_bh_call).  But that may not be
>> the central issue, because...
>>
>>> and to preserve consistency - BH cannot be processed before
>>> it is scheduled by qemu core.
>>
>> ... you are processing them differently anyway between record mode
>> (where the BH is scheduled by the core) and replay (where the BH is
>> called directly).
> 
> In record it also called through replay. It is scheduled into the replay
> events queue and called at checkpoint, where queue is flushed.
> 
>> In fact, I don't understand what introduces the difference between
>> record and replay that requires special handling of ptimers' bottom
>> halves.  In both cases, the ptimer triggers at the desired time (based
>> on checkpoints) and then the bottom half is called as soon as possible.
>>  Why is a separate async event necessary?
> 
> We want to preserve order of all events that affect virtual machine behavior,
> not only instructions execution. These events include processing of
> interrupts, exceptions, and bottom halves.
> That is why bottom halves are bind to checkpoints and recorded into the log.
> 
>> Because we only care about bottom halves from ptimers, their order
>> should be the same for both record and replay.
>>
>> If bottom halves async events could be removed, that would simplify a
>> lot the code, and it would make it a lot easier to understand for me.
> 
> I added ptimer handling because replay didn't work when I removed BH queuing.

Ok, got it.  I still want to understand exactly the need for the init
and reset checkpoints, and the placement of qemu_clock_warp calls, but
apart from that the patches are good to go for 2.5.  Thanks for your
persistence!

Paolo

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 07/10/2015 11:50, Pavel Dovgaluk wrote:
> >> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> >> On 07/10/2015 10:21, Pavel Dovgaluk wrote:
> >>> There are two kinds of events:
> >>> - read from the log and injected immediately (user input, network input)
> >>> - read from the log and wait for corresponding event in the queue (BH)
> >>>
> >>> We cannot inject BH event immediately because we do not have any information
> >>> about callback
> >>
> >> Actually we do (indirectly, through aio_bh_call).  But that may not be
> >> the central issue, because...
> >>
> >>> and to preserve consistency - BH cannot be processed before
> >>> it is scheduled by qemu core.
> >>
> >> ... you are processing them differently anyway between record mode
> >> (where the BH is scheduled by the core) and replay (where the BH is
> >> called directly).
> >
> > In record it also called through replay. It is scheduled into the replay
> > events queue and called at checkpoint, where queue is flushed.
> >
> >> In fact, I don't understand what introduces the difference between
> >> record and replay that requires special handling of ptimers' bottom
> >> halves.  In both cases, the ptimer triggers at the desired time (based
> >> on checkpoints) and then the bottom half is called as soon as possible.
> >>  Why is a separate async event necessary?
> >
> > We want to preserve order of all events that affect virtual machine behavior,
> > not only instructions execution. These events include processing of
> > interrupts, exceptions, and bottom halves.
> > That is why bottom halves are bind to checkpoints and recorded into the log.
> >
> >> Because we only care about bottom halves from ptimers, their order
> >> should be the same for both record and replay.
> >>
> >> If bottom halves async events could be removed, that would simplify a
> >> lot the code, and it would make it a lot easier to understand for me.
> >
> > I added ptimer handling because replay didn't work when I removed BH queuing.
> 
> Ok, got it.  I still want to understand exactly the need for the init
> and reset checkpoints, and the placement of qemu_clock_warp calls, but
> apart from that the patches are good to go for 2.5.  Thanks for your
> persistence!

Init checkpoint is needed to separate initialization events (mostly coming from block
devices) from execution ones.
Reset checkpoint is used for synchronization of machine reset call.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Paolo Bonzini]

On 07/10/2015 12:42, Pavel Dovgaluk wrote:
>> > Ok, got it.  I still want to understand exactly the need for the init
>> > and reset checkpoints, and the placement of qemu_clock_warp calls, but
>> > apart from that the patches are good to go for 2.5.  Thanks for your
>> > persistence!
> Init checkpoint is needed to separate initialization events (mostly coming from block
> devices) from execution ones.
> Reset checkpoint is used for synchronization of machine reset call.

Why do they need to be separate on startup?  Does initialization hang?
My reasoning was that QEMU_CLOCK_VIRTUAL is zero anyway at both init and
reset.

Paolo

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 07/10/2015 12:42, Pavel Dovgaluk wrote:
> >> > Ok, got it.  I still want to understand exactly the need for the init
> >> > and reset checkpoints, and the placement of qemu_clock_warp calls, but
> >> > apart from that the patches are good to go for 2.5.  Thanks for your
> >> > persistence!
> > Init checkpoint is needed to separate initialization events (mostly coming from block
> > devices) from execution ones.
> > Reset checkpoint is used for synchronization of machine reset call.
> 
> Why do they need to be separate on startup?  Does initialization hang?
> My reasoning was that QEMU_CLOCK_VIRTUAL is zero anyway at both init and
> reset.

I'm not sure about current (only core functions) version, but full version
requires this because of loading virtual machine state and block devices initialization.
Events from these actions interfere with each other and with machine execution.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH 4/4] events doubts

[Paolo Bonzini]

On 07/10/2015 12:51, Pavel Dovgaluk wrote:
> > Why do they need to be separate on startup?  Does initialization hang?
> > My reasoning was that QEMU_CLOCK_VIRTUAL is zero anyway at both init and
> > reset.
>
> I'm not sure about current (only core functions) version, but full version
> requires this because of loading virtual machine state and block devices initialization.
> Events from these actions interfere with each other and with machine execution.

Ok, thanks.  Perhaps we could use the run state to only record/replay
after these events have happened.

Paolo

Re: [Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Pavel Dovgaluk]

There is one more fix.
Sometimes replay cannot continue after stopping/restarting of the virtual machine.
This happens because warp on stopped machine and on running machine behaves differently.
Timers deadline calculation depends on enabled flag of the virtual timer.
The following patch fixes the problem - it disables warp when machine is stopped.

index 5130806..7a337d9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -411,7 +411,7 @@ void qemu_clock_warp(QEMUClockType type)
     }

     /* warp clock deterministically in record/replay mode */
-    if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
+    if (!runstate_is_running() || !replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
         return;
     }

Pavel Dovgalyuk


> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Tuesday, October 06, 2015 11:01 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru
> Subject: [RFH PATCH 0/4] record/replay fixups and doubts
> 
> These are some comments I have about the record/replay code.  I can
> integrate these in your patches myself, but I need an ack/tested-by and
> in some case more answers...  Please take a look.
> 
> Paolo Bonzini (4):
>   replay: generalize ptimer event to bottom halves
>   more replay fixes
>   why is runstate_is_running needed?
>   events doubts
> 
>  Makefile.objs                       |  2 ++
>  Makefile.target                     |  1 -
>  cpu-exec.c                          |  2 +-
>  cpus.c                              |  2 +-
>  exec.c                              |  2 +-
>  hw/bt/hci.c                         |  4 ++--
>  hw/core/ptimer.c                    |  8 ++------
>  include/qapi/qmp/qerror.h           |  2 +-
>  {replay => include/sysemu}/replay.h |  4 ++--
>  qapi/common.json                    |  6 +-----
>  qemu-timer.c                        | 23 +++++++++--------------
>  replay/Makefile.objs                | 11 +++++------
>  replay/replay-events.c              | 24 +++++++++++++++---------
>  replay/replay-input.c               |  2 +-
>  replay/replay-internal.c            |  4 ++--
>  replay/replay-internal.h            |  2 +-
>  replay/replay-time.c                |  2 +-
>  replay/replay.c                     |  2 +-
>  stubs/Makefile.objs                 |  1 +
>  {replay => stubs}/replay-user.c     |  6 +-----
>  stubs/replay.c                      | 10 +++++-----
>  ui/input.c                          |  2 +-
>  vl.c                                |  6 +++---
>  23 files changed, 59 insertions(+), 69 deletions(-)
>  rename {replay => include/sysemu}/replay.h (97%)
>  mode change 100755 => 100644 replay/Makefile.objs
>  mode change 100755 => 100644 replay/replay-events.c
>  mode change 100755 => 100644 replay/replay-input.c
>  mode change 100755 => 100644 replay/replay-internal.c
>  mode change 100755 => 100644 replay/replay-internal.h
>  mode change 100755 => 100644 replay/replay-time.c
>  mode change 100755 => 100644 replay/replay.c
>  rename {replay => stubs}/replay-user.c (90%)
> 
> --
> 2.5.0

Re: [Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Paolo Bonzini]

On 13/10/2015 10:10, Pavel Dovgaluk wrote:
> Sometimes replay cannot continue after stopping/restarting of the virtual machine.
> This happens because warp on stopped machine and on running machine behaves differently.
> Timers deadline calculation depends on enabled flag of the virtual timer.
> The following patch fixes the problem - it disables warp when machine is stopped.
> 
> index 5130806..7a337d9 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -411,7 +411,7 @@ void qemu_clock_warp(QEMUClockType type)
>      }
> 
>      /* warp clock deterministically in record/replay mode */
> -    if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
> +    if (!runstate_is_running() || !replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
>          return;
>      }

Thanks, I'm adding a comment too:

@@ -410,6 +410,13 @@ void qemu_clock_warp(QEMUClockType type)
         return;
     }

+    /* Nothing to do if the VM is stopped: QEMU_CLOCK_VIRTUAL timers
+     * do not fire, so computing the deadline does not make sense.
+     */
+    if (!runstate_is_running()) {
+        return;
+    }
+
     /* warp clock deterministically in record/replay mode */
     if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
         return;

Re: [Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Pavel Dovgaluk]

Hi, Paolo!

Will you pull these patches into 2.5?

Pavel Dovgalyuk

> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Tuesday, October 06, 2015 11:01 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru
> Subject: [RFH PATCH 0/4] record/replay fixups and doubts
> 
> These are some comments I have about the record/replay code.  I can
> integrate these in your patches myself, but I need an ack/tested-by and
> in some case more answers...  Please take a look.
> 
> Paolo Bonzini (4):
>   replay: generalize ptimer event to bottom halves
>   more replay fixes
>   why is runstate_is_running needed?
>   events doubts
> 
>  Makefile.objs                       |  2 ++
>  Makefile.target                     |  1 -
>  cpu-exec.c                          |  2 +-
>  cpus.c                              |  2 +-
>  exec.c                              |  2 +-
>  hw/bt/hci.c                         |  4 ++--
>  hw/core/ptimer.c                    |  8 ++------
>  include/qapi/qmp/qerror.h           |  2 +-
>  {replay => include/sysemu}/replay.h |  4 ++--
>  qapi/common.json                    |  6 +-----
>  qemu-timer.c                        | 23 +++++++++--------------
>  replay/Makefile.objs                | 11 +++++------
>  replay/replay-events.c              | 24 +++++++++++++++---------
>  replay/replay-input.c               |  2 +-
>  replay/replay-internal.c            |  4 ++--
>  replay/replay-internal.h            |  2 +-
>  replay/replay-time.c                |  2 +-
>  replay/replay.c                     |  2 +-
>  stubs/Makefile.objs                 |  1 +
>  {replay => stubs}/replay-user.c     |  6 +-----
>  stubs/replay.c                      | 10 +++++-----
>  ui/input.c                          |  2 +-
>  vl.c                                |  6 +++---
>  23 files changed, 59 insertions(+), 69 deletions(-)
>  rename {replay => include/sysemu}/replay.h (97%)
>  mode change 100755 => 100644 replay/Makefile.objs
>  mode change 100755 => 100644 replay/replay-events.c
>  mode change 100755 => 100644 replay/replay-input.c
>  mode change 100755 => 100644 replay/replay-internal.c
>  mode change 100755 => 100644 replay/replay-internal.h
>  mode change 100755 => 100644 replay/replay-time.c
>  mode change 100755 => 100644 replay/replay.c
>  rename {replay => stubs}/replay-user.c (90%)
> 
> --
> 2.5.0

Re: [Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Paolo Bonzini]

On 23/10/2015 09:28, Pavel Dovgaluk wrote:
> Hi, Paolo!
> 
> Will you pull these patches into 2.5?

Yes.

Paolo