Series comparison

-[Qemu-devel] [PULL 0/2] Block patches
+[PULL 0/1] Block patches
-The following changes since commit 41feb5b955f0d7c9d071b2c5adbc404ae2895c7a:
+The following changes since commit 887cba855bb6ff4775256f7968409281350b568c:
-  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-3.0-pull-request' into staging (2018-06-05 10:38:33 +0100)
+  configure: Fix cross-building for RISCV host (v5) (2023-07-11 17:56:09 +0100)
-are available in the git repository at:
+are available in the Git repository at:
-  git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 68acc99f143b60ec4faa2903065b187d4d3c4bf3:
+for you to fetch changes up to 75dcb4d790bbe5327169fd72b185960ca58e2fa6:
-  sheepdog: remove huge BSS object (2018-06-05 10:15:12 -0400)
+  virtio-blk: fix host notifier issues during dataplane start/stop (2023-07-12 15:20:32 -0400)
 ----------------------------------------------------------------
-Sheepdog patches
+Pull request
 ----------------------------------------------------------------
-Paolo Bonzini (2):
+Stefan Hajnoczi (1):
-  sheepdog: cleanup repeated expression
+  virtio-blk: fix host notifier issues during dataplane start/stop
   sheepdog: remove huge BSS object
- block/sheepdog.c | 28 +++++++++++++++-------------
+ hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
-file changed, 15 insertions(+), 13 deletions(-)
+file changed, 38 insertions(+), 29 deletions(-)
 --
-.13.6
+.40.1

-[Qemu-devel] [PULL 1/2] sheepdog: cleanup repeated expression
+[PULL 1/1] virtio-blk: fix host notifier issues during dataplane start/stop
-From: Paolo Bonzini <pbonzini@redhat.com>
+The main loop thread can consume 100% CPU when using --device
 virtio-blk-pci,iothread=<iothread>. ppoll() constantly returns but
 reading virtqueue host notifiers fails with EAGAIN. The file descriptors
 are stale and remain registered with the AioContext because of bugs in
 the virtio-blk dataplane start/stop code.
-The expression "SD_INODE_SIZE - sizeof(inode.data_vdi_id)" already has a macro
+The problem is that the dataplane start/stop code involves drain
-defined for the same value (though with a nicer definition using offsetof).
+operations, which call virtio_blk_drained_begin() and
-Replace it.
+virtio_blk_drained_end() at points where the host notifier is not
 operational:
 - In virtio_blk_data_plane_start(), blk_set_aio_context() drains after
   vblk->dataplane_started has been set to true but the host notifier has
   not been attached yet.
 - In virtio_blk_data_plane_stop(), blk_drain() and blk_set_aio_context()
   drain after the host notifier has already been detached but with
   vblk->dataplane_started still set to true.
-Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
+I would like to simplify ->ioeventfd_start/stop() to avoid interactions
-Reviewed-by: Fam Zheng <famz@redhat.com>
+with drain entirely, but couldn't find a way to do that. Instead, this
-Reviewed-by: Jeff Cody <jcody@redhat.com>
+patch accepts the fragile nature of the code and reorders it so that
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+vblk->dataplane_started is false during drain operations. This way the
-Message-Id: <20180523160721.14018-2-pbonzini@redhat.com>
+virtio_blk_drained_begin() and virtio_blk_drained_end() calls don't
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+touch the host notifier. The result is that
 virtio_blk_data_plane_start() and virtio_blk_data_plane_stop() have
 complete control over the host notifier and stale file descriptors are
 no longer left in the AioContext.
 This patch fixes the 100% CPU consumption in the main loop thread and
 correctly moves host notifier processing to the IOThread.
 Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()")
 Reported-by: Lukáš Doktor <ldoktor@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Tested-by: Lukas Doktor <ldoktor@redhat.com>
 Message-id: 20230704151527.193586-1-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 6 +++---
+ hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
-file changed, 3 insertions(+), 3 deletions(-)
+file changed, 38 insertions(+), 29 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/hw/block/dataplane/virtio-blk.c
-+++ b/block/sheepdog.c
++++ b/hw/block/dataplane/virtio-blk.c
-@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset,
+@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
      memory_region_transaction_commit();
 -    /*
 -     * These fields are visible to the IOThread so we rely on implicit barriers
 -     * in aio_context_acquire() on the write side and aio_notify_accept() on
 -     * the read side.
 -     */
 -    s->starting = false;
 -    vblk->dataplane_started = true;
      trace_virtio_blk_data_plane_start(s);
      old_context = blk_get_aio_context(s->conf->conf.blk);
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
          event_notifier_set(virtio_queue_get_host_notifier(vq));
      }
-     /* we don't need to update entire object */
++    /*
--    datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
++     * These fields must be visible to the IOThread when it processes the
-+    datalen = SD_INODE_HEADER_SIZE;
++     * virtqueue, otherwise it will think dataplane has not started yet.
-     s->inode.vdi_size = offset;
++     *
-     ret = write_object(fd, s->bs, (char *)&s->inode,
++     * Make sure ->dataplane_started is false when blk_set_aio_context() is
-                        vid_to_vdi_oid(s->inode.vdi_id), s->inode.nr_copies,
++     * called above so that draining does not cause the host notifier to be
-@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
++     * detached/attached prematurely.
-      */
++     */
-     strncpy(s->inode.tag, sn_info->name, sizeof(s->inode.tag));
++    s->starting = false;
-     /* we don't need to update entire object */
++    vblk->dataplane_started = true;
--    datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
++    smp_wmb(); /* paired with aio_notify_accept() on the read side */
-+    datalen = SD_INODE_HEADER_SIZE;
++
-     inode = g_malloc(datalen);
+     /* Get this show started by hooking up our callbacks */
+     if (!blk_in_drain(s->conf->conf.blk)) {
-     /* refresh inode. */
+         aio_context_acquire(s->ctx);
-@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
+@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
-         /* we don't need to read entire object */
+   fail_guest_notifiers:
-         ret = read_object(fd, s->bs, (char *)&inode,
+     vblk->dataplane_disabled = true;
-                           vid_to_vdi_oid(vid),
+     s->starting = false;
--                          0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0,
+-    vblk->dataplane_started = true;
-+                          0, SD_INODE_HEADER_SIZE, 0,
+     return -ENOSYS;
-                           s->cache_flags);
+ }
-         if (ret) {
+@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
          aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
      }
 +    /*
 +     * Batch all the host notifiers in a single transaction to avoid
 +     * quadratic time complexity in address_space_update_ioeventfds().
 +     */
 +    memory_region_transaction_begin();
 +
 +    for (i = 0; i < nvqs; i++) {
 +        virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
 +    }
 +
 +    /*
 +     * The transaction expects the ioeventfds to be open when it
 +     * commits. Do it now, before the cleanup loop.
 +     */
 +    memory_region_transaction_commit();
 +
 +    for (i = 0; i < nvqs; i++) {
 +        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
 +    }
 +
 +    /*
 +     * Set ->dataplane_started to false before draining so that host notifiers
 +     * are not detached/attached anymore.
 +     */
 +    vblk->dataplane_started = false;
 +
      aio_context_acquire(s->ctx);
      /* Wait for virtio_blk_dma_restart_bh() and in flight I/O to complete */
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
      aio_context_release(s->ctx);
 -    /*
 -     * Batch all the host notifiers in a single transaction to avoid
 -     * quadratic time complexity in address_space_update_ioeventfds().
 -     */
 -    memory_region_transaction_begin();
 -
 -    for (i = 0; i < nvqs; i++) {
 -        virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
 -    }
 -
 -    /*
 -     * The transaction expects the ioeventfds to be open when it
 -     * commits. Do it now, before the cleanup loop.
 -     */
 -    memory_region_transaction_commit();
 -
 -    for (i = 0; i < nvqs; i++) {
 -        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
 -    }
 -
      qemu_bh_cancel(s->bh);
      notify_guest_bh(s); /* final chance to notify guest */
      /* Clean up guest notifier (irq) */
      k->set_guest_notifiers(qbus->parent, nvqs, false);
 -    vblk->dataplane_started = false;
      s->stopping = false;
  }
 --
