Series comparison

-[Qemu-devel] [PULL 0/7] Block patches
+[PULL 0/8] Block patches
-The following changes since commit 0ab4537f08e09b13788db67efd760592fb7db769:
+The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95:
-  Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2018-03-07-1' into staging (2018-03-08 12:56:39 +0000)
+  Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100)
 are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268:
+for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21:
-  vl: introduce vm_shutdown() (2018-03-08 17:38:51 +0000)
+  coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100)
 ----------------------------------------------------------------
 Pull request
 (Resent due to an email preparation mistake.)
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Paolo Bonzini (6):
   coroutine-sleep: use a stack-allocated timer
   coroutine-sleep: disallow NULL QemuCoSleepState** argument
   coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
   coroutine-sleep: move timer out of QemuCoSleepState
   coroutine-sleep: replace QemuCoSleepState pointer with struct in the
     API
   coroutine-sleep: introduce qemu_co_sleep
-Deepa Srinivasan (1):
+Philippe Mathieu-Daudé (1):
-  block: Fix qemu crash when using scsi-block
+  bitops.h: Improve find_xxx_bit() documentation
-Fam Zheng (1):
+Zenghui Yu (1):
-  README: Fix typo 'git-publish'
+  multi-process: Initialize variables declared with g_auto*
-Sergio Lopez (1):
+ include/qemu/bitops.h       | 15 ++++++--
-  virtio-blk: dataplane: Don't batch notifications if EVENT_IDX is
+ include/qemu/coroutine.h    | 27 ++++++++-----
-    present
+ block/block-copy.c          | 10 ++---
+ block/nbd.c                 | 14 +++----
-Stefan Hajnoczi (4):
+ hw/remote/memory.c          |  5 +--
-  block: add aio_wait_bh_oneshot()
+ hw/remote/proxy.c           |  3 +-
-  virtio-blk: fix race between .ioeventfd_stop() and vq handler
+ util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------
-  virtio-scsi: fix race between .ioeventfd_stop() and vq handler
+files changed, 79 insertions(+), 70 deletions(-)
   vl: introduce vm_shutdown()
  include/block/aio-wait.h        | 13 +++++++++++
  include/sysemu/iothread.h       |  1 -
  include/sysemu/sysemu.h         |  1 +
  block/block-backend.c           | 51 ++++++++++++++++++++---------------------
  cpus.c                          | 16 ++++++++++---
  hw/block/dataplane/virtio-blk.c | 39 +++++++++++++++++++++++--------
  hw/scsi/virtio-scsi-dataplane.c |  9 ++++----
  iothread.c                      | 31 -------------------------
  util/aio-wait.c                 | 31 +++++++++++++++++++++++++
  vl.c                            | 13 +++--------
  README                          |  2 +-
 files changed, 122 insertions(+), 85 deletions(-)
 --
-.14.3
+.31.1

-New patch
+[PULL 1/8] multi-process: Initialize variables declared with g_auto*
+From: Zenghui Yu <yuzenghui@huawei.com>
+Quote docs/devel/style.rst (section "Automatic memory deallocation"):
+* Variables declared with g_auto* MUST always be initialized,
+  otherwise the cleanup function will use uninitialized stack memory
+Initialize @name properly to get rid of the compilation error (using
+gcc-7.3.0 on CentOS):
+../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
+/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
+   g_free (*pp);
+   ^~~~~~~~~~~~
+../hw/remote/proxy.c:350:30: note: 'name' was declared here
+             g_autofree char *name;
+                              ^~~~
+Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
+Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
+Message-id: 20210312112143.1369-1-yuzenghui@huawei.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ hw/remote/memory.c | 5 ++---
+ hw/remote/proxy.c  | 3 +--
+files changed, 3 insertions(+), 5 deletions(-)
+diff --git a/hw/remote/memory.c b/hw/remote/memory.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/remote/memory.c
++++ b/hw/remote/memory.c
+@@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
+     remote_sysmem_reset();
+-    for (region = 0; region < msg->num_fds; region++) {
+-        g_autofree char *name;
++    for (region = 0; region < msg->num_fds; region++, suffix++) {
++        g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
+         subregion = g_new(MemoryRegion, 1);
+-        name = g_strdup_printf("remote-mem-%u", suffix++);
+         memory_region_init_ram_from_fd(subregion, NULL,
+                                        name, sysmem_info->sizes[region],
+                                        true, msg->fds[region],
+diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/remote/proxy.c
++++ b/hw/remote/proxy.c
+@@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
+                    PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
+         if (size) {
+-            g_autofree char *name;
++            g_autofree char *name = g_strdup_printf("bar-region-%d", i);
+             pdev->region[i].dev = pdev;
+             pdev->region[i].present = true;
+             if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
+                 pdev->region[i].memory = true;
+             }
+-            name = g_strdup_printf("bar-region-%d", i);
+             memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
+                                   &proxy_mr_ops, &pdev->region[i],
+                                   name, size);
+--
+.31.1

-[Qemu-devel] [PULL 6/7] virtio-scsi: fix race between .ioeventfd_stop() and vq handler
+[PULL 2/8] bitops.h: Improve find_xxx_bit() documentation
-If the main loop thread invokes .ioeventfd_stop() just as the vq handler
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
 function begins in the IOThread then the handler may lose the race for
 the AioContext lock.  By the time the vq handler is able to acquire the
 AioContext lock the ioeventfd has already been removed and the handler
 isn't supposed to run anymore!
-Use the new aio_wait_bh_oneshot() function to perform ioeventfd removal
+Document the following functions return the bitmap size
-from within the IOThread.  This way no races with the vq handler are
+if no matching bit is found:
 possible.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+- find_first_bit
-Reviewed-by: Fam Zheng <famz@redhat.com>
+- find_next_bit
-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
+- find_last_bit
-Message-id: 20180307144205.20619-4-stefanha@redhat.com
+- find_first_zero_bit
 - find_next_zero_bit
 Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20210510200758.2623154-2-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- hw/scsi/virtio-scsi-dataplane.c | 9 +++++----
+ include/qemu/bitops.h | 15 ++++++++++++---
-file changed, 5 insertions(+), 4 deletions(-)
+file changed, 12 insertions(+), 3 deletions(-)
-diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
+diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
 index XXXXXXX..XXXXXXX 100644
---- a/hw/scsi/virtio-scsi-dataplane.c
+--- a/include/qemu/bitops.h
-+++ b/hw/scsi/virtio-scsi-dataplane.c
++++ b/include/qemu/bitops.h
-@@ -XXX,XX +XXX,XX @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
+@@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr)
-     return 0;
+  * @addr: The address to start the search at
- }
+  * @size: The maximum size to search
+  *
--/* assumes s->ctx held */
+- * Returns the bit number of the first set bit, or size.
--static void virtio_scsi_clear_aio(VirtIOSCSI *s)
++ * Returns the bit number of the last set bit,
-+/* Context: BH in IOThread */
++ * or @size if there is no set bit in the bitmap.
-+static void virtio_scsi_dataplane_stop_bh(void *opaque)
+  */
- {
+ unsigned long find_last_bit(const unsigned long *addr,
-+    VirtIOSCSI *s = opaque;
+                             unsigned long size);
-     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+@@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr,
-     int i;
+  * @addr: The address to base the search on
+  * @offset: The bitnumber to start searching at
-@@ -XXX,XX +XXX,XX @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
+  * @size: The bitmap size in bits
-     return 0;
++ *
++ * Returns the bit number of the next set bit,
- fail_vrings:
++ * or @size if there are no further set bits in the bitmap.
--    virtio_scsi_clear_aio(s);
+  */
-+    aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_stop_bh, s);
+ unsigned long find_next_bit(const unsigned long *addr,
-     aio_context_release(s->ctx);
+                             unsigned long size,
-     for (i = 0; i < vs->conf.num_queues + 2; i++) {
+@@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr,
-         virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+  * @addr: The address to base the search on
-@@ -XXX,XX +XXX,XX @@ void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
+  * @offset: The bitnumber to start searching at
-     s->dataplane_stopping = true;
+  * @size: The bitmap size in bits
++ *
-     aio_context_acquire(s->ctx);
++ * Returns the bit number of the next cleared bit,
--    virtio_scsi_clear_aio(s);
++ * or @size if there are no further clear bits in the bitmap.
-+    aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_stop_bh, s);
+  */
-     aio_context_release(s->ctx);
+ unsigned long find_next_zero_bit(const unsigned long *addr,
-     blk_drain_all(); /* ensure there are no in-flight requests */
+@@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr,
   * @addr: The address to start the search at
   * @size: The maximum size to search
   *
 - * Returns the bit number of the first set bit.
 + * Returns the bit number of the first set bit,
 + * or @size if there is no set bit in the bitmap.
   */
  static inline unsigned long find_first_bit(const unsigned long *addr,
                                             unsigned long size)
@@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr,
   * @addr: The address to start the search at
   * @size: The maximum size to search
   *
 - * Returns the bit number of the first cleared bit.
 + * Returns the bit number of the first cleared bit,
 + * or @size if there is no clear bit in the bitmap.
   */
  static inline unsigned long find_first_zero_bit(const unsigned long *addr,
                                                  unsigned long size)
 --
-.14.3
+.31.1

-[Qemu-devel] [PULL 5/7] virtio-blk: fix race between .ioeventfd_stop() and vq handler
+[PULL 3/8] coroutine-sleep: use a stack-allocated timer
-If the main loop thread invokes .ioeventfd_stop() just as the vq handler
+From: Paolo Bonzini <pbonzini@redhat.com>
 function begins in the IOThread then the handler may lose the race for
 the AioContext lock.  By the time the vq handler is able to acquire the
 AioContext lock the ioeventfd has already been removed and the handler
 isn't supposed to run anymore!
-Use the new aio_wait_bh_oneshot() function to perform ioeventfd removal
+The lifetime of the timer is well-known (it cannot outlive
-from within the IOThread.  This way no races with the vq handler are
+qemu_co_sleep_ns_wakeable, because it's deleted by the time the
-possible.
+coroutine resumes), so it is not necessary to place it on the heap.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Fam Zheng <famz@redhat.com>
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
+Message-id: 20210517100548.28806-2-pbonzini@redhat.com
 Message-id: 20180307144205.20619-3-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- hw/block/dataplane/virtio-blk.c | 24 +++++++++++++++++-------
+ util/qemu-coroutine-sleep.c | 9 ++++-----
-file changed, 17 insertions(+), 7 deletions(-)
+file changed, 4 insertions(+), 5 deletions(-)
-diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 index XXXXXXX..XXXXXXX 100644
---- a/hw/block/dataplane/virtio-blk.c
+--- a/util/qemu-coroutine-sleep.c
-+++ b/hw/block/dataplane/virtio-blk.c
++++ b/util/qemu-coroutine-sleep.c
-@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
+@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
-     return -ENOSYS;
  struct QemuCoSleepState {
      Coroutine *co;
 -    QEMUTimer *ts;
 +    QEMUTimer ts;
      QemuCoSleepState **user_state_pointer;
  };
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
      if (sleep_state->user_state_pointer) {
          *sleep_state->user_state_pointer = NULL;
      }
 -    timer_del(sleep_state->ts);
 +    timer_del(&sleep_state->ts);
      aio_co_wake(sleep_state->co);
  }
-+/* Stop notifications for new requests from guest.
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-+ *
+     AioContext *ctx = qemu_get_current_aio_context();
-+ * Context: BH in IOThread
+     QemuCoSleepState state = {
-+ */
+         .co = qemu_coroutine_self(),
-+static void virtio_blk_data_plane_stop_bh(void *opaque)
+-        .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),
-+{
+         .user_state_pointer = sleep_state,
-+    VirtIOBlockDataPlane *s = opaque;
+     };
-+    unsigned i;
-+
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-+    for (i = 0; i < s->conf->num_queues; i++) {
+         abort();
-+        VirtQueue *vq = virtio_get_queue(s->vdev, i);
+     }
-+
-+        virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, NULL);
++    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
-+    }
+     if (sleep_state) {
-+}
+         *sleep_state = &state;
-+
+     }
- /* Context: QEMU global mutex held */
+-    timer_mod(state.ts, qemu_clock_get_ns(type) + ns);
- void virtio_blk_data_plane_stop(VirtIODevice *vdev)
++    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
- {
+     qemu_coroutine_yield();
-@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
+     if (sleep_state) {
-     trace_virtio_blk_data_plane_stop(s);
+         /*
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-     aio_context_acquire(s->ctx);
+          */
--
+         assert(*sleep_state == NULL);
--    /* Stop notifications for new requests from guest */
+     }
--    for (i = 0; i < nvqs; i++) {
+-    timer_free(state.ts);
--        VirtQueue *vq = virtio_get_queue(s->vdev, i);
+ }
 -
 -        virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, NULL);
 -    }
 +    aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
      /* Drain and switch bs back to the QEMU main loop */
      blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
 --
