virtio_scsi_dataplane_stop() calls blk_drain_all(), which invokes
->drained_begin()/->drained_end() after we've already detached the host
notifier. virtio_scsi_drained_end() currently attaches the host notifier
again and leaves it dangling after dataplane has stopped.

This results in the following assertion failure because
virtio_scsi_defer_to_dataplane() is called from the IOThread instead of
the main loop thread:

  qemu-system-x86_64: ../softmmu/memory.c:1111: memory_region_transaction_commit: Assertion `qemu_mutex_iothread_locked()' failed.

Reported-by: Jean-Louis Dupond <jean-louis@dupond.be>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/virtio-scsi.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 9c8ef0aaa6..45b95ea070 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -1125,7 +1125,16 @@ static void virtio_scsi_drained_begin(SCSIBus *bus)
     uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED +
                             s->parent_obj.conf.num_queues;
 
-    if (!s->dataplane_started) {
+    /*
+     * Drain is called when stopping dataplane but the host notifier has
+     * already been detached. Detaching multiple times is a no-op if nothing
+     * else is using the monitoring same file descriptor, but avoid it just in
+     * case.
+     *
+     * Also, don't detach if dataplane has not even been started yet because
+     * the host notifier isn't attached.
+     */
+    if (s->dataplane_stopping || !s->dataplane_started) {
         return;
     }
 
@@ -1143,7 +1152,14 @@ static void virtio_scsi_drained_end(SCSIBus *bus)
     uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED +
                             s->parent_obj.conf.num_queues;
 
-    if (!s->dataplane_started) {
+    /*
+     * Drain is called when stopping dataplane. Keep the host notifier detached
+     * so it's not left dangling after dataplane is stopped.
+     *
+     * Also, don't attach if dataplane has not even been started yet. We're not
+     * ready.
+     */
+    if (s->dataplane_stopping || !s->dataplane_started) {
         return;
     }
 
-- 
2.40.1

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1680

On Sun, Jun 11, 2023, 15:39 Stefan Hajnoczi <stefanha@redhat.com> wrote:

> virtio_scsi_dataplane_stop() calls blk_drain_all(), which invokes
> ->drained_begin()/->drained_end() after we've already detached the host
> notifier. virtio_scsi_drained_end() currently attaches the host notifier
> again and leaves it dangling after dataplane has stopped.
>
> This results in the following assertion failure because
> virtio_scsi_defer_to_dataplane() is called from the IOThread instead of
> the main loop thread:
>
>   qemu-system-x86_64: ../softmmu/memory.c:1111:
> memory_region_transaction_commit: Assertion `qemu_mutex_iothread_locked()'
> failed.
>
> Reported-by: Jean-Louis Dupond <jean-louis@dupond.be>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/scsi/virtio-scsi.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 9c8ef0aaa6..45b95ea070 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -1125,7 +1125,16 @@ static void virtio_scsi_drained_begin(SCSIBus *bus)
>      uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED +
>                              s->parent_obj.conf.num_queues;
>
> -    if (!s->dataplane_started) {
> +    /*
> +     * Drain is called when stopping dataplane but the host notifier has
> +     * already been detached. Detaching multiple times is a no-op if
> nothing
> +     * else is using the monitoring same file descriptor, but avoid it
> just in
> +     * case.
> +     *
> +     * Also, don't detach if dataplane has not even been started yet
> because
> +     * the host notifier isn't attached.
> +     */
> +    if (s->dataplane_stopping || !s->dataplane_started) {
>          return;
>      }
>
> @@ -1143,7 +1152,14 @@ static void virtio_scsi_drained_end(SCSIBus *bus)
>      uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED +
>                              s->parent_obj.conf.num_queues;
>
> -    if (!s->dataplane_started) {
> +    /*
> +     * Drain is called when stopping dataplane. Keep the host notifier
> detached
> +     * so it's not left dangling after dataplane is stopped.
> +     *
> +     * Also, don't attach if dataplane has not even been started yet.
> We're not
> +     * ready.
> +     */
> +    if (s->dataplane_stopping || !s->dataplane_started) {
>          return;
>      }
>
> --
> 2.40.1
>
>
>