-.13.6
+.40.1

-[Qemu-devel] [PULL 2/2] sheepdog: remove huge BSS object
+Deleted patch
-From: Paolo Bonzini <pbonzini@redhat.com>
-block/sheepdog.o has a 4M static variable that is 90% of QEMU's whole .bss
-section.  Replace it with a heap-allocated block, and make it smaller too
-since only the inode header is actually being used.
-bss size goes down from 4464280 to 269976.
-Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
-Reviewed-by: Jeff Cody <jcody@redhat.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Message-Id: <20180523160721.14018-3-pbonzini@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
----
- block/sheepdog.c | 22 ++++++++++++----------
-file changed, 12 insertions(+), 10 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
-+++ b/block/sheepdog.c
-@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
-     QEMUSnapshotInfo *sn_tab = NULL;
-     unsigned wlen, rlen;
-     int found = 0;
--    static SheepdogInode inode;
-+    SheepdogInode *inode;
-     unsigned long *vdi_inuse;
-     unsigned int start_nr;
-     uint64_t hval;
-     uint32_t vid;
-     vdi_inuse = g_malloc(max);
-+    inode = g_malloc(SD_INODE_HEADER_SIZE);
-     fd = connect_to_sdog(s, &local_err);
-     if (fd < 0) {
-@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
-         }
-         /* we don't need to read entire object */
--        ret = read_object(fd, s->bs, (char *)&inode,
-+        ret = read_object(fd, s->bs, (char *)inode,
-                           vid_to_vdi_oid(vid),
-, SD_INODE_HEADER_SIZE, 0,
-                           s->cache_flags);
-@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
-             continue;
-         }
--        if (!strcmp(inode.name, s->name) && is_snapshot(&inode)) {
--            sn_tab[found].date_sec = inode.snap_ctime >> 32;
--            sn_tab[found].date_nsec = inode.snap_ctime & 0xffffffff;
--            sn_tab[found].vm_state_size = inode.vm_state_size;
--            sn_tab[found].vm_clock_nsec = inode.vm_clock_nsec;
-+        if (!strcmp(inode->name, s->name) && is_snapshot(inode)) {
-+            sn_tab[found].date_sec = inode->snap_ctime >> 32;
-+            sn_tab[found].date_nsec = inode->snap_ctime & 0xffffffff;
-+            sn_tab[found].vm_state_size = inode->vm_state_size;
-+            sn_tab[found].vm_clock_nsec = inode->vm_clock_nsec;
-             snprintf(sn_tab[found].id_str, sizeof(sn_tab[found].id_str),
--                     "%" PRIu32, inode.snap_id);
-+                     "%" PRIu32, inode->snap_id);
-             pstrcpy(sn_tab[found].name,
--                    MIN(sizeof(sn_tab[found].name), sizeof(inode.tag)),
--                    inode.tag);
-+                    MIN(sizeof(sn_tab[found].name), sizeof(inode->tag)),
-+                    inode->tag);
-             found++;
-         }
-     }
-@@ -XXX,XX +XXX,XX @@ out:
-     *psn_tab = sn_tab;
-     g_free(vdi_inuse);
-+    g_free(inode);
-     if (ret < 0) {
-         return ret;
---
-.13.6

From: Paolo Bonzini <pbonzini@redhat.com>

The expression "SD_INODE_SIZE - sizeof(inode.data_vdi_id)" already has a macro
defined for the same value (though with a nicer definition using offsetof).
Replace it.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20180523160721.14018-2-pbonzini@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset,
     }
 
     /* we don't need to update entire object */