-.14.3
+.31.1

-[Qemu-devel] [PULL 3/7] virtio-blk: dataplane: Don't batch notifications if EVENT_IDX is present
+[PULL 4/8] coroutine-sleep: disallow NULL QemuCoSleepState** argument
-From: Sergio Lopez <slp@redhat.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-Commit 5b2ffbe4d99843fd8305c573a100047a8c962327 ("virtio-blk: dataplane:
+Simplify the code by removing conditionals.  qemu_co_sleep_ns
-notify guest as a batch") deferred guest notification to a BH in order
+can simply point the argument to an on-stack temporary.
 batch notifications, with purpose of avoiding flooding the guest with
 interruptions.
-This optimization came with a cost. The average latency perceived in the
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-guest is increased by a few microseconds, but also when multiple IO
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-operations finish at the same time, the guest won't be notified until
+Message-id: 20210517100548.28806-3-pbonzini@redhat.com
 all completions from each operation has been run. On the contrary,
 virtio-scsi issues the notification at the end of each completion.
 On the other hand, nowadays we have the EVENT_IDX feature that allows a
 better coordination between QEMU and the Guest OS to avoid sending
 unnecessary interruptions.
 With this change, virtio-blk/dataplane only batches notifications if the
 EVENT_IDX feature is not present.
 Some numbers obtained with fio (ioengine=sync, iodepth=1, direct=1):
  - Test specs:
    * fio-3.4 (ioengine=sync, iodepth=1, direct=1)
    * qemu master
    * virtio-blk with a dedicated iothread (default poll-max-ns)
    * backend: null_blk nr_devices=1 irqmode=2 completion_nsec=280000
    * 8 vCPUs pinned to isolated physical cores
    * Emulator and iothread also pinned to separate isolated cores
    * variance between runs < 1%
  - Not patched
    * numjobs=1:  lat_avg=327.32  irqs=29998
    * numjobs=4:  lat_avg=337.89  irqs=29073
    * numjobs=8:  lat_avg=342.98  irqs=28643
  - Patched:
    * numjobs=1:  lat_avg=323.92  irqs=30262
    * numjobs=4:  lat_avg=332.65  irqs=29520
    * numjobs=8:  lat_avg=335.54  irqs=29323
 Signed-off-by: Sergio Lopez <slp@redhat.com>
 Message-id: 20180307114459.26636-1-slp@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- hw/block/dataplane/virtio-blk.c | 15 +++++++++++++--
+ include/qemu/coroutine.h    |  5 +++--
-file changed, 13 insertions(+), 2 deletions(-)
+ util/qemu-coroutine-sleep.c | 18 +++++-------------
 files changed, 8 insertions(+), 15 deletions(-)
-diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
+diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
 index XXXXXXX..XXXXXXX 100644
---- a/hw/block/dataplane/virtio-blk.c
+--- a/include/qemu/coroutine.h
-+++ b/hw/block/dataplane/virtio-blk.c
++++ b/include/qemu/coroutine.h
-@@ -XXX,XX +XXX,XX @@ struct VirtIOBlockDataPlane {
+@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState;
-     VirtIODevice *vdev;
-     QEMUBH *bh;                     /* bh for guest notification */
+ /**
-     unsigned long *batch_notify_vqs;
+  * Yield the coroutine for a given duration. During this yield, @sleep_state
-+    bool batch_notifications;
+- * (if not NULL) is set to an opaque pointer, which may be used for
++ * is set to an opaque pointer, which may be used for
-     /* Note that these EventNotifiers are assigned by value.  This is
+  * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
-      * fine as long as you do not call event_notifier_cleanup on them
+  * timer fires. Don't save the obtained value to other variables and don't call
-@@ -XXX,XX +XXX,XX @@ struct VirtIOBlockDataPlane {
+  * qemu_co_sleep_wake from another aio context.
- /* Raise an interrupt to signal guest, if necessary */
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
- void virtio_blk_data_plane_notify(VirtIOBlockDataPlane *s, VirtQueue *vq)
+                                             QemuCoSleepState **sleep_state);
  static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  {
--    set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
+-    qemu_co_sleep_ns_wakeable(type, ns, NULL);
--    qemu_bh_schedule(s->bh);
++    QemuCoSleepState *unused = NULL;
-+    if (s->batch_notifications) {
++    qemu_co_sleep_ns_wakeable(type, ns, &unused);
 +        set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
 +        qemu_bh_schedule(s->bh);
 +    } else {
 +        virtio_notify_irqfd(s->vdev, vq);
 +    }
  }
- static void notify_guest_bh(void *opaque)
+ /**
-@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
+index XXXXXXX..XXXXXXX 100644
-     s->starting = true;
+--- a/util/qemu-coroutine-sleep.c
++++ b/util/qemu-coroutine-sleep.c
-+    if (!virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-+        s->batch_notifications = true;
+                                            qemu_co_sleep_ns__scheduled, NULL);
-+    } else {
-+        s->batch_notifications = false;
+     assert(scheduled == qemu_co_sleep_ns__scheduled);
-+    }
+-    if (sleep_state->user_state_pointer) {
 -        *sleep_state->user_state_pointer = NULL;
 -    }
 +    *sleep_state->user_state_pointer = NULL;
      timer_del(&sleep_state->ts);
      aio_co_wake(sleep_state->co);
  }
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
      }
      aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
 -    if (sleep_state) {
 -        *sleep_state = &state;
 -    }
 +    *sleep_state = &state;
      timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
      qemu_coroutine_yield();
 -    if (sleep_state) {
 -        /*
 -         * Note that *sleep_state is cleared during qemu_co_sleep_wake
 -         * before resuming this coroutine.
 -         */
 -        assert(*sleep_state == NULL);
 -    }
 +
-     /* Set up guest notifier (irq) */
++    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
-     r = k->set_guest_notifiers(qbus->parent, nvqs, true);
++    assert(*sleep_state == NULL);
-     if (r != 0) {
+ }
 --
-.14.3
+.31.1

-[Qemu-devel] [PULL 1/7] block: Fix qemu crash when using scsi-block
+[PULL 5/8] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
-From: Deepa Srinivasan <deepa.srinivasan@oracle.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-Starting qemu with the following arguments causes qemu to segfault:
+All callers of qemu_co_sleep_wake are checking whether they are passing
-... -device lsi,id=lsi0 -drive file=iscsi:<...>,format=raw,if=none,node-name=
+a NULL argument inside the pointer-to-pointer: do the check in
-iscsi1 -device scsi-block,bus=lsi0.0,id=<...>,drive=iscsi1
+qemu_co_sleep_wake itself.
-This patch fixes blk_aio_ioctl() so it does not pass stack addresses to
+As a side effect, qemu_co_sleep_wake can be called more than once and
-blk_aio_ioctl_entry() which may be invoked after blk_aio_ioctl() returns. More
+it will only wake the coroutine once; after the first time, the argument
-details about the bug follow.
+will be set to NULL via *sleep_state->user_state_pointer.  However, this
 would not be safe unless co_sleep_cb keeps using the QemuCoSleepState*
 directly, so make it go through the pointer-to-pointer instead.
-blk_aio_ioctl() invokes blk_aio_prwv() with blk_aio_ioctl_entry as the
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-coroutine parameter. blk_aio_prwv() ultimately calls aio_co_enter().
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Message-id: 20210517100548.28806-4-pbonzini@redhat.com
 When blk_aio_ioctl() is executed from within a coroutine context (e.g.
 iscsi_bh_cb()), aio_co_enter() adds the coroutine (blk_aio_ioctl_entry) to
 the current coroutine's wakeup queue. blk_aio_ioctl() then returns.
 When blk_aio_ioctl_entry() executes later, it accesses an invalid pointer:
 ....
     BlkRwCo *rwco = &acb->rwco;
     rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
                              rwco->qiov->iov[0].iov_base);  <--- qiov is
                                                                  invalid here
 ...
 In the case when blk_aio_ioctl() is called from a non-coroutine context,
 blk_aio_ioctl_entry() executes immediately. But if bdrv_co_ioctl() calls
 qemu_coroutine_yield(), blk_aio_ioctl() will return. When the coroutine
 execution is complete, control returns to blk_aio_ioctl_entry() after the call
 to blk_co_ioctl(). There is no invalid reference after this point, but the
 function is still holding on to invalid pointers.
 The fix is to change blk_aio_prwv() to accept a void pointer for the IO buffer
 rather than a QEMUIOVector. blk_aio_prwv() passes this through in BlkRwCo and the
 coroutine function casts it to QEMUIOVector or uses the void pointer directly.
 Signed-off-by: Deepa Srinivasan <deepa.srinivasan@oracle.com>
 Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
 Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
 Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/block-backend.c | 51 +++++++++++++++++++++++++--------------------------
