[PULL 00/31] Block patches

[PULL 00/31] Block patches

[Stefan Hajnoczi]

The following changes since commit 9ac5df20f51fabcba0d902025df4bd7ea987c158:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200221-1' into staging (2020-02-21 16:18:38 +0000)

are available in the Git repository at:

  https://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to e5c59355ae9f724777c61c859292ec9db2c8c2ab:

  fuzz: add documentation to docs/devel/ (2020-02-22 08:26:48 +0000)

----------------------------------------------------------------
Pull request

This pull request contains a virtio-blk/scsi performance optimization, event
loop scalability improvements, and a qtest-based device fuzzing framework.  I
am including the fuzzing patches because I have reviewed them and Thomas Huth
is currently away on leave.

----------------------------------------------------------------

Alexander Bulekov (22):
  softmmu: move vl.c to softmmu/
  softmmu: split off vl.c:main() into main.c
  module: check module wasn't already initialized
  fuzz: add FUZZ_TARGET module type
  qtest: add qtest_server_send abstraction
  libqtest: add a layer of abstraction to send/recv
  libqtest: make bufwrite rely on the TransportOps
  qtest: add in-process incoming command handler
  libqos: rename i2c_send and i2c_recv
  libqos: split qos-test and libqos makefile vars
  libqos: move useful qos-test funcs to qos_external
  fuzz: add fuzzer skeleton
  exec: keep ram block across fork when using qtest
  main: keep rcu_atfork callback enabled for qtest
  fuzz: support for fork-based fuzzing.
  fuzz: add support for qos-assisted fuzz targets
  fuzz: add target/fuzz makefile rules
  fuzz: add configure flag --enable-fuzzing
  fuzz: add i440fx fuzz targets
  fuzz: add virtio-net fuzz target
  fuzz: add virtio-scsi fuzz target
  fuzz: add documentation to docs/devel/

Denis Plotnikov (1):
  virtio: increase virtqueue size for virtio-scsi and virtio-blk

Paolo Bonzini (1):
  rcu_queue: add QSLIST functions

Stefan Hajnoczi (7):
  aio-posix: avoid reacquiring rcu_read_lock() when polling
  util/async: make bh_aio_poll() O(1)
  aio-posix: fix use after leaving scope in aio_poll()
  aio-posix: don't pass ns timeout to epoll_wait()
  qemu/queue.h: add QLIST_SAFE_REMOVE()
  aio-posix: make AioHandler deletion O(1)
  aio-posix: make AioHandler dispatch O(1) with epoll

 MAINTAINERS                         |  11 +-
 Makefile                            |  15 +-
 Makefile.objs                       |   2 -
 Makefile.target                     |  19 ++-
 block.c                             |   5 +-
 chardev/spice.c                     |   4 +-
 configure                           |  39 +++++
 docs/devel/fuzzing.txt              | 116 ++++++++++++++
 exec.c                              |  12 +-
 hw/block/virtio-blk.c               |   2 +-
 hw/core/machine.c                   |   2 +
 hw/scsi/virtio-scsi.c               |   2 +-
 include/block/aio.h                 |  26 ++-
 include/qemu/module.h               |   4 +-
 include/qemu/queue.h                |  32 +++-
 include/qemu/rcu_queue.h            |  47 ++++++
 include/sysemu/qtest.h              |   4 +
 include/sysemu/sysemu.h             |   4 +
 qtest.c                             |  31 +++-
 scripts/checkpatch.pl               |   2 +-
 scripts/get_maintainer.pl           |   3 +-
 softmmu/Makefile.objs               |   3 +
 softmmu/main.c                      |  53 +++++++
 vl.c => softmmu/vl.c                |  48 +++---
 tests/Makefile.include              |   2 +
 tests/qtest/Makefile.include        |  72 +++++----
 tests/qtest/fuzz/Makefile.include   |  18 +++
 tests/qtest/fuzz/fork_fuzz.c        |  55 +++++++
 tests/qtest/fuzz/fork_fuzz.h        |  23 +++
 tests/qtest/fuzz/fork_fuzz.ld       |  37 +++++
 tests/qtest/fuzz/fuzz.c             | 179 +++++++++++++++++++++
 tests/qtest/fuzz/fuzz.h             |  95 +++++++++++
 tests/qtest/fuzz/i440fx_fuzz.c      | 193 ++++++++++++++++++++++
 tests/qtest/fuzz/qos_fuzz.c         | 234 +++++++++++++++++++++++++++
 tests/qtest/fuzz/qos_fuzz.h         |  33 ++++
 tests/qtest/fuzz/virtio_net_fuzz.c  | 198 +++++++++++++++++++++++
 tests/qtest/fuzz/virtio_scsi_fuzz.c | 213 +++++++++++++++++++++++++
 tests/qtest/libqos/i2c.c            |  10 +-
 tests/qtest/libqos/i2c.h            |   4 +-
 tests/qtest/libqos/qos_external.c   | 168 ++++++++++++++++++++
 tests/qtest/libqos/qos_external.h   |  28 ++++
 tests/qtest/libqtest.c              | 119 ++++++++++++--
 tests/qtest/libqtest.h              |   4 +
 tests/qtest/pca9552-test.c          |  10 +-
 tests/qtest/qos-test.c              | 132 +---------------
 tests/test-aio.c                    |   3 +-
 tests/test-rcu-list.c               |  16 ++
 tests/test-rcu-slist.c              |   2 +
 util/aio-posix.c                    | 187 +++++++++++++++-------
 util/async.c                        | 237 ++++++++++++++++------------
 util/module.c                       |   7 +
 51 files changed, 2365 insertions(+), 400 deletions(-)
 create mode 100644 docs/devel/fuzzing.txt
 create mode 100644 softmmu/Makefile.objs
 create mode 100644 softmmu/main.c
 rename vl.c => softmmu/vl.c (99%)
 create mode 100644 tests/qtest/fuzz/Makefile.include
 create mode 100644 tests/qtest/fuzz/fork_fuzz.c
 create mode 100644 tests/qtest/fuzz/fork_fuzz.h
 create mode 100644 tests/qtest/fuzz/fork_fuzz.ld
 create mode 100644 tests/qtest/fuzz/fuzz.c
 create mode 100644 tests/qtest/fuzz/fuzz.h
 create mode 100644 tests/qtest/fuzz/i440fx_fuzz.c
 create mode 100644 tests/qtest/fuzz/qos_fuzz.c
 create mode 100644 tests/qtest/fuzz/qos_fuzz.h
 create mode 100644 tests/qtest/fuzz/virtio_net_fuzz.c
 create mode 100644 tests/qtest/fuzz/virtio_scsi_fuzz.c
 create mode 100644 tests/qtest/libqos/qos_external.c
 create mode 100644 tests/qtest/libqos/qos_external.h
 create mode 100644 tests/test-rcu-slist.c

-- 
2.24.1

[PULL 01/31] virtio: increase virtqueue size for virtio-scsi and virtio-blk

[Stefan Hajnoczi]

From: Denis Plotnikov <dplotnikov@virtuozzo.com>

The goal is to reduce the amount of requests issued by a guest on
1M reads/writes. This rises the performance up to 4% on that kind of
disk access pattern.

The maximum chunk size to be used for the guest disk accessing is
limited with seg_max parameter, which represents the max amount of
pices in the scatter-geather list in one guest disk request.

Since seg_max is virqueue_size dependent, increasing the virtqueue
size increases seg_max, which, in turn, increases the maximum size
of data to be read/write from a guest disk.

More details in the original problem statment:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html

Suggested-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Message-id: 20200214074648.958-1-dplotnikov@virtuozzo.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 2 +-
 hw/core/machine.c     | 2 ++
 hw/scsi/virtio-scsi.c | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..142863a3b2 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
     DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
                     true),
     DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
-    DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+    DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
     DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
     DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
                      IOThread *),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 84812a1d1c..ce403ccea9 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@
 #include "hw/mem/nvdimm.h"
 
 GlobalProperty hw_compat_4_2[] = {
+    { "virtio-blk-device", "queue-size", "128"},
+    { "virtio-scsi-device", "virtqueue_size", "128"},
     { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
     { "virtio-blk-device", "seg-max-adjust", "off"},
     { "virtio-scsi-device", "seg_max_adjust", "off"},
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..472bbd233b 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
 static Property virtio_scsi_properties[] = {
     DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
     DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
-                                         parent_obj.conf.virtqueue_size, 128),
+                                         parent_obj.conf.virtqueue_size, 256),
     DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
                       parent_obj.conf.seg_max_adjust, true),
     DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
-- 
2.24.1

[PULL 02/31] aio-posix: avoid reacquiring rcu_read_lock() when polling

[Stefan Hajnoczi]

The first rcu_read_lock/unlock() is expensive.  Nested calls are cheap.

This optimization increases IOPS from 73k to 162k with a Linux guest
that has 2 virtio-blk,num-queues=1 and 99 virtio-blk,num-queues=32
devices.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20200218182708.914552-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/aio-posix.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index a4977f538e..f67f5b34e9 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -15,6 +15,7 @@
 
 #include "qemu/osdep.h"
 #include "block/block.h"
+#include "qemu/rcu.h"
 #include "qemu/rcu_queue.h"
 #include "qemu/sockets.h"
 #include "qemu/cutils.h"
@@ -514,6 +515,16 @@ static bool run_poll_handlers_once(AioContext *ctx, int64_t *timeout)
     bool progress = false;
     AioHandler *node;
 
