[PATCH v3 0/3] unbreak non-tcg builds

[PATCH v3 0/3] unbreak non-tcg builds

[Claudio Fontana]

This series now unbreaks current non-tcg builds
(!CONFIG_TCG).

tests Makefiles need to avoid relying on all non-native
archs binaries to be present,

bios-tables-test needs to skip tests that are tcg-only,

and notably the replay framework needs to consider that
it might not be functional (or its code present at all)
without TCG.

Tested ok target x86_64-softmmu on x86_64 host with:

./configure --enable-tcg --disable-kvm
./configure --enable-kvm --disable-tcg
./configure --enable-tcg --enable-kvm

running make check-qtest

v2 => v3:

* do not alter the replay api, provide a block wrapper
instead to call into replay events handling
or the normal call flow depending on whether replay events are enabled.

v1: initial RFC


Claudio Fontana (2):
  qtest: unbreak non-TCG builds in bios-tables-test
  replay: do not build if TCG is not available

Paolo Bonzini (1):
  tests/Makefile.include: unbreak non-tcg builds

 block/block-backend.c          |  9 ++--
 block/io.c                     | 52 ++++++++++++------
 block/iscsi.c                  |  5 +-
 block/meson.build              |  3 +-
 block/nfs.c                    |  6 +--
 block/null.c                   |  4 +-
 block/nvme.c                   |  6 +--
 block/rbd.c                    |  5 +-
 hw/ide/core.c                  |  8 +--
 hw/ide/ioport.c                |  1 -
 include/block/block.h          | 29 ++++++++++
 migration/savevm.c             | 11 ++--
 net/meson.build                |  3 +-
 replay/meson.build             |  2 +-
 replay/replay-events.c         | 20 +++----
 replay/replay-input.c          |  4 +-
 stubs/meson.build              |  1 -
 stubs/replay-user.c            |  9 ----
 stubs/replay.c                 | 96 ++++++++++++++++++++++++++++++++++
 tests/Makefile.include         |  2 +-
 tests/ptimer-test-stubs.c      |  5 --
 tests/qtest/bios-tables-test.c | 10 ++++
 tests/qtest/qmp-cmd-test.c     |  3 ++
 ui/input.c                     | 12 ++++-
 24 files changed, 223 insertions(+), 83 deletions(-)
 delete mode 100644 stubs/replay-user.c

-- 
2.26.2

[PATCH v3 1/3] tests/Makefile.include: unbreak non-tcg builds

[Claudio Fontana]

From: Paolo Bonzini <pbonzini@redhat.com>

remove dependency of check-block from non-native archs

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 5aca98e60c..4037490b69 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -140,7 +140,7 @@ QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = tests/qemu-iotests/socket_scm_helper$(EXE
 check: check-block
 check-block: $(SRC_PATH)/tests/check-block.sh qemu-img$(EXESUF) \
 		qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) \
-		$(patsubst %-softmmu,qemu-system-%,$(filter %-softmmu,$(TARGET_DIRS)))
+		$(filter qemu-system-%, $(ninja-targets-c_LINKER) $(ninja-targets-cpp_LINKER))
 	@$<
 endif
 
-- 
2.26.2

[PATCH v3 2/3] qtest: unbreak non-TCG builds in bios-tables-test

[Claudio Fontana]