-    datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
+    datalen = SD_INODE_HEADER_SIZE;
     s->inode.vdi_size = offset;
     ret = write_object(fd, s->bs, (char *)&s->inode,
                        vid_to_vdi_oid(s->inode.vdi_id), s->inode.nr_copies,
@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
      */
     strncpy(s->inode.tag, sn_info->name, sizeof(s->inode.tag));
     /* we don't need to update entire object */
-    datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
+    datalen = SD_INODE_HEADER_SIZE;
     inode = g_malloc(datalen);
 
     /* refresh inode. */
@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
         /* we don't need to read entire object */
         ret = read_object(fd, s->bs, (char *)&inode,
                           vid_to_vdi_oid(vid),
-                          0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0,
+                          0, SD_INODE_HEADER_SIZE, 0,
                           s->cache_flags);
 
         if (ret) {
-- 
2.13.6

From: Paolo Bonzini <pbonzini@redhat.com>

block/sheepdog.o has a 4M static variable that is 90% of QEMU's whole .bss
section.  Replace it with a heap-allocated block, and make it smaller too
since only the inode header is actually being used.

bss size goes down from 4464280 to 269976.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20180523160721.14018-3-pbonzini@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
     QEMUSnapshotInfo *sn_tab = NULL;
     unsigned wlen, rlen;
     int found = 0;
-    static SheepdogInode inode;
+    SheepdogInode *inode;
     unsigned long *vdi_inuse;
     unsigned int start_nr;
     uint64_t hval;
     uint32_t vid;
 
     vdi_inuse = g_malloc(max);
+    inode = g_malloc(SD_INODE_HEADER_SIZE);
 
     fd = connect_to_sdog(s, &local_err);
     if (fd < 0) {
@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
         }
 
         /* we don't need to read entire object */
-        ret = read_object(fd, s->bs, (char *)&inode,
+        ret = read_object(fd, s->bs, (char *)inode,
                           vid_to_vdi_oid(vid),
                           0, SD_INODE_HEADER_SIZE, 0,
                           s->cache_flags);
@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
             continue;
         }
 
-        if (!strcmp(inode.name, s->name) && is_snapshot(&inode)) {
-            sn_tab[found].date_sec = inode.snap_ctime >> 32;
-            sn_tab[found].date_nsec = inode.snap_ctime & 0xffffffff;
-            sn_tab[found].vm_state_size = inode.vm_state_size;
-            sn_tab[found].vm_clock_nsec = inode.vm_clock_nsec;
+        if (!strcmp(inode->name, s->name) && is_snapshot(inode)) {
+            sn_tab[found].date_sec = inode->snap_ctime >> 32;
+            sn_tab[found].date_nsec = inode->snap_ctime & 0xffffffff;
+            sn_tab[found].vm_state_size = inode->vm_state_size;
+            sn_tab[found].vm_clock_nsec = inode->vm_clock_nsec;
 
             snprintf(sn_tab[found].id_str, sizeof(sn_tab[found].id_str),
-                     "%" PRIu32, inode.snap_id);
+                     "%" PRIu32, inode->snap_id);
             pstrcpy(sn_tab[found].name,
-                    MIN(sizeof(sn_tab[found].name), sizeof(inode.tag)),
-                    inode.tag);
+                    MIN(sizeof(sn_tab[found].name), sizeof(inode->tag)),
+                    inode->tag);
             found++;
         }
     }
@@ -XXX,XX +XXX,XX @@ out:
     *psn_tab = sn_tab;
 
     g_free(vdi_inuse);