+    /*
+     * Optimization: ->io_poll() handlers often contain RCU read critical
+     * sections and we therefore see many rcu_read_lock() -> rcu_read_unlock()
+     * -> rcu_read_lock() -> ... sequences with expensive memory
+     * synchronization primitives.  Make the entire polling loop an RCU
+     * critical section because nested rcu_read_lock()/rcu_read_unlock() calls
+     * are cheap.
+     */
+    RCU_READ_LOCK_GUARD();
+
     QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
         if (!node->deleted && node->io_poll &&
             aio_node_check(ctx, node->is_external) &&
-- 
2.24.1

[PULL 03/31] rcu_queue: add QSLIST functions

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

QSLIST is the only family of lists for which we do not have RCU-friendly accessors,
add them.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20200220103828.24525-1-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/queue.h     | 15 +++++++++++--
 include/qemu/rcu_queue.h | 47 ++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.include   |  2 ++
 tests/test-rcu-list.c    | 16 ++++++++++++++
 tests/test-rcu-slist.c   |  2 ++
 5 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 tests/test-rcu-slist.c

diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index 19425f973f..fcecb70228 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -211,9 +211,20 @@ struct {                                                                \
         (head)->slh_first = (head)->slh_first->field.sle_next;          \
 } while (/*CONSTCOND*/0)
 
-#define QSLIST_REMOVE_AFTER(slistelm, field) do {                        \
+#define QSLIST_REMOVE_AFTER(slistelm, field) do {                       \
         (slistelm)->field.sle_next =                                    \
-            QSLIST_NEXT(QSLIST_NEXT((slistelm), field), field);           \
+            QSLIST_NEXT(QSLIST_NEXT((slistelm), field), field);         \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_REMOVE(head, elm, type, field) do {                      \
+    if ((head)->slh_first == (elm)) {                                   \
+        QSLIST_REMOVE_HEAD((head), field);                              \
+    } else {                                                            \
+        struct type *curelm = (head)->slh_first;                        \
+        while (curelm->field.sle_next != (elm))                         \
+            curelm = curelm->field.sle_next;                            \
+        curelm->field.sle_next = curelm->field.sle_next->field.sle_next; \
+    }                                                                   \
 } while (/*CONSTCOND*/0)
 
 #define QSLIST_FOREACH(var, head, field)                                 \
diff --git a/include/qemu/rcu_queue.h b/include/qemu/rcu_queue.h
index 2d386f303e..558961cc27 100644
--- a/include/qemu/rcu_queue.h
+++ b/include/qemu/rcu_queue.h
@@ -262,6 +262,53 @@ extern "C" {
          (var) && ((next) = atomic_rcu_read(&(var)->field.tqe_next), 1); \
          (var) = (next))
 
+/*
+ * RCU singly-linked list
+ */
+
+/* Singly-linked list access methods */
+#define QSLIST_EMPTY_RCU(head)      (atomic_read(&(head)->slh_first) == NULL)
+#define QSLIST_FIRST_RCU(head)       atomic_rcu_read(&(head)->slh_first)
+#define QSLIST_NEXT_RCU(elm, field)  atomic_rcu_read(&(elm)->field.sle_next)
+
+/* Singly-linked list functions */
+#define QSLIST_INSERT_HEAD_RCU(head, elm, field) do {           \
+    (elm)->field.sle_next = (head)->slh_first;                  \
+    atomic_rcu_set(&(head)->slh_first, (elm));                  \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_INSERT_AFTER_RCU(head, listelm, elm, field) do {         \
+    (elm)->field.sle_next = (listelm)->field.sle_next;                  \
+    atomic_rcu_set(&(listelm)->field.sle_next, (elm));                  \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_REMOVE_HEAD_RCU(head, field) do {                       \
+    atomic_set(&(head)->slh_first, (head)->slh_first->field.sle_next); \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_REMOVE_RCU(head, elm, type, field) do {              \
+    if ((head)->slh_first == (elm)) {                               \
+        QSLIST_REMOVE_HEAD_RCU((head), field);                      \
+    } else {                                                        \
+        struct type *curr = (head)->slh_first;                      \
+        while (curr->field.sle_next != (elm)) {                     \
+            curr = curr->field.sle_next;                            \
+        }                                                           \
+        atomic_set(&curr->field.sle_next,                           \
+                   curr->field.sle_next->field.sle_next);           \
+    }                                                               \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_FOREACH_RCU(var, head, field)                          \
+    for ((var) = atomic_rcu_read(&(head)->slh_first);                   \
+         (var);                                                         \
+         (var) = atomic_rcu_read(&(var)->field.sle_next))
+
+#define QSLIST_FOREACH_SAFE_RCU(var, head, field, next)                \
+    for ((var) = atomic_rcu_read(&(head)->slh_first);                    \
+         (var) && ((next) = atomic_rcu_read(&(var)->field.sle_next), 1); \
+         (var) = (next))
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 2f1cafed72..edcbd475aa 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -98,6 +98,7 @@ check-unit-y += tests/rcutorture$(EXESUF)
 check-unit-y += tests/test-rcu-list$(EXESUF)
 check-unit-y += tests/test-rcu-simpleq$(EXESUF)
 check-unit-y += tests/test-rcu-tailq$(EXESUF)
+check-unit-y += tests/test-rcu-slist$(EXESUF)
 check-unit-y += tests/test-qdist$(EXESUF)
 check-unit-y += tests/test-qht$(EXESUF)
 check-unit-y += tests/test-qht-par$(EXESUF)
@@ -415,6 +416,7 @@ tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
 tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
 tests/test-rcu-simpleq$(EXESUF): tests/test-rcu-simpleq.o $(test-util-obj-y)
 tests/test-rcu-tailq$(EXESUF): tests/test-rcu-tailq.o $(test-util-obj-y)
+tests/test-rcu-slist$(EXESUF): tests/test-rcu-slist.o $(test-util-obj-y)
 tests/test-qdist$(EXESUF): tests/test-qdist.o $(test-util-obj-y)
 tests/test-qht$(EXESUF): tests/test-qht.o $(test-util-obj-y)
 tests/test-qht-par$(EXESUF): tests/test-qht-par.o tests/qht-bench$(EXESUF) $(test-util-obj-y)
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index 6f076473e0..1442c0c982 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -93,6 +93,8 @@ struct list_element {
     QSIMPLEQ_ENTRY(list_element) entry;
 #elif TEST_LIST_TYPE == 3
     QTAILQ_ENTRY(list_element) entry;
+#elif TEST_LIST_TYPE == 4
+    QSLIST_ENTRY(list_element) entry;
 #else
 #error Invalid TEST_LIST_TYPE
 #endif
@@ -144,6 +146,20 @@ static QTAILQ_HEAD(, list_element) Q_list_head;
 #define TEST_LIST_INSERT_HEAD_RCU   QTAILQ_INSERT_HEAD_RCU
 #define TEST_LIST_FOREACH_RCU       QTAILQ_FOREACH_RCU
 #define TEST_LIST_FOREACH_SAFE_RCU  QTAILQ_FOREACH_SAFE_RCU
+
+#elif TEST_LIST_TYPE == 4
+static QSLIST_HEAD(, list_element) Q_list_head;
+
+#define TEST_NAME "qslist"
+#define TEST_LIST_REMOVE_RCU(el, f)                              \
+	 QSLIST_REMOVE_RCU(&Q_list_head, el, list_element, f)
+
+#define TEST_LIST_INSERT_AFTER_RCU(list_el, el, f)               \
+         QSLIST_INSERT_AFTER_RCU(&Q_list_head, list_el, el, f)
+
+#define TEST_LIST_INSERT_HEAD_RCU   QSLIST_INSERT_HEAD_RCU
+#define TEST_LIST_FOREACH_RCU       QSLIST_FOREACH_RCU
+#define TEST_LIST_FOREACH_SAFE_RCU  QSLIST_FOREACH_SAFE_RCU
 #else
 #error Invalid TEST_LIST_TYPE
 #endif
diff --git a/tests/test-rcu-slist.c b/tests/test-rcu-slist.c
new file mode 100644
index 0000000000..868e1e472e
--- /dev/null
+++ b/tests/test-rcu-slist.c
@@ -0,0 +1,2 @@
+#define TEST_LIST_TYPE 4
+#include "test-rcu-list.c"
-- 
2.24.1

[PULL 04/31] util/async: make bh_aio_poll() O(1)

[Stefan Hajnoczi]

The ctx->first_bh list contains all created BHs, including those that
are not scheduled.  The list is iterated by the event loop and therefore
has O(n) time complexity with respected to the number of created BHs.

Rewrite BHs so that only scheduled or deleted BHs are enqueued.
Only BHs that actually require action will be iterated.

One semantic change is required: qemu_bh_delete() enqueues the BH and
therefore invokes aio_notify().  The
tests/test-aio.c:test_source_bh_delete_from_cb() test case assumed that
g_main_context_iteration(NULL, false) returns false after
qemu_bh_delete() but it now returns true for one iteration.  Fix up the
test case.

This patch makes aio_compute_timeout() and aio_bh_poll() drop from a CPU
profile reported by perf-top(1).  Previously they combined to 9% CPU
utilization when AioContext polling is commented out and the guest has 2
virtio-blk,num-queues=1 and 99 virtio-blk,num-queues=32 devices.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20200221093951.1414693-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/aio.h |  20 +++-
 tests/test-aio.c    |   3 +-
 util/async.c        | 237 ++++++++++++++++++++++++++------------------
 3 files changed, 158 insertions(+), 102 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 7ba9bd7874..1a2ce9ca26 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -51,6 +51,19 @@ struct ThreadPool;
 struct LinuxAioState;
 struct LuringState;
 
+/*
+ * Each aio_bh_poll() call carves off a slice of the BH list, so that newly
+ * scheduled BHs are not processed until the next aio_bh_poll() call.  All
+ * active aio_bh_poll() calls chain their slices together in a list, so that
+ * nested aio_bh_poll() calls process all scheduled bottom halves.
+ */
+typedef QSLIST_HEAD(, QEMUBH) BHList;
+typedef struct BHListSlice BHListSlice;
+struct BHListSlice {
+    BHList bh_list;
+    QSIMPLEQ_ENTRY(BHListSlice) next;
+};
+
 struct AioContext {
     GSource source;
 
@@ -91,8 +104,11 @@ struct AioContext {
      */
     QemuLockCnt list_lock;
 
-    /* Anchor of the list of Bottom Halves belonging to the context */
-    struct QEMUBH *first_bh;
+    /* Bottom Halves pending aio_bh_poll() processing */
+    BHList bh_list;
+
+    /* Chained BH list slices for each nested aio_bh_poll() call */
+    QSIMPLEQ_HEAD(, BHListSlice) bh_slice_list;
 
     /* Used by aio_notify.
      *
diff --git a/tests/test-aio.c b/tests/test-aio.c
index 86fb73b3d5..8a46078463 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -615,7 +615,8 @@ static void test_source_bh_delete_from_cb(void)
     g_assert_cmpint(data1.n, ==, data1.max);
     g_assert(data1.bh == NULL);
 
-    g_assert(!g_main_context_iteration(NULL, false));
+    assert(g_main_context_iteration(NULL, false));
+    assert(!g_main_context_iteration(NULL, false));
 }
 
 static void test_source_bh_delete_from_cb_many(void)
diff --git a/util/async.c b/util/async.c
index c192a24a61..b94518b948 100644
--- a/util/async.c
+++ b/util/async.c
@@ -29,6 +29,7 @@
 #include "block/thread-pool.h"
 #include "qemu/main-loop.h"
 #include "qemu/atomic.h"
+#include "qemu/rcu_queue.h"
 #include "block/raw-aio.h"
 #include "qemu/coroutine_int.h"
 #include "trace.h"
@@ -36,16 +37,76 @@
 /***********************************************************/
 /* bottom halves (can be seen as timers which expire ASAP) */
 
+/* QEMUBH::flags values */
+enum {
+    /* Already enqueued and waiting for aio_bh_poll() */
+    BH_PENDING   = (1 << 0),
+
+    /* Invoke the callback */
+    BH_SCHEDULED = (1 << 1),
+
+    /* Delete without invoking callback */
+    BH_DELETED   = (1 << 2),
+
+    /* Delete after invoking callback */
+    BH_ONESHOT   = (1 << 3),
+
+    /* Schedule periodically when the event loop is idle */
+    BH_IDLE      = (1 << 4),
+};
+
 struct QEMUBH {
     AioContext *ctx;
     QEMUBHFunc *cb;
     void *opaque;
-    QEMUBH *next;
-    bool scheduled;
-    bool idle;
-    bool deleted;
+    QSLIST_ENTRY(QEMUBH) next;
+    unsigned flags;
 };
 
+/* Called concurrently from any thread */
+static void aio_bh_enqueue(QEMUBH *bh, unsigned new_flags)
+{
+    AioContext *ctx = bh->ctx;
+    unsigned old_flags;
+
+    /*
+     * The memory barrier implicit in atomic_fetch_or makes sure that:
+     * 1. idle & any writes needed by the callback are done before the
+     *    locations are read in the aio_bh_poll.
+     * 2. ctx is loaded before the callback has a chance to execute and bh
+     *    could be freed.
+     */
+    old_flags = atomic_fetch_or(&bh->flags, BH_PENDING | new_flags);
+    if (!(old_flags & BH_PENDING)) {
+        QSLIST_INSERT_HEAD_ATOMIC(&ctx->bh_list, bh, next);
+    }
+
+    aio_notify(ctx);
+}
+
+/* Only called from aio_bh_poll() and aio_ctx_finalize() */
+static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags)
+{
+    QEMUBH *bh = QSLIST_FIRST_RCU(head);
+
+    if (!bh) {
+        return NULL;
+    }
+
+    QSLIST_REMOVE_HEAD(head, next);
+
+    /*
+     * The atomic_and is paired with aio_bh_enqueue().  The implicit memory
+     * barrier ensures that the callback sees all writes done by the scheduling
+     * thread.  It also ensures that the scheduling thread sees the cleared
+     * flag before bh->cb has run, and thus will call aio_notify again if
+     * necessary.
+     */
+    *flags = atomic_fetch_and(&bh->flags,
+                              ~(BH_PENDING | BH_SCHEDULED | BH_IDLE));
+    return bh;
+}
+
 void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
 {
     QEMUBH *bh;
@@ -55,15 +116,7 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
         .cb = cb,
         .opaque = opaque,
     };
-    qemu_lockcnt_lock(&ctx->list_lock);
-    bh->next = ctx->first_bh;
-    bh->scheduled = 1;
-    bh->deleted = 1;
-    /* Make sure that the members are ready before putting bh into list */
-    smp_wmb();
-    ctx->first_bh = bh;
-    qemu_lockcnt_unlock(&ctx->list_lock);
-    aio_notify(ctx);
+    aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
 }
 
 QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
@@ -75,12 +128,6 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
         .cb = cb,
         .opaque = opaque,
     };
-    qemu_lockcnt_lock(&ctx->list_lock);
-    bh->next = ctx->first_bh;
-    /* Make sure that the members are ready before putting bh into list */
-    smp_wmb();
-    ctx->first_bh = bh;
-    qemu_lockcnt_unlock(&ctx->list_lock);
     return bh;
 }
 
@@ -89,91 +136,56 @@ void aio_bh_call(QEMUBH *bh)
     bh->cb(bh->opaque);
 }
 
-/* Multiple occurrences of aio_bh_poll cannot be called concurrently.
- * The count in ctx->list_lock is incremented before the call, and is
- * not affected by the call.
- */
+/* Multiple occurrences of aio_bh_poll cannot be called concurrently. */
 int aio_bh_poll(AioContext *ctx)
 {
-    QEMUBH *bh, **bhp, *next;
-    int ret;
-    bool deleted = false;
-
-    ret = 0;
-    for (bh = atomic_rcu_read(&ctx->first_bh); bh; bh = next) {
-        next = atomic_rcu_read(&bh->next);
-        /* The atomic_xchg is paired with the one in qemu_bh_schedule.  The
-         * implicit memory barrier ensures that the callback sees all writes
-         * done by the scheduling thread.  It also ensures that the scheduling
-         * thread sees the zero before bh->cb has run, and thus will call
-         * aio_notify again if necessary.
-         */
-        if (atomic_xchg(&bh->scheduled, 0)) {
+    BHListSlice slice;
+    BHListSlice *s;
+    int ret = 0;
+
+    QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
+    QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
+
+    while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
+        QEMUBH *bh;
+        unsigned flags;
+
+        bh = aio_bh_dequeue(&s->bh_list, &flags);
+        if (!bh) {
+            QSIMPLEQ_REMOVE_HEAD(&ctx->bh_slice_list, next);
+            continue;
+        }
+
+        if ((flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
             /* Idle BHs don't count as progress */
-            if (!bh->idle) {
+            if (!(flags & BH_IDLE)) {
                 ret = 1;
             }
-            bh->idle = 0;
             aio_bh_call(bh);
         }
-        if (bh->deleted) {
-            deleted = true;
+        if (flags & (BH_DELETED | BH_ONESHOT)) {
+            g_free(bh);
         }
     }
 
-    /* remove deleted bhs */
-    if (!deleted) {
-        return ret;
-    }
-
-    if (qemu_lockcnt_dec_if_lock(&ctx->list_lock)) {
-        bhp = &ctx->first_bh;
-        while (*bhp) {
-            bh = *bhp;
-            if (bh->deleted && !bh->scheduled) {
-                *bhp = bh->next;
-                g_free(bh);
-            } else {
-                bhp = &bh->next;
-            }
-        }
-        qemu_lockcnt_inc_and_unlock(&ctx->list_lock);
-    }
     return ret;
 }
 
 void qemu_bh_schedule_idle(QEMUBH *bh)
 {
-    bh->idle = 1;
-    /* Make sure that idle & any writes needed by the callback are done
-     * before the locations are read in the aio_bh_poll.
-     */
-    atomic_mb_set(&bh->scheduled, 1);
+    aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
 }
 
 void qemu_bh_schedule(QEMUBH *bh)
 {
-    AioContext *ctx;
-
-    ctx = bh->ctx;
-    bh->idle = 0;
-    /* The memory barrier implicit in atomic_xchg makes sure that:
-     * 1. idle & any writes needed by the callback are done before the
-     *    locations are read in the aio_bh_poll.
-     * 2. ctx is loaded before scheduled is set and the callback has a chance
-     *    to execute.
-     */
-    if (atomic_xchg(&bh->scheduled, 1) == 0) {
-        aio_notify(ctx);
-    }
+    aio_bh_enqueue(bh, BH_SCHEDULED);
 }
 
-
 /* This func is async.
  */
 void qemu_bh_cancel(QEMUBH *bh)
 {
-    atomic_mb_set(&bh->scheduled, 0);
+    atomic_and(&bh->flags, ~BH_SCHEDULED);
 }
 
 /* This func is async.The bottom half will do the delete action at the finial
@@ -181,21 +193,16 @@ void qemu_bh_cancel(QEMUBH *bh)
  */
 void qemu_bh_delete(QEMUBH *bh)
 {
-    bh->scheduled = 0;
-    bh->deleted = 1;
+    aio_bh_enqueue(bh, BH_DELETED);
 }
 
-int64_t
-aio_compute_timeout(AioContext *ctx)
+static int64_t aio_compute_bh_timeout(BHList *head, int timeout)
 {
-    int64_t deadline;
-    int timeout = -1;
     QEMUBH *bh;
 
-    for (bh = atomic_rcu_read(&ctx->first_bh); bh;
-         bh = atomic_rcu_read(&bh->next)) {
-        if (bh->scheduled) {
-            if (bh->idle) {
+    QSLIST_FOREACH_RCU(bh, head, next) {
+        if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
+            if (bh->flags & BH_IDLE) {
                 /* idle bottom halves will be polled at least
                  * every 10ms */
                 timeout = 10000000;
@@ -207,6 +214,28 @@ aio_compute_timeout(AioContext *ctx)
         }
     }
 
+    return timeout;
+}
+
+int64_t
+aio_compute_timeout(AioContext *ctx)
+{
+    BHListSlice *s;
+    int64_t deadline;
+    int timeout = -1;
+
+    timeout = aio_compute_bh_timeout(&ctx->bh_list, timeout);
+    if (timeout == 0) {
+        return 0;
+    }
+
+    QSIMPLEQ_FOREACH(s, &ctx->bh_slice_list, next) {
+        timeout = aio_compute_bh_timeout(&s->bh_list, timeout);
+        if (timeout == 0) {
+            return 0;
+        }
+    }
+
     deadline = timerlistgroup_deadline_ns(&ctx->tlg);
     if (deadline == 0) {
         return 0;
@@ -237,15 +266,24 @@ aio_ctx_check(GSource *source)
 {
     AioContext *ctx = (AioContext *) source;
     QEMUBH *bh;
+    BHListSlice *s;
 
     atomic_and(&ctx->notify_me, ~1);
     aio_notify_accept(ctx);
 
-    for (bh = ctx->first_bh; bh; bh = bh->next) {
-        if (bh->scheduled) {
+    QSLIST_FOREACH_RCU(bh, &ctx->bh_list, next) {
+        if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
             return true;
         }
     }
+
+    QSIMPLEQ_FOREACH(s, &ctx->bh_slice_list, next) {
+        QSLIST_FOREACH_RCU(bh, &s->bh_list, next) {
+            if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
+                return true;
+            }
+        }
+    }
     return aio_pending(ctx) || (timerlistgroup_deadline_ns(&ctx->tlg) == 0);
 }
 
@@ -265,6 +303,8 @@ static void
 aio_ctx_finalize(GSource     *source)
 {
     AioContext *ctx = (AioContext *) source;
+    QEMUBH *bh;
+    unsigned flags;
 
     thread_pool_free(ctx->thread_pool);
 
@@ -287,18 +327,15 @@ aio_ctx_finalize(GSource     *source)
     assert(QSLIST_EMPTY(&ctx->scheduled_coroutines));
     qemu_bh_delete(ctx->co_schedule_bh);
 
-    qemu_lockcnt_lock(&ctx->list_lock);
-    assert(!qemu_lockcnt_count(&ctx->list_lock));
-    while (ctx->first_bh) {
-        QEMUBH *next = ctx->first_bh->next;
+    /* There must be no aio_bh_poll() calls going on */
+    assert(QSIMPLEQ_EMPTY(&ctx->bh_slice_list));
 
+    while ((bh = aio_bh_dequeue(&ctx->bh_list, &flags))) {
         /* qemu_bh_delete() must have been called on BHs in this AioContext */
-        assert(ctx->first_bh->deleted);
+        assert(flags & BH_DELETED);
 
-        g_free(ctx->first_bh);
-        ctx->first_bh = next;
+        g_free(bh);
     }
-    qemu_lockcnt_unlock(&ctx->list_lock);
 
     aio_set_event_notifier(ctx, &ctx->notifier, false, NULL, NULL);
     event_notifier_cleanup(&ctx->notifier);
@@ -445,6 +482,8 @@ AioContext *aio_context_new(Error **errp)
     AioContext *ctx;
 
     ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
+    QSLIST_INIT(&ctx->bh_list);
+    QSIMPLEQ_INIT(&ctx->bh_slice_list);
     aio_context_setup(ctx);
 
     ret = event_notifier_init(&ctx->notifier, false);
-- 
2.24.1

[PULL 05/31] aio-posix: fix use after leaving scope in aio_poll()

[Stefan Hajnoczi]

epoll_handler is a stack variable and must not be accessed after it goes
out of scope:

      if (aio_epoll_check_poll(ctx, pollfds, npfd, timeout)) {
          AioHandler epoll_handler;
          ...
          add_pollfd(&epoll_handler);
          ret = aio_epoll(ctx, pollfds, npfd, timeout);
      } ...

  ...

  /* if we have any readable fds, dispatch event */
  if (ret > 0) {
      for (i = 0; i < npfd; i++) {
          nodes[i]->pfd.revents = pollfds[i].revents;
      }
  }

nodes[0] is &epoll_handler, which has already gone out of scope.

There is no need to use pollfds[] for epoll.  We don't need an
AioHandler for the epoll fd.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200214171712.541358-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/aio-posix.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index f67f5b34e9..c964627a03 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -105,17 +105,18 @@ static void aio_epoll_update(AioContext *ctx, AioHandler *node, bool is_new)
     }
 }
 
-static int aio_epoll(AioContext *ctx, GPollFD *pfds,
-                     unsigned npfd, int64_t timeout)
+static int aio_epoll(AioContext *ctx, int64_t timeout)
 {
+    GPollFD pfd = {
+        .fd = ctx->epollfd,
+        .events = G_IO_IN | G_IO_OUT | G_IO_HUP | G_IO_ERR,
+    };
     AioHandler *node;
     int i, ret = 0;
     struct epoll_event events[128];
 
-    assert(npfd == 1);
-    assert(pfds[0].fd == ctx->epollfd);
     if (timeout > 0) {
-        ret = qemu_poll_ns(pfds, npfd, timeout);
+        ret = qemu_poll_ns(&pfd, 1, timeout);
     }
     if (timeout <= 0 || ret > 0) {
         ret = epoll_wait(ctx->epollfd, events,
@@ -669,13 +670,8 @@ bool aio_poll(AioContext *ctx, bool blocking)
 
         /* wait until next event */
         if (aio_epoll_check_poll(ctx, pollfds, npfd, timeout)) {
-            AioHandler epoll_handler;
-
-            epoll_handler.pfd.fd = ctx->epollfd;
-            epoll_handler.pfd.events = G_IO_IN | G_IO_OUT | G_IO_HUP | G_IO_ERR;
-            npfd = 0;
-            add_pollfd(&epoll_handler);
-            ret = aio_epoll(ctx, pollfds, npfd, timeout);
+            npfd = 0; /* pollfds[] is not being used */
+            ret = aio_epoll(ctx, timeout);
         } else  {
             ret = qemu_poll_ns(pollfds, npfd, timeout);
         }
-- 
2.24.1

[PULL 06/31] aio-posix: don't pass ns timeout to epoll_wait()

[Stefan Hajnoczi]

Don't pass the nanosecond timeout into epoll_wait(), which expects
milliseconds.

The epoll_wait() timeout value does not matter if qemu_poll_ns()
determined that the poll fd is ready, but passing a value in the wrong
units is still ugly.  Pass a 0 timeout to epoll_wait() instead.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200214171712.541358-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/aio-posix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index c964627a03..58765e581e 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -117,6 +117,9 @@ static int aio_epoll(AioContext *ctx, int64_t timeout)
 
     if (timeout > 0) {
         ret = qemu_poll_ns(&pfd, 1, timeout);
+        if (ret > 0) {
+            timeout = 0;
+        }
     }
     if (timeout <= 0 || ret > 0) {
         ret = epoll_wait(ctx->epollfd, events,
-- 
2.24.1

[PULL 07/31] qemu/queue.h: add QLIST_SAFE_REMOVE()

[Stefan Hajnoczi]

QLIST_REMOVE() assumes the element is in a list.  It also leaves the
element's linked list pointers dangling.

Introduce a safe version of QLIST_REMOVE() and convert open-coded
instances of this pattern.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200214171712.541358-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c              |  5 +----
 chardev/spice.c      |  4 +---
 include/qemu/queue.h | 14 ++++++++++++++
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 308a91c96b..1bdb9c679d 100644
--- a/block.c
+++ b/block.c
@@ -2636,10 +2636,7 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
 
 static void bdrv_detach_child(BdrvChild *child)
 {
-    if (child->next.le_prev) {
-        QLIST_REMOVE(child, next);
-        child->next.le_prev = NULL;
-    }
+    QLIST_SAFE_REMOVE(child, next);
 
     bdrv_replace_child(child, NULL);
 
diff --git a/chardev/spice.c b/chardev/spice.c
index 241e2b7770..bf7ea1e294 100644
--- a/chardev/spice.c
+++ b/chardev/spice.c
@@ -216,9 +216,7 @@ static void char_spice_finalize(Object *obj)
 
     vmc_unregister_interface(s);
 
-    if (s->next.le_prev) {
-        QLIST_REMOVE(s, next);
-    }
+    QLIST_SAFE_REMOVE(s, next);
 
     g_free((char *)s->sin.subtype);
     g_free((char *)s->sin.portname);
diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index fcecb70228..60e794a4e3 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -144,6 +144,20 @@ struct {                                                                \
         *(elm)->field.le_prev = (elm)->field.le_next;                   \
 } while (/*CONSTCOND*/0)
 
+/*
+ * Like QLIST_REMOVE() but safe to call when elm is not in a list
+ */
+#define QLIST_SAFE_REMOVE(elm, field) do {                              \
+        if ((elm)->field.le_prev != NULL) {                             \
+                if ((elm)->field.le_next != NULL)                       \
+                        (elm)->field.le_next->field.le_prev =           \
+                            (elm)->field.le_prev;                       \
+                *(elm)->field.le_prev = (elm)->field.le_next;           \
+                (elm)->field.le_next = NULL;                            \
+                (elm)->field.le_prev = NULL;                            \
+        }                                                               \
+} while (/*CONSTCOND*/0)
+
 #define QLIST_FOREACH(var, head, field)                                 \
         for ((var) = ((head)->lh_first);                                \
                 (var);                                                  \
-- 
2.24.1

[PULL 08/31] aio-posix: make AioHandler deletion O(1)

[Stefan Hajnoczi]

It is not necessary to scan all AioHandlers for deletion.  Keep a list
of deleted handlers instead of scanning the full list of all handlers.

The AioHandler->deleted field can be dropped.  Let's check if the
handler has been inserted into the deleted list instead.  Add a new
QLIST_IS_INSERTED() API for this check.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200214171712.541358-5-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/aio.h  |  6 ++++-
 include/qemu/queue.h |  3 +++
 util/aio-posix.c     | 53 +++++++++++++++++++++++++++++---------------
 3 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 1a2ce9ca26..9dd61cee7e 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -42,6 +42,7 @@ void qemu_aio_unref(void *p);
 void qemu_aio_ref(void *p);
 
 typedef struct AioHandler AioHandler;
+typedef QLIST_HEAD(, AioHandler) AioHandlerList;
 typedef void QEMUBHFunc(void *opaque);
 typedef bool AioPollFn(void *opaque);
 typedef void IOHandler(void *opaque);
@@ -71,7 +72,10 @@ struct AioContext {
     QemuRecMutex lock;
 
     /* The list of registered AIO handlers.  Protected by ctx->list_lock. */
-    QLIST_HEAD(, AioHandler) aio_handlers;
+    AioHandlerList aio_handlers;
+
+    /* The list of AIO handlers to be deleted.  Protected by ctx->list_lock. */
+    AioHandlerList deleted_aio_handlers;
 
     /* Used to avoid unnecessary event_notifier_set calls in aio_notify;
      * accessed with atomic primitives.  If this field is 0, everything
diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index 60e794a4e3..294db54eb1 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -158,6 +158,9 @@ struct {                                                                \
         }                                                               \
 } while (/*CONSTCOND*/0)
 
+/* Is elm in a list? */
+#define QLIST_IS_INSERTED(elm, field) ((elm)->field.le_prev != NULL)
+
 #define QLIST_FOREACH(var, head, field)                                 \
         for ((var) = ((head)->lh_first);                                \
                 (var);                                                  \
diff --git a/util/aio-posix.c b/util/aio-posix.c
index 58765e581e..b5cfdbd2f6 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -32,10 +32,10 @@ struct AioHandler
     AioPollFn *io_poll;
     IOHandler *io_poll_begin;
     IOHandler *io_poll_end;
-    int deleted;
     void *opaque;
     bool is_external;
     QLIST_ENTRY(AioHandler) node;
+    QLIST_ENTRY(AioHandler) node_deleted;
 };
 
 #ifdef CONFIG_EPOLL_CREATE1
@@ -68,7 +68,7 @@ static bool aio_epoll_try_enable(AioContext *ctx)
 
     QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
         int r;
-        if (node->deleted || !node->pfd.events) {
+        if (QLIST_IS_INSERTED(node, node_deleted) || !node->pfd.events) {
             continue;
         }
         event.events = epoll_events_from_pfd(node->pfd.events);
@@ -196,9 +196,11 @@ static AioHandler *find_aio_handler(AioContext *ctx, int fd)
     AioHandler *node;
 
     QLIST_FOREACH(node, &ctx->aio_handlers, node) {
-        if (node->pfd.fd == fd)
-            if (!node->deleted)
+        if (node->pfd.fd == fd) {
+            if (!QLIST_IS_INSERTED(node, node_deleted)) {
                 return node;
+            }
+        }
     }
 
     return NULL;
@@ -217,7 +219,7 @@ static bool aio_remove_fd_handler(AioContext *ctx, AioHandler *node)
 
     /* If a read is in progress, just mark the node as deleted */
     if (qemu_lockcnt_count(&ctx->list_lock)) {
-        node->deleted = 1;
+        QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted);
         node->pfd.revents = 0;
         return false;
     }
@@ -359,7 +361,7 @@ static void poll_set_started(AioContext *ctx, bool started)
     QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
         IOHandler *fn;
 
-        if (node->deleted) {
+        if (QLIST_IS_INSERTED(node, node_deleted)) {
             continue;
         }
 
@@ -416,6 +418,26 @@ bool aio_pending(AioContext *ctx)
     return result;
 }
 
+static void aio_free_deleted_handlers(AioContext *ctx)
+{
+    AioHandler *node;
+
+    if (QLIST_EMPTY_RCU(&ctx->deleted_aio_handlers)) {
+        return;
+    }
+    if (!qemu_lockcnt_dec_if_lock(&ctx->list_lock)) {
+        return; /* we are nested, let the parent do the freeing */
+    }
+
+    while ((node = QLIST_FIRST_RCU(&ctx->deleted_aio_handlers))) {
+        QLIST_REMOVE(node, node);
+        QLIST_REMOVE(node, node_deleted);
+        g_free(node);
+    }
+
+    qemu_lockcnt_inc_and_unlock(&ctx->list_lock);
+}
+
 static bool aio_dispatch_handlers(AioContext *ctx)
 {
     AioHandler *node, *tmp;
@@ -427,7 +449,7 @@ static bool aio_dispatch_handlers(AioContext *ctx)
         revents = node->pfd.revents & node->pfd.events;
         node->pfd.revents = 0;
 
-        if (!node->deleted &&
+        if (!QLIST_IS_INSERTED(node, node_deleted) &&
             (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) &&
             aio_node_check(ctx, node->is_external) &&
             node->io_read) {
@@ -438,21 +460,13 @@ static bool aio_dispatch_handlers(AioContext *ctx)
                 progress = true;
             }
         }
-        if (!node->deleted &&
+        if (!QLIST_IS_INSERTED(node, node_deleted) &&
             (revents & (G_IO_OUT | G_IO_ERR)) &&
             aio_node_check(ctx, node->is_external) &&
             node->io_write) {
             node->io_write(node->opaque);
             progress = true;
         }
-
-        if (node->deleted) {
-            if (qemu_lockcnt_dec_if_lock(&ctx->list_lock)) {
-                QLIST_REMOVE(node, node);
-                g_free(node);
-                qemu_lockcnt_inc_and_unlock(&ctx->list_lock);
-            }
-        }
     }
 
     return progress;
@@ -463,6 +477,7 @@ void aio_dispatch(AioContext *ctx)
     qemu_lockcnt_inc(&ctx->list_lock);
     aio_bh_poll(ctx);
     aio_dispatch_handlers(ctx);
+    aio_free_deleted_handlers(ctx);
     qemu_lockcnt_dec(&ctx->list_lock);
 
     timerlistgroup_run_timers(&ctx->tlg);
@@ -530,7 +545,7 @@ static bool run_poll_handlers_once(AioContext *ctx, int64_t *timeout)
     RCU_READ_LOCK_GUARD();
 
     QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
-        if (!node->deleted && node->io_poll &&
+        if (!QLIST_IS_INSERTED(node, node_deleted) && node->io_poll &&
             aio_node_check(ctx, node->is_external) &&
             node->io_poll(node->opaque)) {
             /*
@@ -664,7 +679,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
 
         if (!aio_epoll_enabled(ctx)) {
             QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
-                if (!node->deleted && node->pfd.events
+                if (!QLIST_IS_INSERTED(node, node_deleted) && node->pfd.events
                     && aio_node_check(ctx, node->is_external)) {
                     add_pollfd(node);
                 }
@@ -741,6 +756,8 @@ bool aio_poll(AioContext *ctx, bool blocking)
         progress |= aio_dispatch_handlers(ctx);
     }
 
+    aio_free_deleted_handlers(ctx);
+
     qemu_lockcnt_dec(&ctx->list_lock);
 
     progress |= timerlistgroup_run_timers(&ctx->tlg);
-- 
2.24.1

[PULL 09/31] aio-posix: make AioHandler dispatch O(1) with epoll

[Stefan Hajnoczi]

File descriptor monitoring is O(1) with epoll(7), but
aio_dispatch_handlers() still scans all AioHandlers instead of
dispatching just those that are ready.  This makes aio_poll() O(n) with
respect to the total number of registered handlers.

Add a local ready_list to aio_poll() so that each nested aio_poll()
builds a list of handlers ready to be dispatched.  Since file descriptor
polling is level-triggered, nested aio_poll() calls also see fds that
were ready in the parent but not yet dispatched.  This guarantees that
nested aio_poll() invocations will dispatch all fds, even those that
became ready before the nested invocation.

Since only handlers ready to be dispatched are placed onto the
ready_list, the new aio_dispatch_ready_handlers() function provides O(1)
dispatch.

Note that AioContext polling is still O(n) and currently cannot be fully
disabled.  This still needs to be fixed before aio_poll() is fully O(1).

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200214171712.541358-6-stefanha@redhat.com
[Fix compilation error on macOS where there is no epoll(87).  The
aio_epoll() prototype was out of date and aio_add_ready_list() needed to
be moved outside the ifdef.
--Stefan]
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/aio-posix.c | 110 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 32 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index b5cfdbd2f6..9e1befc0c0 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -35,9 +35,20 @@ struct AioHandler
     void *opaque;
     bool is_external;
     QLIST_ENTRY(AioHandler) node;
+    QLIST_ENTRY(AioHandler) node_ready; /* only used during aio_poll() */
     QLIST_ENTRY(AioHandler) node_deleted;
 };
 
+/* Add a handler to a ready list */
+static void add_ready_handler(AioHandlerList *ready_list,
+                              AioHandler *node,
+                              int revents)
+{
+    QLIST_SAFE_REMOVE(node, node_ready); /* remove from nested parent's list */
+    node->pfd.revents = revents;
+    QLIST_INSERT_HEAD(ready_list, node, node_ready);
+}
+
 #ifdef CONFIG_EPOLL_CREATE1
 
 /* The fd number threshold to switch to epoll */
@@ -105,7 +116,8 @@ static void aio_epoll_update(AioContext *ctx, AioHandler *node, bool is_new)
     }
 }
 
-static int aio_epoll(AioContext *ctx, int64_t timeout)
+static int aio_epoll(AioContext *ctx, AioHandlerList *ready_list,
+                     int64_t timeout)
 {
     GPollFD pfd = {
         .fd = ctx->epollfd,
@@ -130,11 +142,13 @@ static int aio_epoll(AioContext *ctx, int64_t timeout)
         }
         for (i = 0; i < ret; i++) {
             int ev = events[i].events;
+            int revents = (ev & EPOLLIN ? G_IO_IN : 0) |
+                          (ev & EPOLLOUT ? G_IO_OUT : 0) |
+                          (ev & EPOLLHUP ? G_IO_HUP : 0) |
+                          (ev & EPOLLERR ? G_IO_ERR : 0);
+
             node = events[i].data.ptr;
-            node->pfd.revents = (ev & EPOLLIN ? G_IO_IN : 0) |
-                (ev & EPOLLOUT ? G_IO_OUT : 0) |
-                (ev & EPOLLHUP ? G_IO_HUP : 0) |
-                (ev & EPOLLERR ? G_IO_ERR : 0);
+            add_ready_handler(ready_list, node, revents);
         }
     }
 out:
@@ -172,8 +186,8 @@ static void aio_epoll_update(AioContext *ctx, AioHandler *node, bool is_new)
 {
 }
 
-static int aio_epoll(AioContext *ctx, GPollFD *pfds,
-                     unsigned npfd, int64_t timeout)
+static int aio_epoll(AioContext *ctx, AioHandlerList *ready_list,
+                     int64_t timeout)
 {
     assert(false);
 }
@@ -438,36 +452,63 @@ static void aio_free_deleted_handlers(AioContext *ctx)
     qemu_lockcnt_inc_and_unlock(&ctx->list_lock);
 }
 
-static bool aio_dispatch_handlers(AioContext *ctx)
+static bool aio_dispatch_handler(AioContext *ctx, AioHandler *node)
 {
-    AioHandler *node, *tmp;
     bool progress = false;
+    int revents;
 
-    QLIST_FOREACH_SAFE_RCU(node, &ctx->aio_handlers, node, tmp) {
-        int revents;
+    revents = node->pfd.revents & node->pfd.events;
+    node->pfd.revents = 0;
 
-        revents = node->pfd.revents & node->pfd.events;
-        node->pfd.revents = 0;
+    if (!QLIST_IS_INSERTED(node, node_deleted) &&
+        (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) &&
+        aio_node_check(ctx, node->is_external) &&
+        node->io_read) {
+        node->io_read(node->opaque);
 
-        if (!QLIST_IS_INSERTED(node, node_deleted) &&
-            (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) &&
-            aio_node_check(ctx, node->is_external) &&
-            node->io_read) {
-            node->io_read(node->opaque);
-
-            /* aio_notify() does not count as progress */
-            if (node->opaque != &ctx->notifier) {
-                progress = true;
-            }
-        }
-        if (!QLIST_IS_INSERTED(node, node_deleted) &&
-            (revents & (G_IO_OUT | G_IO_ERR)) &&
-            aio_node_check(ctx, node->is_external) &&
-            node->io_write) {
-            node->io_write(node->opaque);
+        /* aio_notify() does not count as progress */
+        if (node->opaque != &ctx->notifier) {
             progress = true;
         }
     }
+    if (!QLIST_IS_INSERTED(node, node_deleted) &&
+        (revents & (G_IO_OUT | G_IO_ERR)) &&
+        aio_node_check(ctx, node->is_external) &&
+        node->io_write) {
+        node->io_write(node->opaque);
+        progress = true;
+    }
+
+    return progress;
+}
+
+/*
+ * If we have a list of ready handlers then this is more efficient than
+ * scanning all handlers with aio_dispatch_handlers().
+ */
+static bool aio_dispatch_ready_handlers(AioContext *ctx,
+                                        AioHandlerList *ready_list)
+{
+    bool progress = false;
+    AioHandler *node;
+
+    while ((node = QLIST_FIRST(ready_list))) {
+        QLIST_SAFE_REMOVE(node, node_ready);
+        progress = aio_dispatch_handler(ctx, node) || progress;
+    }
+
+    return progress;
+}
+
+/* Slower than aio_dispatch_ready_handlers() but only used via glib */
+static bool aio_dispatch_handlers(AioContext *ctx)
+{
+    AioHandler *node, *tmp;
+    bool progress = false;
+
+    QLIST_FOREACH_SAFE_RCU(node, &ctx->aio_handlers, node, tmp) {
+        progress = aio_dispatch_handler(ctx, node) || progress;
+    }
 
     return progress;
 }
@@ -639,6 +680,7 @@ static bool try_poll_mode(AioContext *ctx, int64_t *timeout)
 
 bool aio_poll(AioContext *ctx, bool blocking)
 {
+    AioHandlerList ready_list = QLIST_HEAD_INITIALIZER(ready_list);
     AioHandler *node;
     int i;
     int ret = 0;
@@ -689,7 +731,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
         /* wait until next event */
         if (aio_epoll_check_poll(ctx, pollfds, npfd, timeout)) {
             npfd = 0; /* pollfds[] is not being used */
-            ret = aio_epoll(ctx, timeout);
+            ret = aio_epoll(ctx, &ready_list, timeout);
         } else  {
             ret = qemu_poll_ns(pollfds, npfd, timeout);
         }
@@ -744,7 +786,11 @@ bool aio_poll(AioContext *ctx, bool blocking)
     /* if we have any readable fds, dispatch event */
     if (ret > 0) {
         for (i = 0; i < npfd; i++) {
-            nodes[i]->pfd.revents = pollfds[i].revents;
+            int revents = pollfds[i].revents;
+
+            if (revents) {
+                add_ready_handler(&ready_list, nodes[i], revents);
+            }
         }
     }
 
@@ -753,7 +799,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
     progress |= aio_bh_poll(ctx);
 
     if (ret > 0) {
-        progress |= aio_dispatch_handlers(ctx);
+        progress |= aio_dispatch_ready_handlers(ctx, &ready_list);
     }
 
     aio_free_deleted_handlers(ctx);
-- 
2.24.1

[PULL 10/31] softmmu: move vl.c to softmmu/

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Move vl.c to a separate directory, similar to linux-user/
Update the chechpatch and get_maintainer scripts, since they relied on
/vl.c for top_of_tree checks.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-2-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS               | 2 +-
 Makefile.objs             | 2 --
 Makefile.target           | 1 +
 scripts/checkpatch.pl     | 2 +-
 scripts/get_maintainer.pl | 3 ++-
 softmmu/Makefile.objs     | 2 ++
 vl.c => softmmu/vl.c      | 0
 7 files changed, 7 insertions(+), 5 deletions(-)
 create mode 100644 softmmu/Makefile.objs
 rename vl.c => softmmu/vl.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1740a4fddc..6e26d2ec2a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2031,7 +2031,7 @@ F: include/qemu/main-loop.h
 F: include/sysemu/runstate.h
 F: util/main-loop.c
 F: util/qemu-timer.c
-F: vl.c
+F: softmmu/vl.c
 F: qapi/run-state.json
 
 Human Monitor (HMP)
diff --git a/Makefile.objs b/Makefile.objs
index 26b9cff954..8a1cbe8000 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -58,8 +58,6 @@ common-obj-y += ui/
 common-obj-m += ui/
 
 common-obj-y += dma-helpers.o
-common-obj-y += vl.o
-vl.o-cflags := $(GPROF_CFLAGS) $(SDL_CFLAGS)
 common-obj-$(CONFIG_TPM) += tpm.o
 
 common-obj-y += backends/
diff --git a/Makefile.target b/Makefile.target
index 6e61f607b1..06c36d1161 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -160,6 +160,7 @@ obj-y += qapi/
 obj-y += memory.o
 obj-y += memory_mapping.o
 obj-y += migration/ram.o
+obj-y += softmmu/
 LIBS := $(libs_softmmu) $(LIBS)
 
 # Hardware support
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 11512a8a09..b27e4ff5e9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -462,7 +462,7 @@ sub top_of_kernel_tree {
 	my @tree_check = (
 		"COPYING", "MAINTAINERS", "Makefile",
 		"README.rst", "docs", "VERSION",
-		"vl.c"
+		"linux-user", "softmmu"
 	);
 
 	foreach my $check (@tree_check) {
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 27991eb1cf..271f5ff42a 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -795,7 +795,8 @@ sub top_of_tree {
         && (-f "${lk_path}Makefile")
         && (-d "${lk_path}docs")
         && (-f "${lk_path}VERSION")
-        && (-f "${lk_path}vl.c")) {
+        && (-d "${lk_path}linux-user/")
+        && (-d "${lk_path}softmmu/")) {
 	return 1;
     }
     return 0;
diff --git a/softmmu/Makefile.objs b/softmmu/Makefile.objs
new file mode 100644
index 0000000000..d80a5ffe5a
--- /dev/null
+++ b/softmmu/Makefile.objs
@@ -0,0 +1,2 @@
+obj-y += vl.o
+vl.o-cflags := $(GPROF_CFLAGS) $(SDL_CFLAGS)
diff --git a/vl.c b/softmmu/vl.c
similarity index 100%
rename from vl.c
rename to softmmu/vl.c
-- 
2.24.1

[PULL 11/31] softmmu: split off vl.c:main() into main.c

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

A program might rely on functions implemented in vl.c, but implement its
own main(). By placing main into a separate source file, there are no
complaints about duplicate main()s when linking against vl.o. For
example, the virtual-device fuzzer uses a main() provided by libfuzzer,
and needs to perform some initialization before running the softmmu
initialization. Now, main simply calls three vl.c functions which
handle the guest initialization, main loop and cleanup.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-3-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS             |  1 +
 Makefile.target         |  2 +-
 include/sysemu/sysemu.h |  4 ++++
 softmmu/Makefile.objs   |  1 +
 softmmu/main.c          | 53 +++++++++++++++++++++++++++++++++++++++++
 softmmu/vl.c            | 36 +++++++---------------------
 6 files changed, 69 insertions(+), 28 deletions(-)
 create mode 100644 softmmu/main.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6e26d2ec2a..969329f83e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2032,6 +2032,7 @@ F: include/sysemu/runstate.h
 F: util/main-loop.c
 F: util/qemu-timer.c
 F: softmmu/vl.c
+F: softmmu/main.c
 F: qapi/run-state.json
 
 Human Monitor (HMP)
diff --git a/Makefile.target b/Makefile.target
index 06c36d1161..6f4dd72022 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -203,7 +203,7 @@ endif
 COMMON_LDADDS = ../libqemuutil.a
 
 # build either PROG or PROGW
-$(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS)
+$(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS) $(softmmu-main-y)
 	$(call LINK, $(filter-out %.mak, $^))
 ifdef CONFIG_DARWIN
 	$(call quiet-command,Rez -append $(SRC_PATH)/pc-bios/qemu.rsrc -o $@,"REZ","$(TARGET_DIR)$@")
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index c0678c1ca3..dec64fcc17 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -115,6 +115,10 @@ QemuOpts *qemu_get_machine_opts(void);
 
 bool defaults_enabled(void);
 
+void qemu_init(int argc, char **argv, char **envp);
+void qemu_main_loop(void);
+void qemu_cleanup(void);
+
 extern QemuOptsList qemu_legacy_drive_opts;
 extern QemuOptsList qemu_common_drive_opts;
 extern QemuOptsList qemu_drive_opts;
diff --git a/softmmu/Makefile.objs b/softmmu/Makefile.objs
index d80a5ffe5a..dd15c24346 100644
--- a/softmmu/Makefile.objs
+++ b/softmmu/Makefile.objs
@@ -1,2 +1,3 @@
+softmmu-main-y = softmmu/main.o
 obj-y += vl.o
 vl.o-cflags := $(GPROF_CFLAGS) $(SDL_CFLAGS)
diff --git a/softmmu/main.c b/softmmu/main.c
new file mode 100644
index 0000000000..7adc530c73
--- /dev/null
+++ b/softmmu/main.c
@@ -0,0 +1,53 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2020 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+
+#ifdef CONFIG_SDL
+#if defined(__APPLE__) || defined(main)
+#include <SDL.h>
+int main(int argc, char **argv)
+{
+    return qemu_main(argc, argv, NULL);
+}
+#undef main
+#define main qemu_main
+#endif
+#endif /* CONFIG_SDL */
+
+#ifdef CONFIG_COCOA
+#undef main
+#define main qemu_main
+#endif /* CONFIG_COCOA */
+
+int main(int argc, char **argv, char **envp)
+{
+    qemu_init(argc, argv, envp);
+    qemu_main_loop();
+    qemu_cleanup();
+
+    return 0;
+}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 794f2e5733..080d3b5106 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -36,25 +36,6 @@
 #include "sysemu/seccomp.h"
 #include "sysemu/tcg.h"
 
-#ifdef CONFIG_SDL
-#if defined(__APPLE__) || defined(main)
-#include <SDL.h>
-int qemu_main(int argc, char **argv, char **envp);
-int main(int argc, char **argv)
-{
-    return qemu_main(argc, argv, NULL);
-}
-#undef main
-#define main qemu_main
-#endif
-#endif /* CONFIG_SDL */
-
-#ifdef CONFIG_COCOA
-#undef main
-#define main qemu_main
-#endif /* CONFIG_COCOA */
-
-
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
 #include "sysemu/accel.h"
@@ -1670,7 +1651,7 @@ static bool main_loop_should_exit(void)
     return false;
 }
 
-static void main_loop(void)
+void qemu_main_loop(void)
 {
 #ifdef CONFIG_PROFILER
     int64_t ti;
@@ -2805,7 +2786,7 @@ static void configure_accelerators(const char *progname)
     }
 }
 
-int main(int argc, char **argv, char **envp)
+void qemu_init(int argc, char **argv, char **envp)
 {
     int i;
     int snapshot, linux_boot;
@@ -3357,7 +3338,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_watchdog:
                 if (watchdog) {
                     error_report("only one watchdog option may be given");
-                    return 1;
+                    exit(1);
                 }
                 watchdog = optarg;
                 break;
@@ -4269,7 +4250,7 @@ int main(int argc, char **argv, char **envp)
     parse_numa_opts(current_machine);
 
     /* do monitor/qmp handling at preconfig state if requested */
-    main_loop();
+    qemu_main_loop();
 
     audio_init_audiodevs();
 
@@ -4387,7 +4368,7 @@ int main(int argc, char **argv, char **envp)
     if (vmstate_dump_file) {
         /* dump and exit */
         dump_vmstate_json_to_file(vmstate_dump_file);
-        return 0;
+        exit(0);
     }
 
     if (incoming) {
@@ -4404,8 +4385,11 @@ int main(int argc, char **argv, char **envp)
     accel_setup_post(current_machine);
     os_setup_post();
 
-    main_loop();
+    return;
+}
 
+void qemu_cleanup(void)
+{
     gdbserver_cleanup();
 
     /*
@@ -4442,6 +4426,4 @@ int main(int argc, char **argv, char **envp)
     qemu_chr_cleanup();
     user_creatable_cleanup();
     /* TODO: unref root container, check all devices are ok */
-
-    return 0;
 }
-- 
2.24.1

[PULL 12/31] module: check module wasn't already initialized

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

The virtual-device fuzzer must initialize QOM, prior to running
vl:qemu_init, so that it can use the qos_graph to identify the arguments
required to initialize a guest for libqos-assisted fuzzing. This change
prevents errors when vl:qemu_init tries to (re)initialize the previously
initialized QOM module.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200220041118.23264-4-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/module.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/util/module.c b/util/module.c
index 8c5315a7a3..236a7bb52a 100644
--- a/util/module.c
+++ b/util/module.c
@@ -30,6 +30,7 @@ typedef struct ModuleEntry
 typedef QTAILQ_HEAD(, ModuleEntry) ModuleTypeList;
 
 static ModuleTypeList init_type_list[MODULE_INIT_MAX];
+static bool modules_init_done[MODULE_INIT_MAX];
 
 static ModuleTypeList dso_init_list;
 
@@ -91,11 +92,17 @@ void module_call_init(module_init_type type)
     ModuleTypeList *l;
     ModuleEntry *e;
 
+    if (modules_init_done[type]) {
+        return;
+    }
+
     l = find_type(type);
 
     QTAILQ_FOREACH(e, l, node) {
         e->init();
     }
+
+    modules_init_done[type] = true;
 }
 
 #ifdef CONFIG_MODULES
-- 
2.24.1

[PULL 13/31] fuzz: add FUZZ_TARGET module type

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-5-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/module.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/qemu/module.h b/include/qemu/module.h
index 65ba596e46..684753d808 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -46,6 +46,7 @@ typedef enum {
     MODULE_INIT_TRACE,
     MODULE_INIT_XEN_BACKEND,
     MODULE_INIT_LIBQOS,
+    MODULE_INIT_FUZZ_TARGET,
     MODULE_INIT_MAX
 } module_init_type;
 
@@ -56,7 +57,8 @@ typedef enum {
 #define xen_backend_init(function) module_init(function, \
                                                MODULE_INIT_XEN_BACKEND)
 #define libqos_init(function) module_init(function, MODULE_INIT_LIBQOS)
-
+#define fuzz_target_init(function) module_init(function, \
+                                               MODULE_INIT_FUZZ_TARGET)
 #define block_module_load_one(lib) module_load_one("block-", lib)
 #define ui_module_load_one(lib) module_load_one("ui-", lib)
 #define audio_module_load_one(lib) module_load_one("audio-", lib)
-- 
2.24.1

[PULL 14/31] qtest: add qtest_server_send abstraction

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

qtest_server_send is a function pointer specifying the handler used to
transmit data to the qtest client. In the standard configuration, this
calls the CharBackend handler, but now it is possible for other types of
handlers, e.g direct-function calls if the qtest client and server
exist within the same process (inproc)

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200220041118.23264-6-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/qtest.h |  3 +++
 qtest.c                | 18 ++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 5ed09c80b1..e2f1047fd7 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -26,4 +26,7 @@ bool qtest_driver(void);
 
 void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp);
 
+void qtest_server_set_send_handler(void (*send)(void *, const char *),
+                                 void *opaque);
+
 #endif
diff --git a/qtest.c b/qtest.c
index 587dcbb4b5..8a4ba59828 100644
--- a/qtest.c
+++ b/qtest.c
@@ -43,6 +43,8 @@ static GString *inbuf;
 static int irq_levels[MAX_IRQ];
 static qemu_timeval start_time;
 static bool qtest_opened;
+static void (*qtest_server_send)(void*, const char*);
+static void *qtest_server_send_opaque;
 
 #define FMT_timeval "%ld.%06ld"
 
@@ -229,8 +231,10 @@ static void GCC_FMT_ATTR(1, 2) qtest_log_send(const char *fmt, ...)
     va_end(ap);
 }
 
-static void do_qtest_send(CharBackend *chr, const char *str, size_t len)
+static void qtest_server_char_be_send(void *opaque, const char *str)
 {
+    size_t len = strlen(str);
+    CharBackend* chr = (CharBackend *)opaque;
     qemu_chr_fe_write_all(chr, (uint8_t *)str, len);
     if (qtest_log_fp && qtest_opened) {
         fprintf(qtest_log_fp, "%s", str);
@@ -239,7 +243,7 @@ static void do_qtest_send(CharBackend *chr, const char *str, size_t len)
 
 static void qtest_send(CharBackend *chr, const char *str)
 {
-    do_qtest_send(chr, str, strlen(str));
+    qtest_server_send(qtest_server_send_opaque, str);
 }
 
 static void GCC_FMT_ATTR(2, 3) qtest_sendf(CharBackend *chr,
@@ -784,6 +788,16 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
     qemu_chr_fe_set_echo(&qtest_chr, true);
 
     inbuf = g_string_new("");
+
+    if (!qtest_server_send) {
+        qtest_server_set_send_handler(qtest_server_char_be_send, &qtest_chr);
+    }
+}
+
+void qtest_server_set_send_handler(void (*send)(void*, const char*), void *opaque)
+{
+    qtest_server_send = send;
+    qtest_server_send_opaque = opaque;
 }
 
 bool qtest_driver(void)
-- 
2.24.1

[PULL 15/31] libqtest: add a layer of abstraction to send/recv

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

This makes it simple to swap the transport functions for qtest commands
to and from the qtest client. For example, now it is possible to
directly pass qtest commands to a server handler that exists within the
same process, without the standard way of writing to a file descriptor.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-7-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/libqtest.c | 48 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 76c9f8eade..e5056a1d0f 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -35,6 +35,15 @@
 #define SOCKET_TIMEOUT 50
 #define SOCKET_MAX_FDS 16
 
+
+typedef void (*QTestSendFn)(QTestState *s, const char *buf);
+typedef GString* (*QTestRecvFn)(QTestState *);
+
+typedef struct QTestClientTransportOps {
+    QTestSendFn     send;      /* for sending qtest commands */
+    QTestRecvFn     recv_line; /* for receiving qtest command responses */
+} QTestTransportOps;
+
 struct QTestState
 {
     int fd;
@@ -45,6 +54,7 @@ struct QTestState
     bool big_endian;
     bool irq_level[MAX_IRQ];
     GString *rx;
+    QTestTransportOps ops;
 };
 
 static GHookList abrt_hooks;
@@ -52,6 +62,14 @@ static struct sigaction sigact_old;
 
 static int qtest_query_target_endianness(QTestState *s);
 
+static void qtest_client_socket_send(QTestState*, const char *buf);
+static void socket_send(int fd, const char *buf, size_t size);
+
+static GString *qtest_client_socket_recv_line(QTestState *);
+
+static void qtest_client_set_tx_handler(QTestState *s, QTestSendFn send);
+static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv);
+
 static int init_socket(const char *socket_path)
 {
     struct sockaddr_un addr;
@@ -234,6 +252,9 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
+    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
+    qtest_client_set_tx_handler(s, qtest_client_socket_send);
+
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
     command = g_strdup_printf("exec %s "
@@ -379,13 +400,9 @@ static void socket_send(int fd, const char *buf, size_t size)
     }
 }
 
-static void socket_sendf(int fd, const char *fmt, va_list ap)
+static void qtest_client_socket_send(QTestState *s, const char *buf)
 {
-    gchar *str = g_strdup_vprintf(fmt, ap);
-    size_t size = strlen(str);
-
-    socket_send(fd, str, size);
-    g_free(str);
+    socket_send(s->fd, buf, strlen(buf));
 }
 
 static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
@@ -393,8 +410,11 @@ static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
     va_list ap;
 
     va_start(ap, fmt);
-    socket_sendf(s->fd, fmt, ap);
+    gchar *str = g_strdup_vprintf(fmt, ap);
     va_end(ap);
+
+    s->ops.send(s, str);
+    g_free(str);
 }
 
 /* Sends a message and file descriptors to the socket.
@@ -431,7 +451,7 @@ static void socket_send_fds(int socket_fd, int *fds, size_t fds_num,
     g_assert_cmpint(ret, >, 0);
 }
 
-static GString *qtest_recv_line(QTestState *s)
+static GString *qtest_client_socket_recv_line(QTestState *s)
 {
     GString *line;
     size_t offset;
@@ -468,7 +488,7 @@ static gchar **qtest_rsp(QTestState *s, int expected_args)
     int i;
 
 redo:
-    line = qtest_recv_line(s);
+    line = s->ops.recv_line(s);
     words = g_strsplit(line->str, " ", 0);
     g_string_free(line, TRUE);
 
@@ -1337,3 +1357,13 @@ void qmp_assert_error_class(QDict *rsp, const char *class)
 
     qobject_unref(rsp);
 }
+
+static void qtest_client_set_tx_handler(QTestState *s,
+                    QTestSendFn send)
+{
+    s->ops.send = send;
+}
+static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv)
+{
+    s->ops.recv_line = recv;
+}
-- 
2.24.1

[PULL 16/31] libqtest: make bufwrite rely on the TransportOps

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

When using qtest "in-process" communication, qtest_sendf directly calls
a function in the server (qtest.c). Previously, bufwrite used
socket_send, which bypasses the TransportOps enabling the call into
qtest.c. This change replaces the socket_send calls with ops->send,
maintaining the benefits of the direct socket_send call, while adding
support for in-process qtest calls.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-8-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/libqtest.c | 71 ++++++++++++++++++++++++++++++++++++++++--
 tests/qtest/libqtest.h |  4 +++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index e5056a1d0f..49075b55a1 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -37,10 +37,18 @@
 
 
 typedef void (*QTestSendFn)(QTestState *s, const char *buf);
+typedef void (*ExternalSendFn)(void *s, const char *buf);
 typedef GString* (*QTestRecvFn)(QTestState *);
 
 typedef struct QTestClientTransportOps {
     QTestSendFn     send;      /* for sending qtest commands */
+
+    /*
+     * use external_send to send qtest command strings through functions which
+     * do not accept a QTestState as the first parameter.
+     */
+    ExternalSendFn  external_send;
+
     QTestRecvFn     recv_line; /* for receiving qtest command responses */
 } QTestTransportOps;
 
@@ -1078,8 +1086,8 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
 
     bdata = g_base64_encode(data, size);
     qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
-    socket_send(s->fd, bdata, strlen(bdata));
-    socket_send(s->fd, "\n", 1);
+    s->ops.send(s, bdata);
+    s->ops.send(s, "\n");
     qtest_rsp(s, 0);
     g_free(bdata);
 }
@@ -1367,3 +1375,62 @@ static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv)
 {
     s->ops.recv_line = recv;
 }
+/* A type-safe wrapper for s->send() */
+static void send_wrapper(QTestState *s, const char *buf)
+{
+    s->ops.external_send(s, buf);
+}
+
+static GString *qtest_client_inproc_recv_line(QTestState *s)
+{
+    GString *line;
+    size_t offset;
+    char *eol;
+
+    eol = strchr(s->rx->str, '\n');
+    offset = eol - s->rx->str;
+    line = g_string_new_len(s->rx->str, offset);
+    g_string_erase(s->rx, 0, offset + 1);
+    return line;
+}
+
+QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
+                    void (*send)(void*, const char*))
+{
+    QTestState *qts;
+    qts = g_new0(QTestState, 1);
+    *s = qts; /* Expose qts early on, since the query endianness relies on it */
+    qts->wstatus = 0;
+    for (int i = 0; i < MAX_IRQ; i++) {
+        qts->irq_level[i] = false;
+    }
+
+    qtest_client_set_rx_handler(qts, qtest_client_inproc_recv_line);
+
+    /* send() may not have a matching protoype, so use a type-safe wrapper */
+    qts->ops.external_send = send;
+    qtest_client_set_tx_handler(qts, send_wrapper);
+
+    qts->big_endian = qtest_query_target_endianness(qts);
+
+    /*
+     * Set a dummy path for QTEST_QEMU_BINARY. Doesn't need to exist, but this
+     * way, qtest_get_arch works for inproc qtest.
+     */
+    gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
+    setenv("QTEST_QEMU_BINARY", bin_path, 0);
+    g_free(bin_path);
+
+    return qts;
+}
+
+void qtest_client_inproc_recv(void *opaque, const char *str)
+{
+    QTestState *qts = *(QTestState **)opaque;
+
+    if (!qts->rx) {
+        qts->rx = g_string_new(NULL);
+    }
+    g_string_append(qts->rx, str);
+    return;
+}
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index c9e21e05b3..f5cf93c386 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -729,4 +729,8 @@ bool qtest_probe_child(QTestState *s);
  */
 void qtest_set_expected_status(QTestState *s, int status);
 
+QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
+                    void (*send)(void*, const char*));
+
+void qtest_client_inproc_recv(void *opaque, const char *str);
 #endif
-- 
2.24.1

[PULL 17/31] qtest: add in-process incoming command handler

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

The handler allows a qtest client to send commands to the server by
directly calling a function, rather than using a file/CharBackend

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-9-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/qtest.h |  1 +
 qtest.c                | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index e2f1047fd7..eedd3664f0 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -28,5 +28,6 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
 
 void qtest_server_set_send_handler(void (*send)(void *, const char *),
                                  void *opaque);
+void qtest_server_inproc_recv(void *opaque, const char *buf);
 
 #endif
diff --git a/qtest.c b/qtest.c
index 8a4ba59828..43bb90f53e 100644
--- a/qtest.c
+++ b/qtest.c
@@ -804,3 +804,16 @@ bool qtest_driver(void)
 {
     return qtest_chr.chr != NULL;
 }
+
+void qtest_server_inproc_recv(void *dummy, const char *buf)
+{
+    static GString *gstr;
+    if (!gstr) {
+        gstr = g_string_new(NULL);
+    }
+    g_string_append(gstr, buf);
+    if (gstr->str[gstr->len - 1] == '\n') {
+        qtest_process_inbuf(NULL, gstr);
+        g_string_truncate(gstr, 0);
+    }
+}
-- 
2.24.1

[PULL 18/31] libqos: rename i2c_send and i2c_recv

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

The names i2c_send and i2c_recv collide with functions defined in
hw/i2c/core.c. This causes an error when linking against libqos and
softmmu simultaneously (for example when using qtest inproc). Rename the
libqos functions to avoid this.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200220041118.23264-10-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/libqos/i2c.c   | 10 +++++-----
 tests/qtest/libqos/i2c.h   |  4 ++--
 tests/qtest/pca9552-test.c | 10 +++++-----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/qtest/libqos/i2c.c b/tests/qtest/libqos/i2c.c
index 156114e745..38f800dbab 100644
--- a/tests/qtest/libqos/i2c.c
+++ b/tests/qtest/libqos/i2c.c
@@ -10,12 +10,12 @@
 #include "libqos/i2c.h"
 #include "libqtest.h"
 
-void i2c_send(QI2CDevice *i2cdev, const uint8_t *buf, uint16_t len)
+void qi2c_send(QI2CDevice *i2cdev, const uint8_t *buf, uint16_t len)
 {
     i2cdev->bus->send(i2cdev->bus, i2cdev->addr, buf, len);
 }
 
-void i2c_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
+void qi2c_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
 {
     i2cdev->bus->recv(i2cdev->bus, i2cdev->addr, buf, len);
 }
@@ -23,8 +23,8 @@ void i2c_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
 void i2c_read_block(QI2CDevice *i2cdev, uint8_t reg,
                     uint8_t *buf, uint16_t len)
 {
-    i2c_send(i2cdev, &reg, 1);
-    i2c_recv(i2cdev, buf, len);
+    qi2c_send(i2cdev, &reg, 1);
+    qi2c_recv(i2cdev, buf, len);
 }
 
 void i2c_write_block(QI2CDevice *i2cdev, uint8_t reg,
@@ -33,7 +33,7 @@ void i2c_write_block(QI2CDevice *i2cdev, uint8_t reg,
     uint8_t *cmd = g_malloc(len + 1);
     cmd[0] = reg;
     memcpy(&cmd[1], buf, len);
-    i2c_send(i2cdev, cmd, len + 1);
+    qi2c_send(i2cdev, cmd, len + 1);
     g_free(cmd);
 }
 
diff --git a/tests/qtest/libqos/i2c.h b/tests/qtest/libqos/i2c.h
index 945b65b34c..c65f087834 100644
--- a/tests/qtest/libqos/i2c.h
+++ b/tests/qtest/libqos/i2c.h
@@ -47,8 +47,8 @@ struct QI2CDevice {
 void *i2c_device_create(void *i2c_bus, QGuestAllocator *alloc, void *addr);
 void add_qi2c_address(QOSGraphEdgeOptions *opts, QI2CAddress *addr);
 
-void i2c_send(QI2CDevice *dev, const uint8_t *buf, uint16_t len);
-void i2c_recv(QI2CDevice *dev, uint8_t *buf, uint16_t len);
+void qi2c_send(QI2CDevice *dev, const uint8_t *buf, uint16_t len);
+void qi2c_recv(QI2CDevice *dev, uint8_t *buf, uint16_t len);
 
 void i2c_read_block(QI2CDevice *dev, uint8_t reg,
                     uint8_t *buf, uint16_t len);
diff --git a/tests/qtest/pca9552-test.c b/tests/qtest/pca9552-test.c
index 4b800d3c3e..d80ed93cd3 100644
--- a/tests/qtest/pca9552-test.c
+++ b/tests/qtest/pca9552-test.c
@@ -32,22 +32,22 @@ static void receive_autoinc(void *obj, void *data, QGuestAllocator *alloc)
 
     pca9552_init(i2cdev);
 
-    i2c_send(i2cdev, &reg, 1);
+    qi2c_send(i2cdev, &reg, 1);
 
     /* PCA9552_LS0 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x54);
 
     /* PCA9552_LS1 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x55);
 
     /* PCA9552_LS2 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x55);
 
     /* PCA9552_LS3 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x54);
 }
 
-- 
2.24.1

[PULL 19/31] libqos: split qos-test and libqos makefile vars

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Most qos-related objects were specified in the qos-test-obj-y variable.
qos-test-obj-y also included qos-test.o which defines a main().
This made it difficult to repurpose qos-test-obj-y to link anything
beside tests/qos-test against libqos. This change separates objects that
are libqos-specific and ones that are qos-test specific into different
variables.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200220041118.23264-11-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/Makefile.include | 71 ++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index eb0f23b108..838618e6f9 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -157,52 +157,53 @@ check-qtest-s390x-y += migration-test
 # libqos / qgraph :
 libqgraph-obj-y = tests/qtest/libqos/qgraph.o
 
-libqos-obj-y = $(libqgraph-obj-y) tests/qtest/libqos/pci.o tests/qtest/libqos/fw_cfg.o
-libqos-obj-y += tests/qtest/libqos/malloc.o
-libqos-obj-y += tests/qtest/libqos/libqos.o
-libqos-spapr-obj-y = $(libqos-obj-y) tests/qtest/libqos/malloc-spapr.o
+libqos-core-obj-y = $(libqgraph-obj-y) tests/qtest/libqos/pci.o tests/qtest/libqos/fw_cfg.o
+libqos-core-obj-y += tests/qtest/libqos/malloc.o
+libqos-core-obj-y += tests/qtest/libqos/libqos.o
+libqos-spapr-obj-y = $(libqos-core-obj-y) tests/qtest/libqos/malloc-spapr.o
 libqos-spapr-obj-y += tests/qtest/libqos/libqos-spapr.o
 libqos-spapr-obj-y += tests/qtest/libqos/rtas.o
 libqos-spapr-obj-y += tests/qtest/libqos/pci-spapr.o
-libqos-pc-obj-y = $(libqos-obj-y) tests/qtest/libqos/pci-pc.o
+libqos-pc-obj-y = $(libqos-core-obj-y) tests/qtest/libqos/pci-pc.o
 libqos-pc-obj-y += tests/qtest/libqos/malloc-pc.o tests/qtest/libqos/libqos-pc.o
 libqos-pc-obj-y += tests/qtest/libqos/ahci.o
 libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/qtest/libqos/usb.o
 
 # qos devices:
-qos-test-obj-y = tests/qtest/qos-test.o $(libqgraph-obj-y)
-qos-test-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
-qos-test-obj-y += tests/qtest/libqos/e1000e.o
-qos-test-obj-y += tests/qtest/libqos/i2c.o
-qos-test-obj-y += tests/qtest/libqos/i2c-imx.o
-qos-test-obj-y += tests/qtest/libqos/i2c-omap.o
-qos-test-obj-y += tests/qtest/libqos/sdhci.o
-qos-test-obj-y += tests/qtest/libqos/tpci200.o
-qos-test-obj-y += tests/qtest/libqos/virtio.o
-qos-test-obj-$(CONFIG_VIRTFS) += tests/qtest/libqos/virtio-9p.o
-qos-test-obj-y += tests/qtest/libqos/virtio-balloon.o
-qos-test-obj-y += tests/qtest/libqos/virtio-blk.o
-qos-test-obj-y += tests/qtest/libqos/virtio-mmio.o
-qos-test-obj-y += tests/qtest/libqos/virtio-net.o
-qos-test-obj-y += tests/qtest/libqos/virtio-pci.o
-qos-test-obj-y += tests/qtest/libqos/virtio-pci-modern.o
-qos-test-obj-y += tests/qtest/libqos/virtio-rng.o
-qos-test-obj-y += tests/qtest/libqos/virtio-scsi.o
-qos-test-obj-y += tests/qtest/libqos/virtio-serial.o
+libqos-obj-y =  $(libqgraph-obj-y)
+libqos-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
+libqos-obj-y += tests/qtest/libqos/e1000e.o
+libqos-obj-y += tests/qtest/libqos/i2c.o
+libqos-obj-y += tests/qtest/libqos/i2c-imx.o
+libqos-obj-y += tests/qtest/libqos/i2c-omap.o
+libqos-obj-y += tests/qtest/libqos/sdhci.o
+libqos-obj-y += tests/qtest/libqos/tpci200.o
+libqos-obj-y += tests/qtest/libqos/virtio.o
+libqos-obj-$(CONFIG_VIRTFS) += tests/qtest/libqos/virtio-9p.o
+libqos-obj-y += tests/qtest/libqos/virtio-balloon.o
+libqos-obj-y += tests/qtest/libqos/virtio-blk.o
+libqos-obj-y += tests/qtest/libqos/virtio-mmio.o
+libqos-obj-y += tests/qtest/libqos/virtio-net.o
+libqos-obj-y += tests/qtest/libqos/virtio-pci.o
+libqos-obj-y += tests/qtest/libqos/virtio-pci-modern.o
+libqos-obj-y += tests/qtest/libqos/virtio-rng.o
+libqos-obj-y += tests/qtest/libqos/virtio-scsi.o
+libqos-obj-y += tests/qtest/libqos/virtio-serial.o
 
 # qos machines:
-qos-test-obj-y += tests/qtest/libqos/aarch64-xlnx-zcu102-machine.o
-qos-test-obj-y += tests/qtest/libqos/arm-imx25-pdk-machine.o
-qos-test-obj-y += tests/qtest/libqos/arm-n800-machine.o
-qos-test-obj-y += tests/qtest/libqos/arm-raspi2-machine.o
-qos-test-obj-y += tests/qtest/libqos/arm-sabrelite-machine.o
-qos-test-obj-y += tests/qtest/libqos/arm-smdkc210-machine.o
-qos-test-obj-y += tests/qtest/libqos/arm-virt-machine.o
-qos-test-obj-y += tests/qtest/libqos/arm-xilinx-zynq-a9-machine.o
-qos-test-obj-y += tests/qtest/libqos/ppc64_pseries-machine.o
-qos-test-obj-y += tests/qtest/libqos/x86_64_pc-machine.o
+libqos-obj-y += tests/qtest/libqos/aarch64-xlnx-zcu102-machine.o
+libqos-obj-y += tests/qtest/libqos/arm-imx25-pdk-machine.o
+libqos-obj-y += tests/qtest/libqos/arm-n800-machine.o
+libqos-obj-y += tests/qtest/libqos/arm-raspi2-machine.o
+libqos-obj-y += tests/qtest/libqos/arm-sabrelite-machine.o
+libqos-obj-y += tests/qtest/libqos/arm-smdkc210-machine.o
+libqos-obj-y += tests/qtest/libqos/arm-virt-machine.o
+libqos-obj-y += tests/qtest/libqos/arm-xilinx-zynq-a9-machine.o
+libqos-obj-y += tests/qtest/libqos/ppc64_pseries-machine.o
+libqos-obj-y += tests/qtest/libqos/x86_64_pc-machine.o
 
 # qos tests:
+qos-test-obj-y += tests/qtest/qos-test.o
 qos-test-obj-y += tests/qtest/ac97-test.o
 qos-test-obj-y += tests/qtest/ds1338-test.o
 qos-test-obj-y += tests/qtest/e1000-test.o
@@ -234,7 +235,7 @@ check-unit-y += tests/test-qgraph$(EXESUF)
 tests/test-qgraph$(EXESUF): tests/test-qgraph.o $(libqgraph-obj-y)
 
 check-qtest-generic-y += qos-test
-tests/qtest/qos-test$(EXESUF): $(qos-test-obj-y)
+tests/qtest/qos-test$(EXESUF): $(qos-test-obj-y) $(libqos-obj-y)
 
 # QTest dependencies:
 tests/qtest/qmp-test$(EXESUF): tests/qtest/qmp-test.o
-- 
2.24.1

[PULL 20/31] libqos: move useful qos-test funcs to qos_external

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

The moved functions are not specific to qos-test and might be useful
elsewhere. For example the virtual-device fuzzer makes use of them for
qos-assisted fuzz-targets.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-12-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/Makefile.include      |   1 +
 tests/qtest/libqos/qos_external.c | 168 ++++++++++++++++++++++++++++++
 tests/qtest/libqos/qos_external.h |  28 +++++
 tests/qtest/qos-test.c            | 132 +----------------------
 4 files changed, 198 insertions(+), 131 deletions(-)
 create mode 100644 tests/qtest/libqos/qos_external.c
 create mode 100644 tests/qtest/libqos/qos_external.h

diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index 838618e6f9..e769c1ad70 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -172,6 +172,7 @@ libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/qtest/libqos/u
 # qos devices:
 libqos-obj-y =  $(libqgraph-obj-y)
 libqos-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
+libqos-obj-y += tests/qtest/libqos/qos_external.o
 libqos-obj-y += tests/qtest/libqos/e1000e.o
 libqos-obj-y += tests/qtest/libqos/i2c.o
 libqos-obj-y += tests/qtest/libqos/i2c-imx.o
diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
new file mode 100644
index 0000000000..398556dde0
--- /dev/null
+++ b/tests/qtest/libqos/qos_external.c
@@ -0,0 +1,168 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include <getopt.h>
+#include "libqtest.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qbool.h"
+#include "qapi/qmp/qstring.h"
+#include "qemu/module.h"
+#include "qapi/qmp/qlist.h"
+#include "libqos/malloc.h"
+#include "libqos/qgraph.h"
+#include "libqos/qgraph_internal.h"
+#include "libqos/qos_external.h"
+
+
+
+void apply_to_node(const char *name, bool is_machine, bool is_abstract)
+{
+    char *machine_name = NULL;
+    if (is_machine) {
+        const char *arch = qtest_get_arch();
+        machine_name = g_strconcat(arch, "/", name, NULL);
+        name = machine_name;
+    }
+    qos_graph_node_set_availability(name, true);
+    if (is_abstract) {
+        qos_delete_cmd_line(name);
+    }
+    g_free(machine_name);
+}
+
+/**
+ * apply_to_qlist(): using QMP queries QEMU for a list of
+ * machines and devices available, and sets the respective node
+ * as true. If a node is found, also all its produced and contained
+ * child are marked available.
+ *
+ * See qos_graph_node_set_availability() for more info
+ */
+void apply_to_qlist(QList *list, bool is_machine)
+{
+    const QListEntry *p;
+    const char *name;
+    bool abstract;
+    QDict *minfo;
+    QObject *qobj;
+    QString *qstr;
+    QBool *qbool;
+
+    for (p = qlist_first(list); p; p = qlist_next(p)) {
+        minfo = qobject_to(QDict, qlist_entry_obj(p));
+        qobj = qdict_get(minfo, "name");
+        qstr = qobject_to(QString, qobj);
+        name = qstring_get_str(qstr);
+
+        qobj = qdict_get(minfo, "abstract");
+        if (qobj) {
+            qbool = qobject_to(QBool, qobj);
+            abstract = qbool_get_bool(qbool);
+        } else {
+            abstract = false;
+        }
+
+        apply_to_node(name, is_machine, abstract);
+        qobj = qdict_get(minfo, "alias");
+        if (qobj) {
+            qstr = qobject_to(QString, qobj);
+            name = qstring_get_str(qstr);
+            apply_to_node(name, is_machine, abstract);
+        }
+    }
+}
+
+QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
+{
+    return obj->get_driver(obj, "memory");
+}
+
+/**
+ * allocate_objects(): given an array of nodes @arg,
+ * walks the path invoking all constructors and
+ * passing the corresponding parameter in order to
+ * continue the objects allocation.
+ * Once the test is reached, return the object it consumes.
+ *
+ * Since the machine and QEDGE_CONSUMED_BY nodes allocate
+ * memory in the constructor, g_test_queue_destroy is used so
+ * that after execution they can be safely free'd.  (The test's
+ * ->before callback is also welcome to use g_test_queue_destroy).
+ *
+ * Note: as specified in walk_path() too, @arg is an array of
+ * char *, where arg[0] is a pointer to the command line
+ * string that will be used to properly start QEMU when executing
+ * the test, and the remaining elements represent the actual objects
+ * that will be allocated.
+ */
+void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc)
+{
+    int current = 0;
+    QGuestAllocator *alloc;
+    QOSGraphObject *parent = NULL;
+    QOSGraphEdge *edge;
+    QOSGraphNode *node;
+    void *edge_arg;
+    void *obj;
+
+    node = qos_graph_get_node(path[current]);
+    g_assert(node->type == QNODE_MACHINE);
+
+    obj = qos_machine_new(node, qts);
+    qos_object_queue_destroy(obj);
+
+    alloc = get_machine_allocator(obj);
+    if (p_alloc) {
+        *p_alloc = alloc;
+    }
+
+    for (;;) {
+        if (node->type != QNODE_INTERFACE) {
+            qos_object_start_hw(obj);
+            parent = obj;
+        }
+
+        /* follow edge and get object for next node constructor */
+        current++;
+        edge = qos_graph_get_edge(path[current - 1], path[current]);
+        node = qos_graph_get_node(path[current]);
+
+        if (node->type == QNODE_TEST) {
+            g_assert(qos_graph_edge_get_type(edge) == QEDGE_CONSUMED_BY);
+            return obj;
+        }
+
+        switch (qos_graph_edge_get_type(edge)) {
+        case QEDGE_PRODUCES:
+            obj = parent->get_driver(parent, path[current]);
+            break;
+
+        case QEDGE_CONSUMED_BY:
+            edge_arg = qos_graph_edge_get_arg(edge);
+            obj = qos_driver_new(node, obj, alloc, edge_arg);
+            qos_object_queue_destroy(obj);
+            break;
+
+        case QEDGE_CONTAINS:
+            obj = parent->get_device(parent, path[current]);
+            break;
+        }
+    }
+}
+
diff --git a/tests/qtest/libqos/qos_external.h b/tests/qtest/libqos/qos_external.h
new file mode 100644
index 0000000000..7b44930c55
--- /dev/null
+++ b/tests/qtest/libqos/qos_external.h
@@ -0,0 +1,28 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef QOS_EXTERNAL_H
+#define QOS_EXTERNAL_H
+#include "libqos/qgraph.h"
+
+void apply_to_node(const char *name, bool is_machine, bool is_abstract);
+void apply_to_qlist(QList *list, bool is_machine);
+QGuestAllocator *get_machine_allocator(QOSGraphObject *obj);
+void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc);
+
+#endif
diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
index fd70d73ea5..ad193f43a5 100644
--- a/tests/qtest/qos-test.c
+++ b/tests/qtest/qos-test.c
@@ -27,65 +27,11 @@
 #include "libqos/malloc.h"
 #include "libqos/qgraph.h"
 #include "libqos/qgraph_internal.h"
+#include "libqos/qos_external.h"
 
 static char *old_path;
 
-static void apply_to_node(const char *name, bool is_machine, bool is_abstract)
-{
-    char *machine_name = NULL;
-    if (is_machine) {
-        const char *arch = qtest_get_arch();
-        machine_name = g_strconcat(arch, "/", name, NULL);
-        name = machine_name;
-    }
-    qos_graph_node_set_availability(name, true);
-    if (is_abstract) {
-        qos_delete_cmd_line(name);
-    }
-    g_free(machine_name);
-}
 
-/**
- * apply_to_qlist(): using QMP queries QEMU for a list of
- * machines and devices available, and sets the respective node
- * as true. If a node is found, also all its produced and contained
- * child are marked available.
- *
- * See qos_graph_node_set_availability() for more info
- */
-static void apply_to_qlist(QList *list, bool is_machine)
-{
-    const QListEntry *p;
-    const char *name;
-    bool abstract;
-    QDict *minfo;
-    QObject *qobj;
-    QString *qstr;
-    QBool *qbool;
-
-    for (p = qlist_first(list); p; p = qlist_next(p)) {
-        minfo = qobject_to(QDict, qlist_entry_obj(p));
-        qobj = qdict_get(minfo, "name");
-        qstr = qobject_to(QString, qobj);
-        name = qstring_get_str(qstr);
-
-        qobj = qdict_get(minfo, "abstract");
-        if (qobj) {
-            qbool = qobject_to(QBool, qobj);
-            abstract = qbool_get_bool(qbool);
-        } else {
-            abstract = false;
-        }
-
-        apply_to_node(name, is_machine, abstract);
-        qobj = qdict_get(minfo, "alias");
-        if (qobj) {
-            qstr = qobject_to(QString, qobj);
-            name = qstring_get_str(qstr);
-            apply_to_node(name, is_machine, abstract);
-        }
-    }
-}
 
 /**
  * qos_set_machines_devices_available(): sets availability of qgraph
@@ -129,10 +75,6 @@ static void qos_set_machines_devices_available(void)
     qobject_unref(response);
 }
 
-static QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
-{
-    return obj->get_driver(obj, "memory");
-}
 
 static void restart_qemu_or_continue(char *path)
 {
@@ -159,78 +101,6 @@ void qos_invalidate_command_line(void)
     old_path = NULL;
 }
 
-/**
- * allocate_objects(): given an array of nodes @arg,
- * walks the path invoking all constructors and
- * passing the corresponding parameter in order to
- * continue the objects allocation.
- * Once the test is reached, return the object it consumes.
- *
- * Since the machine and QEDGE_CONSUMED_BY nodes allocate
- * memory in the constructor, g_test_queue_destroy is used so
- * that after execution they can be safely free'd.  (The test's
- * ->before callback is also welcome to use g_test_queue_destroy).
- *
- * Note: as specified in walk_path() too, @arg is an array of
- * char *, where arg[0] is a pointer to the command line
- * string that will be used to properly start QEMU when executing
- * the test, and the remaining elements represent the actual objects
- * that will be allocated.
- */
-static void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc)
-{
-    int current = 0;
-    QGuestAllocator *alloc;
-    QOSGraphObject *parent = NULL;
-    QOSGraphEdge *edge;
-    QOSGraphNode *node;
-    void *edge_arg;
-    void *obj;
-
-    node = qos_graph_get_node(path[current]);
-    g_assert(node->type == QNODE_MACHINE);
-
-    obj = qos_machine_new(node, qts);
-    qos_object_queue_destroy(obj);
-
-    alloc = get_machine_allocator(obj);
-    if (p_alloc) {
-        *p_alloc = alloc;
-    }
-
-    for (;;) {
-        if (node->type != QNODE_INTERFACE) {
-            qos_object_start_hw(obj);
-            parent = obj;
-        }
-
-        /* follow edge and get object for next node constructor */
-        current++;
-        edge = qos_graph_get_edge(path[current - 1], path[current]);
-        node = qos_graph_get_node(path[current]);
-
-        if (node->type == QNODE_TEST) {
-            g_assert(qos_graph_edge_get_type(edge) == QEDGE_CONSUMED_BY);
-            return obj;
-        }
-
-        switch (qos_graph_edge_get_type(edge)) {
-        case QEDGE_PRODUCES:
-            obj = parent->get_driver(parent, path[current]);
-            break;
-
-        case QEDGE_CONSUMED_BY:
-            edge_arg = qos_graph_edge_get_arg(edge);
-            obj = qos_driver_new(node, obj, alloc, edge_arg);
-            qos_object_queue_destroy(obj);
-            break;
-
-        case QEDGE_CONTAINS:
-            obj = parent->get_device(parent, path[current]);
-            break;
-        }
-    }
-}
 
 /* The argument to run_one_test, which is the test function that is registered
  * with GTest, is a vector of strings.  The first item is the initial command
-- 
2.24.1

[PULL 21/31] fuzz: add fuzzer skeleton

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

tests/fuzz/fuzz.c serves as the entry point for the virtual-device
fuzzer. Namely, libfuzzer invokes the LLVMFuzzerInitialize and
LLVMFuzzerTestOneInput functions, both of which are defined in this
file. This change adds a "FuzzTarget" struct, along with the
fuzz_add_target function, which should be used to define new fuzz
targets.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-13-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS                       |   8 ++
 tests/qtest/fuzz/Makefile.include |   6 +
 tests/qtest/fuzz/fuzz.c           | 179 ++++++++++++++++++++++++++++++
 tests/qtest/fuzz/fuzz.h           |  95 ++++++++++++++++
 4 files changed, 288 insertions(+)
 create mode 100644 tests/qtest/fuzz/Makefile.include
 create mode 100644 tests/qtest/fuzz/fuzz.c
 create mode 100644 tests/qtest/fuzz/fuzz.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 969329f83e..195dd58cac 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2184,6 +2184,14 @@ F: qtest.c
 F: accel/qtest.c
 F: tests/qtest/
 
+Device Fuzzing
+M: Alexander Bulekov <alxndr@bu.edu>
+R: Paolo Bonzini <pbonzini@redhat.com>
+R: Bandan Das <bsd@redhat.com>
+R: Stefan Hajnoczi <stefanha@redhat.com>
+S: Maintained
+F: tests/qtest/fuzz/
+
 Register API
 M: Alistair Francis <alistair@alistair23.me>
 S: Maintained
diff --git a/tests/qtest/fuzz/Makefile.include b/tests/qtest/fuzz/Makefile.include
new file mode 100644
index 0000000000..8632bb89f4
--- /dev/null
+++ b/tests/qtest/fuzz/Makefile.include
@@ -0,0 +1,6 @@
+QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
+
+fuzz-obj-y += tests/qtest/libqtest.o
+fuzz-obj-y += tests/qtest/fuzz/fuzz.o # Fuzzer skeleton
+
+FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
new file mode 100644
index 0000000000..0d78ac8d36
--- /dev/null
+++ b/tests/qtest/fuzz/fuzz.c
@@ -0,0 +1,179 @@
+/*
+ * fuzzing driver
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include <wordexp.h>
+
+#include "sysemu/qtest.h"
+#include "sysemu/runstate.h"
+#include "sysemu/sysemu.h"
+#include "qemu/main-loop.h"
+#include "tests/qtest/libqtest.h"
+#include "tests/qtest/libqos/qgraph.h"
+#include "fuzz.h"
+
+#define MAX_EVENT_LOOPS 10
+
+typedef struct FuzzTargetState {
+        FuzzTarget *target;
+        QSLIST_ENTRY(FuzzTargetState) target_list;
+} FuzzTargetState;
+
+typedef QSLIST_HEAD(, FuzzTargetState) FuzzTargetList;
+
+static const char *fuzz_arch = TARGET_NAME;
+
+static FuzzTargetList *fuzz_target_list;
+static FuzzTarget *fuzz_target;
+static QTestState *fuzz_qts;
+
+
+
+void flush_events(QTestState *s)
+{
+    int i = MAX_EVENT_LOOPS;
+    while (g_main_context_pending(NULL) && i-- > 0) {
+        main_loop_wait(false);
+    }
+}
+
+static QTestState *qtest_setup(void)
+{
+    qtest_server_set_send_handler(&qtest_client_inproc_recv, &fuzz_qts);
+    return qtest_inproc_init(&fuzz_qts, false, fuzz_arch,
+            &qtest_server_inproc_recv);
+}
+
+void fuzz_add_target(const FuzzTarget *target)
+{
+    FuzzTargetState *tmp;
+    FuzzTargetState *target_state;
+    if (!fuzz_target_list) {
+        fuzz_target_list = g_new0(FuzzTargetList, 1);
+    }
+
+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
+        if (g_strcmp0(tmp->target->name, target->name) == 0) {
+            fprintf(stderr, "Error: Fuzz target name %s already in use\n",
+                    target->name);
+            abort();
+        }
+    }
+    target_state = g_new0(FuzzTargetState, 1);
+    target_state->target = g_new0(FuzzTarget, 1);
+    *(target_state->target) = *target;
+    QSLIST_INSERT_HEAD(fuzz_target_list, target_state, target_list);
+}
+
+
+
+static void usage(char *path)
+{
+    printf("Usage: %s --fuzz-target=FUZZ_TARGET [LIBFUZZER ARGUMENTS]\n", path);
+    printf("where FUZZ_TARGET is one of:\n");
+    FuzzTargetState *tmp;
+    if (!fuzz_target_list) {
+        fprintf(stderr, "Fuzz target list not initialized\n");
+        abort();
+    }
+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
+        printf(" * %s  : %s\n", tmp->target->name,
+                tmp->target->description);
+    }
+    exit(0);
+}
+
+static FuzzTarget *fuzz_get_target(char* name)
+{
+    FuzzTargetState *tmp;
+    if (!fuzz_target_list) {
+        fprintf(stderr, "Fuzz target list not initialized\n");
+        abort();
+    }
+
+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
+        if (strcmp(tmp->target->name, name) == 0) {
+            return tmp->target;
+        }
+    }
+    return NULL;
+}
+
+
+/* Executed for each fuzzing-input */
+int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size)
+{
+    /*
+     * Do the pre-fuzz-initialization before the first fuzzing iteration,
+     * instead of before the actual fuzz loop. This is needed since libfuzzer
+     * may fork off additional workers, prior to the fuzzing loop, and if
+     * pre_fuzz() sets up e.g. shared memory, this should be done for the
+     * individual worker processes
+     */
+    static int pre_fuzz_done;
+    if (!pre_fuzz_done && fuzz_target->pre_fuzz) {
+        fuzz_target->pre_fuzz(fuzz_qts);
+        pre_fuzz_done = true;
+    }
+
+    fuzz_target->fuzz(fuzz_qts, Data, Size);
+    return 0;
+}
+
+/* Executed once, prior to fuzzing */
+int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
+{
+
+    char *target_name;
+
+    /* Initialize qgraph and modules */
+    qos_graph_init();
+    module_call_init(MODULE_INIT_FUZZ_TARGET);
+    module_call_init(MODULE_INIT_QOM);
+    module_call_init(MODULE_INIT_LIBQOS);
+
+    if (*argc <= 1) {
+        usage(**argv);
+    }
+
+    /* Identify the fuzz target */
+    target_name = (*argv)[1];
+    if (!strstr(target_name, "--fuzz-target=")) {
+        usage(**argv);
+    }
+
+    target_name += strlen("--fuzz-target=");
+
+    fuzz_target = fuzz_get_target(target_name);
+    if (!fuzz_target) {
+        usage(**argv);
+    }
+
+    fuzz_qts = qtest_setup();
+
+    if (fuzz_target->pre_vm_init) {
+        fuzz_target->pre_vm_init();
+    }
+
+    /* Run QEMU's softmmu main with the fuzz-target dependent arguments */
+    const char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target);
+
+    /* Split the runcmd into an argv and argc */
+    wordexp_t result;
+    wordexp(init_cmdline, &result, 0);
+
+    qemu_init(result.we_wordc, result.we_wordv, NULL);
+
+    return 0;
+}
diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h
new file mode 100644
index 0000000000..03901d414e
--- /dev/null
+++ b/tests/qtest/fuzz/fuzz.h
@@ -0,0 +1,95 @@
+/*
+ * fuzzing driver
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef FUZZER_H_
+#define FUZZER_H_
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+
+#include "tests/qtest/libqtest.h"
+
+/**
+ * A libfuzzer fuzzing target
+ *
+ * The QEMU fuzzing binary is built with all available targets, each
+ * with a unique @name that can be specified on the command-line to
+ * select which target should run.
+ *
+ * A target must implement ->fuzz() to process a random input.  If QEMU
+ * crashes in ->fuzz() then libfuzzer will record a failure.
+ *
+ * Fuzzing targets are registered with fuzz_add_target():
+ *
+ *   static const FuzzTarget fuzz_target = {
+ *       .name = "my-device-fifo",
+ *       .description = "Fuzz the FIFO buffer registers of my-device",
+ *       ...
+ *   };
+ *
+ *   static void register_fuzz_target(void)
+ *   {
+ *       fuzz_add_target(&fuzz_target);
+ *   }
+ *   fuzz_target_init(register_fuzz_target);
+ */
+typedef struct FuzzTarget {
+    const char *name;         /* target identifier (passed to --fuzz-target=)*/
+    const char *description;  /* help text */
+
+
+    /*
+     * returns the arg-list that is passed to qemu/softmmu init()
+     * Cannot be NULL
+     */
+    const char* (*get_init_cmdline)(struct FuzzTarget *);
+
+    /*
+     * will run once, prior to running qemu/softmmu init.
+     * eg: set up shared-memory for communication with the child-process
+     * Can be NULL
+     */
+    void(*pre_vm_init)(void);
+
+    /*
+     * will run once, after QEMU has been initialized, prior to the fuzz-loop.
+     * eg: detect the memory map
+     * Can be NULL
+     */
+    void(*pre_fuzz)(QTestState *);
+
+    /*
+     * accepts and executes an input from libfuzzer. this is repeatedly
+     * executed during the fuzzing loop. Its should handle setup, input
+     * execution and cleanup.
+     * Cannot be NULL
+     */
+    void(*fuzz)(QTestState *, const unsigned char *, size_t);
+
+} FuzzTarget;
+
+void flush_events(QTestState *);
+void reboot(QTestState *);
+
+/*
+ * makes a copy of *target and adds it to the target-list.
+ * i.e. fine to set up target on the caller's stack
+ */
+void fuzz_add_target(const FuzzTarget *target);
+
+int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size);
+int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp);
+
+#endif
+
-- 
2.24.1

[PULL 22/31] exec: keep ram block across fork when using qtest

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Ram blocks were marked MADV_DONTFORK breaking fuzzing-tests which
execute each test-input in a forked process.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-14-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 exec.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 8e9cc3b47c..c930040f83 100644
--- a/exec.c
+++ b/exec.c
@@ -35,6 +35,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/tcg.h"
+#include "sysemu/qtest.h"
 #include "qemu/timer.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
@@ -2305,8 +2306,15 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
     if (new_block->host) {
         qemu_ram_setup_dump(new_block->host, new_block->max_length);
         qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_HUGEPAGE);
-        /* MADV_DONTFORK is also needed by KVM in absence of synchronous MMU */
-        qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_DONTFORK);
+        /*
+         * MADV_DONTFORK is also needed by KVM in absence of synchronous MMU
+         * Configure it unless the machine is a qtest server, in which case
+         * KVM is not used and it may be forked (eg for fuzzing purposes).
+         */
+        if (!qtest_enabled()) {
+            qemu_madvise(new_block->host, new_block->max_length,
+                         QEMU_MADV_DONTFORK);
+        }
         ram_block_notify_add(new_block->host, new_block->max_length);
     }
 }
-- 
2.24.1

[PULL 23/31] main: keep rcu_atfork callback enabled for qtest

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

The qtest-based fuzzer makes use of forking to reset-state between
tests. Keep the callback enabled, so the call_rcu thread gets created
within the child process.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20200220041118.23264-15-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 softmmu/vl.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 080d3b5106..92c7b3a6e9 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3782,7 +3782,17 @@ void qemu_init(int argc, char **argv, char **envp)
     set_memory_options(&ram_slots, &maxram_size, machine_class);
 
     os_daemonize();
-    rcu_disable_atfork();
+
+    /*
+     * If QTest is enabled, keep the rcu_atfork enabled, since system processes
+     * may be forked testing purposes (e.g. fork-server based fuzzing) The fork
+     * should happen before a signle cpu instruction is executed, to prevent
+     * deadlocks. See commit 73c6e40, rcu: "completely disable pthread_atfork
+     * callbacks as soon as possible"
+     */
+    if (!qtest_enabled()) {
+        rcu_disable_atfork();
+    }
 
     if (pid_file && !qemu_write_pidfile(pid_file, &err)) {
         error_reportf_err(err, "cannot create PID file: ");
-- 
2.24.1

[PULL 24/31] fuzz: support for fork-based fuzzing.

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

fork() is a simple way to ensure that state does not leak in between
fuzzing runs. Unfortunately, the fuzzer mutation engine relies on
bitmaps which contain coverage information for each fuzzing run, and
these bitmaps should be copied from the child to the parent(where the
mutation occurs). These bitmaps are created through compile-time
instrumentation and they are not shared with fork()-ed processes, by
default. To address this, we create a shared memory region, adjust its
size and map it _over_ the counter region. Furthermore, libfuzzer
doesn't generally expose the globals that specify the location of the
counters/coverage bitmap. As a workaround, we rely on a custom linker
script which forces all of the bitmaps we care about to be placed in a
contiguous region, which is easy to locate and mmap over.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-16-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/fuzz/Makefile.include |  5 +++
 tests/qtest/fuzz/fork_fuzz.c      | 55 +++++++++++++++++++++++++++++++
 tests/qtest/fuzz/fork_fuzz.h      | 23 +++++++++++++
 tests/qtest/fuzz/fork_fuzz.ld     | 37 +++++++++++++++++++++
 4 files changed, 120 insertions(+)
 create mode 100644 tests/qtest/fuzz/fork_fuzz.c
 create mode 100644 tests/qtest/fuzz/fork_fuzz.h
 create mode 100644 tests/qtest/fuzz/fork_fuzz.ld

diff --git a/tests/qtest/fuzz/Makefile.include b/tests/qtest/fuzz/Makefile.include
index 8632bb89f4..a90915d56d 100644
--- a/tests/qtest/fuzz/Makefile.include
+++ b/tests/qtest/fuzz/Makefile.include
@@ -2,5 +2,10 @@ QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
 
 fuzz-obj-y += tests/qtest/libqtest.o
 fuzz-obj-y += tests/qtest/fuzz/fuzz.o # Fuzzer skeleton
+fuzz-obj-y += tests/qtest/fuzz/fork_fuzz.o
 
 FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest
+
+# Linker Script to force coverage-counters into known regions which we can mark
+# shared
+FUZZ_LDFLAGS += -Xlinker -T$(SRC_PATH)/tests/qtest/fuzz/fork_fuzz.ld
diff --git a/tests/qtest/fuzz/fork_fuzz.c b/tests/qtest/fuzz/fork_fuzz.c
new file mode 100644
index 0000000000..2bd0851903
--- /dev/null
+++ b/tests/qtest/fuzz/fork_fuzz.c
@@ -0,0 +1,55 @@
+/*
+ * Fork-based fuzzing helpers
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "fork_fuzz.h"
+
+
+void counter_shm_init(void)
+{
+    char *shm_path = g_strdup_printf("/qemu-fuzz-cntrs.%d", getpid());
+    int fd = shm_open(shm_path, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR);
+    g_free(shm_path);
+
+    if (fd == -1) {
+        perror("Error: ");
+        exit(1);
+    }
+    if (ftruncate(fd, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START) == -1) {
+        perror("Error: ");
+        exit(1);
+    }
+    /* Copy what's in the counter region to the shm.. */
+    void *rptr = mmap(NULL ,
+            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
+            PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+    memcpy(rptr,
+           &__FUZZ_COUNTERS_START,
+           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
+
+    munmap(rptr, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
+
+    /* And map the shm over the counter region */
+    rptr = mmap(&__FUZZ_COUNTERS_START,
+            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
+            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, fd, 0);
+
+    close(fd);
+
+    if (!rptr) {
+        perror("Error: ");
+        exit(1);
+    }
+}
+
+
diff --git a/tests/qtest/fuzz/fork_fuzz.h b/tests/qtest/fuzz/fork_fuzz.h
new file mode 100644
index 0000000000..9ecb8b58ef
--- /dev/null
+++ b/tests/qtest/fuzz/fork_fuzz.h
@@ -0,0 +1,23 @@
+/*
+ * Fork-based fuzzing helpers
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef FORK_FUZZ_H
+#define FORK_FUZZ_H
+
+extern uint8_t __FUZZ_COUNTERS_START;
+extern uint8_t __FUZZ_COUNTERS_END;
+
+void counter_shm_init(void);
+
+#endif
+
diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld
new file mode 100644
index 0000000000..b23a59f194
--- /dev/null
+++ b/tests/qtest/fuzz/fork_fuzz.ld
@@ -0,0 +1,37 @@
+/* We adjust linker script modification to place all of the stuff that needs to
+ * persist across fuzzing runs into a contiguous seciton of memory. Then, it is
+ * easy to re-map the counter-related memory as shared.
+*/
+
+SECTIONS
+{
+  .data.fuzz_start : ALIGN(4K)
+  {
+      __FUZZ_COUNTERS_START = .;
+      __start___sancov_cntrs = .;
+      *(_*sancov_cntrs);
+      __stop___sancov_cntrs = .;
+
+      /* Lowest stack counter */
+      *(__sancov_lowest_stack);
+  }
+  .data.fuzz_ordered :
+  {
+      /* Coverage counters. They're not necessary for fuzzing, but are useful
+       * for analyzing the fuzzing performance
+       */
+      __start___llvm_prf_cnts = .;
+      *(*llvm_prf_cnts);
+      __stop___llvm_prf_cnts = .;
+
+      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap */
+      FuzzerTracePC*(.bss*);
+  }
+  .data.fuzz_end : ALIGN(4K)
+  {
+      __FUZZ_COUNTERS_END = .;
+  }
+}
+/* Dont overwrite the SECTIONS in the default linker script. Instead insert the
+ * above into the default script */
+INSERT AFTER .data;
-- 
2.24.1

[PULL 25/31] fuzz: add support for qos-assisted fuzz targets

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-17-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/fuzz/Makefile.include |   2 +
 tests/qtest/fuzz/qos_fuzz.c       | 234 ++++++++++++++++++++++++++++++
 tests/qtest/fuzz/qos_fuzz.h       |  33 +++++
 3 files changed, 269 insertions(+)
 create mode 100644 tests/qtest/fuzz/qos_fuzz.c
 create mode 100644 tests/qtest/fuzz/qos_fuzz.h

diff --git a/tests/qtest/fuzz/Makefile.include b/tests/qtest/fuzz/Makefile.include
index a90915d56d..e3bdd33ff4 100644
--- a/tests/qtest/fuzz/Makefile.include
+++ b/tests/qtest/fuzz/Makefile.include
@@ -1,8 +1,10 @@
 QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
 
 fuzz-obj-y += tests/qtest/libqtest.o
+fuzz-obj-y += $(libqos-obj-y)
 fuzz-obj-y += tests/qtest/fuzz/fuzz.o # Fuzzer skeleton
 fuzz-obj-y += tests/qtest/fuzz/fork_fuzz.o
+fuzz-obj-y += tests/qtest/fuzz/qos_fuzz.o
 
 FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest
 
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
new file mode 100644
index 0000000000..bbb17470ff
--- /dev/null
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -0,0 +1,234 @@
+/*
+ * QOS-assisted fuzzing helpers
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "exec/memory.h"
+#include "exec/address-spaces.h"
+#include "sysemu/sysemu.h"
+#include "qemu/main-loop.h"
+
+#include "tests/qtest/libqtest.h"
+#include "tests/qtest/libqos/malloc.h"
+#include "tests/qtest/libqos/qgraph.h"
+#include "tests/qtest/libqos/qgraph_internal.h"
+#include "tests/qtest/libqos/qos_external.h"
+
+#include "fuzz.h"
+#include "qos_fuzz.h"
+
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qmp/qlist.h"
+
+
+void *fuzz_qos_obj;
+QGuestAllocator *fuzz_qos_alloc;
+
+static const char *fuzz_target_name;
+static char **fuzz_path_vec;
+
+/*
+ * Replaced the qmp commands with direct qmp_marshal calls.
+ * Probably there is a better way to do this
+ */
+static void qos_set_machines_devices_available(void)
+{
+    QDict *req = qdict_new();
+    QObject *response;
+    QDict *args = qdict_new();
+    QList *lst;
+    Error *err = NULL;
+
+    qmp_marshal_query_machines(NULL, &response, &err);
+    assert(!err);
+    lst = qobject_to(QList, response);
+    apply_to_qlist(lst, true);
+
+    qobject_unref(response);
+
+
+    qdict_put_str(req, "execute", "qom-list-types");
+    qdict_put_str(args, "implements", "device");
+    qdict_put_bool(args, "abstract", true);
+    qdict_put_obj(req, "arguments", (QObject *) args);
+
+    qmp_marshal_qom_list_types(args, &response, &err);
+    assert(!err);
+    lst = qobject_to(QList, response);
+    apply_to_qlist(lst, false);
+    qobject_unref(response);
+    qobject_unref(req);
+}
+
+static char **current_path;
+
+void *qos_allocate_objects(QTestState *qts, QGuestAllocator **p_alloc)
+{
+    return allocate_objects(qts, current_path + 1, p_alloc);
+}
+
+static const char *qos_build_main_args(void)
+{
+    char **path = fuzz_path_vec;
+    QOSGraphNode *test_node;
+    GString *cmd_line = g_string_new(path[0]);
+    void *test_arg;
+
+    if (!path) {
+        fprintf(stderr, "QOS Path not found\n");
+        abort();
+    }
+
+    /* Before test */
+    current_path = path;
+    test_node = qos_graph_get_node(path[(g_strv_length(path) - 1)]);
+    test_arg = test_node->u.test.arg;
+    if (test_node->u.test.before) {
+        test_arg = test_node->u.test.before(cmd_line, test_arg);
+    }
+    /* Prepend the arguments that we need */
+    g_string_prepend(cmd_line,
+            TARGET_NAME " -display none -machine accel=qtest -m 64 ");
+    return cmd_line->str;
+}
+
+/*
+ * This function is largely a copy of qos-test.c:walk_path. Since walk_path
+ * is itself a callback, its a little annoying to add another argument/layer of
+ * indirection
+ */
+static void walk_path(QOSGraphNode *orig_path, int len)
+{
+    QOSGraphNode *path;
+    QOSGraphEdge *edge;
+
+    /* etype set to QEDGE_CONSUMED_BY so that machine can add to the command line */
+    QOSEdgeType etype = QEDGE_CONSUMED_BY;
+
+    /* twice QOS_PATH_MAX_ELEMENT_SIZE since each edge can have its arg */
+    char **path_vec = g_new0(char *, (QOS_PATH_MAX_ELEMENT_SIZE * 2));
+    int path_vec_size = 0;
+
+    char *after_cmd, *before_cmd, *after_device;
+    GString *after_device_str = g_string_new("");
+    char *node_name = orig_path->name, *path_str;
+
+    GString *cmd_line = g_string_new("");
+    GString *cmd_line2 = g_string_new("");
+
+    path = qos_graph_get_node(node_name); /* root */
+    node_name = qos_graph_edge_get_dest(path->path_edge); /* machine name */
+
+    path_vec[path_vec_size++] = node_name;
+    path_vec[path_vec_size++] = qos_get_machine_type(node_name);
+
+    for (;;) {
+        path = qos_graph_get_node(node_name);
+        if (!path->path_edge) {
+            break;
+        }
+
+        node_name = qos_graph_edge_get_dest(path->path_edge);
+
+        /* append node command line + previous edge command line */
+        if (path->command_line && etype == QEDGE_CONSUMED_BY) {
+            g_string_append(cmd_line, path->command_line);
+            g_string_append(cmd_line, after_device_str->str);
+            g_string_truncate(after_device_str, 0);
+        }
+
+        path_vec[path_vec_size++] = qos_graph_edge_get_name(path->path_edge);
+        /* detect if edge has command line args */
+        after_cmd = qos_graph_edge_get_after_cmd_line(path->path_edge);
+        after_device = qos_graph_edge_get_extra_device_opts(path->path_edge);
+        before_cmd = qos_graph_edge_get_before_cmd_line(path->path_edge);
+        edge = qos_graph_get_edge(path->name, node_name);
+        etype = qos_graph_edge_get_type(edge);
+
+        if (before_cmd) {
+            g_string_append(cmd_line, before_cmd);
+        }
+        if (after_cmd) {
+            g_string_append(cmd_line2, after_cmd);
+        }
+        if (after_device) {
+            g_string_append(after_device_str, after_device);
+        }
+    }
+
+    path_vec[path_vec_size++] = NULL;
+    g_string_append(cmd_line, after_device_str->str);
+    g_string_free(after_device_str, true);
+
+    g_string_append(cmd_line, cmd_line2->str);
+    g_string_free(cmd_line2, true);
+
+    /*
+     * here position 0 has <arch>/<machine>, position 1 has <machine>.
+     * The path must not have the <arch>, qtest_add_data_func adds it.
+     */
+    path_str = g_strjoinv("/", path_vec + 1);
+
+    /* Check that this is the test we care about: */
+    char *test_name = strrchr(path_str, '/') + 1;
+    if (strcmp(test_name, fuzz_target_name) == 0) {
+        /*
+         * put arch/machine in position 1 so run_one_test can do its work
+         * and add the command line at position 0.
+         */
+        path_vec[1] = path_vec[0];
+        path_vec[0] = g_string_free(cmd_line, false);
+
+        fuzz_path_vec = path_vec;
+    } else {
+        g_free(path_vec);
+    }
+
+    g_free(path_str);
+}
+
+static const char *qos_get_cmdline(FuzzTarget *t)
+{
+    /*
+     * Set a global variable that we use to identify the qos_path for our
+     * fuzz_target
+     */
+    fuzz_target_name = t->name;
+    qos_set_machines_devices_available();
+    qos_graph_foreach_test_path(walk_path);
+    return qos_build_main_args();
+}
+
+void fuzz_add_qos_target(
+        FuzzTarget *fuzz_opts,
+        const char *interface,
+        QOSGraphTestOptions *opts
+        )
+{
+    qos_add_test(fuzz_opts->name, interface, NULL, opts);
+    fuzz_opts->get_init_cmdline = qos_get_cmdline;
+    fuzz_add_target(fuzz_opts);
+}
+
+void qos_init_path(QTestState *s)
+{
+    fuzz_qos_obj = qos_allocate_objects(s , &fuzz_qos_alloc);
+}
diff --git a/tests/qtest/fuzz/qos_fuzz.h b/tests/qtest/fuzz/qos_fuzz.h
new file mode 100644
index 0000000000..477f11b02b
--- /dev/null
+++ b/tests/qtest/fuzz/qos_fuzz.h
@@ -0,0 +1,33 @@
+/*
+ * QOS-assisted fuzzing helpers
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef _QOS_FUZZ_H_
+#define _QOS_FUZZ_H_
+
+#include "tests/qtest/fuzz/fuzz.h"
+#include "tests/qtest/libqos/qgraph.h"
+
+int qos_fuzz(const unsigned char *Data, size_t Size);
+void qos_setup(void);
+
+extern void *fuzz_qos_obj;
+extern QGuestAllocator *fuzz_qos_alloc;
+
+void fuzz_add_qos_target(
+        FuzzTarget *fuzz_opts,
+        const char *interface,
+        QOSGraphTestOptions *opts
+        );
+
+void qos_init_path(QTestState *);
+
+#endif
-- 
2.24.1

[PULL 26/31] fuzz: add target/fuzz makefile rules

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20200220041118.23264-18-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 Makefile        | 15 ++++++++++++++-
 Makefile.target | 16 ++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b5a7377cb1..18e82cca85 100644
--- a/Makefile
+++ b/Makefile
@@ -477,7 +477,7 @@ config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"GEN","$@")
 
-TARGET_DIRS_RULES := $(foreach t, all clean install, $(addsuffix /$(t), $(TARGET_DIRS)))
+TARGET_DIRS_RULES := $(foreach t, all fuzz clean install, $(addsuffix /$(t), $(TARGET_DIRS)))
 
 SOFTMMU_ALL_RULES=$(filter %-softmmu/all, $(TARGET_DIRS_RULES))
 $(SOFTMMU_ALL_RULES): $(authz-obj-y)
@@ -490,6 +490,15 @@ ifdef DECOMPRESS_EDK2_BLOBS
 $(SOFTMMU_ALL_RULES): $(edk2-decompressed)
 endif
 
+SOFTMMU_FUZZ_RULES=$(filter %-softmmu/fuzz, $(TARGET_DIRS_RULES))
+$(SOFTMMU_FUZZ_RULES): $(authz-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(block-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(chardev-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(crypto-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(io-obj-y)
+$(SOFTMMU_FUZZ_RULES): config-all-devices.mak
+$(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
+
 .PHONY: $(TARGET_DIRS_RULES)
 # The $(TARGET_DIRS_RULES) are of the form SUBDIR/GOAL, so that
 # $(dir $@) yields the sub-directory, and $(notdir $@) yields the sub-goal
@@ -540,6 +549,9 @@ subdir-slirp: slirp/all
 $(filter %/all, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
 	$(qom-obj-y)
 
+$(filter %/fuzz, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
+	$(qom-obj-y) $(crypto-user-obj-$(CONFIG_USER_ONLY))
+
 ROM_DIRS = $(addprefix pc-bios/, $(ROMS))
 ROM_DIRS_RULES=$(foreach t, all clean, $(addsuffix /$(t), $(ROM_DIRS)))
 # Only keep -O and -g cflags
@@ -549,6 +561,7 @@ $(ROM_DIRS_RULES):
 
 .PHONY: recurse-all recurse-clean recurse-install
 recurse-all: $(addsuffix /all, $(TARGET_DIRS) $(ROM_DIRS))
+recurse-fuzz: $(addsuffix /fuzz, $(TARGET_DIRS) $(ROM_DIRS))
 recurse-clean: $(addsuffix /clean, $(TARGET_DIRS) $(ROM_DIRS))
 recurse-install: $(addsuffix /install, $(TARGET_DIRS))
 $(addsuffix /install, $(TARGET_DIRS)): all
diff --git a/Makefile.target b/Makefile.target
index 6f4dd72022..2d43dc586a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -228,6 +228,22 @@ ifdef CONFIG_TRACE_SYSTEMTAP
 	rm -f *.stp
 endif
 
+ifdef CONFIG_FUZZ
+include $(SRC_PATH)/tests/qtest/fuzz/Makefile.include
+include $(SRC_PATH)/tests/qtest/Makefile.include
+
+fuzz: fuzz-vars
+fuzz-vars: QEMU_CFLAGS := $(FUZZ_CFLAGS) $(QEMU_CFLAGS)
+fuzz-vars: QEMU_LDFLAGS := $(FUZZ_LDFLAGS) $(QEMU_LDFLAGS)
+fuzz-vars: $(QEMU_PROG_FUZZ)
+dummy := $(call unnest-vars,, fuzz-obj-y)
+
+
+$(QEMU_PROG_FUZZ): config-devices.mak $(all-obj-y) $(COMMON_LDADDS) $(fuzz-obj-y)
+	$(call LINK, $(filter-out %.mak, $^))
+
+endif
+
 install: all
 ifneq ($(PROGS),)
 	$(call install-prog,$(PROGS),$(DESTDIR)$(bindir))
-- 
2.24.1

[PULL 27/31] fuzz: add configure flag --enable-fuzzing

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-19-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 configure | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/configure b/configure
index d57261e3ad..48d6f89d57 100755
--- a/configure
+++ b/configure
@@ -505,6 +505,7 @@ debug_mutex="no"
 libpmem=""
 default_devices="yes"
 plugins="no"
+fuzzing="no"
 
 supported_cpu="no"
 supported_os="no"
@@ -635,6 +636,15 @@ int main(void) { return 0; }
 EOF
 }
 
+write_c_fuzzer_skeleton() {
+    cat > $TMPC <<EOF
+#include <stdint.h>
+#include <sys/types.h>
+int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
+int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { return 0; }
+EOF
+}
+
 if check_define __linux__ ; then
   targetos="Linux"
 elif check_define _WIN32 ; then
@@ -1558,6 +1568,10 @@ for opt do
   ;;
   --disable-containers) use_containers="no"
   ;;
+  --enable-fuzzing) fuzzing=yes
+  ;;
+  --disable-fuzzing) fuzzing=no
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -6072,6 +6086,15 @@ EOF
   fi
 fi
 
+##########################################
+# checks for fuzzer
+if test "$fuzzing" = "yes" ; then
+  write_c_fuzzer_skeleton
+  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address,fuzzer" ""; then
+      have_fuzzer=yes
+  fi
+fi
+
 ##########################################
 # check for libpmem
 
@@ -6666,6 +6689,7 @@ echo "libpmem support   $libpmem"
 echo "libudev           $libudev"
 echo "default devices   $default_devices"
 echo "plugin support    $plugins"
+echo "fuzzing support   $fuzzing"
 
 if test "$supported_cpu" = "no"; then
     echo
@@ -7504,6 +7528,16 @@ fi
 if test "$sheepdog" = "yes" ; then
   echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
 fi
+if test "$fuzzing" = "yes" ; then
+  if test "$have_fuzzer" = "yes"; then
+    FUZZ_LDFLAGS=" -fsanitize=address,fuzzer"
+    FUZZ_CFLAGS=" -fsanitize=address,fuzzer"
+    CFLAGS=" -fsanitize=address,fuzzer-no-link"
+  else
+    error_exit "Your compiler doesn't support -fsanitize=address,fuzzer"
+    exit 1
+  fi
+fi
 
 if test "$plugins" = "yes" ; then
     echo "CONFIG_PLUGIN=y" >> $config_host_mak
@@ -7605,6 +7639,11 @@ if test "$libudev" != "no"; then
     echo "CONFIG_LIBUDEV=y" >> $config_host_mak
     echo "LIBUDEV_LIBS=$libudev_libs" >> $config_host_mak
 fi
+if test "$fuzzing" != "no"; then
+    echo "CONFIG_FUZZ=y" >> $config_host_mak
+    echo "FUZZ_CFLAGS=$FUZZ_CFLAGS" >> $config_host_mak
+    echo "FUZZ_LDFLAGS=$FUZZ_LDFLAGS" >> $config_host_mak
+fi
 
 if test "$edk2_blobs" = "yes" ; then
   echo "DECOMPRESS_EDK2_BLOBS=y" >> $config_host_mak
-- 
2.24.1

[PULL 28/31] fuzz: add i440fx fuzz targets

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

These three targets should simply fuzz reads/writes to a couple ioports,
but they mostly serve as examples of different ways to write targets.
They demonstrate using qtest and qos for fuzzing, as well as using
rebooting and forking to reset state, or not resetting it at all.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-20-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/fuzz/Makefile.include |   3 +
 tests/qtest/fuzz/i440fx_fuzz.c    | 193 ++++++++++++++++++++++++++++++
 2 files changed, 196 insertions(+)
 create mode 100644 tests/qtest/fuzz/i440fx_fuzz.c

diff --git a/tests/qtest/fuzz/Makefile.include b/tests/qtest/fuzz/Makefile.include
index e3bdd33ff4..38b8cdd9f1 100644
--- a/tests/qtest/fuzz/Makefile.include
+++ b/tests/qtest/fuzz/Makefile.include
@@ -6,6 +6,9 @@ fuzz-obj-y += tests/qtest/fuzz/fuzz.o # Fuzzer skeleton
 fuzz-obj-y += tests/qtest/fuzz/fork_fuzz.o
 fuzz-obj-y += tests/qtest/fuzz/qos_fuzz.o
 
+# Targets
+fuzz-obj-y += tests/qtest/fuzz/i440fx_fuzz.o
+
 FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest
 
 # Linker Script to force coverage-counters into known regions which we can mark
diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
new file mode 100644
index 0000000000..ab5f112584
--- /dev/null
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -0,0 +1,193 @@
+/*
+ * I440FX Fuzzing Target
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "qemu/main-loop.h"
+#include "tests/qtest/libqtest.h"
+#include "tests/qtest/libqos/pci.h"
+#include "tests/qtest/libqos/pci-pc.h"
+#include "fuzz.h"
+#include "fuzz/qos_fuzz.h"
+#include "fuzz/fork_fuzz.h"
+
+
+#define I440FX_PCI_HOST_BRIDGE_CFG 0xcf8
+#define I440FX_PCI_HOST_BRIDGE_DATA 0xcfc
+
+/*
+ * the input to the fuzzing functions below is a buffer of random bytes. we
+ * want to convert these bytes into a sequence of qtest or qos calls. to do
+ * this we define some opcodes:
+ */
+enum action_id {
+    WRITEB,
+    WRITEW,
+    WRITEL,
+    READB,
+    READW,
+    READL,
+    ACTION_MAX
+};
+
+static void i440fx_fuzz_qtest(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+    /*
+     * loop over the Data, breaking it up into actions. each action has an
+     * opcode, address offset and value
+     */
+    typedef struct QTestFuzzAction {
+        uint8_t opcode;
+        uint8_t addr;
+        uint32_t value;
+    } QTestFuzzAction;
+    QTestFuzzAction a;
+
+    while (Size >= sizeof(a)) {
+        /* make a copy of the action so we can normalize the values in-place */
+        memcpy(&a, Data, sizeof(a));
+        /* select between two i440fx Port IO addresses */
+        uint16_t addr = a.addr % 2 ? I440FX_PCI_HOST_BRIDGE_CFG :
+                                      I440FX_PCI_HOST_BRIDGE_DATA;
+        switch (a.opcode % ACTION_MAX) {
+        case WRITEB:
+            qtest_outb(s, addr, (uint8_t)a.value);
+            break;
+        case WRITEW:
+            qtest_outw(s, addr, (uint16_t)a.value);
+            break;
+        case WRITEL:
+            qtest_outl(s, addr, (uint32_t)a.value);
+            break;
+        case READB:
+            qtest_inb(s, addr);
+            break;
+        case READW:
+            qtest_inw(s, addr);
+            break;
+        case READL:
+            qtest_inl(s, addr);
+            break;
+        }
+        /* Move to the next operation */
+        Size -= sizeof(a);
+        Data += sizeof(a);
+    }
+    flush_events(s);
+}
+
+static void i440fx_fuzz_qos(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+    /*
+     * Same as i440fx_fuzz_qtest, but using QOS. devfn is incorporated into the
+     * value written over Port IO
+     */
+    typedef struct QOSFuzzAction {
+        uint8_t opcode;
+        uint8_t offset;
+        int devfn;
+        uint32_t value;
+    } QOSFuzzAction;
+
+    static QPCIBus *bus;
+    if (!bus) {
+        bus = qpci_new_pc(s, fuzz_qos_alloc);
+    }
+
+    QOSFuzzAction a;
+    while (Size >= sizeof(a)) {
+        memcpy(&a, Data, sizeof(a));
+        switch (a.opcode % ACTION_MAX) {
+        case WRITEB:
+            bus->config_writeb(bus, a.devfn, a.offset, (uint8_t)a.value);
+            break;
+        case WRITEW:
+            bus->config_writew(bus, a.devfn, a.offset, (uint16_t)a.value);
+            break;
+        case WRITEL:
+            bus->config_writel(bus, a.devfn, a.offset, (uint32_t)a.value);
+            break;
+        case READB:
+            bus->config_readb(bus, a.devfn, a.offset);
+            break;
+        case READW:
+            bus->config_readw(bus, a.devfn, a.offset);
+            break;
+        case READL:
+            bus->config_readl(bus, a.devfn, a.offset);
+            break;
+        }
+        Size -= sizeof(a);
+        Data += sizeof(a);
+    }
+    flush_events(s);
+}
+
+static void i440fx_fuzz_qos_fork(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+    if (fork() == 0) {
+        i440fx_fuzz_qos(s, Data, Size);
+        _Exit(0);
+    } else {
+        wait(NULL);
+    }
+}
+
+static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
+                                       "-m 0 -display none";
+static const char *i440fx_argv(FuzzTarget *t)
+{
+    return i440fx_qtest_argv;
+}
+
+static void fork_init(void)
+{
+    counter_shm_init();
+}
+
+static void register_pci_fuzz_targets(void)
+{
+    /* Uses simple qtest commands and reboots to reset state */
+    fuzz_add_target(&(FuzzTarget){
+                .name = "i440fx-qtest-reboot-fuzz",
+                .description = "Fuzz the i440fx using raw qtest commands and"
+                               "rebooting after each run",
+                .get_init_cmdline = i440fx_argv,
+                .fuzz = i440fx_fuzz_qtest});
+
+    /* Uses libqos and forks to prevent state leakage */
+    fuzz_add_qos_target(&(FuzzTarget){
+                .name = "i440fx-qos-fork-fuzz",
+                .description = "Fuzz the i440fx using raw qtest commands and"
+                               "rebooting after each run",
+                .pre_vm_init = &fork_init,
+                .fuzz = i440fx_fuzz_qos_fork,},
+                "i440FX-pcihost",
+                &(QOSGraphTestOptions){}
+                );
+
+    /*
+     * Uses libqos. Doesn't do anything to reset state. Note that if we were to
+     * reboot after each run, we would also have to redo the qos-related
+     * initialization (qos_init_path)
+     */
+    fuzz_add_qos_target(&(FuzzTarget){
+                .name = "i440fx-qos-noreset-fuzz",
+                .description = "Fuzz the i440fx using raw qtest commands and"
+                               "rebooting after each run",
+                .fuzz = i440fx_fuzz_qos,},
+                "i440FX-pcihost",
+                &(QOSGraphTestOptions){}
+                );
+}
+
+fuzz_target_init(register_pci_fuzz_targets);
-- 
2.24.1

[PULL 29/31] fuzz: add virtio-net fuzz target

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

The virtio-net fuzz target feeds inputs to all three virtio-net
virtqueues, and uses forking to avoid leaking state between fuzz runs.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-21-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/fuzz/Makefile.include  |   1 +
 tests/qtest/fuzz/virtio_net_fuzz.c | 198 +++++++++++++++++++++++++++++
 2 files changed, 199 insertions(+)
 create mode 100644 tests/qtest/fuzz/virtio_net_fuzz.c

diff --git a/tests/qtest/fuzz/Makefile.include b/tests/qtest/fuzz/Makefile.include
index 38b8cdd9f1..77385777ef 100644
--- a/tests/qtest/fuzz/Makefile.include
+++ b/tests/qtest/fuzz/Makefile.include
@@ -8,6 +8,7 @@ fuzz-obj-y += tests/qtest/fuzz/qos_fuzz.o
 
 # Targets
 fuzz-obj-y += tests/qtest/fuzz/i440fx_fuzz.o
+fuzz-obj-y += tests/qtest/fuzz/virtio_net_fuzz.o
 
 FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest
 
diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
new file mode 100644
index 0000000000..d08a47e278
--- /dev/null
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -0,0 +1,198 @@
+/*
+ * virtio-net Fuzzing Target
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "standard-headers/linux/virtio_config.h"
+#include "tests/qtest/libqtest.h"
+#include "tests/qtest/libqos/virtio-net.h"
+#include "fuzz.h"
+#include "fork_fuzz.h"
+#include "qos_fuzz.h"
+
+
+#define QVIRTIO_NET_TIMEOUT_US (30 * 1000 * 1000)
+#define QVIRTIO_RX_VQ 0
+#define QVIRTIO_TX_VQ 1
+#define QVIRTIO_CTRL_VQ 2
+
+static int sockfds[2];
+static bool sockfds_initialized;
+
+static void virtio_net_fuzz_multi(QTestState *s,
+        const unsigned char *Data, size_t Size, bool check_used)
+{
+    typedef struct vq_action {
+        uint8_t queue;
+        uint8_t length;
+        uint8_t write;
+        uint8_t next;
+        uint8_t rx;
+    } vq_action;
+
+    uint32_t free_head = 0;
+
+    QGuestAllocator *t_alloc = fuzz_qos_alloc;
+
+    QVirtioNet *net_if = fuzz_qos_obj;
+    QVirtioDevice *dev = net_if->vdev;
+    QVirtQueue *q;
+    vq_action vqa;
+    while (Size >= sizeof(vqa)) {
+        memcpy(&vqa, Data, sizeof(vqa));
+        Data += sizeof(vqa);
+        Size -= sizeof(vqa);
+
+        q = net_if->queues[vqa.queue % 3];
+
+        vqa.length = vqa.length >= Size ? Size :  vqa.length;
+
+        /*
+         * Only attempt to write incoming packets, when using the socket
+         * backend. Otherwise, always place the input on a virtqueue.
+         */
+        if (vqa.rx && sockfds_initialized) {
+            write(sockfds[0], Data, vqa.length);
+        } else {
+            vqa.rx = 0;
+            uint64_t req_addr = guest_alloc(t_alloc, vqa.length);
+            /*
+             * If checking used ring, ensure that the fuzzer doesn't trigger
+             * trivial asserion failure on zero-zied buffer
+             */
+            qtest_memwrite(s, req_addr, Data, vqa.length);
+
+
+            free_head = qvirtqueue_add(s, q, req_addr, vqa.length,
+                    vqa.write, vqa.next);
+            qvirtqueue_add(s, q, req_addr, vqa.length, vqa.write , vqa.next);
+            qvirtqueue_kick(s, dev, q, free_head);
+        }
+
+        /* Run the main loop */
+        qtest_clock_step(s, 100);
+        flush_events(s);
+
+        /* Wait on used descriptors */
+        if (check_used && !vqa.rx) {
+            gint64 start_time = g_get_monotonic_time();
+            /*
+             * normally, we could just use qvirtio_wait_used_elem, but since we
+             * must manually run the main-loop for all the bhs to run, we use
+             * this hack with flush_events(), to run the main_loop
+             */
+            while (!vqa.rx && q != net_if->queues[QVIRTIO_RX_VQ]) {
+                uint32_t got_desc_idx;
+                /* Input led to a virtio_error */
+                if (dev->bus->get_status(dev) & VIRTIO_CONFIG_S_NEEDS_RESET) {
+                    break;
+                }
+                if (dev->bus->get_queue_isr_status(dev, q) &&
+                        qvirtqueue_get_buf(s, q, &got_desc_idx, NULL)) {
+                    g_assert_cmpint(got_desc_idx, ==, free_head);
+                    break;
+                }
+                g_assert(g_get_monotonic_time() - start_time
+                        <= QVIRTIO_NET_TIMEOUT_US);
+
+                /* Run the main loop */
+                qtest_clock_step(s, 100);
+                flush_events(s);
+            }
+        }
+        Data += vqa.length;
+        Size -= vqa.length;
+    }
+}
+
+static void virtio_net_fork_fuzz(QTestState *s,
+        const unsigned char *Data, size_t Size)
+{
+    if (fork() == 0) {
+        virtio_net_fuzz_multi(s, Data, Size, false);
+        flush_events(s);
+        _Exit(0);
+    } else {
+        wait(NULL);
+    }
+}
+
+static void virtio_net_fork_fuzz_check_used(QTestState *s,
+        const unsigned char *Data, size_t Size)
+{
+    if (fork() == 0) {
+        virtio_net_fuzz_multi(s, Data, Size, true);
+        flush_events(s);
+        _Exit(0);
+    } else {
+        wait(NULL);
+    }
+}
+
+static void virtio_net_pre_fuzz(QTestState *s)
+{
+    qos_init_path(s);
+    counter_shm_init();
+}
+
+static void *virtio_net_test_setup_socket(GString *cmd_line, void *arg)
+{
+    int ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sockfds);
+    g_assert_cmpint(ret, !=, -1);
+    fcntl(sockfds[0], F_SETFL, O_NONBLOCK);
+    sockfds_initialized = true;
+    g_string_append_printf(cmd_line, " -netdev socket,fd=%d,id=hs0 ",
+                           sockfds[1]);
+    return arg;
+}
+
+static void *virtio_net_test_setup_user(GString *cmd_line, void *arg)
+{
+    g_string_append_printf(cmd_line, " -netdev user,id=hs0 ");
+    return arg;
+}
+
+static void register_virtio_net_fuzz_targets(void)
+{
+    fuzz_add_qos_target(&(FuzzTarget){
+            .name = "virtio-net-socket",
+            .description = "Fuzz the virtio-net virtual queues. Fuzz incoming "
+            "traffic using the socket backend",
+            .pre_fuzz = &virtio_net_pre_fuzz,
+            .fuzz = virtio_net_fork_fuzz,},
+            "virtio-net",
+            &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
+            );
+
+    fuzz_add_qos_target(&(FuzzTarget){
+            .name = "virtio-net-socket-check-used",
+            .description = "Fuzz the virtio-net virtual queues. Wait for the "
+            "descriptors to be used. Timeout may indicate improperly handled "
+            "input",
+            .pre_fuzz = &virtio_net_pre_fuzz,
+            .fuzz = virtio_net_fork_fuzz_check_used,},
+            "virtio-net",
+            &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
+            );
+    fuzz_add_qos_target(&(FuzzTarget){
+            .name = "virtio-net-slirp",
+            .description = "Fuzz the virtio-net virtual queues with the slirp "
+            " backend. Warning: May result in network traffic emitted from the "
+            " process. Run in an isolated network environment.",
+            .pre_fuzz = &virtio_net_pre_fuzz,
+            .fuzz = virtio_net_fork_fuzz,},
+            "virtio-net",
+            &(QOSGraphTestOptions){.before = virtio_net_test_setup_user}
+            );
+}
+
+fuzz_target_init(register_virtio_net_fuzz_targets);
-- 
2.24.1

[PULL 30/31] fuzz: add virtio-scsi fuzz target

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

The virtio-scsi fuzz target sets up and fuzzes the available virtio-scsi
queues. After an element is placed on a queue, the fuzzer can select
whether to perform a kick, or continue adding elements.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-22-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/fuzz/Makefile.include   |   1 +
 tests/qtest/fuzz/virtio_scsi_fuzz.c | 213 ++++++++++++++++++++++++++++
 2 files changed, 214 insertions(+)
 create mode 100644 tests/qtest/fuzz/virtio_scsi_fuzz.c

diff --git a/tests/qtest/fuzz/Makefile.include b/tests/qtest/fuzz/Makefile.include
index 77385777ef..cde3e9636c 100644
--- a/tests/qtest/fuzz/Makefile.include
+++ b/tests/qtest/fuzz/Makefile.include
@@ -9,6 +9,7 @@ fuzz-obj-y += tests/qtest/fuzz/qos_fuzz.o
 # Targets
 fuzz-obj-y += tests/qtest/fuzz/i440fx_fuzz.o
 fuzz-obj-y += tests/qtest/fuzz/virtio_net_fuzz.o
+fuzz-obj-y += tests/qtest/fuzz/virtio_scsi_fuzz.o
 
 FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest
 
diff --git a/tests/qtest/fuzz/virtio_scsi_fuzz.c b/tests/qtest/fuzz/virtio_scsi_fuzz.c
new file mode 100644
index 0000000000..3b95247f12
--- /dev/null
+++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
@@ -0,0 +1,213 @@
+/*
+ * virtio-serial Fuzzing Target
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "tests/qtest/libqtest.h"
+#include "libqos/virtio-scsi.h"
+#include "libqos/virtio.h"
+#include "libqos/virtio-pci.h"
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_pci.h"
+#include "standard-headers/linux/virtio_scsi.h"
+#include "fuzz.h"
+#include "fork_fuzz.h"
+#include "qos_fuzz.h"
+
+#define PCI_SLOT                0x02
+#define PCI_FN                  0x00
+#define QVIRTIO_SCSI_TIMEOUT_US (1 * 1000 * 1000)
+
+#define MAX_NUM_QUEUES 64
+
+/* Based on tests/virtio-scsi-test.c */
+typedef struct {
+    int num_queues;
+    QVirtQueue *vq[MAX_NUM_QUEUES + 2];
+} QVirtioSCSIQueues;
+
+static QVirtioSCSIQueues *qvirtio_scsi_init(QVirtioDevice *dev, uint64_t mask)
+{
+    QVirtioSCSIQueues *vs;
+    uint64_t feat;
+    int i;
+
+    vs = g_new0(QVirtioSCSIQueues, 1);
+
+    feat = qvirtio_get_features(dev);
+    if (mask) {
+        feat &= ~QVIRTIO_F_BAD_FEATURE | mask;
+    } else {
+        feat &= ~(QVIRTIO_F_BAD_FEATURE | (1ull << VIRTIO_RING_F_EVENT_IDX));
+    }
+    qvirtio_set_features(dev, feat);
+
+    vs->num_queues = qvirtio_config_readl(dev, 0);
+
+    for (i = 0; i < vs->num_queues + 2; i++) {
+        vs->vq[i] = qvirtqueue_setup(dev, fuzz_qos_alloc, i);
+    }
+
+    qvirtio_set_driver_ok(dev);
+
+    return vs;
+}
+
+static void virtio_scsi_fuzz(QTestState *s, QVirtioSCSIQueues* queues,
+        const unsigned char *Data, size_t Size)
+{
+    /*
+     * Data is a sequence of random bytes. We split them up into "actions",
+     * followed by data:
+     * [vqa][dddddddd][vqa][dddd][vqa][dddddddddddd] ...
+     * The length of the data is specified by the preceding vqa.length
+     */
+    typedef struct vq_action {
+        uint8_t queue;
+        uint8_t length;
+        uint8_t write;
+        uint8_t next;
+        uint8_t kick;
+    } vq_action;
+
+    /* Keep track of the free head for each queue we interact with */
+    bool vq_touched[MAX_NUM_QUEUES + 2] = {0};
+    uint32_t free_head[MAX_NUM_QUEUES + 2];
+
+    QGuestAllocator *t_alloc = fuzz_qos_alloc;
+
+    QVirtioSCSI *scsi = fuzz_qos_obj;
+    QVirtioDevice *dev = scsi->vdev;
+    QVirtQueue *q;
+    vq_action vqa;
+    while (Size >= sizeof(vqa)) {
+        /* Copy the action, so we can normalize length, queue and flags */
+        memcpy(&vqa, Data, sizeof(vqa));
+
+        Data += sizeof(vqa);
+        Size -= sizeof(vqa);
+
+        vqa.queue = vqa.queue % queues->num_queues;
+        /* Cap length at the number of remaining bytes in data */
+        vqa.length = vqa.length >= Size ? Size : vqa.length;
+        vqa.write = vqa.write & 1;
+        vqa.next = vqa.next & 1;
+        vqa.kick = vqa.kick & 1;
+
+
+        q = queues->vq[vqa.queue];
+
+        /* Copy the data into ram, and place it on the virtqueue */
+        uint64_t req_addr = guest_alloc(t_alloc, vqa.length);
+        qtest_memwrite(s, req_addr, Data, vqa.length);
+        if (vq_touched[vqa.queue] == 0) {
+            vq_touched[vqa.queue] = 1;
+            free_head[vqa.queue] = qvirtqueue_add(s, q, req_addr, vqa.length,
+                    vqa.write, vqa.next);
+        } else {
+            qvirtqueue_add(s, q, req_addr, vqa.length, vqa.write , vqa.next);
+        }
+
+        if (vqa.kick) {
+            qvirtqueue_kick(s, dev, q, free_head[vqa.queue]);
+            free_head[vqa.queue] = 0;
+        }
+        Data += vqa.length;
+        Size -= vqa.length;
+    }
+    /* In the end, kick each queue we interacted with */
+    for (int i = 0; i < MAX_NUM_QUEUES + 2; i++) {
+        if (vq_touched[i]) {
+            qvirtqueue_kick(s, dev, queues->vq[i], free_head[i]);
+        }
+    }
+}
+
+static void virtio_scsi_fork_fuzz(QTestState *s,
+        const unsigned char *Data, size_t Size)
+{
+    QVirtioSCSI *scsi = fuzz_qos_obj;
+    static QVirtioSCSIQueues *queues;
+    if (!queues) {
+        queues = qvirtio_scsi_init(scsi->vdev, 0);
+    }
+    if (fork() == 0) {
+        virtio_scsi_fuzz(s, queues, Data, Size);
+        flush_events(s);
+        _Exit(0);
+    } else {
+        wait(NULL);
+    }
+}
+
+static void virtio_scsi_with_flag_fuzz(QTestState *s,
+        const unsigned char *Data, size_t Size)
+{
+    QVirtioSCSI *scsi = fuzz_qos_obj;
+    static QVirtioSCSIQueues *queues;
+
+    if (fork() == 0) {
+        if (Size >= sizeof(uint64_t)) {
+            queues = qvirtio_scsi_init(scsi->vdev, *(uint64_t *)Data);
+            virtio_scsi_fuzz(s, queues,
+                             Data + sizeof(uint64_t), Size - sizeof(uint64_t));
+            flush_events(s);
+        }
+        _Exit(0);
+    } else {
+        wait(NULL);
+    }
+}
+
+static void virtio_scsi_pre_fuzz(QTestState *s)
+{
+    qos_init_path(s);
+    counter_shm_init();
+}
+
+static void *virtio_scsi_test_setup(GString *cmd_line, void *arg)
+{
+    g_string_append(cmd_line,
+                    " -drive file=blkdebug::null-co://,"
+                    "file.image.read-zeroes=on,"
+                    "if=none,id=dr1,format=raw,file.align=4k "
+                    "-device scsi-hd,drive=dr1,lun=0,scsi-id=1");
+    return arg;
+}
+
+
+static void register_virtio_scsi_fuzz_targets(void)
+{
+    fuzz_add_qos_target(&(FuzzTarget){
+                .name = "virtio-scsi-fuzz",
+                .description = "Fuzz the virtio-scsi virtual queues, forking"
+                                "for each fuzz run",
+                .pre_vm_init = &counter_shm_init,
+                .pre_fuzz = &virtio_scsi_pre_fuzz,
+                .fuzz = virtio_scsi_fork_fuzz,},
+                "virtio-scsi",
+                &(QOSGraphTestOptions){.before = virtio_scsi_test_setup}
+                );
+
+    fuzz_add_qos_target(&(FuzzTarget){
+                .name = "virtio-scsi-flags-fuzz",
+                .description = "Fuzz the virtio-scsi virtual queues, forking"
+                "for each fuzz run (also fuzzes the virtio flags)",
+                .pre_vm_init = &counter_shm_init,
+                .pre_fuzz = &virtio_scsi_pre_fuzz,
+                .fuzz = virtio_scsi_with_flag_fuzz,},
+                "virtio-scsi",
+                &(QOSGraphTestOptions){.before = virtio_scsi_test_setup}
+                );
+}
+
+fuzz_target_init(register_virtio_scsi_fuzz_targets);
-- 
2.24.1

[PULL 31/31] fuzz: add documentation to docs/devel/

[Stefan Hajnoczi]

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20200220041118.23264-23-alxndr@bu.edu
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/devel/fuzzing.txt | 116 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)
 create mode 100644 docs/devel/fuzzing.txt

diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
new file mode 100644
index 0000000000..324d2cd92b
--- /dev/null
+++ b/docs/devel/fuzzing.txt
@@ -0,0 +1,116 @@
+= Fuzzing =
+
+== Introduction ==
+
+This document describes the virtual-device fuzzing infrastructure in QEMU and
+how to use it to implement additional fuzzers.
+
+== Basics ==
+
+Fuzzing operates by passing inputs to an entry point/target function. The
+fuzzer tracks the code coverage triggered by the input. Based on these
+findings, the fuzzer mutates the input and repeats the fuzzing.
+
+To fuzz QEMU, we rely on libfuzzer. Unlike other fuzzers such as AFL, libfuzzer
+is an _in-process_ fuzzer. For the developer, this means that it is their
+responsibility to ensure that state is reset between fuzzing-runs.
+
+== Building the fuzzers ==
+
+NOTE: If possible, build a 32-bit binary. When forking, the 32-bit fuzzer is
+much faster, since the page-map has a smaller size. This is due to the fact that
+AddressSanitizer mmaps ~20TB of memory, as part of its detection. This results
+in a large page-map, and a much slower fork().
+
+To build the fuzzers, install a recent version of clang:
+Configure with (substitute the clang binaries with the version you installed):
+
+    CC=clang-8 CXX=clang++-8 /path/to/configure --enable-fuzzing
+
+Fuzz targets are built similarly to system/softmmu:
+
+    make i386-softmmu/fuzz
+
+This builds ./i386-softmmu/qemu-fuzz-i386
+
+The first option to this command is: --fuzz_taget=FUZZ_NAME
+To list all of the available fuzzers run qemu-fuzz-i386 with no arguments.
+
+eg:
+    ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=virtio-net-fork-fuzz
+
+Internally, libfuzzer parses all arguments that do not begin with "--".
+Information about these is available by passing -help=1
+
+Now the only thing left to do is wait for the fuzzer to trigger potential
+crashes.
+
+== Adding a new fuzzer ==
+Coverage over virtual devices can be improved by adding additional fuzzers.
+Fuzzers are kept in tests/qtest/fuzz/ and should be added to
+tests/qtest/fuzz/Makefile.include
+
+Fuzzers can rely on both qtest and libqos to communicate with virtual devices.
+
+1. Create a new source file. For example ``tests/qtest/fuzz/foo-device-fuzz.c``.
+
+2. Write the fuzzing code using the libqtest/libqos API. See existing fuzzers
+for reference.
+
+3. Register the fuzzer in ``tests/fuzz/Makefile.include`` by appending the
+corresponding object to fuzz-obj-y
+
+Fuzzers can be more-or-less thought of as special qtest programs which can
+modify the qtest commands and/or qtest command arguments based on inputs
+provided by libfuzzer. Libfuzzer passes a byte array and length. Commonly the
+fuzzer loops over the byte-array interpreting it as a list of qtest commands,
+addresses, or values.
+
+= Implementation Details =
+
+== The Fuzzer's Lifecycle ==
+
+The fuzzer has two entrypoints that libfuzzer calls. libfuzzer provides it's
+own main(), which performs some setup, and calls the entrypoints:
+
+LLVMFuzzerInitialize: called prior to fuzzing. Used to initialize all of the
+necessary state
+
+LLVMFuzzerTestOneInput: called for each fuzzing run. Processes the input and
+resets the state at the end of each run.
+
+In more detail:
+
+LLVMFuzzerInitialize parses the arguments to the fuzzer (must start with two
+dashes, so they are ignored by libfuzzer main()). Currently, the arguments
+select the fuzz target. Then, the qtest client is initialized. If the target
+requires qos, qgraph is set up and the QOM/LIBQOS modules are initialized.
+Then the QGraph is walked and the QEMU cmd_line is determined and saved.
+
+After this, the vl.c:qemu__main is called to set up the guest. There are
+target-specific hooks that can be called before and after qemu_main, for
+additional setup(e.g. PCI setup, or VM snapshotting).
+
+LLVMFuzzerTestOneInput: Uses qtest/qos functions to act based on the fuzz
+input. It is also responsible for manually calling the main loop/main_loop_wait
+to ensure that bottom halves are executed and any cleanup required before the
+next input.
+
+Since the same process is reused for many fuzzing runs, QEMU state needs to
+be reset at the end of each run. There are currently two implemented
+options for resetting state:
+1. Reboot the guest between runs.
+   Pros: Straightforward and fast for simple fuzz targets.
+   Cons: Depending on the device, does not reset all device state. If the
+   device requires some initialization prior to being ready for fuzzing
+   (common for QOS-based targets), this initialization needs to be done after
+   each reboot.
+   Example target: i440fx-qtest-reboot-fuzz
+2. Run each test case in a separate forked process and copy the coverage
+   information back to the parent. This is fairly similar to AFL's "deferred"
+   fork-server mode [3]
+   Pros: Relatively fast. Devices only need to be initialized once. No need
+   to do slow reboots or vmloads.
+   Cons: Not officially supported by libfuzzer. Does not work well for devices
+   that rely on dedicated threads.
+   Example target: virtio-net-fork-fuzz
-- 
2.24.1

Re: [PULL 00/31] Block patches

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200222085030.1760640-1-stefanha@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PULL 00/31] Block patches
Message-id: 20200222085030.1760640-1-stefanha@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200222085030.1760640-1-stefanha@redhat.com -> patchew/20200222085030.1760640-1-stefanha@redhat.com
Switched to a new branch 'test'
dc382b1 fuzz: add documentation to docs/devel/
003bbd0 fuzz: add virtio-scsi fuzz target
e8802f9 fuzz: add virtio-net fuzz target
69e13ad fuzz: add i440fx fuzz targets
4083dcb fuzz: add configure flag --enable-fuzzing
c3d2240 fuzz: add target/fuzz makefile rules
7c4d2fb fuzz: add support for qos-assisted fuzz targets
8518c8d fuzz: support for fork-based fuzzing.
6a286ed main: keep rcu_atfork callback enabled for qtest
ed941c7 exec: keep ram block across fork when using qtest
e24313f fuzz: add fuzzer skeleton
408df07 libqos: move useful qos-test funcs to qos_external
d7daac7 libqos: split qos-test and libqos makefile vars
4bceb01 libqos: rename i2c_send and i2c_recv
4ea92f2 qtest: add in-process incoming command handler
0adfc33 libqtest: make bufwrite rely on the TransportOps
780da9f libqtest: add a layer of abstraction to send/recv
bcc084b qtest: add qtest_server_send abstraction
266c2c5 fuzz: add FUZZ_TARGET module type
66b6939 module: check module wasn't already initialized
009494e softmmu: split off vl.c:main() into main.c
28796ad softmmu: move vl.c to softmmu/
6b8a3f7 aio-posix: make AioHandler dispatch O(1) with epoll
b839357 aio-posix: make AioHandler deletion O(1)
5c9397d qemu/queue.h: add QLIST_SAFE_REMOVE()
8cd11b4 aio-posix: don't pass ns timeout to epoll_wait()
4252dd3 aio-posix: fix use after leaving scope in aio_poll()
e850ad8 util/async: make bh_aio_poll() O(1)
3ae8e2d rcu_queue: add QSLIST functions
b593eb3 aio-posix: avoid reacquiring rcu_read_lock() when polling
6fbbd86 virtio: increase virtqueue size for virtio-scsi and virtio-blk

=== OUTPUT BEGIN ===
1/31 Checking commit 6fbbd869010e (virtio: increase virtqueue size for virtio-scsi and virtio-blk)
2/31 Checking commit b593eb3a32e9 (aio-posix: avoid reacquiring rcu_read_lock() when polling)
3/31 Checking commit 3ae8e2d89202 (rcu_queue: add QSLIST functions)
WARNING: Block comments use a leading /* on a separate line
#29: FILE: include/qemu/queue.h:217:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#64: FILE: include/qemu/rcu_queue.h:278:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#69: FILE: include/qemu/rcu_queue.h:283:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#73: FILE: include/qemu/rcu_queue.h:287:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#86: FILE: include/qemu/rcu_queue.h:300:
+} while (/*CONSTCOND*/0)

