Re: [Qemu-devel] [PATCH v18 00/21] Deterministic replay core

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/10/2015 17:09, Paolo Bonzini wrote:
> 
> 
> On 21/09/2015 09:12, Pavel Dovgaluk wrote:
>> Hi!
>>
>> Paolo, have you reviewed these patches?
> 
> Pavel,
> 
> I think this is ready to go in.  Here are my final changes,
> can you ack them?

Hmm, there are a few other issues that the patch I pasted doesn't fix.

Thanks,

Paolo

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 11954b9..ab0be16 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 c56078d..1d7aa9b 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..80f8231 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
@@ -488,20 +488,20 @@ 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) ||
+            (replay_mode == REPLAY_MODE_PLAY && !runstate_is_running())) {
             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) ||
+            (replay_mode == REPLAY_MODE_PLAY && !runstate_is_running())) {
             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) ||
+            (replay_mode == REPLAY_MODE_PLAY && !runstate_is_running())) {
             goto out;
         }
         break;
@@ -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 23f3b12..ad4f621
--- 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 15871bf..8224f7b 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -22,6 +22,7 @@ stub-obj-y += monitor-init.o
 stub-obj-y += notify-event.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);
     }
 }