the tests assume TCG is available, thus breaking
for TCG-only tests, where only the TCG accelerator option
is passed to the QEMU binary.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 tests/qtest/bios-tables-test.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 5647624492..18f535d4e8 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -127,6 +127,9 @@ static void free_test_data(test_data *data)
 {
     int i;
 
+    if (!data->tables) {
+        return;
+    }
     for (i = 0; i < data->tables->len; ++i) {
         cleanup_table_descriptor(&g_array_index(data->tables, AcpiSdtTable, i));
     }
@@ -656,6 +659,13 @@ static void test_acpi_one(const char *params, test_data *data)
     char *args;
     bool use_uefi = data->uefi_fl1 && data->uefi_fl2;
 
+#ifndef CONFIG_TCG
+    if (data->tcg_only) {
+        g_test_skip("TCG disabled, skipping ACPI tcg_only test");
+        return;
+    }
+#endif /* CONFIG_TCG */
+
     if (use_uefi) {
         /*
          * TODO: convert '-drive if=pflash' to new syntax (see e33763be7cd3)
-- 
2.26.2

[PATCH v3 3/3] replay: do not build if TCG is not available

[Claudio Fontana]

this fixes non-TCG builds broken recently by replay reverse debugging.

stub the needed functions in stub/, including errors for hmp and qmp.

change hooks for the case when replay code is disabled (!CONFIG_TCG),
as we need to avoid sinking all the events in replay in this case.

Surprisingly, only _one_ qtest was affected by this, ide-test.c, which
resulted in a buzz as the bh events were never delivered, and the bh
never executed.

Many other subsystems _should_ have been affected.

This fixes the immediate issue, however a better way to group replay
functionality to TCG-only code could be developed in the long term.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 block/block-backend.c      |  9 ++--
 block/io.c                 | 52 ++++++++++++++-------
 block/iscsi.c              |  5 +-
 block/meson.build          |  3 +-
 block/nfs.c                |  6 +--
 block/null.c               |  4 +-
 block/nvme.c               |  6 +--
 block/rbd.c                |  5 +-
 hw/ide/core.c              |  8 ++--
 hw/ide/ioport.c            |  1 -
 include/block/block.h      | 29 ++++++++++++
 migration/savevm.c         | 11 +++--
 net/meson.build            |  3 +-
 replay/meson.build         |  2 +-
 replay/replay-events.c     | 20 ++++----
 replay/replay-input.c      |  4 +-
 stubs/meson.build          |  1 -
 stubs/replay-user.c        |  9 ----
 stubs/replay.c             | 96 ++++++++++++++++++++++++++++++++++++++
 tests/ptimer-test-stubs.c  |  5 --
 tests/qtest/qmp-cmd-test.c |  3 ++
 ui/input.c                 | 12 ++++-
 22 files changed, 212 insertions(+), 82 deletions(-)
 delete mode 100644 stubs/replay-user.c

diff --git a/block/block-backend.c b/block/block-backend.c
index ce78d30794..57ec5224dc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -19,7 +19,6 @@
 #include "sysemu/blockdev.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/replay.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-block.h"
 #include "qemu/id.h"
@@ -1381,9 +1380,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
     acb = blk_aio_get(&block_backend_aiocb_info, blk, cb, opaque);
     acb->blk = blk;
     acb->ret = ret;
-
-    replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
-                                     error_callback_bh, acb);
+    bdrv_bh_schedule_oneshot(blk_get_aio_context(blk), error_callback_bh, acb);
     return &acb->common;
 }
 
@@ -1447,8 +1444,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
 
     acb->has_returned = true;
     if (acb->rwco.ret != NOT_DONE) {
-        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
-                                         blk_aio_complete_bh, acb);
+        bdrv_bh_schedule_oneshot(blk_get_aio_context(blk),
+                                 blk_aio_complete_bh, acb);
     }
 
     return &acb->common;
diff --git a/block/io.c b/block/io.c
index 54f0968aee..0347ff6b10 100644
--- a/block/io.c
+++ b/block/io.c
@@ -368,8 +368,8 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
     if (bs) {
         bdrv_inc_in_flight(bs);
     }
-    replay_bh_schedule_oneshot_event(bdrv_get_aio_context(bs),
-                                     bdrv_co_drain_bh_cb, &data);
+    bdrv_bh_schedule_oneshot(bdrv_get_aio_context(bs),
+                             bdrv_co_drain_bh_cb, &data);
 
     qemu_coroutine_yield();
     /* If we are resumed from some other event (such as an aio completion or a
@@ -600,12 +600,12 @@ void bdrv_drain_all_begin(void)
         return;
     }
 
-    /*
-     * bdrv queue is managed by record/replay,
-     * waiting for finishing the I/O requests may
-     * be infinite
-     */
     if (replay_events_enabled()) {
+        /*
+         * bdrv queue is managed by record/replay,
+         * waiting for finishing the I/O requests may
+         * be infinite
+         */
         return;
     }
 
@@ -638,12 +638,12 @@ void bdrv_drain_all_end(void)
     BlockDriverState *bs = NULL;
     int drained_end_counter = 0;
 
-    /*
-     * bdrv queue is managed by record/replay,
-     * waiting for finishing the I/O requests may
-     * be endless
-     */
     if (replay_events_enabled()) {
+        /*
+         * bdrv queue is managed by record/replay,
+         * waiting for finishing the I/O requests may
+         * be endless
+         */
         return;
     }
 
@@ -2122,12 +2122,12 @@ int bdrv_flush_all(void)
     BlockDriverState *bs = NULL;
     int result = 0;
 
-    /*
-     * bdrv queue is managed by record/replay,
-     * creating new flush request for stopping
-     * the VM may break the determinism
-     */
     if (replay_events_enabled()) {
+        /*
+         * bdrv queue is managed by record/replay,
+         * creating new flush request for stopping
+         * the VM may break the determinism
+         */
         return result;
     }
 
@@ -3215,3 +3215,21 @@ out:
 
     return ret;
 }
+
+void bdrv_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
+{
+    if (replay_events_enabled()) {
+        replay_bh_schedule_oneshot_event(ctx, cb, opaque);
+    } else {
+        aio_bh_schedule_oneshot(ctx, cb, opaque);
+    }
+}
+
+void bdrv_bh_schedule(QEMUBH *bh)
+{
+    if (replay_events_enabled()) {
+        replay_bh_schedule_event(bh);
+    } else {
+        qemu_bh_schedule(bh);
+    }
+}
diff --git a/block/iscsi.c b/block/iscsi.c
index e30a7e3606..fec8cb9255 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -40,7 +40,6 @@
 #include "qemu/module.h"
 #include "qemu/option.h"
 #include "qemu/uuid.h"
-#include "sysemu/replay.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine.h"
 #include "qapi/qmp/qdict.h"
@@ -283,8 +282,8 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
     }
 
     if (iTask->co) {
-        replay_bh_schedule_oneshot_event(iTask->iscsilun->aio_context,
-                                         iscsi_co_generic_bh_cb, iTask);
+        bdrv_bh_schedule_oneshot(iTask->iscsilun->aio_context,
+                                 iscsi_co_generic_bh_cb, iTask);
     } else {
         iTask->complete = 1;
     }