+ block/block-copy.c          |  4 +---
-file changed, 25 insertions(+), 26 deletions(-)
+ block/nbd.c                 |  8 ++------
  util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
 files changed, 15 insertions(+), 18 deletions(-)
-diff --git a/block/block-backend.c b/block/block-backend.c
+diff --git a/block/block-copy.c b/block/block-copy.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/block-backend.c
+--- a/block/block-copy.c
-+++ b/block/block-backend.c
++++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
+@@ -XXX,XX +XXX,XX @@ out:
- typedef struct BlkRwCo {
-     BlockBackend *blk;
+ void block_copy_kick(BlockCopyCallState *call_state)
      int64_t offset;
 -    QEMUIOVector *qiov;
 +    void *iobuf;
      int ret;
      BdrvRequestFlags flags;
  } BlkRwCo;
@@ -XXX,XX +XXX,XX @@ typedef struct BlkRwCo {
  static void blk_read_entry(void *opaque)
  {
-     BlkRwCo *rwco = opaque;
+-    if (call_state->sleep_state) {
-+    QEMUIOVector *qiov = rwco->iobuf;
+-        qemu_co_sleep_wake(call_state->sleep_state);
+-    }
--    rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, rwco->qiov->size,
++    qemu_co_sleep_wake(call_state->sleep_state);
 -                              rwco->qiov, rwco->flags);
 +    rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, qiov->size,
 +                              qiov, rwco->flags);
  }
- static void blk_write_entry(void *opaque)
+ /*
 diff --git a/block/nbd.c b/block/nbd.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/nbd.c
 +++ b/block/nbd.c
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
      BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
      s->drained = true;
 -    if (s->connection_co_sleep_ns_state) {
 -        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 -    }
 +    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
      nbd_co_establish_connection_cancel(bs, false);
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
      s->state = NBD_CLIENT_QUIT;
      if (s->connection_co) {
 -        if (s->connection_co_sleep_ns_state) {
 -            qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 -        }
 +        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
          nbd_co_establish_connection_cancel(bs, true);
      }
      if (qemu_in_coroutine()) {
 diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/qemu-coroutine-sleep.c
 +++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ struct QemuCoSleepState {
  void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
  {
-     BlkRwCo *rwco = opaque;
+-    /* Write of schedule protected by barrier write in aio_co_schedule */
-+    QEMUIOVector *qiov = rwco->iobuf;
+-    const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
+-                                           qemu_co_sleep_ns__scheduled, NULL);
--    rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, rwco->qiov->size,
++    if (sleep_state) {
--                               rwco->qiov, rwco->flags);
++        /* Write of schedule protected by barrier write in aio_co_schedule */
-+    rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, qiov->size,
++        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
-+                               qiov, rwco->flags);
++                                                qemu_co_sleep_ns__scheduled, NULL);
 -    assert(scheduled == qemu_co_sleep_ns__scheduled);
 -    *sleep_state->user_state_pointer = NULL;
 -    timer_del(&sleep_state->ts);
 -    aio_co_wake(sleep_state->co);
 +        assert(scheduled == qemu_co_sleep_ns__scheduled);
 +        *sleep_state->user_state_pointer = NULL;
 +        timer_del(&sleep_state->ts);
 +        aio_co_wake(sleep_state->co);
 +    }
  }
- static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
+ static void co_sleep_cb(void *opaque)
-@@ -XXX,XX +XXX,XX @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
+ {
-     rwco = (BlkRwCo) {
+-    qemu_co_sleep_wake(opaque);
-         .blk    = blk,
++    QemuCoSleepState **sleep_state = opaque;
-         .offset = offset,
++    qemu_co_sleep_wake(*sleep_state);
 -        .qiov   = &qiov,
 +        .iobuf  = &qiov,
          .flags  = flags,
          .ret    = NOT_DONE,
      };
@@ -XXX,XX +XXX,XX @@ static void blk_aio_complete_bh(void *opaque)
  }
- static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
+ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
--                                QEMUIOVector *qiov, CoroutineEntry co_entry,
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-+                                void *iobuf, CoroutineEntry co_entry,
+         abort();
-                                 BdrvRequestFlags flags,
+     }
-                                 BlockCompletionFunc *cb, void *opaque)
- {
+-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
-@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
++    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-     acb->rwco = (BlkRwCo) {
+     *sleep_state = &state;
-         .blk    = blk,
+     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
-         .offset = offset,
+     qemu_coroutine_yield();
 -        .qiov   = qiov,
 +        .iobuf  = iobuf,
          .flags  = flags,
          .ret    = NOT_DONE,
      };
@@ -XXX,XX +XXX,XX @@ static void blk_aio_read_entry(void *opaque)
  {
      BlkAioEmAIOCB *acb = opaque;
      BlkRwCo *rwco = &acb->rwco;
 +    QEMUIOVector *qiov = rwco->iobuf;
 -    assert(rwco->qiov->size == acb->bytes);
 +    assert(qiov->size == acb->bytes);
      rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, acb->bytes,
 -                              rwco->qiov, rwco->flags);
 +                              qiov, rwco->flags);
      blk_aio_complete(acb);
  }
@@ -XXX,XX +XXX,XX @@ static void blk_aio_write_entry(void *opaque)
  {
      BlkAioEmAIOCB *acb = opaque;
      BlkRwCo *rwco = &acb->rwco;
 +    QEMUIOVector *qiov = rwco->iobuf;
 -    assert(!rwco->qiov || rwco->qiov->size == acb->bytes);
 +    assert(!qiov || qiov->size == acb->bytes);
      rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, acb->bytes,
 -                               rwco->qiov, rwco->flags);
 +                               qiov, rwco->flags);
      blk_aio_complete(acb);
  }
@@ -XXX,XX +XXX,XX @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
  static void blk_ioctl_entry(void *opaque)
  {
      BlkRwCo *rwco = opaque;
 +    QEMUIOVector *qiov = rwco->iobuf;
 +
      rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
 -                             rwco->qiov->iov[0].iov_base);
 +                             qiov->iov[0].iov_base);
  }
  int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
@@ -XXX,XX +XXX,XX @@ static void blk_aio_ioctl_entry(void *opaque)
      BlkAioEmAIOCB *acb = opaque;
      BlkRwCo *rwco = &acb->rwco;
 -    rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
 -                             rwco->qiov->iov[0].iov_base);
 +    rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset, rwco->iobuf);
 +
      blk_aio_complete(acb);
  }
  BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
                            BlockCompletionFunc *cb, void *opaque)
  {
 -    QEMUIOVector qiov;
 -    struct iovec iov;
 -
 -    iov = (struct iovec) {
 -        .iov_base = buf,
 -        .iov_len = 0,
 -    };
 -    qemu_iovec_init_external(&qiov, &iov, 1);
 -
 -    return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb, opaque);
 +    return blk_aio_prwv(blk, req, 0, buf, blk_aio_ioctl_entry, 0, cb, opaque);
  }
  int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
@@ -XXX,XX +XXX,XX @@ int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
  static void blk_pdiscard_entry(void *opaque)
  {
      BlkRwCo *rwco = opaque;
 -    rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size);
 +    QEMUIOVector *qiov = rwco->iobuf;
 +
 +    rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, qiov->size);
  }
  int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
 --
-.14.3
+.31.1