ERROR: code indent should never use tabs
#144: FILE: tests/test-rcu-list.c:155:
+^I QSLIST_REMOVE_RCU(&Q_list_head, el, list_element, f)$

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#156: 
new file mode 100644

total: 1 errors, 6 warnings, 119 lines checked

Patch 3/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/31 Checking commit e850ad83574b (util/async: make bh_aio_poll() O(1))
5/31 Checking commit 4252dd376d4c (aio-posix: fix use after leaving scope in aio_poll())
6/31 Checking commit 8cd11b46c0f8 (aio-posix: don't pass ns timeout to epoll_wait())
7/31 Checking commit 5c9397dad943 (qemu/queue.h: add QLIST_SAFE_REMOVE())
WARNING: Block comments use a leading /* on a separate line
#70: FILE: include/qemu/queue.h:159:
+} while (/*CONSTCOND*/0)

total: 0 errors, 1 warnings, 41 lines checked

Patch 7/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/31 Checking commit b83935774545 (aio-posix: make AioHandler deletion O(1))
9/31 Checking commit 6b8a3f79ba86 (aio-posix: make AioHandler dispatch O(1) with epoll)
10/31 Checking commit 28796ad5d822 (softmmu: move vl.c to softmmu/)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#83: 
new file mode 100644

total: 0 errors, 1 warnings, 42 lines checked