diff --git a/block/meson.build b/block/meson.build
index 78e8b25232..01fe6f84d2 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -7,7 +7,6 @@ block_ss.add(files(
   'backup-top.c',
   'blkdebug.c',
   'blklogwrites.c',
-  'blkreplay.c',
   'blkverify.c',
   'block-backend.c',
   'block-copy.c',
@@ -42,6 +41,8 @@ block_ss.add(files(
   'write-threshold.c',
 ), zstd, zlib)
 
+block_ss.add(when: 'CONFIG_TCG', if_true: files('blkreplay.c'))
+
 block_ss.add(when: 'CONFIG_QCOW1', if_true: files('qcow.c'))
 block_ss.add(when: 'CONFIG_VDI', if_true: files('vdi.c'))
 block_ss.add(when: 'CONFIG_CLOOP', if_true: files('cloop.c'))
diff --git a/block/nfs.c b/block/nfs.c
index f86e660374..90a737fe64 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -38,7 +38,6 @@
 #include "qemu/uri.h"
 #include "qemu/cutils.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/replay.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
@@ -259,8 +258,9 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
     if (task->ret < 0) {
         error_report("NFS Error: %s", nfs_get_error(nfs));
     }
-    replay_bh_schedule_oneshot_event(task->client->aio_context,
-                                     nfs_co_generic_bh_cb, task);
+
+    bdrv_bh_schedule_oneshot(task->client->aio_context,
+                             nfs_co_generic_bh_cb, task);
 }
 
 static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
diff --git a/block/null.c b/block/null.c
index cc9b1d4ea7..1fa713c42d 100644
--- a/block/null.c
+++ b/block/null.c
@@ -17,7 +17,6 @@
 #include "qemu/module.h"
 #include "qemu/option.h"
 #include "block/block_int.h"
-#include "sysemu/replay.h"
 
 #define NULL_OPT_LATENCY "latency-ns"
 #define NULL_OPT_ZEROES  "read-zeroes"
@@ -180,8 +179,7 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
         timer_mod_ns(&acb->timer,
                      qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + s->latency_ns);
     } else {
-        replay_bh_schedule_oneshot_event(bdrv_get_aio_context(bs),
-                                         null_bh_cb, acb);
+        bdrv_bh_schedule_oneshot(bdrv_get_aio_context(bs), null_bh_cb, acb);
     }
     return &acb->common;
 }
diff --git a/block/nvme.c b/block/nvme.c
index b48f6f2588..c13ad92b7d 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -23,7 +23,6 @@
 #include "qemu/option.h"
 #include "qemu/vfio-helpers.h"
 #include "block/block_int.h"
-#include "sysemu/replay.h"
 #include "trace.h"
 
 #include "block/nvme.h"
@@ -313,8 +312,7 @@ static void nvme_put_free_req_locked(NVMeQueuePair *q, NVMeRequest *req)
 static void nvme_wake_free_req_locked(NVMeQueuePair *q)
 {
     if (!qemu_co_queue_empty(&q->free_req_queue)) {
-        replay_bh_schedule_oneshot_event(q->s->aio_context,
-                nvme_free_req_queue_cb, q);
+        bdrv_bh_schedule_oneshot(q->s->aio_context, nvme_free_req_queue_cb, q);
     }
 }
 
@@ -1068,7 +1066,7 @@ static void nvme_rw_cb(void *opaque, int ret)
         /* The rw coroutine hasn't yielded, don't try to enter. */
         return;
     }
-    replay_bh_schedule_oneshot_event(data->ctx, nvme_rw_cb_bh, data);
+    bdrv_bh_schedule_oneshot(data->ctx, nvme_rw_cb_bh, data);
 }
 
 static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,
diff --git a/block/rbd.c b/block/rbd.c
index 9bd2bce716..11d082b7cc 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -22,7 +22,6 @@
 #include "block/qdict.h"
 #include "crypto/secret.h"
 #include "qemu/cutils.h"
-#include "sysemu/replay.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
@@ -862,8 +861,8 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb)
     rcb->ret = rbd_aio_get_return_value(c);
     rbd_aio_release(c);
 