-[Qemu-devel] [PULL 7/7] vl: introduce vm_shutdown()
+[PULL 6/8] coroutine-sleep: move timer out of QemuCoSleepState
-Commit 00d09fdbbae5f7864ce754913efc84c12fdf9f1a ("vl: pause vcpus before
+From: Paolo Bonzini <pbonzini@redhat.com>
 stopping iothreads") and commit dce8921b2baaf95974af8176406881872067adfa
 ("iothread: Stop threads before main() quits") tried to work around the
 fact that emulation was still active during termination by stopping
 iothreads.  They suffer from race conditions:
 . virtio_scsi_handle_cmd_vq() racing with iothread_stop_all() hits the
    virtio_scsi_ctx_check() assertion failure because the BDS AioContext
    has been modified by iothread_stop_all().
 . Guest vq kick racing with main loop termination leaves a readable
    ioeventfd that is handled by the next aio_poll() when external
    clients are enabled again, resulting in unwanted emulation activity.
-This patch obsoletes those commits by fully disabling emulation activity
+This simplification is enabled by the previous patch.  Now aio_co_wake
-when vcpus are stopped.
+will only be called once, therefore we do not care about a spurious
 firing of the timer after a qemu_co_sleep_wake.
-Use the new vm_shutdown() function instead of pause_all_vcpus() so that
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-vm change state handlers are invoked too.  Virtio devices will now stop
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-their ioeventfds, preventing further emulation activity after vm_stop().
+Message-id: 20210517100548.28806-5-pbonzini@redhat.com
 Note that vm_stop(RUN_STATE_SHUTDOWN) cannot be used because it emits a
 QMP STOP event that may affect existing clients.
 It is no longer necessary to call replay_disable_events() directly since
 vm_shutdown() does so already.
 Drop iothread_stop_all() since it is no longer used.
 Cc: Fam Zheng <famz@redhat.com>
 Cc: Kevin Wolf <kwolf@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Fam Zheng <famz@redhat.com>
 Acked-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20180307144205.20619-5-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- include/sysemu/iothread.h |  1 -
+ util/qemu-coroutine-sleep.c | 8 ++++----
- include/sysemu/sysemu.h   |  1 +
+file changed, 4 insertions(+), 4 deletions(-)
  cpus.c                    | 16 +++++++++++++---
  iothread.c                | 31 -------------------------------
  vl.c                      | 13 +++----------
 files changed, 17 insertions(+), 45 deletions(-)
-diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/sysemu/iothread.h
+--- a/util/qemu-coroutine-sleep.c
-+++ b/include/sysemu/iothread.h
++++ b/util/qemu-coroutine-sleep.c
-@@ -XXX,XX +XXX,XX @@ typedef struct {
+@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
- char *iothread_get_id(IOThread *iothread);
- IOThread *iothread_by_id(const char *id);
+ struct QemuCoSleepState {
- AioContext *iothread_get_aio_context(IOThread *iothread);
+     Coroutine *co;
--void iothread_stop_all(void);
+-    QEMUTimer ts;
- GMainContext *iothread_get_g_main_context(IOThread *iothread);
+     QemuCoSleepState **user_state_pointer;
+ };
- /*
-diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
+@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-index XXXXXXX..XXXXXXX 100644
---- a/include/sysemu/sysemu.h
+         assert(scheduled == qemu_co_sleep_ns__scheduled);
-+++ b/include/sysemu/sysemu.h
+         *sleep_state->user_state_pointer = NULL;
-@@ -XXX,XX +XXX,XX @@ void vm_start(void);
+-        timer_del(&sleep_state->ts);
- int vm_prepare_start(void);
+         aio_co_wake(sleep_state->co);
  int vm_stop(RunState state);
  int vm_stop_force_state(RunState state);
 +int vm_shutdown(void);
  typedef enum WakeupReason {
      /* Always keep QEMU_WAKEUP_REASON_NONE = 0 */
 diff --git a/cpus.c b/cpus.c
 index XXXXXXX..XXXXXXX 100644
 --- a/cpus.c
 +++ b/cpus.c
@@ -XXX,XX +XXX,XX @@ void cpu_synchronize_all_pre_loadvm(void)
      }
  }
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
--static int do_vm_stop(RunState state)
+                                             QemuCoSleepState **sleep_state)
 +static int do_vm_stop(RunState state, bool send_stop)
  {
-     int ret = 0;
+     AioContext *ctx = qemu_get_current_aio_context();
++    QEMUTimer ts;
-@@ -XXX,XX +XXX,XX @@ static int do_vm_stop(RunState state)
+     QemuCoSleepState state = {
-         pause_all_vcpus();
+         .co = qemu_coroutine_self(),
-         runstate_set(state);
+         .user_state_pointer = sleep_state,
-         vm_state_notify(0, state);
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
--        qapi_event_send_stop(&error_abort);
+         abort();
 +        if (send_stop) {
 +            qapi_event_send_stop(&error_abort);
 +        }
      }
-     bdrv_drain_all();
+-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-@@ -XXX,XX +XXX,XX @@ static int do_vm_stop(RunState state)
++    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-     return ret;
+     *sleep_state = &state;
- }
+-    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
++    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
-+/* Special vm_stop() variant for terminating the process.  Historically clients
+     qemu_coroutine_yield();
-+ * did not expect a QMP STOP event and so we need to retain compatibility.
++    timer_del(&ts);
-+ */
-+int vm_shutdown(void)
+     /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
-+{
+     assert(*sleep_state == NULL);
 +    return do_vm_stop(RUN_STATE_SHUTDOWN, false);
 +}
 +
  static bool cpu_can_run(CPUState *cpu)
  {
      if (cpu->stop) {
@@ -XXX,XX +XXX,XX @@ int vm_stop(RunState state)
          return 0;
      }
 -    return do_vm_stop(state);
 +    return do_vm_stop(state, true);
  }
  /**
 diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
 --- a/iothread.c
 +++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ void iothread_stop(IOThread *iothread)
      qemu_thread_join(&iothread->thread);
  }
 -static int iothread_stop_iter(Object *object, void *opaque)
 -{
 -    IOThread *iothread;
 -
 -    iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
 -    if (!iothread) {
 -        return 0;
 -    }
 -    iothread_stop(iothread);
 -    return 0;
 -}
 -
  static void iothread_instance_init(Object *obj)
  {
      IOThread *iothread = IOTHREAD(obj);
@@ -XXX,XX +XXX,XX @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)
      return head;
  }
 -void iothread_stop_all(void)
 -{
 -    Object *container = object_get_objects_root();
 -    BlockDriverState *bs;
 -    BdrvNextIterator it;
 -
 -    for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
 -        AioContext *ctx = bdrv_get_aio_context(bs);
 -        if (ctx == qemu_get_aio_context()) {
 -            continue;
 -        }
 -        aio_context_acquire(ctx);
 -        bdrv_set_aio_context(bs, qemu_get_aio_context());
 -        aio_context_release(ctx);
 -    }
 -
 -    object_child_foreach(container, iothread_stop_iter, NULL);
 -}
 -
  static gpointer iothread_g_main_context_init(gpointer opaque)
  {
      AioContext *ctx;
 diff --git a/vl.c b/vl.c
 index XXXXXXX..XXXXXXX 100644
 --- a/vl.c
 +++ b/vl.c
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
      os_setup_post();
      main_loop();
 -    replay_disable_events();
 -    /* The ordering of the following is delicate.  Stop vcpus to prevent new
 -     * I/O requests being queued by the guest.  Then stop IOThreads (this
 -     * includes a drain operation and completes all request processing).  At
 -     * this point emulated devices are still associated with their IOThreads
 -     * (if any) but no longer have any work to do.  Only then can we close
 -     * block devices safely because we know there is no more I/O coming.
 -     */
 -    pause_all_vcpus();
 -    iothread_stop_all();
 +    /* No more vcpu or device emulation activity beyond this point */
 +    vm_shutdown();
 +
      bdrv_close_all();
      res_free();
 --
-.14.3
+.31.1

-[Qemu-devel] [PULL 2/7] README: Fix typo 'git-publish'
+[PULL 7/8] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API
-From: Fam Zheng <famz@redhat.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-Reported-by: Alberto Garcia <berto@igalia.com>
+Right now, users of qemu_co_sleep_ns_wakeable are simply passing
-Signed-off-by: Fam Zheng <famz@redhat.com>
+a pointer to QemuCoSleepState by reference to the function.  But
-Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
+QemuCoSleepState really is just a Coroutine*; making the
-Message-id: 20180306024328.19195-1-famz@redhat.com
+content of the struct public is just as efficient and lets us
 skip the user_state_pointer indirection.
 Since the usage is changed, take the occasion to rename the
 struct to QemuCoSleep.
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20210517100548.28806-6-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- README | 2 +-
+ include/qemu/coroutine.h    | 23 +++++++++++----------
-file changed, 1 insertion(+), 1 deletion(-)
+ block/block-copy.c          |  8 ++++----
+ block/nbd.c                 | 10 ++++-----
-diff --git a/README b/README
+ util/qemu-coroutine-sleep.c | 41 ++++++++++++++++---------------------
-index XXXXXXX..XXXXXXX 100644
+files changed, 39 insertions(+), 43 deletions(-)
---- a/README
-+++ b/README
+diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
-@@ -XXX,XX +XXX,XX @@ The QEMU website is also maintained under source control.
+index XXXXXXX..XXXXXXX 100644
-   git clone git://git.qemu.org/qemu-web.git
+--- a/include/qemu/coroutine.h
-   https://www.qemu.org/2017/02/04/the-new-qemu-website-is-up/
++++ b/include/qemu/coroutine.h
+@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
--A 'git-profile' utility was created to make above process less
+  */
-+A 'git-publish' utility was created to make above process less
+ void qemu_co_rwlock_unlock(CoRwlock *lock);
- cumbersome, and is highly recommended for making regular contributions,
- or even just for sending consecutive patch series revisions. It also
+-typedef struct QemuCoSleepState QemuCoSleepState;
- requires a working 'git send-email' setup, and by default doesn't
++typedef struct QemuCoSleep {
 +    Coroutine *to_wake;
 +} QemuCoSleep;
  /**
 - * Yield the coroutine for a given duration. During this yield, @sleep_state
 - * is set to an opaque pointer, which may be used for
 - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
 - * timer fires. Don't save the obtained value to other variables and don't call
 - * qemu_co_sleep_wake from another aio context.
 + * Yield the coroutine for a given duration. Initializes @w so that,
 + * during this yield, it can be passed to qemu_co_sleep_wake() to
 + * terminate the sleep.
   */
 -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
 -                                            QemuCoSleepState **sleep_state);
 +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
 +                                            QEMUClockType type, int64_t ns);
 +
  static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  {
 -    QemuCoSleepState *unused = NULL;
 -    qemu_co_sleep_ns_wakeable(type, ns, &unused);
 +    QemuCoSleep w = { 0 };
 +    qemu_co_sleep_ns_wakeable(&w, type, ns);
  }
  /**
@@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
   * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
   * qemu_co_sleep_wake().
   */
 -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
 +void qemu_co_sleep_wake(QemuCoSleep *w);
  /**
   * Yield until a file descriptor becomes readable
 diff --git a/block/block-copy.c b/block/block-copy.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/block-copy.c
 +++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyCallState {
      /* State */
      int ret;
      bool finished;
 -    QemuCoSleepState *sleep_state;
 +    QemuCoSleep sleep;
      bool cancelled;
      /* OUT parameters */
@@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
                  if (ns > 0) {
                      block_copy_task_end(task, -EAGAIN);
                      g_free(task);
 -                    qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
 -                                              &call_state->sleep_state);
 +                    qemu_co_sleep_ns_wakeable(&call_state->sleep,
 +                                              QEMU_CLOCK_REALTIME, ns);
                      continue;
                  }
              }
@@ -XXX,XX +XXX,XX @@ out:
  void block_copy_kick(BlockCopyCallState *call_state)
  {
 -    qemu_co_sleep_wake(call_state->sleep_state);
 +    qemu_co_sleep_wake(&call_state->sleep);
  }
  /*
 diff --git a/block/nbd.c b/block/nbd.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/nbd.c
 +++ b/block/nbd.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVNBDState {
      CoQueue free_sema;
      Coroutine *connection_co;
      Coroutine *teardown_co;
 -    QemuCoSleepState *connection_co_sleep_ns_state;
 +    QemuCoSleep reconnect_sleep;
      bool drained;
      bool wait_drained_end;
      int in_flight;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
      BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
      s->drained = true;
 -    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 +    qemu_co_sleep_wake(&s->reconnect_sleep);
      nbd_co_establish_connection_cancel(bs, false);
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
      s->state = NBD_CLIENT_QUIT;
      if (s->connection_co) {
 -        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 +        qemu_co_sleep_wake(&s->reconnect_sleep);
          nbd_co_establish_connection_cancel(bs, true);
      }
      if (qemu_in_coroutine()) {
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
              }
              bdrv_inc_in_flight(s->bs);
          } else {
 -            qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
 -                                      &s->connection_co_sleep_ns_state);
 +            qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
 +                                      QEMU_CLOCK_REALTIME, timeout);
              if (s->drained) {
                  continue;
              }
 diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/qemu-coroutine-sleep.c
 +++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@
  static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 -struct QemuCoSleepState {
 +void qemu_co_sleep_wake(QemuCoSleep *w)
 +{
      Coroutine *co;
 -    QemuCoSleepState **user_state_pointer;
 -};
 -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
 -{
 -    if (sleep_state) {
 +    co = w->to_wake;
 +    w->to_wake = NULL;
 +    if (co) {
          /* Write of schedule protected by barrier write in aio_co_schedule */
 -        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
 +        const char *scheduled = qatomic_cmpxchg(&co->scheduled,
                                                  qemu_co_sleep_ns__scheduled, NULL);
          assert(scheduled == qemu_co_sleep_ns__scheduled);
 -        *sleep_state->user_state_pointer = NULL;
 -        aio_co_wake(sleep_state->co);
 +        aio_co_wake(co);
      }
  }
  static void co_sleep_cb(void *opaque)
  {
 -    QemuCoSleepState **sleep_state = opaque;
 -    qemu_co_sleep_wake(*sleep_state);
 +    QemuCoSleep *w = opaque;
 +    qemu_co_sleep_wake(w);
  }
 -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
 -                                            QemuCoSleepState **sleep_state)
 +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
 +                                            QEMUClockType type, int64_t ns)
  {
 +    Coroutine *co = qemu_coroutine_self();
      AioContext *ctx = qemu_get_current_aio_context();
      QEMUTimer ts;
 -    QemuCoSleepState state = {
 -        .co = qemu_coroutine_self(),
 -        .user_state_pointer = sleep_state,
 -    };
 -    const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
 -                                           qemu_co_sleep_ns__scheduled);
 +    const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
 +                                            qemu_co_sleep_ns__scheduled);
      if (scheduled) {
          fprintf(stderr,
                  "%s: Co-routine was already scheduled in '%s'\n",
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
          abort();
      }
 -    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
 -    *sleep_state = &state;
 +    w->to_wake = co;
 +    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
      timer_mod(&ts, qemu_clock_get_ns(type) + ns);
      qemu_coroutine_yield();
      timer_del(&ts);
 -    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
 -    assert(*sleep_state == NULL);
 +    /* w->to_wake is cleared before resuming this coroutine.  */
 +    assert(w->to_wake == NULL);
  }
 --