Patch 10/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/31 Checking commit 009494eddb10 (softmmu: split off vl.c:main() into main.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#71: 
new file mode 100644

WARNING: architecture specific defines should be avoided
#105: FILE: softmmu/main.c:30:
+#if defined(__APPLE__) || defined(main)

total: 0 errors, 2 warnings, 164 lines checked

Patch 11/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/31 Checking commit 66b69398ca5c (module: check module wasn't already initialized)
13/31 Checking commit 266c2c51ef94 (fuzz: add FUZZ_TARGET module type)
14/31 Checking commit bcc084b75586 (qtest: add qtest_server_send abstraction)
WARNING: line over 80 characters
#77: FILE: qtest.c:797:
+void qtest_server_set_send_handler(void (*send)(void*, const char*), void *opaque)

total: 0 errors, 1 warnings, 50 lines checked

Patch 14/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/31 Checking commit 780da9f5c54b (libqtest: add a layer of abstraction to send/recv)
16/31 Checking commit 0adfc33900fe (libqtest: make bufwrite rely on the TransportOps)
17/31 Checking commit 4ea92f2da690 (qtest: add in-process incoming command handler)
18/31 Checking commit 4bceb011189a (libqos: rename i2c_send and i2c_recv)
19/31 Checking commit d7daac746814 (libqos: split qos-test and libqos makefile vars)
20/31 Checking commit 408df0768420 (libqos: move useful qos-test funcs to qos_external)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

total: 0 errors, 1 warnings, 357 lines checked

Patch 20/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
21/31 Checking commit e24313ff11c7 (fuzz: add fuzzer skeleton)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

total: 0 errors, 1 warnings, 294 lines checked

Patch 21/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
22/31 Checking commit ed941c7f4d98 (exec: keep ram block across fork when using qtest)
23/31 Checking commit 6a286ed521a5 (main: keep rcu_atfork callback enabled for qtest)
24/31 Checking commit 8518c8d513af (fuzz: support for fork-based fuzzing.)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#43: 
new file mode 100644

total: 0 errors, 1 warnings, 125 lines checked

Patch 24/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
25/31 Checking commit 7c4d2fbb0984 (fuzz: add support for qos-assisted fuzz targets)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#30: 
new file mode 100644

WARNING: line over 80 characters
#157: FILE: tests/qtest/fuzz/qos_fuzz.c:123:
+    /* etype set to QEDGE_CONSUMED_BY so that machine can add to the command line */

total: 0 errors, 2 warnings, 277 lines checked

Patch 25/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/31 Checking commit c3d2240eaf7b (fuzz: add target/fuzz makefile rules)
27/31 Checking commit 4083dcb64bc1 (fuzz: add configure flag --enable-fuzzing)
28/31 Checking commit 69e13ad02539 (fuzz: add i440fx fuzz targets)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#34: 
new file mode 100644

total: 0 errors, 1 warnings, 202 lines checked

Patch 28/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
29/31 Checking commit e8802f951a1c (fuzz: add virtio-net fuzz target)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#30: 
new file mode 100644

total: 0 errors, 1 warnings, 205 lines checked

Patch 29/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
30/31 Checking commit 003bbd0f4e44 (fuzz: add virtio-scsi fuzz target)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#30: 
new file mode 100644

total: 0 errors, 1 warnings, 220 lines checked

Patch 30/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
31/31 Checking commit dc382b154f25 (fuzz: add documentation to docs/devel/)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#15: 
new file mode 100644

total: 0 errors, 1 warnings, 116 lines checked

Patch 31/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200222085030.1760640-1-stefanha@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PULL 24/31] fuzz: support for fork-based fuzzing.

[Eric Blake]

On 2/22/20 2:50 AM, Stefan Hajnoczi wrote:
> From: Alexander Bulekov <alxndr@bu.edu>
> 
> fork() is a simple way to ensure that state does not leak in between
> fuzzing runs. Unfortunately, the fuzzer mutation engine relies on
> bitmaps which contain coverage information for each fuzzing run, and
> these bitmaps should be copied from the child to the parent(where the
> mutation occurs). These bitmaps are created through compile-time
> instrumentation and they are not shared with fork()-ed processes, by
> default. To address this, we create a shared memory region, adjust its
> size and map it _over_ the counter region. Furthermore, libfuzzer
> doesn't generally expose the globals that specify the location of the
> counters/coverage bitmap. As a workaround, we rely on a custom linker
> script which forces all of the bitmaps we care about to be placed in a
> contiguous region, which is easy to locate and mmap over.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> Message-id: 20200220041118.23264-16-alxndr@bu.edu
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---

Random drive-by observation:

> +++ b/tests/qtest/fuzz/fork_fuzz.ld
> @@ -0,0 +1,37 @@
> +/* We adjust linker script modification to place all of the stuff that needs to
> + * persist across fuzzing runs into a contiguous seciton of memory. Then, it is

section


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PULL 00/31] Block patches

[Stefan Hajnoczi]

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

On Sat, Feb 22, 2020 at 01:13:32AM -0800, no-reply@patchew.org wrote:
> This series seems to have some coding style problems. See output below for
> more information:

The checkpatch warnings are benign:

 * The include/qemu/queue.h coding style warnings are because this is
   code imported to QEMU from elsewhere (BSD?).

 * The 80 character line warnings can be fixed by Alex in a follow-up
   patch.

 * MAINTAINERS changes have been made, but the check is known to report
   false positives (especially if a wildcard M: line is added and then
   new files are introduced later in the series).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 24/31] fuzz: support for fork-based fuzzing.

[Stefan Hajnoczi]

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

On Sat, Feb 22, 2020 at 05:34:29AM -0600, Eric Blake wrote:
> On 2/22/20 2:50 AM, Stefan Hajnoczi wrote:
> > From: Alexander Bulekov <alxndr@bu.edu>
> > 
> > fork() is a simple way to ensure that state does not leak in between
> > fuzzing runs. Unfortunately, the fuzzer mutation engine relies on
> > bitmaps which contain coverage information for each fuzzing run, and
> > these bitmaps should be copied from the child to the parent(where the
> > mutation occurs). These bitmaps are created through compile-time
> > instrumentation and they are not shared with fork()-ed processes, by
> > default. To address this, we create a shared memory region, adjust its
> > size and map it _over_ the counter region. Furthermore, libfuzzer
> > doesn't generally expose the globals that specify the location of the
> > counters/coverage bitmap. As a workaround, we rely on a custom linker
> > script which forces all of the bitmaps we care about to be placed in a
> > contiguous region, which is easy to locate and mmap over.
> > 
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> > Message-id: 20200220041118.23264-16-alxndr@bu.edu
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> 
> Random drive-by observation:
> 
> > +++ b/tests/qtest/fuzz/fork_fuzz.ld
> > @@ -0,0 +1,37 @@
> > +/* We adjust linker script modification to place all of the stuff that needs to
> > + * persist across fuzzing runs into a contiguous seciton of memory. Then, it is
> 
> section

Thanks, Eric!

Alex, please send follow-up patches to fix this typo and the 80
character line limit issues identified by patchew (see patch email reply
to this email thread).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 00/31] Block patches

[Peter Maydell]

On Sat, 22 Feb 2020 at 08:50, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The following changes since commit 9ac5df20f51fabcba0d902025df4bd7ea987c158:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200221-1' into staging (2020-02-21 16:18:38 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to e5c59355ae9f724777c61c859292ec9db2c8c2ab:
>
>   fuzz: add documentation to docs/devel/ (2020-02-22 08:26:48 +0000)
>
> ----------------------------------------------------------------
> Pull request
>
> This pull request contains a virtio-blk/scsi performance optimization, event
> loop scalability improvements, and a qtest-based device fuzzing framework.  I
> am including the fuzzing patches because I have reviewed them and Thomas Huth
> is currently away on leave.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM

Re: [PULL 24/31] fuzz: support for fork-based fuzzing.

[Alexander Bulekov]

On 200224 1135, Stefan Hajnoczi wrote:
> On Sat, Feb 22, 2020 at 05:34:29AM -0600, Eric Blake wrote:
> > On 2/22/20 2:50 AM, Stefan Hajnoczi wrote:
> > > From: Alexander Bulekov <alxndr@bu.edu>
> > > 
> > > fork() is a simple way to ensure that state does not leak in between
> > > fuzzing runs. Unfortunately, the fuzzer mutation engine relies on
> > > bitmaps which contain coverage information for each fuzzing run, and
> > > these bitmaps should be copied from the child to the parent(where the
> > > mutation occurs). These bitmaps are created through compile-time
> > > instrumentation and they are not shared with fork()-ed processes, by
> > > default. To address this, we create a shared memory region, adjust its
> > > size and map it _over_ the counter region. Furthermore, libfuzzer
> > > doesn't generally expose the globals that specify the location of the
> > > counters/coverage bitmap. As a workaround, we rely on a custom linker
> > > script which forces all of the bitmaps we care about to be placed in a
> > > contiguous region, which is easy to locate and mmap over.
> > > 
> > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> > > Message-id: 20200220041118.23264-16-alxndr@bu.edu
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > ---
> > 
> > Random drive-by observation:
> > 
> > > +++ b/tests/qtest/fuzz/fork_fuzz.ld
> > > @@ -0,0 +1,37 @@
> > > +/* We adjust linker script modification to place all of the stuff that needs to
> > > + * persist across fuzzing runs into a contiguous seciton of memory. Then, it is
> > 
> > section
> 
> Thanks, Eric!
> 
> Alex, please send follow-up patches to fix this typo and the 80
> character line limit issues identified by patchew (see patch email reply
> to this email thread).

Thank you Eric, Stefan!
Just sent out some fixes.

> Stefan

Re: [PULL 04/31] util/async: make bh_aio_poll() O(1)

[Marc-André Lureau]

Hi

On Sat, Feb 22, 2020 at 9:51 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The ctx->first_bh list contains all created BHs, including those that
> are not scheduled.  The list is iterated by the event loop and therefore
> has O(n) time complexity with respected to the number of created BHs.
>
> Rewrite BHs so that only scheduled or deleted BHs are enqueued.
> Only BHs that actually require action will be iterated.
>
> One semantic change is required: qemu_bh_delete() enqueues the BH and
> therefore invokes aio_notify().  The
> tests/test-aio.c:test_source_bh_delete_from_cb() test case assumed that
> g_main_context_iteration(NULL, false) returns false after
> qemu_bh_delete() but it now returns true for one iteration.  Fix up the
> test case.
>
> This patch makes aio_compute_timeout() and aio_bh_poll() drop from a CPU
> profile reported by perf-top(1).  Previously they combined to 9% CPU
> utilization when AioContext polling is commented out and the guest has 2
> virtio-blk,num-queues=1 and 99 virtio-blk,num-queues=32 devices.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-id: 20200221093951.1414693-1-stefanha@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---

Current master migration-test fails with --enable-sanitizers. qemu
exit 1 when 0 is expected.
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
tests/qtest/migration-test  -p /x86_64/migration/postcopy/recovery
tests/qtest/libqtest.c:140: kill_qemu() tried to terminate QEMU
process but encountered exit status 1 (expected 0)

Leak is

=================================================================
==2082571==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7f25971dfc58 in __interceptor_malloc (/lib64/libasan.so.5+0x10dc58)
    #1 0x7f2596d08358 in g_malloc (/lib64/libglib-2.0.so.0+0x57358)
    #2 0x560970d006f8 in qemu_bh_new /home/elmarco/src/qemu/util/main-loop.c:532
    #3 0x5609704afa02 in migrate_fd_connect
/home/elmarco/src/qemu/migration/migration.c:3407
    #4 0x5609704b6b6f in migration_channel_connect
/home/elmarco/src/qemu/migration/channel.c:92
    #5 0x5609704b2bfb in socket_outgoing_migration
/home/elmarco/src/qemu/migration/socket.c:108
    #6 0x560970b9bd6c in qio_task_complete /home/elmarco/src/qemu/io/task.c:196
    #7 0x560970b9aa97 in qio_task_thread_result
/home/elmarco/src/qemu/io/task.c:111
    #8 0x7f2596cfee3a  (/lib64/libglib-2.0.so.0+0x4de3a)

Any idea?

>  include/block/aio.h |  20 +++-
>  tests/test-aio.c    |   3 +-
>  util/async.c        | 237 ++++++++++++++++++++++++++------------------
>  3 files changed, 158 insertions(+), 102 deletions(-)
>
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 7ba9bd7874..1a2ce9ca26 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -51,6 +51,19 @@ struct ThreadPool;
>  struct LinuxAioState;
>  struct LuringState;
>
> +/*
> + * Each aio_bh_poll() call carves off a slice of the BH list, so that newly
> + * scheduled BHs are not processed until the next aio_bh_poll() call.  All
> + * active aio_bh_poll() calls chain their slices together in a list, so that
> + * nested aio_bh_poll() calls process all scheduled bottom halves.
> + */
> +typedef QSLIST_HEAD(, QEMUBH) BHList;
> +typedef struct BHListSlice BHListSlice;
> +struct BHListSlice {
> +    BHList bh_list;
> +    QSIMPLEQ_ENTRY(BHListSlice) next;
> +};
> +
>  struct AioContext {
>      GSource source;
>
> @@ -91,8 +104,11 @@ struct AioContext {
>       */
>      QemuLockCnt list_lock;
>
> -    /* Anchor of the list of Bottom Halves belonging to the context */
> -    struct QEMUBH *first_bh;
> +    /* Bottom Halves pending aio_bh_poll() processing */
> +    BHList bh_list;
> +
> +    /* Chained BH list slices for each nested aio_bh_poll() call */
> +    QSIMPLEQ_HEAD(, BHListSlice) bh_slice_list;
>
>      /* Used by aio_notify.
>       *
> diff --git a/tests/test-aio.c b/tests/test-aio.c
> index 86fb73b3d5..8a46078463 100644
> --- a/tests/test-aio.c
> +++ b/tests/test-aio.c
> @@ -615,7 +615,8 @@ static void test_source_bh_delete_from_cb(void)
>      g_assert_cmpint(data1.n, ==, data1.max);
>      g_assert(data1.bh == NULL);
>
> -    g_assert(!g_main_context_iteration(NULL, false));
> +    assert(g_main_context_iteration(NULL, false));
> +    assert(!g_main_context_iteration(NULL, false));
>  }
>
>  static void test_source_bh_delete_from_cb_many(void)
> diff --git a/util/async.c b/util/async.c
> index c192a24a61..b94518b948 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -29,6 +29,7 @@
>  #include "block/thread-pool.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/atomic.h"
> +#include "qemu/rcu_queue.h"
>  #include "block/raw-aio.h"
>  #include "qemu/coroutine_int.h"
>  #include "trace.h"
> @@ -36,16 +37,76 @@
>  /***********************************************************/
>  /* bottom halves (can be seen as timers which expire ASAP) */
>
> +/* QEMUBH::flags values */
> +enum {
> +    /* Already enqueued and waiting for aio_bh_poll() */
> +    BH_PENDING   = (1 << 0),
> +
> +    /* Invoke the callback */
> +    BH_SCHEDULED = (1 << 1),
> +
> +    /* Delete without invoking callback */
> +    BH_DELETED   = (1 << 2),
> +
> +    /* Delete after invoking callback */
> +    BH_ONESHOT   = (1 << 3),
> +
> +    /* Schedule periodically when the event loop is idle */
> +    BH_IDLE      = (1 << 4),
> +};
> +
>  struct QEMUBH {
>      AioContext *ctx;
>      QEMUBHFunc *cb;
>      void *opaque;
> -    QEMUBH *next;
> -    bool scheduled;
> -    bool idle;
> -    bool deleted;
> +    QSLIST_ENTRY(QEMUBH) next;
> +    unsigned flags;
>  };
>
> +/* Called concurrently from any thread */
> +static void aio_bh_enqueue(QEMUBH *bh, unsigned new_flags)
> +{
> +    AioContext *ctx = bh->ctx;
> +    unsigned old_flags;
> +
> +    /*
> +     * The memory barrier implicit in atomic_fetch_or makes sure that:
> +     * 1. idle & any writes needed by the callback are done before the
> +     *    locations are read in the aio_bh_poll.
> +     * 2. ctx is loaded before the callback has a chance to execute and bh
> +     *    could be freed.
> +     */
> +    old_flags = atomic_fetch_or(&bh->flags, BH_PENDING | new_flags);
> +    if (!(old_flags & BH_PENDING)) {
> +        QSLIST_INSERT_HEAD_ATOMIC(&ctx->bh_list, bh, next);
> +    }
> +
> +    aio_notify(ctx);
> +}
> +
> +/* Only called from aio_bh_poll() and aio_ctx_finalize() */
> +static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags)
> +{
> +    QEMUBH *bh = QSLIST_FIRST_RCU(head);
> +
> +    if (!bh) {
> +        return NULL;
> +    }
> +
> +    QSLIST_REMOVE_HEAD(head, next);
> +
> +    /*
> +     * The atomic_and is paired with aio_bh_enqueue().  The implicit memory
> +     * barrier ensures that the callback sees all writes done by the scheduling
> +     * thread.  It also ensures that the scheduling thread sees the cleared
> +     * flag before bh->cb has run, and thus will call aio_notify again if
> +     * necessary.
> +     */
> +    *flags = atomic_fetch_and(&bh->flags,
> +                              ~(BH_PENDING | BH_SCHEDULED | BH_IDLE));
> +    return bh;
> +}
> +
>  void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
>  {
>      QEMUBH *bh;
> @@ -55,15 +116,7 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
>          .cb = cb,
>          .opaque = opaque,
>      };
> -    qemu_lockcnt_lock(&ctx->list_lock);
> -    bh->next = ctx->first_bh;
> -    bh->scheduled = 1;
> -    bh->deleted = 1;
> -    /* Make sure that the members are ready before putting bh into list */
> -    smp_wmb();
> -    ctx->first_bh = bh;
> -    qemu_lockcnt_unlock(&ctx->list_lock);
> -    aio_notify(ctx);
> +    aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
>  }
>
>  QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
> @@ -75,12 +128,6 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
>          .cb = cb,
>          .opaque = opaque,
>      };
> -    qemu_lockcnt_lock(&ctx->list_lock);
> -    bh->next = ctx->first_bh;
> -    /* Make sure that the members are ready before putting bh into list */
> -    smp_wmb();
> -    ctx->first_bh = bh;
> -    qemu_lockcnt_unlock(&ctx->list_lock);
>      return bh;
>  }
>
> @@ -89,91 +136,56 @@ void aio_bh_call(QEMUBH *bh)
>      bh->cb(bh->opaque);
>  }
>
> -/* Multiple occurrences of aio_bh_poll cannot be called concurrently.
> - * The count in ctx->list_lock is incremented before the call, and is
> - * not affected by the call.
> - */
> +/* Multiple occurrences of aio_bh_poll cannot be called concurrently. */
>  int aio_bh_poll(AioContext *ctx)
>  {
> -    QEMUBH *bh, **bhp, *next;
> -    int ret;
> -    bool deleted = false;
> -
> -    ret = 0;
> -    for (bh = atomic_rcu_read(&ctx->first_bh); bh; bh = next) {
> -        next = atomic_rcu_read(&bh->next);
> -        /* The atomic_xchg is paired with the one in qemu_bh_schedule.  The
> -         * implicit memory barrier ensures that the callback sees all writes
> -         * done by the scheduling thread.  It also ensures that the scheduling
> -         * thread sees the zero before bh->cb has run, and thus will call
> -         * aio_notify again if necessary.
> -         */
> -        if (atomic_xchg(&bh->scheduled, 0)) {
> +    BHListSlice slice;
> +    BHListSlice *s;
> +    int ret = 0;
> +
> +    QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
> +    QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> +
> +    while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
> +        QEMUBH *bh;
> +        unsigned flags;
> +
> +        bh = aio_bh_dequeue(&s->bh_list, &flags);
> +        if (!bh) {
> +            QSIMPLEQ_REMOVE_HEAD(&ctx->bh_slice_list, next);
> +            continue;
> +        }
> +
> +        if ((flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
>              /* Idle BHs don't count as progress */
> -            if (!bh->idle) {
> +            if (!(flags & BH_IDLE)) {
>                  ret = 1;
>              }
> -            bh->idle = 0;
>              aio_bh_call(bh);
>          }
> -        if (bh->deleted) {
> -            deleted = true;
> +        if (flags & (BH_DELETED | BH_ONESHOT)) {
> +            g_free(bh);
>          }
>      }
>
> -    /* remove deleted bhs */
> -    if (!deleted) {
> -        return ret;
> -    }
> -
> -    if (qemu_lockcnt_dec_if_lock(&ctx->list_lock)) {
> -        bhp = &ctx->first_bh;
> -        while (*bhp) {
> -            bh = *bhp;
> -            if (bh->deleted && !bh->scheduled) {
> -                *bhp = bh->next;
> -                g_free(bh);
> -            } else {
> -                bhp = &bh->next;
> -            }
> -        }
> -        qemu_lockcnt_inc_and_unlock(&ctx->list_lock);
> -    }
>      return ret;
>  }
>
>  void qemu_bh_schedule_idle(QEMUBH *bh)
>  {
> -    bh->idle = 1;
> -    /* Make sure that idle & any writes needed by the callback are done
> -     * before the locations are read in the aio_bh_poll.
> -     */
> -    atomic_mb_set(&bh->scheduled, 1);
> +    aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
>  }
>
>  void qemu_bh_schedule(QEMUBH *bh)
>  {
> -    AioContext *ctx;
> -
> -    ctx = bh->ctx;
> -    bh->idle = 0;
> -    /* The memory barrier implicit in atomic_xchg makes sure that:
> -     * 1. idle & any writes needed by the callback are done before the
> -     *    locations are read in the aio_bh_poll.
> -     * 2. ctx is loaded before scheduled is set and the callback has a chance
> -     *    to execute.
> -     */
> -    if (atomic_xchg(&bh->scheduled, 1) == 0) {
> -        aio_notify(ctx);
> -    }
> +    aio_bh_enqueue(bh, BH_SCHEDULED);
>  }
>
> -
>  /* This func is async.
>   */
>  void qemu_bh_cancel(QEMUBH *bh)
>  {
> -    atomic_mb_set(&bh->scheduled, 0);
> +    atomic_and(&bh->flags, ~BH_SCHEDULED);
>  }
>
>  /* This func is async.The bottom half will do the delete action at the finial
> @@ -181,21 +193,16 @@ void qemu_bh_cancel(QEMUBH *bh)
>   */
>  void qemu_bh_delete(QEMUBH *bh)
>  {
> -    bh->scheduled = 0;
> -    bh->deleted = 1;
> +    aio_bh_enqueue(bh, BH_DELETED);
>  }
>
> -int64_t
> -aio_compute_timeout(AioContext *ctx)
> +static int64_t aio_compute_bh_timeout(BHList *head, int timeout)
>  {
> -    int64_t deadline;
> -    int timeout = -1;
>      QEMUBH *bh;
>
> -    for (bh = atomic_rcu_read(&ctx->first_bh); bh;
> -         bh = atomic_rcu_read(&bh->next)) {
> -        if (bh->scheduled) {
> -            if (bh->idle) {
> +    QSLIST_FOREACH_RCU(bh, head, next) {
> +        if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
> +            if (bh->flags & BH_IDLE) {
>                  /* idle bottom halves will be polled at least
>                   * every 10ms */
>                  timeout = 10000000;
> @@ -207,6 +214,28 @@ aio_compute_timeout(AioContext *ctx)
>          }
>      }
>
> +    return timeout;
> +}
> +
> +int64_t
> +aio_compute_timeout(AioContext *ctx)
> +{
> +    BHListSlice *s;
> +    int64_t deadline;
> +    int timeout = -1;
> +
> +    timeout = aio_compute_bh_timeout(&ctx->bh_list, timeout);
> +    if (timeout == 0) {
> +        return 0;
> +    }
> +
> +    QSIMPLEQ_FOREACH(s, &ctx->bh_slice_list, next) {
> +        timeout = aio_compute_bh_timeout(&s->bh_list, timeout);
> +        if (timeout == 0) {
> +            return 0;
> +        }
> +    }
> +
>      deadline = timerlistgroup_deadline_ns(&ctx->tlg);
>      if (deadline == 0) {
>          return 0;
> @@ -237,15 +266,24 @@ aio_ctx_check(GSource *source)
>  {
>      AioContext *ctx = (AioContext *) source;
>      QEMUBH *bh;
> +    BHListSlice *s;
>
>      atomic_and(&ctx->notify_me, ~1);
>      aio_notify_accept(ctx);
>
> -    for (bh = ctx->first_bh; bh; bh = bh->next) {
> -        if (bh->scheduled) {
> +    QSLIST_FOREACH_RCU(bh, &ctx->bh_list, next) {
> +        if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
>              return true;
>          }
>      }
> +
> +    QSIMPLEQ_FOREACH(s, &ctx->bh_slice_list, next) {
> +        QSLIST_FOREACH_RCU(bh, &s->bh_list, next) {
> +            if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
> +                return true;
> +            }
> +        }
> +    }
>      return aio_pending(ctx) || (timerlistgroup_deadline_ns(&ctx->tlg) == 0);
>  }
>
> @@ -265,6 +303,8 @@ static void
>  aio_ctx_finalize(GSource     *source)
>  {
>      AioContext *ctx = (AioContext *) source;
> +    QEMUBH *bh;
> +    unsigned flags;
>
>      thread_pool_free(ctx->thread_pool);
>
> @@ -287,18 +327,15 @@ aio_ctx_finalize(GSource     *source)
>      assert(QSLIST_EMPTY(&ctx->scheduled_coroutines));
>      qemu_bh_delete(ctx->co_schedule_bh);
>
> -    qemu_lockcnt_lock(&ctx->list_lock);
> -    assert(!qemu_lockcnt_count(&ctx->list_lock));
> -    while (ctx->first_bh) {
> -        QEMUBH *next = ctx->first_bh->next;
> +    /* There must be no aio_bh_poll() calls going on */
> +    assert(QSIMPLEQ_EMPTY(&ctx->bh_slice_list));
>
> +    while ((bh = aio_bh_dequeue(&ctx->bh_list, &flags))) {
>          /* qemu_bh_delete() must have been called on BHs in this AioContext */
> -        assert(ctx->first_bh->deleted);
> +        assert(flags & BH_DELETED);
>
> -        g_free(ctx->first_bh);
> -        ctx->first_bh = next;
> +        g_free(bh);
>      }
> -    qemu_lockcnt_unlock(&ctx->list_lock);
>
>      aio_set_event_notifier(ctx, &ctx->notifier, false, NULL, NULL);
>      event_notifier_cleanup(&ctx->notifier);
> @@ -445,6 +482,8 @@ AioContext *aio_context_new(Error **errp)
>      AioContext *ctx;
>
>      ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
> +    QSLIST_INIT(&ctx->bh_list);
> +    QSIMPLEQ_INIT(&ctx->bh_slice_list);
>      aio_context_setup(ctx);
>
>      ret = event_notifier_init(&ctx->notifier, false);
> --
> 2.24.1
>


-- 
Marc-André Lureau

Re: [PULL 00/31] Block patches

[Maxim Levitsky]

On Wed, 2020-07-08 at 18:11 +0300, Maxim Levitsky wrote:
> On Tue, 2020-07-07 at 21:40 +0100, Peter Maydell wrote:
> > On Mon, 6 Jul 2020 at 11:04, Max Reitz <mreitz@redhat.com> wrote:
> > > The following changes since commit eb6490f544388dd24c0d054a96dd304bc7284450:
> > > 
> > >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200703' into staging (2020-07-04 16:08:41 +0100)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   https://github.com/XanClic/qemu.git tags/pull-block-2020-07-06
> > > 
> > > for you to fetch changes up to 365fed5111b06d31c1632af63c7528dfe49d62a2:
> > > 
> > >   qed: Simplify backing reads (2020-07-06 10:34:14 +0200)
> > > 
> > > ----------------------------------------------------------------
> > > Block patches for 5.1:
> > > - LUKS keyslot amendment
> > >   (+ patches to make the iotests pass on non-Linux systems, and to keep
> > >      the tests passing for qcow v1, and to skip LUKS tests (including
> > >      qcow2 LUKS) when the built qemu does not support it)
> > > - Refactoring in the block layer: Drop the basically unnecessary
> > >   unallocated_blocks_are_zero field from BlockDriverInfo
> > > - Fix qcow2 preallocation when the image size is not a multiple of the
> > >   cluster size
> > > - Fix in block-copy code
> > > 
> > 
> > Applied, thanks.
> > 
> > Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
> > for any user-visible changes.
> 
> Since this includes my luks patches, I guess I need to add them to this changelog.
> How do I get an account on the wiki?

Kevin Wolf gave me an account, so I'll write today a short description in the changelog.
Thanks for everyone that helped me get this merged upstream!

Best regards,
	Maxim Levitsky
> 
> Best regards,
> 	Maxim Levitsky
> 
> > -- PMM
> > 

Re: [PULL 00/31] Block patches

[Kevin Wolf]

Am 08.07.2020 um 17:11 hat Maxim Levitsky geschrieben:
> On Tue, 2020-07-07 at 21:40 +0100, Peter Maydell wrote:
> > Applied, thanks.
> > 
> > Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
> > for any user-visible changes.
> 
> Since this includes my luks patches, I guess I need to add them to this changelog.
> How do I get an account on the wiki?

For the record, I contacted Maxim on IRC and created an account for him.

Kevin

Re: [PULL 00/31] Block patches

[Maxim Levitsky]

On Tue, 2020-07-07 at 21:40 +0100, Peter Maydell wrote:
> On Mon, 6 Jul 2020 at 11:04, Max Reitz <mreitz@redhat.com> wrote:
> > The following changes since commit eb6490f544388dd24c0d054a96dd304bc7284450:
> > 
> >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200703' into staging (2020-07-04 16:08:41 +0100)
> > 
> > are available in the Git repository at:
> > 
> >   https://github.com/XanClic/qemu.git tags/pull-block-2020-07-06
> > 
> > for you to fetch changes up to 365fed5111b06d31c1632af63c7528dfe49d62a2:
> > 
> >   qed: Simplify backing reads (2020-07-06 10:34:14 +0200)
> > 
> > ----------------------------------------------------------------
> > Block patches for 5.1:
> > - LUKS keyslot amendment
> >   (+ patches to make the iotests pass on non-Linux systems, and to keep
> >      the tests passing for qcow v1, and to skip LUKS tests (including
> >      qcow2 LUKS) when the built qemu does not support it)
> > - Refactoring in the block layer: Drop the basically unnecessary
> >   unallocated_blocks_are_zero field from BlockDriverInfo
> > - Fix qcow2 preallocation when the image size is not a multiple of the
> >   cluster size
> > - Fix in block-copy code
> > 
> 
> 
> Applied, thanks.
> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
> for any user-visible changes.

Since this includes my luks patches, I guess I need to add them to this changelog.
How do I get an account on the wiki?

Best regards,
	Maxim Levitsky

> 
> -- PMM
> 

Re: [PULL 00/31] Block patches

[Peter Maydell]

On Mon, 6 Jul 2020 at 11:04, Max Reitz <mreitz@redhat.com> wrote:
>
> The following changes since commit eb6490f544388dd24c0d054a96dd304bc7284450:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200703' into staging (2020-07-04 16:08:41 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/XanClic/qemu.git tags/pull-block-2020-07-06
>
> for you to fetch changes up to 365fed5111b06d31c1632af63c7528dfe49d62a2:
>
>   qed: Simplify backing reads (2020-07-06 10:34:14 +0200)
>
> ----------------------------------------------------------------
> Block patches for 5.1:
> - LUKS keyslot amendment
>   (+ patches to make the iotests pass on non-Linux systems, and to keep
>      the tests passing for qcow v1, and to skip LUKS tests (including
>      qcow2 LUKS) when the built qemu does not support it)
> - Refactoring in the block layer: Drop the basically unnecessary
>   unallocated_blocks_are_zero field from BlockDriverInfo
> - Fix qcow2 preallocation when the image size is not a multiple of the
>   cluster size
> - Fix in block-copy code
>



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM

Re: [PULL 00/31] Block patches

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200706100432.2301919-1-mreitz@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PULL 00/31] Block patches
Type: series
Message-id: 20200706100432.2301919-1-mreitz@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200706100432.2301919-1-mreitz@redhat.com -> patchew/20200706100432.2301919-1-mreitz@redhat.com
Switched to a new branch 'test'
61a6805 qed: Simplify backing reads
8160191 block: drop unallocated_blocks_are_zero
d59b973 block/vhdx: drop unallocated_blocks_are_zero
fc425f8 block/file-posix: drop unallocated_blocks_are_zero
ee4e5f6 block/iscsi: drop unallocated_blocks_are_zero
9aacd22 block/crypto: drop unallocated_blocks_are_zero
4d6c6ad block/vpc: return ZERO block-status when appropriate
42d9c04 block/vdi: return ZERO block-status when appropriate
f73a13e block: inline bdrv_unallocated_blocks_are_zero()
d5d6d75 qemu-img: convert: don't use unallocated_blocks_are_zero
ecc887e iotests: add tests for blockdev-amend
ee66938 block/qcow2: implement blockdev-amend
9254299 block/crypto: implement blockdev-amend
f905f40 block/core: add generic infrastructure for x-blockdev-amend qmp command
5737d94 iotests: qemu-img tests for luks key management
1981eea block/qcow2: extend qemu-img amend interface with crypto options
75ebd25 block/crypto: implement the encryption key management
381affe block/crypto: rename two functions
37d5645 block/amend: refactor qcow2 amend options
2057114 block/amend: separate amend and create options for qemu-img
01ead3a block/amend: add 'force' option
23c67be qcrypto/luks: implement encryption key management
255ed50 qcrypto/core: add generic infrastructure for crypto options amendment
2267d63 iotests: Check whether luks works
a28c683 iotests.py: Add (verify|has)_working_luks()
e4821d5 iotests.py: Add qemu_img_pipe_and_status()
e9500aa iotests/common.rc: Add _require_working_luks
b3b46b5 iotests: filter few more luks specific create options
f872aeb iotests: Make _filter_img_create more active
d327a4c qcow2: Fix preallocation on images with unaligned sizes
9e289b8 block/block-copy: block_copy_dirty_clusters: fix failure check

=== OUTPUT BEGIN ===
1/31 Checking commit 9e289b8e0d77 (block/block-copy: block_copy_dirty_clusters: fix failure check)
2/31 Checking commit d327a4c43167 (qcow2: Fix preallocation on images with unaligned sizes)
3/31 Checking commit f872aebcf148 (iotests: Make _filter_img_create more active)
4/31 Checking commit b3b46b52bfec (iotests: filter few more luks specific create options)
5/31 Checking commit e9500aa71b58 (iotests/common.rc: Add _require_working_luks)
6/31 Checking commit e4821d5b92da (iotests.py: Add qemu_img_pipe_and_status())
7/31 Checking commit a28c683849bf (iotests.py: Add (verify|has)_working_luks())
8/31 Checking commit 2267d63fda82 (iotests: Check whether luks works)
9/31 Checking commit 255ed5095c89 (qcrypto/core: add generic infrastructure for crypto options amendment)
10/31 Checking commit 23c67bedd6e4 (qcrypto/luks: implement encryption key management)
11/31 Checking commit 01ead3a1b6b0 (block/amend: add 'force' option)
12/31 Checking commit 2057114b7a98 (block/amend: separate amend and create options for qemu-img)
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#34: FILE: block/qcow2.c:5663:
+#define QCOW_COMMON_OPTIONS                                         \
+    {                                                               \
+        .name = BLOCK_OPT_SIZE,                                     \
+        .type = QEMU_OPT_SIZE,                                      \
+        .help = "Virtual disk size"                                 \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_COMPAT_LEVEL,                             \
+        .type = QEMU_OPT_STRING,                                    \
+        .help = "Compatibility level (v2 [0.10] or v3 [1.1])"       \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_BACKING_FILE,                             \
+        .type = QEMU_OPT_STRING,                                    \
+        .help = "File name of a base image"                         \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_BACKING_FMT,                              \
+        .type = QEMU_OPT_STRING,                                    \
+        .help = "Image format of the base image"                    \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_DATA_FILE,                                \
+        .type = QEMU_OPT_STRING,                                    \
+        .help = "File name of an external data file"                \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_DATA_FILE_RAW,                            \
+        .type = QEMU_OPT_BOOL,                                      \
+        .help = "The external data file must stay valid "           \
+                "as a raw image"                                    \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_ENCRYPT,                                  \
+        .type = QEMU_OPT_BOOL,                                      \
+        .help = "Encrypt the image with format 'aes'. (Deprecated " \
+                "in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)",    \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_ENCRYPT_FORMAT,                           \
+        .type = QEMU_OPT_STRING,                                    \
+        .help = "Encrypt the image, format choices: 'aes', 'luks'", \
+    },                                                              \
+    BLOCK_CRYPTO_OPT_DEF_KEY_SECRET("encrypt.",                     \
+        "ID of secret providing qcow AES key or LUKS passphrase"),  \
+    BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG("encrypt."),               \
+    BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE("encrypt."),              \
+    BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG("encrypt."),                \
+    BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG("encrypt."),           \
+    BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG("encrypt."),                 \
+    BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME("encrypt."),                \
+    {                                                               \
+        .name = BLOCK_OPT_CLUSTER_SIZE,                             \
+        .type = QEMU_OPT_SIZE,                                      \
+        .help = "qcow2 cluster size",                               \
+        .def_value_str = stringify(DEFAULT_CLUSTER_SIZE)            \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_PREALLOC,                                 \
+        .type = QEMU_OPT_STRING,                                    \
+        .help = "Preallocation mode (allowed values: off, "         \
+                "metadata, falloc, full)"                           \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_LAZY_REFCOUNTS,                           \
+        .type = QEMU_OPT_BOOL,                                      \
+        .help = "Postpone refcount updates",                        \
+        .def_value_str = "off"                                      \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_REFCOUNT_BITS,                            \
+        .type = QEMU_OPT_NUMBER,                                    \
+        .help = "Width of a reference count entry in bits",         \
+        .def_value_str = "16"                                       \
+    },                                                              \
+    {                                                               \
+        .name = BLOCK_OPT_COMPRESSION_TYPE,                         \
+        .type = QEMU_OPT_STRING,                                    \
+        .help = "Compression method used for image cluster "        \
+                "compression",                                      \
+        .def_value_str = "zlib"                                     \
+    }

total: 1 errors, 0 warnings, 244 lines checked

Patch 12/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

13/31 Checking commit 37d5645c2d89 (block/amend: refactor qcow2 amend options)
14/31 Checking commit 381affeb1932 (block/crypto: rename two functions)
15/31 Checking commit 75ebd2518718 (block/crypto: implement the encryption key management)
16/31 Checking commit 1981eea00495 (block/qcow2: extend qemu-img amend interface with crypto options)
17/31 Checking commit 5737d9411ca1 (iotests: qemu-img tests for luks key management)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100755

total: 0 errors, 1 warnings, 433 lines checked

Patch 17/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/31 Checking commit f905f403e917 (block/core: add generic infrastructure for x-blockdev-amend qmp command)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#33: 
new file mode 100644

total: 0 errors, 1 warnings, 221 lines checked

Patch 18/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
19/31 Checking commit 925429913f58 (block/crypto: implement blockdev-amend)
20/31 Checking commit ee66938d1193 (block/qcow2: implement blockdev-amend)
21/31 Checking commit ecc887e8c0a4 (iotests: add tests for blockdev-amend)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100755

total: 0 errors, 1 warnings, 593 lines checked

Patch 21/31 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
22/31 Checking commit d5d6d75d306c (qemu-img: convert: don't use unallocated_blocks_are_zero)
23/31 Checking commit f73a13e6c086 (block: inline bdrv_unallocated_blocks_are_zero())
24/31 Checking commit 42d9c04825ab (block/vdi: return ZERO block-status when appropriate)
25/31 Checking commit 4d6c6ad7b389 (block/vpc: return ZERO block-status when appropriate)
26/31 Checking commit 9aacd22b62a4 (block/crypto: drop unallocated_blocks_are_zero)
27/31 Checking commit ee4e5f6a6804 (block/iscsi: drop unallocated_blocks_are_zero)
28/31 Checking commit fc425f8700a0 (block/file-posix: drop unallocated_blocks_are_zero)
29/31 Checking commit d59b973ebbed (block/vhdx: drop unallocated_blocks_are_zero)
30/31 Checking commit 816019134241 (block: drop unallocated_blocks_are_zero)
31/31 Checking commit 61a68058fcb1 (qed: Simplify backing reads)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200706100432.2301919-1-mreitz@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

[PULL 00/31] Block patches

[Max Reitz]

The following changes since commit eb6490f544388dd24c0d054a96dd304bc7284450:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200703' into staging (2020-07-04 16:08:41 +0100)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2020-07-06

for you to fetch changes up to 365fed5111b06d31c1632af63c7528dfe49d62a2:

  qed: Simplify backing reads (2020-07-06 10:34:14 +0200)

----------------------------------------------------------------
Block patches for 5.1:
- LUKS keyslot amendment
  (+ patches to make the iotests pass on non-Linux systems, and to keep
     the tests passing for qcow v1, and to skip LUKS tests (including
     qcow2 LUKS) when the built qemu does not support it)
- Refactoring in the block layer: Drop the basically unnecessary
  unallocated_blocks_are_zero field from BlockDriverInfo
- Fix qcow2 preallocation when the image size is not a multiple of the
  cluster size
- Fix in block-copy code

----------------------------------------------------------------
Alberto Garcia (1):
  qcow2: Fix preallocation on images with unaligned sizes

Eric Blake (1):
  qed: Simplify backing reads

Max Reitz (5):
  iotests: Make _filter_img_create more active
  iotests/common.rc: Add _require_working_luks
  iotests.py: Add qemu_img_pipe_and_status()
  iotests.py: Add (verify|has)_working_luks()
  iotests: Check whether luks works

Maxim Levitsky (14):
  iotests: filter few more luks specific create options
  qcrypto/core: add generic infrastructure for crypto options amendment
  qcrypto/luks: implement encryption key management
  block/amend: add 'force' option
  block/amend: separate amend and create options for qemu-img
  block/amend: refactor qcow2 amend options
  block/crypto: rename two functions
  block/crypto: implement the encryption key management
  block/qcow2: extend qemu-img amend interface with crypto options
  iotests: qemu-img tests for luks key management
  block/core: add generic infrastructure for x-blockdev-amend qmp
    command
  block/crypto: implement blockdev-amend
  block/qcow2: implement blockdev-amend
  iotests: add tests for blockdev-amend

Vladimir Sementsov-Ogievskiy (10):
  block/block-copy: block_copy_dirty_clusters: fix failure check
  qemu-img: convert: don't use unallocated_blocks_are_zero
  block: inline bdrv_unallocated_blocks_are_zero()
  block/vdi: return ZERO block-status when appropriate
  block/vpc: return ZERO block-status when appropriate
  block/crypto: drop unallocated_blocks_are_zero
  block/iscsi: drop unallocated_blocks_are_zero
  block/file-posix: drop unallocated_blocks_are_zero
  block/vhdx: drop unallocated_blocks_are_zero
  block: drop unallocated_blocks_are_zero

 docs/tools/qemu-img.rst          |   5 +-
 qapi/block-core.json             |  68 +++++
 qapi/crypto.json                 |  73 +++++-
 qapi/job.json                    |   4 +-
 block/crypto.h                   |  37 +++
 block/qed.h                      |   1 -
 crypto/blockpriv.h               |   8 +
 include/block/block.h            |   7 +-
 include/block/block_int.h        |  36 ++-
 include/crypto/block.h           |  22 ++
 block.c                          |  19 +-
 block/amend.c                    | 113 +++++++++
 block/block-copy.c               |   4 +-
 block/crypto.c                   | 207 +++++++++++++--
 block/file-posix.c               |   3 -
 block/io.c                       |   8 +-
 block/iscsi.c                    |   1 -
 block/qcow2.c                    | 350 +++++++++++++++-----------
 block/qed.c                      |  65 +----
 block/vdi.c                      |   3 +-
 block/vhdx.c                     |   3 -
 block/vpc.c                      |   3 +-
 crypto/block-luks.c              | 416 ++++++++++++++++++++++++++++++-
 crypto/block.c                   |  29 +++
 qemu-img.c                       |  48 ++--
 block/Makefile.objs              |   2 +-
 qemu-img-cmds.hx                 |   4 +-
 tests/qemu-iotests/049.out       | 102 ++++----
 tests/qemu-iotests/061.out       |  12 +-
 tests/qemu-iotests/082.out       | 185 ++++----------
 tests/qemu-iotests/085.out       |  38 +--
 tests/qemu-iotests/087           |   1 +
 tests/qemu-iotests/087.out       |   6 +-
 tests/qemu-iotests/112.out       |   2 +-
 tests/qemu-iotests/125           |  24 ++
 tests/qemu-iotests/125.out       |   9 +
 tests/qemu-iotests/134.out       |   2 +-
 tests/qemu-iotests/141           |   2 +-
 tests/qemu-iotests/144.out       |   4 +-
 tests/qemu-iotests/146           |  60 +++--
 tests/qemu-iotests/146.out       | 405 ++++++++++++++++++++++++++++--
 tests/qemu-iotests/153           |   9 +-
 tests/qemu-iotests/158.out       |   4 +-
 tests/qemu-iotests/178           |   1 +
 tests/qemu-iotests/182.out       |   2 +-
 tests/qemu-iotests/185.out       |   8 +-
 tests/qemu-iotests/188           |   1 +
 tests/qemu-iotests/188.out       |   2 +-
 tests/qemu-iotests/189           |   1 +
 tests/qemu-iotests/189.out       |   4 +-
 tests/qemu-iotests/198           |   1 +
 tests/qemu-iotests/198.out       |   4 +-
 tests/qemu-iotests/206           |   1 +
 tests/qemu-iotests/255.out       |   8 +-
 tests/qemu-iotests/263           |   1 +
 tests/qemu-iotests/263.out       |   4 +-
 tests/qemu-iotests/274.out       |  46 ++--
 tests/qemu-iotests/280.out       |   2 +-
 tests/qemu-iotests/284           |   1 +
 tests/qemu-iotests/284.out       |   6 +-
 tests/qemu-iotests/293           | 208 ++++++++++++++++
 tests/qemu-iotests/293.out       |  99 ++++++++
 tests/qemu-iotests/294           |  90 +++++++
 tests/qemu-iotests/294.out       |  30 +++
 tests/qemu-iotests/295           | 280 +++++++++++++++++++++
 tests/qemu-iotests/295.out       |  40 +++
 tests/qemu-iotests/296           | 234 +++++++++++++++++
 tests/qemu-iotests/296.out       |  33 +++
 tests/qemu-iotests/common.filter | 106 ++++++--
 tests/qemu-iotests/common.rc     |  30 +++
 tests/qemu-iotests/group         |   4 +
 tests/qemu-iotests/iotests.py    |  84 +++++--
 72 files changed, 3103 insertions(+), 632 deletions(-)
 create mode 100644 block/amend.c
 create mode 100755 tests/qemu-iotests/293
 create mode 100644 tests/qemu-iotests/293.out
 create mode 100755 tests/qemu-iotests/294
 create mode 100644 tests/qemu-iotests/294.out
 create mode 100755 tests/qemu-iotests/295
 create mode 100644 tests/qemu-iotests/295.out
 create mode 100755 tests/qemu-iotests/296
 create mode 100644 tests/qemu-iotests/296.out

-- 
2.26.2