-    replay_bh_schedule_oneshot_event(bdrv_get_aio_context(acb->common.bs),
-                                     rbd_finish_bh, rcb);
+    bdrv_bh_schedule_oneshot(bdrv_get_aio_context(acb->common.bs),
+                             rbd_finish_bh, rcb);
 }
 
 static int rbd_aio_discard_wrapper(rbd_image_t image,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 693b352d5e..4c21e74aac 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -36,7 +36,6 @@
 #include "sysemu/block-backend.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
-#include "sysemu/replay.h"
 #include "sysemu/runstate.h"
 #include "hw/ide/internal.h"
 #include "trace.h"
@@ -493,7 +492,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
 done:
     iocb->aiocb = NULL;
     if (iocb->bh) {
-        replay_bh_schedule_event(iocb->bh);
+        bdrv_bh_schedule(iocb->bh);
     }
 }
 
@@ -2289,8 +2288,9 @@ void ide_ctrl_write(void *opaque, uint32_t addr, uint32_t val)
             s = &bus->ifs[i];
             s->status |= BUSY_STAT;
         }
-        replay_bh_schedule_oneshot_event(qemu_get_aio_context(),
-                                         ide_bus_perform_srst, bus);
+
+        bdrv_bh_schedule_oneshot(qemu_get_aio_context(),
+                                 ide_bus_perform_srst, bus);
     }
 
     bus->cmd = val;
diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
index b613ff3bba..b1cfd1fd85 100644
--- a/hw/ide/ioport.c
+++ b/hw/ide/ioport.c
@@ -33,7 +33,6 @@
 #include "sysemu/block-backend.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
-#include "sysemu/replay.h"
 
 #include "hw/ide/internal.h"
 #include "trace.h"
diff --git a/include/block/block.h b/include/block/block.h
index d16c401cb4..802f2283fa 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -834,4 +834,33 @@ int coroutine_fn bdrv_co_copy_range(BdrvChild *src, uint64_t src_offset,
                                     BdrvChild *dst, uint64_t dst_offset,
                                     uint64_t bytes, BdrvRequestFlags read_flags,
                                     BdrvRequestFlags write_flags);
+
+/**
+ *
+ * bdrv_bh_schedule_oneshot:
+ *
+ * This is a wrapper for aio_bh_schedule_oneshot:
+ * allocate and schedule a oneshot aio bottom half, to be executed as soon as
+ * possible.
+ *
+ * If event-capturing frameworks are present and active,
+ * the event will instead be consumed by their internal queues.
+ *
+ * Arguments are as per aio_bh_schedule_oneshot.
+ **/
+void bdrv_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+
+/**
+ *
+ * bdrv_bh_schedule:
+ *
+ * This is a wrapper for qemu_bh_schedule.
+ *
+ * If event-capturing frameworks are present and active,
+ * the event will instead be consumed by their internal queues.
+ *
+ * Arguments are as per qemu_bh_schedule.
+ **/
+void bdrv_bh_schedule(QEMUBH *bh);
+
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index d2e141f7b1..d9181ca520 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -63,6 +63,7 @@
 #include "migration/colo.h"
 #include "qemu/bitmap.h"
 #include "net/announce.h"
+#include "sysemu/tcg.h"
 
 const unsigned int postcopy_ram_discard_version = 0;
 
@@ -2674,10 +2675,12 @@ int save_snapshot(const char *name, Error **errp)
         return ret;
     }
 