-.14.3
+.31.1

-[Qemu-devel] [PULL 4/7] block: add aio_wait_bh_oneshot()
+[PULL 8/8] coroutine-sleep: introduce qemu_co_sleep
-Sometimes it's necessary for the main loop thread to run a BH in an
+From: Paolo Bonzini <pbonzini@redhat.com>
 IOThread and wait for its completion.  This primitive is useful during
 startup/shutdown to synchronize and avoid race conditions.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
-Reviewed-by: Fam Zheng <famz@redhat.com>
+This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.
-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
-Message-id: 20180307144205.20619-2-stefanha@redhat.com
+In the future we will introduce an API that can work even if the
 sleep and wake happen from different threads.  For now, initializing
 w->to_wake after timer_mod is fine because the timer can only fire in
 the same AioContext.
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20210517100548.28806-7-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- include/block/aio-wait.h | 13 +++++++++++++
+ include/qemu/coroutine.h    |  5 +++++
- util/aio-wait.c          | 31 +++++++++++++++++++++++++++++++
+ util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++-------
-files changed, 44 insertions(+)
+files changed, 24 insertions(+), 7 deletions(-)
-diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
+diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/aio-wait.h
+--- a/include/qemu/coroutine.h
-+++ b/include/block/aio-wait.h
++++ b/include/qemu/coroutine.h
-@@ -XXX,XX +XXX,XX @@ typedef struct {
+@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep {
-  */
+ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
- void aio_wait_kick(AioWait *wait);
+                                             QEMUClockType type, int64_t ns);
 +/**
-+ * aio_wait_bh_oneshot:
++ * Yield the coroutine until the next call to qemu_co_sleep_wake.
 + * @ctx: the aio context
 + * @cb: the BH callback function
 + * @opaque: user data for the BH callback function
 + *
 + * Run a BH in @ctx and wait for it to complete.
 + *
 + * Must be called from the main loop thread with @ctx acquired exactly once.
 + * Note that main loop event processing may occur.
 + */
-+void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
++void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
 +
- #endif /* QEMU_AIO_WAIT */
+ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
-diff --git a/util/aio-wait.c b/util/aio-wait.c
+ {
      QemuCoSleep w = { 0 };
 diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 index XXXXXXX..XXXXXXX 100644
---- a/util/aio-wait.c
+--- a/util/qemu-coroutine-sleep.c
-+++ b/util/aio-wait.c
++++ b/util/qemu-coroutine-sleep.c
-@@ -XXX,XX +XXX,XX @@ void aio_wait_kick(AioWait *wait)
+@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)
-         aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
+     qemu_co_sleep_wake(w);
  }
 -void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
 -                                            QEMUClockType type, int64_t ns)
 +void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
  {
      Coroutine *co = qemu_coroutine_self();
 -    AioContext *ctx = qemu_get_current_aio_context();
 -    QEMUTimer ts;
      const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
                                              qemu_co_sleep_ns__scheduled);
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
      }
+     w->to_wake = co;
+-    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
+-    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
+     qemu_coroutine_yield();
+-    timer_del(&ts);
+     /* w->to_wake is cleared before resuming this coroutine.  */
+     assert(w->to_wake == NULL);
  }
 +
-+typedef struct {
++void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
-+    AioWait wait;
++                                            QEMUClockType type, int64_t ns)
-+    bool done;
++{
-+    QEMUBHFunc *cb;
++    AioContext *ctx = qemu_get_current_aio_context();
-+    void *opaque;
++    QEMUTimer ts;
 +} AioWaitBHData;
 +
-+/* Context: BH in IOThread */
++    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);
-+static void aio_wait_bh(void *opaque)
++    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
 +{
 +    AioWaitBHData *data = opaque;
 +
-+    data->cb(data->opaque);
++    /*
-+
++     * The timer will fire in the current AiOContext, so the callback
-+    data->done = true;
++     * must happen after qemu_co_sleep yields and there is no race
-+    aio_wait_kick(&data->wait);
++     * between timer_mod and qemu_co_sleep.
-+}
++     */
-+
++    qemu_co_sleep(w);
-+void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
++    timer_del(&ts);
 +{
 +    AioWaitBHData data = {
 +        .cb = cb,
 +        .opaque = opaque,
 +    };
 +
 +    assert(qemu_get_current_aio_context() == qemu_get_aio_context());
 +
 +    aio_bh_schedule_oneshot(ctx, aio_wait_bh, &data);
 +    AIO_WAIT_WHILE(&data.wait, ctx, !data.done);
 +}
 --
-.14.3
+.31.1

The following changes since commit 0ab4537f08e09b13788db67efd760592fb7db769:

Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2018-03-07-1' into staging (2018-03-08 12:56:39 +0000)

are available in the Git repository at:

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

for you to fetch changes up to 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268:

vl: introduce vm_shutdown() (2018-03-08 17:38:51 +0000)

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

Deepa Srinivasan (1):
  block: Fix qemu crash when using scsi-block

Fam Zheng (1):
  README: Fix typo 'git-publish'

Sergio Lopez (1):
  virtio-blk: dataplane: Don't batch notifications if EVENT_IDX is
    present

Stefan Hajnoczi (4):
  block: add aio_wait_bh_oneshot()
  virtio-blk: fix race between .ioeventfd_stop() and vq handler
  virtio-scsi: fix race between .ioeventfd_stop() and vq handler
  vl: introduce vm_shutdown()

-- 
2.14.3

From: Deepa Srinivasan <deepa.srinivasan@oracle.com>

Starting qemu with the following arguments causes qemu to segfault:
... -device lsi,id=lsi0 -drive file=iscsi:<...>,format=raw,if=none,node-name=
iscsi1 -device scsi-block,bus=lsi0.0,id=<...>,drive=iscsi1

This patch fixes blk_aio_ioctl() so it does not pass stack addresses to
blk_aio_ioctl_entry() which may be invoked after blk_aio_ioctl() returns. More
details about the bug follow.

blk_aio_ioctl() invokes blk_aio_prwv() with blk_aio_ioctl_entry as the
coroutine parameter. blk_aio_prwv() ultimately calls aio_co_enter().

When blk_aio_ioctl() is executed from within a coroutine context (e.g.
iscsi_bh_cb()), aio_co_enter() adds the coroutine (blk_aio_ioctl_entry) to
the current coroutine's wakeup queue. blk_aio_ioctl() then returns.

When blk_aio_ioctl_entry() executes later, it accesses an invalid pointer:
....
    BlkRwCo *rwco = &acb->rwco;

rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
                             rwco->qiov->iov[0].iov_base);  <--- qiov is
                                                                 invalid here
...

In the case when blk_aio_ioctl() is called from a non-coroutine context,
blk_aio_ioctl_entry() executes immediately. But if bdrv_co_ioctl() calls
qemu_coroutine_yield(), blk_aio_ioctl() will return. When the coroutine
execution is complete, control returns to blk_aio_ioctl_entry() after the call
to blk_co_ioctl(). There is no invalid reference after this point, but the
function is still holding on to invalid pointers.

The fix is to change blk_aio_prwv() to accept a void pointer for the IO buffer
rather than a QEMUIOVector. blk_aio_prwv() passes this through in BlkRwCo and the
coroutine function casts it to QEMUIOVector or uses the void pointer directly.

Signed-off-by: Deepa Srinivasan <deepa.srinivasan@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/block-backend.c | 51 +++++++++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -XXX,XX +XXX,XX @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
 typedef struct BlkRwCo {
     BlockBackend *blk;
     int64_t offset;
-    QEMUIOVector *qiov;
+    void *iobuf;
     int ret;
     BdrvRequestFlags flags;
 } BlkRwCo;
@@ -XXX,XX +XXX,XX @@ typedef struct BlkRwCo {
 static void blk_read_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
+    QEMUIOVector *qiov = rwco->iobuf;
 
-    rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, rwco->qiov->size,
-                              rwco->qiov, rwco->flags);
+    rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, qiov->size,
+                              qiov, rwco->flags);
 }
 
 static void blk_write_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
+    QEMUIOVector *qiov = rwco->iobuf;
 
-    rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, rwco->qiov->size,
-                               rwco->qiov, rwco->flags);
+    rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, qiov->size,
+                               qiov, rwco->flags);
 }
 
 static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
@@ -XXX,XX +XXX,XX @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
     rwco = (BlkRwCo) {
         .blk    = blk,
         .offset = offset,
-        .qiov   = &qiov,
+        .iobuf  = &qiov,
         .flags  = flags,
         .ret    = NOT_DONE,
     };
@@ -XXX,XX +XXX,XX @@ static void blk_aio_complete_bh(void *opaque)
 }
 
 static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
-                                QEMUIOVector *qiov, CoroutineEntry co_entry,
+                                void *iobuf, CoroutineEntry co_entry,
                                 BdrvRequestFlags flags,
                                 BlockCompletionFunc *cb, void *opaque)
 {
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
     acb->rwco = (BlkRwCo) {
         .blk    = blk,
         .offset = offset,
-        .qiov   = qiov,
+        .iobuf  = iobuf,
         .flags  = flags,
         .ret    = NOT_DONE,
     };
@@ -XXX,XX +XXX,XX @@ static void blk_aio_read_entry(void *opaque)
 {
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
+    QEMUIOVector *qiov = rwco->iobuf;
 
-    assert(rwco->qiov->size == acb->bytes);
+    assert(qiov->size == acb->bytes);
     rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, acb->bytes,
-                              rwco->qiov, rwco->flags);
+                              qiov, rwco->flags);
     blk_aio_complete(acb);
 }
 