+    g_free(inode);
 
     if (ret < 0) {
         return ret;
-- 
2.13.6

The main loop thread can consume 100% CPU when using --device
virtio-blk-pci,iothread=<iothread>. ppoll() constantly returns but
reading virtqueue host notifiers fails with EAGAIN. The file descriptors
are stale and remain registered with the AioContext because of bugs in
the virtio-blk dataplane start/stop code.

The problem is that the dataplane start/stop code involves drain
operations, which call virtio_blk_drained_begin() and
virtio_blk_drained_end() at points where the host notifier is not
operational:
- In virtio_blk_data_plane_start(), blk_set_aio_context() drains after
  vblk->dataplane_started has been set to true but the host notifier has
  not been attached yet.
- In virtio_blk_data_plane_stop(), blk_drain() and blk_set_aio_context()
  drain after the host notifier has already been detached but with
  vblk->dataplane_started still set to true.

I would like to simplify ->ioeventfd_start/stop() to avoid interactions
with drain entirely, but couldn't find a way to do that. Instead, this
patch accepts the fragile nature of the code and reorders it so that
vblk->dataplane_started is false during drain operations. This way the
virtio_blk_drained_begin() and virtio_blk_drained_end() calls don't
touch the host notifier. The result is that
virtio_blk_data_plane_start() and virtio_blk_data_plane_stop() have
complete control over the host notifier and stale file descriptors are
no longer left in the AioContext.

This patch fixes the 100% CPU consumption in the main loop thread and
correctly moves host notifier processing to the IOThread.

Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()")
Reported-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Lukas Doktor <ldoktor@redhat.com>
Message-id: 20230704151527.193586-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 29 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)
 
     memory_region_transaction_commit();
 
-    /*
-     * These fields are visible to the IOThread so we rely on implicit barriers
-     * in aio_context_acquire() on the write side and aio_notify_accept() on
-     * the read side.
-     */
-    s->starting = false;
-    vblk->dataplane_started = true;
     trace_virtio_blk_data_plane_start(s);
 
     old_context = blk_get_aio_context(s->conf->conf.blk);
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
         event_notifier_set(virtio_queue_get_host_notifier(vq));
     }
 
+    /*
+     * These fields must be visible to the IOThread when it processes the
+     * virtqueue, otherwise it will think dataplane has not started yet.
+     *
+     * Make sure ->dataplane_started is false when blk_set_aio_context() is
+     * called above so that draining does not cause the host notifier to be
+     * detached/attached prematurely.
+     */
+    s->starting = false;
+    vblk->dataplane_started = true;
+    smp_wmb(); /* paired with aio_notify_accept() on the read side */
+
     /* Get this show started by hooking up our callbacks */
     if (!blk_in_drain(s->conf->conf.blk)) {
         aio_context_acquire(s->ctx);
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
   fail_guest_notifiers:
     vblk->dataplane_disabled = true;
     s->starting = false;
-    vblk->dataplane_started = true;
     return -ENOSYS;
 }
 
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
         aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
     }
 
+    /*
+     * Batch all the host notifiers in a single transaction to avoid
+     * quadratic time complexity in address_space_update_ioeventfds().
+     */
+    memory_region_transaction_begin();
+
+    for (i = 0; i < nvqs; i++) {
+        virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+    }
+
+    /*
+     * The transaction expects the ioeventfds to be open when it
+     * commits. Do it now, before the cleanup loop.
+     */
+    memory_region_transaction_commit();
+
+    for (i = 0; i < nvqs; i++) {
+        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
+    }
+
+    /*
+     * Set ->dataplane_started to false before draining so that host notifiers
+     * are not detached/attached anymore.
+     */
+    vblk->dataplane_started = false;
+
     aio_context_acquire(s->ctx);
 
     /* Wait for virtio_blk_dma_restart_bh() and in flight I/O to complete */
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
 
     aio_context_release(s->ctx);
 
-    /*
-     * Batch all the host notifiers in a single transaction to avoid
-     * quadratic time complexity in address_space_update_ioeventfds().
-     */
-    memory_region_transaction_begin();
-
-    for (i = 0; i < nvqs; i++) {
-        virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
-    }
-
-    /*
-     * The transaction expects the ioeventfds to be open when it
-     * commits. Do it now, before the cleanup loop.
-     */
-    memory_region_transaction_commit();
-
-    for (i = 0; i < nvqs; i++) {
-        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
-    }
-
     qemu_bh_cancel(s->bh);
     notify_guest_bh(s); /* final chance to notify guest */
 
     /* Clean up guest notifier (irq) */
     k->set_guest_notifiers(qbus->parent, nvqs, false);
 
-    vblk->dataplane_started = false;
     s->stopping = false;
 }
-- 
2.40.1