-    if (!replay_can_snapshot()) {
-        error_setg(errp, "Record/replay does not allow making snapshot "
-                   "right now. Try once more later.");
-        return ret;
+    if (tcg_enabled()) {
+        if (!replay_can_snapshot()) {
+            error_setg(errp, "Record/replay does not allow making snapshot "
+                       "right now. Try once more later.");
+            return ret;
+        }
     }
 
     if (!bdrv_all_can_snapshot(&bs)) {
diff --git a/net/meson.build b/net/meson.build
index 1c7e3a3cb9..1076b0a7ab 100644
--- a/net/meson.build
+++ b/net/meson.build
@@ -7,7 +7,6 @@ softmmu_ss.add(files(
   'eth.c',
   'filter-buffer.c',
   'filter-mirror.c',
-  'filter-replay.c',
   'filter-rewriter.c',
   'filter.c',
   'hub.c',
@@ -17,6 +16,8 @@ softmmu_ss.add(files(
   'util.c',
 ))
 
+softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
+
 softmmu_ss.add(when: 'CONFIG_L2TPV3', if_true: files('l2tpv3.c'))
 softmmu_ss.add(when: slirp, if_true: files('slirp.c'))
 softmmu_ss.add(when: ['CONFIG_VDE', vde], if_true: files('vde.c'))
diff --git a/replay/meson.build b/replay/meson.build
index f91163fb1e..cb3207740a 100644
--- a/replay/meson.build
+++ b/replay/meson.build
@@ -1,4 +1,4 @@
-softmmu_ss.add(files(
+softmmu_ss.add(when: 'CONFIG_TCG', if_true: files(
   'replay.c',
   'replay-internal.c',
   'replay-events.c',
diff --git a/replay/replay-events.c b/replay/replay-events.c
index a1c6bb934e..e0a4523e18 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -130,23 +130,19 @@ void replay_add_event(ReplayAsyncEventKind event_kind,
 
 void replay_bh_schedule_event(QEMUBH *bh)
 {
-    if (events_enabled) {
-        uint64_t id = replay_get_current_icount();
-        replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
-    } else {
-        qemu_bh_schedule(bh);
-    }
+    uint64_t id = replay_get_current_icount();
+
+    g_assert(events_enabled);
+    replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
 }
 
 void replay_bh_schedule_oneshot_event(AioContext *ctx,
     QEMUBHFunc *cb, void *opaque)
 {
-    if (events_enabled) {
-        uint64_t id = replay_get_current_icount();
-        replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
-    } else {
-        aio_bh_schedule_oneshot(ctx, cb, opaque);
-    }
+    uint64_t id = replay_get_current_icount();
+
+    g_assert(events_enabled);
+    replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
 }
 
 void replay_add_input_event(struct InputEvent *event)
diff --git a/replay/replay-input.c b/replay/replay-input.c
index 1147e3d34e..5d1fd92e79 100644
--- a/replay/replay-input.c
+++ b/replay/replay-input.c
@@ -124,7 +124,7 @@ void replay_input_event(QemuConsole *src, InputEvent *evt)
     } else if (replay_mode == REPLAY_MODE_RECORD) {
         replay_add_input_event(QAPI_CLONE(InputEvent, evt));
     } else {
-        qemu_input_event_send_impl(src, evt);
+        g_assert_not_reached();
     }
 }
 
@@ -135,6 +135,6 @@ void replay_input_sync_event(void)
     } else if (replay_mode == REPLAY_MODE_RECORD) {
         replay_add_input_sync_event();
     } else {
-        qemu_input_event_sync_impl();
+        g_assert_not_reached();
     }
 }
diff --git a/stubs/meson.build b/stubs/meson.build
index 67f2a8c069..bbd2230d69 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -32,7 +32,6 @@ stub_ss.add(files('qtest.c'))
 stub_ss.add(files('ram-block.c'))
 stub_ss.add(files('ramfb.c'))
 stub_ss.add(files('replay.c'))
-stub_ss.add(files('replay-user.c'))
 stub_ss.add(files('runstate-check.c'))
 stub_ss.add(files('set-fd-handler.c'))
 stub_ss.add(files('sysbus.c'))
diff --git a/stubs/replay-user.c b/stubs/replay-user.c
deleted file mode 100644
index 2ad9e27203..0000000000
--- a/stubs/replay-user.c
+++ /dev/null
@@ -1,9 +0,0 @@
-#include "qemu/osdep.h"
-#include "sysemu/replay.h"
-#include "sysemu/sysemu.h"
-
-void replay_bh_schedule_oneshot_event(AioContext *ctx,
-    QEMUBHFunc *cb, void *opaque)
-{
-    aio_bh_schedule_oneshot(ctx, cb, opaque);
-}
diff --git a/stubs/replay.c b/stubs/replay.c
index 45ebe77fb9..86e8be704b 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -103,3 +103,99 @@ bool replay_reverse_continue(void)
 {
     return false;
 }
+
+void replay_input_event(QemuConsole *src, InputEvent *evt)
+{
+}
+void replay_input_sync_event(void)
+{
+}
+void replay_bh_schedule_event(QEMUBH *bh)
+{
+}
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+                                      QEMUBHFunc *cb, void *opaque)
+{
+}
+void replay_add_blocker(Error *reason)
+{
+}
+void replay_audio_in(size_t *recorded, void *samples, size_t *wpos, size_t size)
+{
+}
+void replay_audio_out(size_t *played)
+{
+}
+void replay_breakpoint(void)
+{
+}
+bool replay_can_snapshot(void)
+{
+    return false;
+}
+void replay_configure(struct QemuOpts *opts)
+{
+}
+void replay_flush_events(void)
+{
+}
+void replay_gdb_attached(void)
+{
+}
+bool replay_running_debug(void)
+{
+    return false;
+}
+void replay_shutdown_request(ShutdownCause cause)
+{
+}
+void replay_start(void)
+{
+}
+void replay_vmstate_init(void)
+{
+}
+
+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
+#include "qapi/qapi-commands-replay.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+
+void hmp_info_replay(Monitor *mon, const QDict *qdict)
+{
+    error_report("replay support not available");
+}
+void hmp_replay_break(Monitor *mon, const QDict *qdict)
+{
+    error_report("replay support not available");
+}
+void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
+{
+    error_report("replay support not available");
+}
+void hmp_replay_seek(Monitor *mon, const QDict *qdict)
+{
+    error_report("replay support not available");
+}
+ReplayInfo *qmp_query_replay(Error **errp)
+{
+    error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+              "replay support not available");
+    return NULL;
+}
+void qmp_replay_break(int64_t icount, Error **errp)
+{
+    error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+              "replay support not available");
+}
+void qmp_replay_delete_break(Error **errp)
+{
+    error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+              "replay support not available");
+}
+void qmp_replay_seek(int64_t icount, Error **errp)
+{
+    error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+              "replay support not available");
+}
diff --git a/tests/ptimer-test-stubs.c b/tests/ptimer-test-stubs.c
index e935a1395e..7f801a4d09 100644
--- a/tests/ptimer-test-stubs.c
+++ b/tests/ptimer-test-stubs.c
@@ -122,8 +122,3 @@ void qemu_bh_delete(QEMUBH *bh)
 {
     g_free(bh);
 }