@@ -XXX,XX +XXX,XX @@ static void blk_aio_write_entry(void *opaque)
 {
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
+    QEMUIOVector *qiov = rwco->iobuf;
 
-    assert(!rwco->qiov || rwco->qiov->size == acb->bytes);
+    assert(!qiov || qiov->size == acb->bytes);
     rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, acb->bytes,
-                               rwco->qiov, rwco->flags);
+                               qiov, rwco->flags);
     blk_aio_complete(acb);
 }
 
@@ -XXX,XX +XXX,XX @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
 static void blk_ioctl_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
+    QEMUIOVector *qiov = rwco->iobuf;
+
     rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
-                             rwco->qiov->iov[0].iov_base);
+                             qiov->iov[0].iov_base);
 }
 
 int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
@@ -XXX,XX +XXX,XX @@ static void blk_aio_ioctl_entry(void *opaque)
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
 
-    rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
-                             rwco->qiov->iov[0].iov_base);
+    rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset, rwco->iobuf);
+
     blk_aio_complete(acb);
 }
 
 BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
                           BlockCompletionFunc *cb, void *opaque)
 {
-    QEMUIOVector qiov;
-    struct iovec iov;
-
-    iov = (struct iovec) {
-        .iov_base = buf,
-        .iov_len = 0,
-    };
-    qemu_iovec_init_external(&qiov, &iov, 1);
-
-    return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb, opaque);
+    return blk_aio_prwv(blk, req, 0, buf, blk_aio_ioctl_entry, 0, cb, opaque);
 }
 
 int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
@@ -XXX,XX +XXX,XX @@ int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
 static void blk_pdiscard_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
-    rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size);
+    QEMUIOVector *qiov = rwco->iobuf;
+
+    rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, qiov->size);
 }
 
 int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
-- 
2.14.3

From: Sergio Lopez <slp@redhat.com>

Commit 5b2ffbe4d99843fd8305c573a100047a8c962327 ("virtio-blk: dataplane:
notify guest as a batch") deferred guest notification to a BH in order
batch notifications, with purpose of avoiding flooding the guest with
interruptions.

This optimization came with a cost. The average latency perceived in the
guest is increased by a few microseconds, but also when multiple IO
operations finish at the same time, the guest won't be notified until
all completions from each operation has been run. On the contrary,
virtio-scsi issues the notification at the end of each completion.

On the other hand, nowadays we have the EVENT_IDX feature that allows a
better coordination between QEMU and the Guest OS to avoid sending
unnecessary interruptions.

With this change, virtio-blk/dataplane only batches notifications if the
EVENT_IDX feature is not present.

Some numbers obtained with fio (ioengine=sync, iodepth=1, direct=1):
 - Test specs:
   * fio-3.4 (ioengine=sync, iodepth=1, direct=1)
   * qemu master
   * virtio-blk with a dedicated iothread (default poll-max-ns)
   * backend: null_blk nr_devices=1 irqmode=2 completion_nsec=280000
   * 8 vCPUs pinned to isolated physical cores
   * Emulator and iothread also pinned to separate isolated cores
   * variance between runs < 1%

- Not patched
   * numjobs=1:  lat_avg=327.32  irqs=29998
   * numjobs=4:  lat_avg=337.89  irqs=29073
   * numjobs=8:  lat_avg=342.98  irqs=28643

- Patched:
   * numjobs=1:  lat_avg=323.92  irqs=30262
   * numjobs=4:  lat_avg=332.65  irqs=29520
   * numjobs=8:  lat_avg=335.54  irqs=29323

Signed-off-by: Sergio Lopez <slp@redhat.com>
Message-id: 20180307114459.26636-1-slp@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ struct VirtIOBlockDataPlane {
     VirtIODevice *vdev;
     QEMUBH *bh;                     /* bh for guest notification */
     unsigned long *batch_notify_vqs;
+    bool batch_notifications;
 
     /* Note that these EventNotifiers are assigned by value.  This is
      * fine as long as you do not call event_notifier_cleanup on them
@@ -XXX,XX +XXX,XX @@ struct VirtIOBlockDataPlane {
 /* Raise an interrupt to signal guest, if necessary */
 void virtio_blk_data_plane_notify(VirtIOBlockDataPlane *s, VirtQueue *vq)
 {
-    set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
-    qemu_bh_schedule(s->bh);
+    if (s->batch_notifications) {
+        set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
+        qemu_bh_schedule(s->bh);
+    } else {
+        virtio_notify_irqfd(s->vdev, vq);
+    }
 }
 
 static void notify_guest_bh(void *opaque)
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
 
     s->starting = true;
 
+    if (!virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+        s->batch_notifications = true;
+    } else {
+        s->batch_notifications = false;
+    }
+
     /* Set up guest notifier (irq) */
     r = k->set_guest_notifiers(qbus->parent, nvqs, true);
     if (r != 0) {
-- 
2.14.3

Sometimes it's necessary for the main loop thread to run a BH in an
IOThread and wait for its completion.  This primitive is useful during
startup/shutdown to synchronize and avoid race conditions.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20180307144205.20619-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/aio-wait.h | 13 +++++++++++++
 util/aio-wait.c          | 31 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/aio-wait.h
+++ b/include/block/aio-wait.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
  */
 void aio_wait_kick(AioWait *wait);
 
+/**
+ * aio_wait_bh_oneshot:
+ * @ctx: the aio context
+ * @cb: the BH callback function
+ * @opaque: user data for the BH callback function
+ *
+ * Run a BH in @ctx and wait for it to complete.
+ *
+ * Must be called from the main loop thread with @ctx acquired exactly once.
+ * Note that main loop event processing may occur.
+ */
+void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+
 #endif /* QEMU_AIO_WAIT */
diff --git a/util/aio-wait.c b/util/aio-wait.c
index XXXXXXX..XXXXXXX 100644
--- a/util/aio-wait.c
+++ b/util/aio-wait.c
@@ -XXX,XX +XXX,XX @@ void aio_wait_kick(AioWait *wait)
         aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
     }
 }
+
+typedef struct {
+    AioWait wait;
+    bool done;
+    QEMUBHFunc *cb;
+    void *opaque;
+} AioWaitBHData;
+
+/* Context: BH in IOThread */
+static void aio_wait_bh(void *opaque)
+{
+    AioWaitBHData *data = opaque;
+
+    data->cb(data->opaque);
+
+    data->done = true;
+    aio_wait_kick(&data->wait);
+}
+
+void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
+{
+    AioWaitBHData data = {
+        .cb = cb,
+        .opaque = opaque,
+    };
+
+    assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+
+    aio_bh_schedule_oneshot(ctx, aio_wait_bh, &data);
+    AIO_WAIT_WHILE(&data.wait, ctx, !data.done);
+}
-- 
2.14.3

If the main loop thread invokes .ioeventfd_stop() just as the vq handler
function begins in the IOThread then the handler may lose the race for
the AioContext lock.  By the time the vq handler is able to acquire the
AioContext lock the ioeventfd has already been removed and the handler
isn't supposed to run anymore!

Use the new aio_wait_bh_oneshot() function to perform ioeventfd removal
from within the IOThread.  This way no races with the vq handler are
possible.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20180307144205.20619-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
     return -ENOSYS;
 }
 
+/* Stop notifications for new requests from guest.
+ *
+ * Context: BH in IOThread
+ */
+static void virtio_blk_data_plane_stop_bh(void *opaque)
+{
+    VirtIOBlockDataPlane *s = opaque;
+    unsigned i;
+
+    for (i = 0; i < s->conf->num_queues; i++) {
+        VirtQueue *vq = virtio_get_queue(s->vdev, i);
+
+        virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, NULL);
+    }
+}
+
 /* Context: QEMU global mutex held */
 void virtio_blk_data_plane_stop(VirtIODevice *vdev)
 {
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
     trace_virtio_blk_data_plane_stop(s);
 
     aio_context_acquire(s->ctx);
-
-    /* Stop notifications for new requests from guest */
-    for (i = 0; i < nvqs; i++) {
-        VirtQueue *vq = virtio_get_queue(s->vdev, i);
-
-        virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, NULL);
-    }
+    aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
 
     /* Drain and switch bs back to the QEMU main loop */
     blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
-- 
2.14.3

Use the new aio_wait_bh_oneshot() function to perform ioeventfd removal
from within the IOThread.  This way no races with the vq handler are
possible.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20180307144205.20619-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/virtio-scsi-dataplane.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -XXX,XX +XXX,XX @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
     return 0;
 }
 
-/* assumes s->ctx held */
-static void virtio_scsi_clear_aio(VirtIOSCSI *s)
+/* Context: BH in IOThread */
+static void virtio_scsi_dataplane_stop_bh(void *opaque)
 {
+    VirtIOSCSI *s = opaque;
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
     int i;
 
@@ -XXX,XX +XXX,XX @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
     return 0;
 
 fail_vrings:
-    virtio_scsi_clear_aio(s);
+    aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_stop_bh, s);
     aio_context_release(s->ctx);
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
         virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
@@ -XXX,XX +XXX,XX @@ void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
     s->dataplane_stopping = true;
 
     aio_context_acquire(s->ctx);
-    virtio_scsi_clear_aio(s);
+    aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_stop_bh, s);
     aio_context_release(s->ctx);
 
     blk_drain_all(); /* ensure there are no in-flight requests */
-- 
2.14.3

Commit 00d09fdbbae5f7864ce754913efc84c12fdf9f1a ("vl: pause vcpus before
stopping iothreads") and commit dce8921b2baaf95974af8176406881872067adfa
("iothread: Stop threads before main() quits") tried to work around the
fact that emulation was still active during termination by stopping
iothreads.  They suffer from race conditions:
1. virtio_scsi_handle_cmd_vq() racing with iothread_stop_all() hits the
   virtio_scsi_ctx_check() assertion failure because the BDS AioContext
   has been modified by iothread_stop_all().
2. Guest vq kick racing with main loop termination leaves a readable
   ioeventfd that is handled by the next aio_poll() when external
   clients are enabled again, resulting in unwanted emulation activity.

This patch obsoletes those commits by fully disabling emulation activity
when vcpus are stopped.

Use the new vm_shutdown() function instead of pause_all_vcpus() so that
vm change state handlers are invoked too.  Virtio devices will now stop
their ioeventfds, preventing further emulation activity after vm_stop().

Note that vm_stop(RUN_STATE_SHUTDOWN) cannot be used because it emits a
QMP STOP event that may affect existing clients.

It is no longer necessary to call replay_disable_events() directly since
vm_shutdown() does so already.

Drop iothread_stop_all() since it is no longer used.

Cc: Fam Zheng <famz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20180307144205.20619-5-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/iothread.h |  1 -
 include/sysemu/sysemu.h   |  1 +
 cpus.c                    | 16 +++++++++++++---
 iothread.c                | 31 -------------------------------
 vl.c                      | 13 +++----------
 5 files changed, 17 insertions(+), 45 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
 char *iothread_get_id(IOThread *iothread);
 IOThread *iothread_by_id(const char *id);
 AioContext *iothread_get_aio_context(IOThread *iothread);
-void iothread_stop_all(void);
 GMainContext *iothread_get_g_main_context(IOThread *iothread);
 
 /*
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -XXX,XX +XXX,XX @@ void vm_start(void);
 int vm_prepare_start(void);
 int vm_stop(RunState state);
 int vm_stop_force_state(RunState state);
+int vm_shutdown(void);
 
 typedef enum WakeupReason {
     /* Always keep QEMU_WAKEUP_REASON_NONE = 0 */
diff --git a/cpus.c b/cpus.c
index XXXXXXX..XXXXXXX 100644
--- a/cpus.c
+++ b/cpus.c
@@ -XXX,XX +XXX,XX @@ void cpu_synchronize_all_pre_loadvm(void)
     }
 }
 
-static int do_vm_stop(RunState state)
+static int do_vm_stop(RunState state, bool send_stop)
 {
     int ret = 0;
 
@@ -XXX,XX +XXX,XX @@ static int do_vm_stop(RunState state)
         pause_all_vcpus();
         runstate_set(state);
         vm_state_notify(0, state);
-        qapi_event_send_stop(&error_abort);
+        if (send_stop) {
+            qapi_event_send_stop(&error_abort);
+        }
     }
 
     bdrv_drain_all();
@@ -XXX,XX +XXX,XX @@ static int do_vm_stop(RunState state)
     return ret;
 }
 
+/* Special vm_stop() variant for terminating the process.  Historically clients
+ * did not expect a QMP STOP event and so we need to retain compatibility.
+ */
+int vm_shutdown(void)
+{
+    return do_vm_stop(RUN_STATE_SHUTDOWN, false);
+}
+
 static bool cpu_can_run(CPUState *cpu)
 {
     if (cpu->stop) {
@@ -XXX,XX +XXX,XX @@ int vm_stop(RunState state)
         return 0;
     }
 
-    return do_vm_stop(state);
+    return do_vm_stop(state, true);
 }
 
 /**
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ void iothread_stop(IOThread *iothread)
     qemu_thread_join(&iothread->thread);
 }
 
-static int iothread_stop_iter(Object *object, void *opaque)
-{
-    IOThread *iothread;
-
-    iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
-    if (!iothread) {
-        return 0;
-    }
-    iothread_stop(iothread);
-    return 0;
-}
-
 static void iothread_instance_init(Object *obj)
 {
     IOThread *iothread = IOTHREAD(obj);
@@ -XXX,XX +XXX,XX @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)
     return head;
 }
 
-void iothread_stop_all(void)
-{
-    Object *container = object_get_objects_root();
-    BlockDriverState *bs;
-    BdrvNextIterator it;
-
-    for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
-        AioContext *ctx = bdrv_get_aio_context(bs);
-        if (ctx == qemu_get_aio_context()) {
-            continue;
-        }
-        aio_context_acquire(ctx);
-        bdrv_set_aio_context(bs, qemu_get_aio_context());
-        aio_context_release(ctx);
-    }
-
-    object_child_foreach(container, iothread_stop_iter, NULL);
-}
-
 static gpointer iothread_g_main_context_init(gpointer opaque)
 {
     AioContext *ctx;
diff --git a/vl.c b/vl.c
index XXXXXXX..XXXXXXX 100644
--- a/vl.c
+++ b/vl.c
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
     os_setup_post();
 
     main_loop();
-    replay_disable_events();
 
-    /* The ordering of the following is delicate.  Stop vcpus to prevent new
-     * I/O requests being queued by the guest.  Then stop IOThreads (this
-     * includes a drain operation and completes all request processing).  At
-     * this point emulated devices are still associated with their IOThreads
-     * (if any) but no longer have any work to do.  Only then can we close
-     * block devices safely because we know there is no more I/O coming.
-     */
-    pause_all_vcpus();
-    iothread_stop_all();
+    /* No more vcpu or device emulation activity beyond this point */
+    vm_shutdown();
+
     bdrv_close_all();
 
     res_free();
-- 
2.14.3

The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95:

Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21:

coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100)

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

(Resent due to an email preparation mistake.)

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

Paolo Bonzini (6):
  coroutine-sleep: use a stack-allocated timer
  coroutine-sleep: disallow NULL QemuCoSleepState** argument
  coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
  coroutine-sleep: move timer out of QemuCoSleepState
  coroutine-sleep: replace QemuCoSleepState pointer with struct in the
    API
  coroutine-sleep: introduce qemu_co_sleep

Philippe Mathieu-Daudé (1):
  bitops.h: Improve find_xxx_bit() documentation

Zenghui Yu (1):
  multi-process: Initialize variables declared with g_auto*

-- 
2.31.1

From: Zenghui Yu <yuzenghui@huawei.com>

Quote docs/devel/style.rst (section "Automatic memory deallocation"):

* Variables declared with g_auto* MUST always be initialized,
  otherwise the cleanup function will use uninitialized stack memory

Initialize @name properly to get rid of the compilation error (using
gcc-7.3.0 on CentOS):

../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   g_free (*pp);
   ^~~~~~~~~~~~
../hw/remote/proxy.c:350:30: note: 'name' was declared here
             g_autofree char *name;
                              ^~~~

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Message-id: 20210312112143.1369-1-yuzenghui@huawei.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/remote/memory.c | 5 ++---
 hw/remote/proxy.c  | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/remote/memory.c b/hw/remote/memory.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/remote/memory.c
+++ b/hw/remote/memory.c
@@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
 
     remote_sysmem_reset();
 
-    for (region = 0; region < msg->num_fds; region++) {
-        g_autofree char *name;
+    for (region = 0; region < msg->num_fds; region++, suffix++) {
+        g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
         subregion = g_new(MemoryRegion, 1);
-        name = g_strdup_printf("remote-mem-%u", suffix++);
         memory_region_init_ram_from_fd(subregion, NULL,
                                        name, sysmem_info->sizes[region],
                                        true, msg->fds[region],
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
                    PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
 
         if (size) {
-            g_autofree char *name;
+            g_autofree char *name = g_strdup_printf("bar-region-%d", i);
             pdev->region[i].dev = pdev;
             pdev->region[i].present = true;
             if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
                 pdev->region[i].memory = true;
             }
-            name = g_strdup_printf("bar-region-%d", i);
             memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
                                   &proxy_mr_ops, &pdev->region[i],
                                   name, size);
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Document the following functions return the bitmap size
if no matching bit is found:

- find_first_bit
- find_next_bit
- find_last_bit
- find_first_zero_bit
- find_next_zero_bit

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20210510200758.2623154-2-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/bitops.h | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr)
  * @addr: The address to start the search at
  * @size: The maximum size to search
  *
- * Returns the bit number of the first set bit, or size.
+ * Returns the bit number of the last set bit,
+ * or @size if there is no set bit in the bitmap.
  */
 unsigned long find_last_bit(const unsigned long *addr,
                             unsigned long size);
@@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr,
  * @addr: The address to base the search on
  * @offset: The bitnumber to start searching at
  * @size: The bitmap size in bits
+ *
+ * Returns the bit number of the next set bit,
+ * or @size if there are no further set bits in the bitmap.
  */
 unsigned long find_next_bit(const unsigned long *addr,
                             unsigned long size,
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr,
  * @addr: The address to base the search on
  * @offset: The bitnumber to start searching at
  * @size: The bitmap size in bits
+ *
+ * Returns the bit number of the next cleared bit,
+ * or @size if there are no further clear bits in the bitmap.
  */
 
 unsigned long find_next_zero_bit(const unsigned long *addr,
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr,
  * @addr: The address to start the search at
  * @size: The maximum size to search
  *
- * Returns the bit number of the first set bit.
+ * Returns the bit number of the first set bit,
+ * or @size if there is no set bit in the bitmap.
  */
 static inline unsigned long find_first_bit(const unsigned long *addr,
                                            unsigned long size)
@@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr,
  * @addr: The address to start the search at
  * @size: The maximum size to search
  *
- * Returns the bit number of the first cleared bit.
+ * Returns the bit number of the first cleared bit,
+ * or @size if there is no clear bit in the bitmap.
  */
 static inline unsigned long find_first_zero_bit(const unsigned long *addr,
                                                 unsigned long size)
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

The lifetime of the timer is well-known (it cannot outlive
qemu_co_sleep_ns_wakeable, because it's deleted by the time the
coroutine resumes), so it is not necessary to place it on the heap.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-2-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine-sleep.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
 struct QemuCoSleepState {
     Coroutine *co;
-    QEMUTimer *ts;
+    QEMUTimer ts;
     QemuCoSleepState **user_state_pointer;
 };
 
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
     if (sleep_state->user_state_pointer) {
         *sleep_state->user_state_pointer = NULL;
     }
-    timer_del(sleep_state->ts);
+    timer_del(&sleep_state->ts);
     aio_co_wake(sleep_state->co);
 }
 
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
     AioContext *ctx = qemu_get_current_aio_context();
     QemuCoSleepState state = {
         .co = qemu_coroutine_self(),
-        .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),
         .user_state_pointer = sleep_state,
     };
 
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
+    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
     if (sleep_state) {
         *sleep_state = &state;
     }
-    timer_mod(state.ts, qemu_clock_get_ns(type) + ns);
+    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
     if (sleep_state) {
         /*
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
          */
         assert(*sleep_state == NULL);
     }
-    timer_free(state.ts);
 }
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Simplify the code by removing conditionals.  qemu_co_sleep_ns
can simply point the argument to an on-stack temporary.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-3-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine.h    |  5 +++--
 util/qemu-coroutine-sleep.c | 18 +++++-------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState;
 
 /**
  * Yield the coroutine for a given duration. During this yield, @sleep_state
- * (if not NULL) is set to an opaque pointer, which may be used for
+ * is set to an opaque pointer, which may be used for
  * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
  * timer fires. Don't save the obtained value to other variables and don't call
  * qemu_co_sleep_wake from another aio context.
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
                                             QemuCoSleepState **sleep_state);
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
-    qemu_co_sleep_ns_wakeable(type, ns, NULL);
+    QemuCoSleepState *unused = NULL;
+    qemu_co_sleep_ns_wakeable(type, ns, &unused);
 }
 
 /**
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
                                            qemu_co_sleep_ns__scheduled, NULL);
 
     assert(scheduled == qemu_co_sleep_ns__scheduled);
-    if (sleep_state->user_state_pointer) {
-        *sleep_state->user_state_pointer = NULL;
-    }
+    *sleep_state->user_state_pointer = NULL;
     timer_del(&sleep_state->ts);
     aio_co_wake(sleep_state->co);
 }
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
     }
 
     aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
-    if (sleep_state) {
-        *sleep_state = &state;
-    }
+    *sleep_state = &state;
     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-    if (sleep_state) {
-        /*
-         * Note that *sleep_state is cleared during qemu_co_sleep_wake
-         * before resuming this coroutine.
-         */
-        assert(*sleep_state == NULL);
-    }
+
+    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
+    assert(*sleep_state == NULL);
 }
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