-
-void replay_bh_schedule_event(QEMUBH *bh)
-{
-    bh->cb(bh->opaque);
-}
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 8a4c570e83..1c7186e53c 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -31,6 +31,9 @@ static int query_error_class(const char *cmd)
 #ifndef CONFIG_SPICE
         { "query-spice", ERROR_CLASS_COMMAND_NOT_FOUND },
 #endif
+#ifndef CONFIG_TCG
+        { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND },
+#endif
 #ifndef CONFIG_VNC
         { "query-vnc", ERROR_CLASS_GENERIC_ERROR },
         { "query-vnc-servers", ERROR_CLASS_GENERIC_ERROR },
diff --git a/ui/input.c b/ui/input.c
index 4791b089c7..8675e8ad09 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -375,7 +375,11 @@ void qemu_input_event_send(QemuConsole *src, InputEvent *evt)
         return;
     }
 
-    replay_input_event(src, evt);
+    if (replay_events_enabled()) {
+        replay_input_event(src, evt);
+    } else {
+        qemu_input_event_send_impl(src, evt);
+    }
 }
 
 void qemu_input_event_sync_impl(void)
@@ -401,7 +405,11 @@ void qemu_input_event_sync(void)
         return;
     }
 
-    replay_input_sync_event();
+    if (replay_events_enabled()) {
+        replay_input_sync_event();
+    } else {
+        qemu_input_event_sync_impl();
+    }
 }
 
 static InputEvent *qemu_input_event_new_key(KeyValue *key, bool down)
-- 
2.26.2

Re: [PATCH v3 3/3] replay: do not build if TCG is not available

[Paolo Bonzini]

On 13/10/20 16:38, Claudio Fontana wrote:
> +void bdrv_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
> +{
> +    if (replay_events_enabled()) {
> +        replay_bh_schedule_oneshot_event(ctx, cb, opaque);
> +    } else {
> +        aio_bh_schedule_oneshot(ctx, cb, opaque);
> +    }
> +}
> +
> +void bdrv_bh_schedule(QEMUBH *bh)
> +{
> +    if (replay_events_enabled()) {
> +        replay_bh_schedule_event(bh);
> +    } else {
> +        qemu_bh_schedule(bh);
> +    }
> +}

This is definitely better, but I'll defer to Kevin with respect to the
naming of the function; having a bdrv_* function that has nothing to do
with the block layer is still smelly of a sub-optimal API, and I'm not
sure why the API change belongs in the series.

Paolo

Re: [PATCH v3 3/3] replay: do not build if TCG is not available

[Claudio Fontana]

On 10/13/20 7:12 PM, Paolo Bonzini wrote:
> On 13/10/20 16:38, Claudio Fontana wrote:
>> +void bdrv_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
>> +{
>> +    if (replay_events_enabled()) {
>> +        replay_bh_schedule_oneshot_event(ctx, cb, opaque);
>> +    } else {
>> +        aio_bh_schedule_oneshot(ctx, cb, opaque);
>> +    }
>> +}
>> +
>> +void bdrv_bh_schedule(QEMUBH *bh)
>> +{
>> +    if (replay_events_enabled()) {
>> +        replay_bh_schedule_event(bh);
>> +    } else {
>> +        qemu_bh_schedule(bh);
>> +    }
>> +}
> 
> This is definitely better, but I'll defer to Kevin with respect to the
> naming of the function; having a bdrv_* function that has nothing to do
> with the block layer is still smelly of a sub-optimal API, and I'm not
> sure why the API change belongs in the series.
> 
> Paolo
> 

Hi Paolo,

I am not attached to the specific name, if someone has a better naming / proposes a better prefix I will replace of course.

Thank you,

Claudio

Re: [PATCH v3 3/3] replay: do not build if TCG is not available

[Paolo Bonzini]

On 13/10/20 19:17, Claudio Fontana wrote:
>> This is definitely better, but I'll defer to Kevin with respect to the
>> naming of the function; having a bdrv_* function that has nothing to do
>> with the block layer is still smelly of a sub-optimal API, and I'm not
>> sure why the API change belongs in the series.
>>
>> Paolo
>>
> Hi Paolo,
> 
> I am not attached to the specific name, if someone has a better naming / proposes a better prefix I will replace of course.

Can you just unbreak TCG in this series, and we can then revisit the
topic of functional code in stubs later?

Paolo

Re: [PATCH v3 3/3] replay: do not build if TCG is not available

[Claudio Fontana]