All callers of qemu_co_sleep_wake are checking whether they are passing
a NULL argument inside the pointer-to-pointer: do the check in
qemu_co_sleep_wake itself.

As a side effect, qemu_co_sleep_wake can be called more than once and
it will only wake the coroutine once; after the first time, the argument
will be set to NULL via *sleep_state->user_state_pointer.  However, this
would not be safe unless co_sleep_cb keeps using the QemuCoSleepState*
directly, so make it go through the pointer-to-pointer instead.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-4-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/block-copy.c          |  4 +---
 block/nbd.c                 |  8 ++------
 util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@ out:
 
 void block_copy_kick(BlockCopyCallState *call_state)
 {
-    if (call_state->sleep_state) {
-        qemu_co_sleep_wake(call_state->sleep_state);
-    }
+    qemu_co_sleep_wake(call_state->sleep_state);
 }
 
 /*
diff --git a/block/nbd.c b/block/nbd.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
 
     s->drained = true;
-    if (s->connection_co_sleep_ns_state) {
-        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
-    }
+    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 
     nbd_co_establish_connection_cancel(bs, false);
 
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
 
     s->state = NBD_CLIENT_QUIT;
     if (s->connection_co) {
-        if (s->connection_co_sleep_ns_state) {
-            qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
-        }
+        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
         nbd_co_establish_connection_cancel(bs, true);
     }
     if (qemu_in_coroutine()) {
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ struct QemuCoSleepState {
 
 void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
 {
-    /* Write of schedule protected by barrier write in aio_co_schedule */
-    const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
-                                           qemu_co_sleep_ns__scheduled, NULL);
+    if (sleep_state) {
+        /* Write of schedule protected by barrier write in aio_co_schedule */
+        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
+                                                qemu_co_sleep_ns__scheduled, NULL);
 
-    assert(scheduled == qemu_co_sleep_ns__scheduled);
-    *sleep_state->user_state_pointer = NULL;
-    timer_del(&sleep_state->ts);
-    aio_co_wake(sleep_state->co);
+        assert(scheduled == qemu_co_sleep_ns__scheduled);
+        *sleep_state->user_state_pointer = NULL;
+        timer_del(&sleep_state->ts);
+        aio_co_wake(sleep_state->co);
+    }
 }
 
 static void co_sleep_cb(void *opaque)
 {
-    qemu_co_sleep_wake(opaque);
+    QemuCoSleepState **sleep_state = opaque;
+    qemu_co_sleep_wake(*sleep_state);
 }
 
 void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
+    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
     *sleep_state = &state;
     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

This simplification is enabled by the previous patch.  Now aio_co_wake
will only be called once, therefore we do not care about a spurious
firing of the timer after a qemu_co_sleep_wake.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-5-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine-sleep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
 struct QemuCoSleepState {
     Coroutine *co;
-    QEMUTimer ts;
     QemuCoSleepState **user_state_pointer;
 };
 
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
 
         assert(scheduled == qemu_co_sleep_ns__scheduled);
         *sleep_state->user_state_pointer = NULL;
-        timer_del(&sleep_state->ts);
         aio_co_wake(sleep_state->co);
     }
 }
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
                                             QemuCoSleepState **sleep_state)
 {
     AioContext *ctx = qemu_get_current_aio_context();
+    QEMUTimer ts;
     QemuCoSleepState state = {
         .co = qemu_coroutine_self(),
         .user_state_pointer = sleep_state,
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
     *sleep_state = &state;
-    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
+    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
+    timer_del(&ts);
 
     /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
     assert(*sleep_state == NULL);
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Right now, users of qemu_co_sleep_ns_wakeable are simply passing
a pointer to QemuCoSleepState by reference to the function.  But
QemuCoSleepState really is just a Coroutine*; making the
content of the struct public is just as efficient and lets us
skip the user_state_pointer indirection.

Since the usage is changed, take the occasion to rename the
struct to QemuCoSleep.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-6-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine.h    | 23 +++++++++++----------
 block/block-copy.c          |  8 ++++----
 block/nbd.c                 | 10 ++++-----
 util/qemu-coroutine-sleep.c | 41 ++++++++++++++++---------------------
 4 files changed, 39 insertions(+), 43 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
  */
 void qemu_co_rwlock_unlock(CoRwlock *lock);
 
-typedef struct QemuCoSleepState QemuCoSleepState;
+typedef struct QemuCoSleep {
+    Coroutine *to_wake;
+} QemuCoSleep;
 
 /**
- * Yield the coroutine for a given duration. During this yield, @sleep_state
- * is set to an opaque pointer, which may be used for
- * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
- * timer fires. Don't save the obtained value to other variables and don't call
- * qemu_co_sleep_wake from another aio context.
+ * Yield the coroutine for a given duration. Initializes @w so that,
+ * during this yield, it can be passed to qemu_co_sleep_wake() to
+ * terminate the sleep.
  */
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-                                            QemuCoSleepState **sleep_state);
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns);
+
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
-    QemuCoSleepState *unused = NULL;
-    qemu_co_sleep_ns_wakeable(type, ns, &unused);
+    QemuCoSleep w = { 0 };
+    qemu_co_sleep_ns_wakeable(&w, type, ns);
 }
 
 /**
@@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
  * qemu_co_sleep_wake().
  */
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
+void qemu_co_sleep_wake(QemuCoSleep *w);
 
 /**
  * Yield until a file descriptor becomes readable
diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyCallState {
     /* State */
     int ret;
     bool finished;
-    QemuCoSleepState *sleep_state;
+    QemuCoSleep sleep;
     bool cancelled;
 
     /* OUT parameters */
@@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
                 if (ns > 0) {
                     block_copy_task_end(task, -EAGAIN);
                     g_free(task);
-                    qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
-                                              &call_state->sleep_state);
+                    qemu_co_sleep_ns_wakeable(&call_state->sleep,
+                                              QEMU_CLOCK_REALTIME, ns);
                     continue;
                 }
             }
@@ -XXX,XX +XXX,XX @@ out:
 
 void block_copy_kick(BlockCopyCallState *call_state)
 {
-    qemu_co_sleep_wake(call_state->sleep_state);
+    qemu_co_sleep_wake(&call_state->sleep);
 }
 
 /*
diff --git a/block/nbd.c b/block/nbd.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVNBDState {
     CoQueue free_sema;
     Coroutine *connection_co;
     Coroutine *teardown_co;
-    QemuCoSleepState *connection_co_sleep_ns_state;
+    QemuCoSleep reconnect_sleep;
     bool drained;
     bool wait_drained_end;
     int in_flight;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
 
     s->drained = true;
-    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+    qemu_co_sleep_wake(&s->reconnect_sleep);
 
     nbd_co_establish_connection_cancel(bs, false);
 
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
 
     s->state = NBD_CLIENT_QUIT;
     if (s->connection_co) {
-        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+        qemu_co_sleep_wake(&s->reconnect_sleep);
         nbd_co_establish_connection_cancel(bs, true);
     }
     if (qemu_in_coroutine()) {
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
             }
             bdrv_inc_in_flight(s->bs);
         } else {
-            qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
-                                      &s->connection_co_sleep_ns_state);
+            qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
+                                      QEMU_CLOCK_REALTIME, timeout);
             if (s->drained) {
                 continue;
             }
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@
 
 static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
-struct QemuCoSleepState {
+void qemu_co_sleep_wake(QemuCoSleep *w)
+{
     Coroutine *co;
-    QemuCoSleepState **user_state_pointer;
-};
 
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-{
-    if (sleep_state) {
+    co = w->to_wake;
+    w->to_wake = NULL;
+    if (co) {
         /* Write of schedule protected by barrier write in aio_co_schedule */
-        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
+        const char *scheduled = qatomic_cmpxchg(&co->scheduled,
                                                 qemu_co_sleep_ns__scheduled, NULL);
 
         assert(scheduled == qemu_co_sleep_ns__scheduled);
-        *sleep_state->user_state_pointer = NULL;
-        aio_co_wake(sleep_state->co);
+        aio_co_wake(co);
     }
 }
 
 static void co_sleep_cb(void *opaque)
 {
-    QemuCoSleepState **sleep_state = opaque;
-    qemu_co_sleep_wake(*sleep_state);
+    QemuCoSleep *w = opaque;
+    qemu_co_sleep_wake(w);
 }
 
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-                                            QemuCoSleepState **sleep_state)
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns)
 {
+    Coroutine *co = qemu_coroutine_self();
     AioContext *ctx = qemu_get_current_aio_context();
     QEMUTimer ts;
-    QemuCoSleepState state = {
-        .co = qemu_coroutine_self(),
-        .user_state_pointer = sleep_state,
-    };
 
-    const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
-                                           qemu_co_sleep_ns__scheduled);
+    const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
+                                            qemu_co_sleep_ns__scheduled);
     if (scheduled) {
         fprintf(stderr,
                 "%s: Co-routine was already scheduled in '%s'\n",
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-    *sleep_state = &state;
+    w->to_wake = co;
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
     timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
     timer_del(&ts);
 
-    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
-    assert(*sleep_state == NULL);
+    /* w->to_wake is cleared before resuming this coroutine.  */
+    assert(w->to_wake == NULL);
 }
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.

In the future we will introduce an API that can work even if the
sleep and wake happen from different threads.  For now, initializing
w->to_wake after timer_mod is fine because the timer can only fire in
the same AioContext.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-7-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine.h    |  5 +++++
 util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++-------
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep {
 void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
                                             QEMUClockType type, int64_t ns);
 
+/**
+ * Yield the coroutine until the next call to qemu_co_sleep_wake.
+ */
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
+
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
     QemuCoSleep w = { 0 };
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)
     qemu_co_sleep_wake(w);
 }
 
-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
-                                            QEMUClockType type, int64_t ns)
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
 {
     Coroutine *co = qemu_coroutine_self();
-    AioContext *ctx = qemu_get_current_aio_context();
-    QEMUTimer ts;
 
     const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
                                             qemu_co_sleep_ns__scheduled);
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
     }
 
     w->to_wake = co;
-    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
-    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-    timer_del(&ts);
 
     /* w->to_wake is cleared before resuming this coroutine.  */
     assert(w->to_wake == NULL);
 }
+
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns)
+{
+    AioContext *ctx = qemu_get_current_aio_context();
+    QEMUTimer ts;
+
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);
+    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
+
+    /*
+     * The timer will fire in the current AiOContext, so the callback
+     * must happen after qemu_co_sleep yields and there is no race
+     * between timer_mod and qemu_co_sleep.
+     */
+    qemu_co_sleep(w);
+    timer_del(&ts);
+}
-- 
2.31.1