On 10/13/20 7:25 PM, Paolo Bonzini wrote:
> On 13/10/20 19:17, Claudio Fontana wrote:
>>> This is definitely better, but I'll defer to Kevin with respect to the
>>> naming of the function; having a bdrv_* function that has nothing to do
>>> with the block layer is still smelly of a sub-optimal API, and I'm not
>>> sure why the API change belongs in the series.
>>>
>>> Paolo
>>>
>> Hi Paolo,
>>
>> I am not attached to the specific name, if someone has a better naming / proposes a better prefix I will replace of course.
> 
> Can you just unbreak TCG in this series, and we can then revisit the
> topic of functional code in stubs later?
> 
> Paolo
> 
> 

Hi Paolo, probably just a typo there, it's non-tcg (--disable-tcg) that's broken, not TCG.

If nothing else would be acceptable and we are in a hurry, sure lets close the nose and do it,
it's just a tiny bit of some duplicated code in stubs.

I'd prefer if we have something a bit better that's all, but if this is hurting badly lets do it!

Ciao,

Claudio

Re: [PATCH v3 0/3] unbreak non-tcg builds

[Philippe Mathieu-Daudé]

On 10/13/20 4:38 PM, Claudio Fontana wrote:
> This series now unbreaks current non-tcg builds
> (!CONFIG_TCG).
> 
> tests Makefiles need to avoid relying on all non-native
> archs binaries to be present,
> 
> bios-tables-test needs to skip tests that are tcg-only,
> 
> and notably the replay framework needs to consider that
> it might not be functional (or its code present at all)
> without TCG.
> 
> Tested ok target x86_64-softmmu on x86_64 host with:
> 
> ./configure --enable-tcg --disable-kvm
> ./configure --enable-kvm --disable-tcg
> ./configure --enable-tcg --enable-kvm

If you want to avoid these configurations to bitrot,
consider covering them by adding Gitlab jobs :)))

Re: [PATCH v3 0/3] unbreak non-tcg builds

[Claudio Fontana]

On 10/13/20 4:55 PM, Philippe Mathieu-Daudé wrote:
> On 10/13/20 4:38 PM, Claudio Fontana wrote:
>> This series now unbreaks current non-tcg builds
>> (!CONFIG_TCG).
>>
>> tests Makefiles need to avoid relying on all non-native
>> archs binaries to be present,
>>
>> bios-tables-test needs to skip tests that are tcg-only,
>>
>> and notably the replay framework needs to consider that
>> it might not be functional (or its code present at all)
>> without TCG.
>>
>> Tested ok target x86_64-softmmu on x86_64 host with:
>>
>> ./configure --enable-tcg --disable-kvm
>> ./configure --enable-kvm --disable-tcg
>> ./configure --enable-tcg --enable-kvm
> 
> If you want to avoid these configurations to bitrot,
> consider covering them by adding Gitlab jobs :)))
> 
> 

That would be, providing a patch to  .travis.yml right?

I guess -j parameter for make is automatically provided depending on infra resources?

Something like the following?

Ciao,

C

diff --git a/.travis.yml b/.travis.yml
index 1054ec5d29..49ec615ed8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -320,6 +320,19 @@ jobs:
         - TEST_CMD="make check-tcg"
         - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
 
+    # Check modular accelerator builds.
+    - name: "modular accelerator builds (x86_64-softmmu, tcg only)"
+      env:
+        - CONFIG="--disable-kvm --enable-tcg --target-list=x86_64-softmmu"
+        - TEST_BUILD_CMD="make"
+        - TEST_CMD="make check"
+
+    - name: "modular accelerator builds (x86_64-softmmu, kvm only)"
+      env:
+        - CONFIG="--enable-kvm --disable-tcg --target-list=x86_64-softmmu"
+        - TEST_BUILD_CMD="make"
+        - TEST_CMD="make check"
+
     - name: "[aarch64] GCC check-tcg"
       arch: arm64
       dist: focal

Re: [PATCH v3 0/3] unbreak non-tcg builds

[Daniel P. Berrangé]

On Tue, Oct 13, 2020 at 06:47:42PM +0200, Claudio Fontana wrote:
> On 10/13/20 4:55 PM, Philippe Mathieu-Daudé wrote:
> > On 10/13/20 4:38 PM, Claudio Fontana wrote:
> >> This series now unbreaks current non-tcg builds
> >> (!CONFIG_TCG).
> >>
> >> tests Makefiles need to avoid relying on all non-native
> >> archs binaries to be present,
> >>
> >> bios-tables-test needs to skip tests that are tcg-only,
> >>
> >> and notably the replay framework needs to consider that
> >> it might not be functional (or its code present at all)
> >> without TCG.
> >>
> >> Tested ok target x86_64-softmmu on x86_64 host with:
> >>
> >> ./configure --enable-tcg --disable-kvm
> >> ./configure --enable-kvm --disable-tcg
> >> ./configure --enable-tcg --enable-kvm
> > 
> > If you want to avoid these configurations to bitrot,
> > consider covering them by adding Gitlab jobs :)))
> > 
> > 
> 
> That would be, providing a patch to  .travis.yml right?

Adding more jobs to Travis is discouraged. GitLab CI is our preferred
CI platform wherever possible. Others are only used where GitLab doesn't
support the required feature.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 0/3] unbreak non-tcg builds

[Claudio Fontana]

On 10/13/20 4:55 PM, Philippe Mathieu-Daudé wrote:
> On 10/13/20 4:38 PM, Claudio Fontana wrote:
>> This series now unbreaks current non-tcg builds
>> (!CONFIG_TCG).
>>
>> tests Makefiles need to avoid relying on all non-native
>> archs binaries to be present,
>>
>> bios-tables-test needs to skip tests that are tcg-only,
>>
>> and notably the replay framework needs to consider that
>> it might not be functional (or its code present at all)
>> without TCG.
>>
>> Tested ok target x86_64-softmmu on x86_64 host with:
>>
>> ./configure --enable-tcg --disable-kvm
>> ./configure --enable-kvm --disable-tcg
>> ./configure --enable-tcg --enable-kvm
> 
> If you want to avoid these configurations to bitrot,
> consider covering them by adding Gitlab jobs :)))
> 
> 

Hi Philippe and all,

latest master now has all the fixes discussed in these series, thanks also to Paolo's help with sorting out the stubs.

Already there is a new breakage though it seems, this time in qmp-cmd-test.

So adding CI coverage seems necessary.

In the .gitlab-ci.yml I see:

build-tcg-disabled:
[...]
    - ../configure --disable-tcg --audio-drv-list="" || { cat config.log meson-logs/meson-log.txt && exit 1; }

So is this supposed to be covered already?

I am confused also by the fact that it seems that suddently I see much "less" stuff in the gitlab CI, did I miss some infra discussion?

Thanks,

Claudio

Re: [PATCH v3 0/3] unbreak non-tcg builds

[Claudio Fontana]

On 10/26/20 4:28 PM, Claudio Fontana wrote:
> On 10/13/20 4:55 PM, Philippe Mathieu-Daudé wrote:
>> On 10/13/20 4:38 PM, Claudio Fontana wrote:
>>> This series now unbreaks current non-tcg builds
>>> (!CONFIG_TCG).
>>>
>>> tests Makefiles need to avoid relying on all non-native
>>> archs binaries to be present,
>>>
>>> bios-tables-test needs to skip tests that are tcg-only,
>>>
>>> and notably the replay framework needs to consider that
>>> it might not be functional (or its code present at all)
>>> without TCG.
>>>
>>> Tested ok target x86_64-softmmu on x86_64 host with:
>>>
>>> ./configure --enable-tcg --disable-kvm
>>> ./configure --enable-kvm --disable-tcg
>>> ./configure --enable-tcg --enable-kvm
>>
>> If you want to avoid these configurations to bitrot,
>> consider covering them by adding Gitlab jobs :)))
>>
>>
> 
> Hi Philippe and all,
> 
> latest master now has all the fixes discussed in these series, thanks also to Paolo's help with sorting out the stubs.
> 
> Already there is a new breakage though it seems, this time in qmp-cmd-test.


Sorry, this is wrong. The changes are not in master yet, that's why stuff breaks (errors from replay qmp commands).


> 
> So adding CI coverage seems necessary.
> 

This is still probably a good idea.

Ciao,

C

> In the .gitlab-ci.yml I see:
> 
> build-tcg-disabled:
> [...]
>     - ../configure --disable-tcg --audio-drv-list="" || { cat config.log meson-logs/meson-log.txt && exit 1; }
> 
> So is this supposed to be covered already?
> 
> I am confused also by the fact that it seems that suddently I see much "less" stuff in the gitlab CI, did I miss some infra discussion?
> 
> Thanks,
> 
> Claudio
> 

Re: [PATCH v3 0/3] unbreak non-tcg builds

[Claudio Fontana]

On 10/13/20 4:55 PM, Philippe Mathieu-Daudé wrote:
> On 10/13/20 4:38 PM, Claudio Fontana wrote:
>> This series now unbreaks current non-tcg builds
>> (!CONFIG_TCG).
>>
>> tests Makefiles need to avoid relying on all non-native
>> archs binaries to be present,
>>
>> bios-tables-test needs to skip tests that are tcg-only,
>>
>> and notably the replay framework needs to consider that
>> it might not be functional (or its code present at all)
>> without TCG.
>>
>> Tested ok target x86_64-softmmu on x86_64 host with:
>>
>> ./configure --enable-tcg --disable-kvm
>> ./configure --enable-kvm --disable-tcg
>> ./configure --enable-tcg --enable-kvm
> 
> If you want to avoid these configurations to bitrot,
> consider covering them by adding Gitlab jobs :)))
> 

Hello Philippe,

happy new year,

I am going back to look at the current code in master, slowly trying to get back a hold on things,
and I remember some time ago you suggested to keep testing "tools" builds,

with

./configure --disable-tcg --disable-kvm --enable-tools

but the drawback of the "tools-only" build is that currently one cannot run make check on it.

It fails with ERRORS in bios-tables-test and others.

Is it supposed to actually work? Is there any make check work that can be done without any accelerator?
I would assume so...

Thanks,

Claudio