All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/6] Block layer patches
@ 2024-03-26 13:54 Kevin Wolf
  2024-03-26 19:46 ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Wolf @ 2024-03-26 13:54 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

The following changes since commit 096ae430a7b5a704af4cd94dca7200d6cb069991:

  Merge tag 'pull-qapi-2024-03-26' of https://repo.or.cz/qemu/armbru into staging (2024-03-26 09:50:21 +0000)

are available in the Git repository at:

  https://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 12d7b3bbd3333cededd3b695501d8d247239d769:

  iotests: add test for stream job with an unaligned prefetch read (2024-03-26 14:21:26 +0100)

----------------------------------------------------------------
Block layer patches

- Fix crash with unaligned prefetch requests (e.g. in stream jobs)
- vdpa-dev: Fix initialisation order to restore VDUSE compatibility
- iotests fixes

----------------------------------------------------------------
Fiona Ebner (3):
      block-backend: fix edge case in bdrv_next() where BDS associated to BB changes
      block-backend: fix edge case in bdrv_next_cleanup() where BDS associated to BB changes
      iotests: add test for stream job with an unaligned prefetch read

Kevin Wolf (1):
      vdpa-dev: Fix initialisation order to restore VDUSE compatibility

Stefan Reiter (1):
      block/io: accept NULL qiov in bdrv_pad_request

Thomas Huth (1):
      tests/qemu-iotests: Test 157 and 227 require virtio-blk

 block/block-backend.c                              | 18 ++---
 block/io.c                                         | 33 +++++----
 hw/net/vhost_net.c                                 | 10 +++
 hw/virtio/vdpa-dev.c                               |  5 +-
 hw/virtio/vhost-vdpa.c                             | 29 +++++++-
 hw/virtio/vhost.c                                  |  8 +-
 hw/virtio/trace-events                             |  2 +-
 tests/qemu-iotests/157                             |  2 +
 tests/qemu-iotests/227                             |  2 +
 tests/qemu-iotests/tests/stream-unaligned-prefetch | 86 ++++++++++++++++++++++
 .../tests/stream-unaligned-prefetch.out            |  5 ++
 11 files changed, 167 insertions(+), 33 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/stream-unaligned-prefetch
 create mode 100644 tests/qemu-iotests/tests/stream-unaligned-prefetch.out



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 0/6] Block layer patches
  2024-03-26 13:54 Kevin Wolf
@ 2024-03-26 19:46 ` Peter Maydell
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2024-03-26 19:46 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-block, qemu-devel

On Tue, 26 Mar 2024 at 13:54, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 096ae430a7b5a704af4cd94dca7200d6cb069991:
>
>   Merge tag 'pull-qapi-2024-03-26' of https://repo.or.cz/qemu/armbru into staging (2024-03-26 09:50:21 +0000)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 12d7b3bbd3333cededd3b695501d8d247239d769:
>
>   iotests: add test for stream job with an unaligned prefetch read (2024-03-26 14:21:26 +0100)
>
> ----------------------------------------------------------------
> Block layer patches
>
> - Fix crash with unaligned prefetch requests (e.g. in stream jobs)
> - vdpa-dev: Fix initialisation order to restore VDUSE compatibility
> - iotests fixes
>


Applied, thanks.

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

-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PULL 0/6] Block layer patches
@ 2026-03-31 15:03 Kevin Wolf
  2026-03-31 15:03 ` [PULL 1/6] ide: Fix potential assertion failure on VM stop for PIO read error Kevin Wolf
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-03-31 15:03 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

The following changes since commit 49ea0100202fe448be1d0f3ea5384cd51b5e6a58:

  Merge tag 'migration-20260330-pull-request' of https://gitlab.com/farosas/qemu into staging (2026-03-31 09:39:52 +0100)

are available in the Git repository at:

  https://gitlab.com/kmwolf/qemu.git tags/for-upstream

for you to fetch changes up to a55402d5c3a8c63c801de86896f86c9abeda0ca8:

  block: Fix crash after setting latency historygram with single bin (2026-03-31 15:47:52 +0200)

----------------------------------------------------------------
Block layer patches

- ide: Fix potential assertion failure on VM stop for PIO read error
- scsi: Don't consider LOGICAL UNIT NOT SUPPORTED guest recoverable
- vhost-user-blk-server: fix opt_io_size=1 causing severe Windows I/O degradation
- monitor: Fix deadlock in monitor_cleanup
- Fix filename references in comments

----------------------------------------------------------------
Kevin Wolf (3):
      ide: Fix potential assertion failure on VM stop for PIO read error
      scsi: Don't consider LOGICAL UNIT NOT SUPPORTED guest recoverable
      block: Fix crash after setting latency historygram with single bin

Max Makarov (1):
      vhost-user-blk-server: fix opt_io_size=1 causing severe Windows I/O degradation

Yunjian Long (1):
      block: Fix references in bdrv_bsc_*() function comments

hongmianquan (1):
      monitor: Fix deadlock in monitor_cleanup

 block.c                              |  6 +++---
 block/accounting.c                   |  9 +++++++++
 block/export/vhost-user-blk-server.c |  4 ++--
 hw/ide/core.c                        |  1 +
 qapi/qmp-dispatch.c                  | 10 ++++++++++
 scsi/utils.c                         |  1 -
 6 files changed, 25 insertions(+), 6 deletions(-)



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PULL 1/6] ide: Fix potential assertion failure on VM stop for PIO read error
  2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
@ 2026-03-31 15:03 ` Kevin Wolf
  2026-03-31 15:03 ` [PULL 2/6] scsi: Don't consider LOGICAL UNIT NOT SUPPORTED guest recoverable Kevin Wolf
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-03-31 15:03 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

ide_sector_read() as well as its callers neglect to call ide_set_retry()
before starting I/O. If the I/O fails, this means that the retry
information is stale. In particular, ide_handle_rw_error() has an
assertion that s->bus->retry_unit == s->unit, which can fail if either
there was no previous request or it came from another device on the bus.
If the assertion weren't there, a wrong request would be retried after
resuming the VM.

Fix this by adding a ide_set_retry() call to ide_sector_read().

This affects only reads because ide_transfer_start() does call
ide_set_retry(). For writes, the data transfer comes first and the I/O
is only started when the data has been read into s->io_buffer, so by
that time, ide_set_retry() has been called. For reads, however, the I/O
comes first and only then the data is transferred to the guest, so the
call in ide_transfer_start() is too late.

Buglink: https://redhat.atlassian.net/browse/RHEL-153537
Reported-by: Tingting Mao <timao@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260326165124.138593-1-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d6719dbf31d..7a15d6cac9b 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -799,6 +799,7 @@ static void ide_sector_read(IDEState *s)
     s->error = 0; /* not needed by IDE spec, but needed by Windows */
     sector_num = ide_get_sector(s);
     n = s->nsector;
+    ide_set_retry(s);
 
     if (n == 0) {
         ide_transfer_stop(s);
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PULL 2/6] scsi: Don't consider LOGICAL UNIT NOT SUPPORTED guest recoverable
  2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
  2026-03-31 15:03 ` [PULL 1/6] ide: Fix potential assertion failure on VM stop for PIO read error Kevin Wolf
@ 2026-03-31 15:03 ` Kevin Wolf
  2026-03-31 15:03 ` [PULL 3/6] block: Fix references in bdrv_bsc_*() function comments Kevin Wolf
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-03-31 15:03 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

When commit bdf9613b introduced scsi_sense_buf_is_guest_recoverable(),
it included LOGICAL UNIT NOT SUPPORTED in the list of guest recoverable
sense codes. It doesn't really explain how the codes to be in the list
were selected.

As the LUN doesn't come from the guest, but from the block backend
(usually the SCSI device on the host that was opened with host_device,
but it could also be the iscsi block driver), there is really no way the
guest could influence this.

It seems that on some storage arrays, LOGICAL UNIT NOT SUPPORTED can
happen during failover operations. When combined with multipath, the
request should be retried on another path instead of being reported to
the guest, which would offline the filesystem in response.

Simply returning false in scsi_sense_buf_is_guest_recoverable() will
enable the retry logic in file-posix, and will also make sure that if
the error persists, the configured error policy is respected so that the
VM can be stopped.

Buglink: https://redhat.atlassian.net/browse/RHEL-158212
Fixes: bdf9613b7f87 ('scsi: explicitly list guest-recoverable sense codes')
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260330121635.49205-1-kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 scsi/utils.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scsi/utils.c b/scsi/utils.c
index 545956f4f95..daee90ecf0e 100644
--- a/scsi/utils.c
+++ b/scsi/utils.c
@@ -373,7 +373,6 @@ static bool scsi_sense_is_guest_recoverable(int key, int asc, int ascq)
     case 0x1a00: /* PARAMETER LIST LENGTH ERROR */
     case 0x2000: /* INVALID OPERATION CODE */
     case 0x2400: /* INVALID FIELD IN CDB */
-    case 0x2500: /* LOGICAL UNIT NOT SUPPORTED */
     case 0x2600: /* INVALID FIELD IN PARAMETER LIST */
 
     case 0x2104: /* UNALIGNED WRITE COMMAND */
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PULL 3/6] block: Fix references in bdrv_bsc_*() function comments
  2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
  2026-03-31 15:03 ` [PULL 1/6] ide: Fix potential assertion failure on VM stop for PIO read error Kevin Wolf
  2026-03-31 15:03 ` [PULL 2/6] scsi: Don't consider LOGICAL UNIT NOT SUPPORTED guest recoverable Kevin Wolf
@ 2026-03-31 15:03 ` Kevin Wolf
  2026-03-31 15:03 ` [PULL 4/6] monitor: Fix deadlock in monitor_cleanup Kevin Wolf
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-03-31 15:03 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

From: Yunjian Long <long.yunjian@zte.com.cn>

Some functions are defined in block_int-io.h, so the correct
annotation is block_int-io.h rather than block_int.h

Signed-off-by: Yunjian Long <long.yunjian@zte.com.cn>
Message-ID: <20260325094204300GFCZYBzAE00cSWaldbDcT@zte.com.cn>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 48a17f393c5..f0a6042e616 100644
--- a/block.c
+++ b/block.c
@@ -8477,7 +8477,7 @@ static bool bdrv_bsc_range_overlaps_locked(BlockDriverState *bs,
 }
 
 /**
- * See block_int.h for this function's documentation.
+ * See block_int-io.h for this function's documentation.
  */
 bool bdrv_bsc_is_data(BlockDriverState *bs, int64_t offset, int64_t *pnum)
 {
@@ -8487,7 +8487,7 @@ bool bdrv_bsc_is_data(BlockDriverState *bs, int64_t offset, int64_t *pnum)
 }
 
 /**
- * See block_int.h for this function's documentation.
+ * See block_int-io.h for this function's documentation.
  */
 void bdrv_bsc_invalidate_range(BlockDriverState *bs,
                                int64_t offset, int64_t bytes)
@@ -8501,7 +8501,7 @@ void bdrv_bsc_invalidate_range(BlockDriverState *bs,
 }
 
 /**
- * See block_int.h for this function's documentation.
+ * See block_int-io.h for this function's documentation.
  */
 void bdrv_bsc_fill(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
                   ` (2 preceding siblings ...)
  2026-03-31 15:03 ` [PULL 3/6] block: Fix references in bdrv_bsc_*() function comments Kevin Wolf
@ 2026-03-31 15:03 ` Kevin Wolf
  2026-04-04  6:02   ` Michael Tokarev
  2026-03-31 15:03 ` [PULL 5/6] vhost-user-blk-server: fix opt_io_size=1 causing severe Windows I/O degradation Kevin Wolf
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Kevin Wolf @ 2026-03-31 15:03 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

From: hongmianquan <hongmianquan@bytedance.com>

During qemu_cleanup, if a non-coroutine QMP command (e.g.,
query-commands) is concurrently received and processed by the
mon_iothread, it can lead to a deadlock in monitor_cleanup.

The root cause is a race condition between the main thread's shutdown
sequence and the coroutine's dispatching mechanism. When handling a
non-coroutine QMP command, qmp_dispatcher_co schedules the actual
command execution as a bottom half in iohandler_ctx and then yields. At
this suspended point, qmp_dispatcher_co_busy remains true.

Subsequently, the main thread in monitor_cleanup(), sets
qmp_dispatcher_co_shutdown, and calls qmp_dispatcher_co_wake(). Since
qmp_dispatcher_co_busy is already true, the aio_co_wake is skipped. The
main thread then enters the AIO_WAIT_WHILE_UNLOCKED loop, it executes
the scheduled BH (do_qmp_dispatch_bh) via aio_poll(iohandler_ctx,
false), which attempts to wake up the coroutine, aio_co_wake schedules a
new wake-up BH in iohandler_ctx. The main thread then blocks
indefinitely in aio_poll(qemu_aio_context, true), while the coroutine's
wake-up BH is starved in iohandler_ctx, qmp_dispatcher_co never reaches
termination, resulting in a deadlock.

The execution sequence is illustrated below:

 IO Thread                 Main Thread (qemu_aio_context)        qmp_dispatcher_co (iohandler_ctx)
    |                                 |                                        |
    |-- query-commands                |                                        |
    |-- qmp_dispatcher_co_wake()      |                                        |
    |    (sets busy = true)           |                                        |
    |                                 |   <-- Wakes up in iohandler_ctx -->    |
    |                                 |                                        |-- qmp_dispatch()
    |                                 |                                        |-- Schedules BH (do_qmp_dispatch_bh)
    |                                 |                                        |-- qemu_coroutine_yield()
    |                                 |                                            [State: Suspended, busy=true]
    |   [ quit triggered ]            |
    |                                 |-- monitor_cleanup()
    |                                 |-- qmp_dispatcher_co_shutdown = true
    |                                 |-- qmp_dispatcher_co_wake()
    |                                 |    -> Checks busy flag. It's TRUE!
    |                                 |    -> Skips aio_co_wake().
    |                                 |
    |                                 |-- AIO_WAIT_WHILE_UNLOCKED:
    |                                 |   |-- aio_poll(iohandler_ctx, false)
    |                                 |   |    -> Executes do_qmp_dispatch_bh
    |                                 |   |    -> Schedules 'co_schedule_bh' in iohandler_ctx
    |                                 |   |
    |                                 |   |-- aio_poll(qemu_aio_context, true)
    |                                 |   |    -> Blocks indefinitely! (Deadlock)
    |                                 |
    |                                 X (Main thread sleeping)                 X (Waiting for next iohandler_ctx poll)

To fix this, we add an explicit aio_wait_kick() in do_qmp_dispatch_bh()
to break the main loop out of its blocking poll, allowing it to evaluate
the loop condition and poll iohandler_ctx.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
Signed-off-by: wubo.bob <wubo.bob@bytedance.com>
Message-ID: <20260327131024.51947-1-hongmianquan@bytedance.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/qmp-dispatch.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 9bb1e6a9f4a..e3897d51977 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -128,6 +128,16 @@ static void do_qmp_dispatch_bh(void *opaque)
     data->cmd->fn(data->args, data->ret, data->errp);
     monitor_set_cur(qemu_coroutine_self(), NULL);
     aio_co_wake(data->co);
+
+    /*
+     * If the QMP dispatcher coroutine is waiting to be scheduled
+     * in iohandler_ctx, we must kick the main loop. This ensures
+     * that AIO_WAIT_WHILE_UNLOCKED() in monitor_cleanup() doesn't
+     * block indefinitely waiting for an event in qemu_aio_context,
+     * but actually gets the chance to poll iohandler_ctx and resume
+     * the coroutine.
+     */
+    aio_wait_kick();
 }
 
 /*
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PULL 5/6] vhost-user-blk-server: fix opt_io_size=1 causing severe Windows I/O degradation
  2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
                   ` (3 preceding siblings ...)
  2026-03-31 15:03 ` [PULL 4/6] monitor: Fix deadlock in monitor_cleanup Kevin Wolf
@ 2026-03-31 15:03 ` Kevin Wolf
  2026-03-31 15:03 ` [PULL 6/6] block: Fix crash after setting latency historygram with single bin Kevin Wolf
  2026-03-31 18:04 ` [PULL 0/6] Block layer patches Peter Maydell
  6 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-03-31 15:03 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

From: Max Makarov <maxpain177@gmail.com>

The QSD vhost-user-blk export sets opt_io_size=1 and min_io_size=1 in
the virtio config. These values are reported to the guest through the
VPD Block Limits page as OptimalTransferLength=1 block (512 bytes)
and OptimalTransferLengthGranularity=1 block.

Windows respects these hints and splits all I/O into ~512-byte
requests, causing ~100x sequential throughput degradation (150 MB/s
instead of 15+ GB/s). Linux is unaffected as its block layer ignores
these values.

Set both to 0 which means "not reported" per the SCSI Block Limits
VPD spec, allowing Windows to use its own optimal I/O size defaults.

Signed-off-by: Max Makarov <maxpain@linux.com>
Message-ID: <20260330193451.76037-1-maxpain@linux.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/export/vhost-user-blk-server.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index e89422bb85a..67912a3e170 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -242,8 +242,8 @@ vu_blk_initialize_config(BlockDriverState *bs,
     config->blk_size = cpu_to_le32(blk_size);
     config->size_max = cpu_to_le32(0);
     config->seg_max = cpu_to_le32(128 - 2);
-    config->min_io_size = cpu_to_le16(1);
-    config->opt_io_size = cpu_to_le32(1);
+    config->min_io_size = cpu_to_le16(0);
+    config->opt_io_size = cpu_to_le32(0);
     config->num_queues = cpu_to_le16(num_queues);
     config->max_discard_sectors =
         cpu_to_le32(VIRTIO_BLK_MAX_DISCARD_SECTORS);
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PULL 6/6] block: Fix crash after setting latency historygram with single bin
  2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
                   ` (4 preceding siblings ...)
  2026-03-31 15:03 ` [PULL 5/6] vhost-user-blk-server: fix opt_io_size=1 causing severe Windows I/O degradation Kevin Wolf
@ 2026-03-31 15:03 ` Kevin Wolf
  2026-03-31 18:04 ` [PULL 0/6] Block layer patches Peter Maydell
  6 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-03-31 15:03 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

Passing an empty list of boundaries to block-latency-histogram-set sets
up a state that leads to a NULL pointer dereference when the next
request should be accounted for. This is not a useful configuration, so
just error out if the user tries to set it.

The crash can easily be reproduced with the following script:

    qmp() {
    cat <<EOF
    {'execute':'qmp_capabilities'}
    {'execute':'block-latency-histogram-set',
     'arguments': {'id':'ide0','boundaries':[]}}
    {'execute':'cont'}
    EOF
    }

    qmp | ./qemu-system-x86_64 -S -qmp stdio \
        -drive if=none,format=raw,file=null-co:// \
        -device ide-hd,drive=none0,id=ide0

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260331102608.60882-1-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/accounting.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/block/accounting.c b/block/accounting.c
index 5cf51f029b1..f00fe997403 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -185,6 +185,15 @@ int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
         prev = entry->value;
     }
 
+    /*
+     * block_latency_histogram_account() assumes that it can always access
+     * hist->boundaries[0], so require at least one boundary. A histogram with
+     * a single bin is useless anyway.
+     */
+    if (new_nbins <= 1) {
+        return -EINVAL;
+    }
+
     hist->nbins = new_nbins;
     g_free(hist->boundaries);
     hist->boundaries = g_new(uint64_t, hist->nbins - 1);
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PULL 0/6] Block layer patches
  2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
                   ` (5 preceding siblings ...)
  2026-03-31 15:03 ` [PULL 6/6] block: Fix crash after setting latency historygram with single bin Kevin Wolf
@ 2026-03-31 18:04 ` Peter Maydell
  6 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2026-03-31 18:04 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-block, qemu-devel

On Tue, 31 Mar 2026 at 16:04, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 49ea0100202fe448be1d0f3ea5384cd51b5e6a58:
>
>   Merge tag 'migration-20260330-pull-request' of https://gitlab.com/farosas/qemu into staging (2026-03-31 09:39:52 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/kmwolf/qemu.git tags/for-upstream
>
> for you to fetch changes up to a55402d5c3a8c63c801de86896f86c9abeda0ca8:
>
>   block: Fix crash after setting latency historygram with single bin (2026-03-31 15:47:52 +0200)
>
> ----------------------------------------------------------------
> Block layer patches
>
> - ide: Fix potential assertion failure on VM stop for PIO read error
> - scsi: Don't consider LOGICAL UNIT NOT SUPPORTED guest recoverable
> - vhost-user-blk-server: fix opt_io_size=1 causing severe Windows I/O degradation
> - monitor: Fix deadlock in monitor_cleanup
> - Fix filename references in comments
>
> ----------------------------------------------------------------



Applied, thanks.

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

-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-03-31 15:03 ` [PULL 4/6] monitor: Fix deadlock in monitor_cleanup Kevin Wolf
@ 2026-04-04  6:02   ` Michael Tokarev
  2026-04-04  6:08     ` Michael Tokarev
  0 siblings, 1 reply; 18+ messages in thread
From: Michael Tokarev @ 2026-04-04  6:02 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: qemu-devel, peter.maydell

On 31.03.2026 18:03, Kevin Wolf wrote:
> From: hongmianquan <hongmianquan@bytedance.com>
> 
> During qemu_cleanup, if a non-coroutine QMP command (e.g.,
> query-commands) is concurrently received and processed by the
> mon_iothread, it can lead to a deadlock in monitor_cleanup.
> 
> The root cause is a race condition between the main thread's shutdown
> sequence and the coroutine's dispatching mechanism. When handling a
> non-coroutine QMP command, qmp_dispatcher_co schedules the actual
> command execution as a bottom half in iohandler_ctx and then yields. At
> this suspended point, qmp_dispatcher_co_busy remains true.
> 
> Subsequently, the main thread in monitor_cleanup(), sets
> qmp_dispatcher_co_shutdown, and calls qmp_dispatcher_co_wake(). Since
> qmp_dispatcher_co_busy is already true, the aio_co_wake is skipped. The
> main thread then enters the AIO_WAIT_WHILE_UNLOCKED loop, it executes
> the scheduled BH (do_qmp_dispatch_bh) via aio_poll(iohandler_ctx,
> false), which attempts to wake up the coroutine, aio_co_wake schedules a
> new wake-up BH in iohandler_ctx. The main thread then blocks
> indefinitely in aio_poll(qemu_aio_context, true), while the coroutine's
> wake-up BH is starved in iohandler_ctx, qmp_dispatcher_co never reaches
> termination, resulting in a deadlock.
...


> To fix this, we add an explicit aio_wait_kick() in do_qmp_dispatch_bh()
> to break the main loop out of its blocking poll, allowing it to evaluate
> the loop condition and poll iohandler_ctx.

This broke qemu guest agent build.

[18/51] Linking target qga/qemu-ga
FAILED: qga/qemu-ga
ld: libqemuutil.a.p/qapi_qmp-dispatch.c.o: in function `do_qmp_dispatch_bh':
qapi/qmp-dispatch.c:140:(.text+0x5c): undefined reference to `aio_wait_kick'
collect2: error: ld returned 1 exit status

This is an interesting situation.
I think we've API boundary separation violation here.

This change assumes qmp is only used by qemu-system binary, but
apparently it isn't.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-04-04  6:02   ` Michael Tokarev
@ 2026-04-04  6:08     ` Michael Tokarev
  2026-04-07 12:04       ` Kevin Wolf
  0 siblings, 1 reply; 18+ messages in thread
From: Michael Tokarev @ 2026-04-04  6:08 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: qemu-devel, peter.maydell

On 04.04.2026 09:02, Michael Tokarev wrote:
..
> This broke qemu guest agent build.
> 
> [18/51] Linking target qga/qemu-ga
> FAILED: qga/qemu-ga
> ld: libqemuutil.a.p/qapi_qmp-dispatch.c.o: in function 
> `do_qmp_dispatch_bh':
> qapi/qmp-dispatch.c:140:(.text+0x5c): undefined reference to 
> `aio_wait_kick'
> collect2: error: ld returned 1 exit status

To clarify: it is --disable-system --disable-user build - like,
tools-and-qga-only.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-04-04  6:08     ` Michael Tokarev
@ 2026-04-07 12:04       ` Kevin Wolf
  2026-04-07 13:30         ` Michael Tokarev
  2026-04-07 16:57         ` Michael Tokarev
  0 siblings, 2 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-04-07 12:04 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: qemu-block, qemu-devel, peter.maydell, pbonzini

Am 04.04.2026 um 08:08 hat Michael Tokarev geschrieben:
> On 04.04.2026 09:02, Michael Tokarev wrote:
> ..
> > This broke qemu guest agent build.
> > 
> > [18/51] Linking target qga/qemu-ga
> > FAILED: qga/qemu-ga
> > ld: libqemuutil.a.p/qapi_qmp-dispatch.c.o: in function
> > `do_qmp_dispatch_bh':
> > qapi/qmp-dispatch.c:140:(.text+0x5c): undefined reference to
> > `aio_wait_kick'
> > collect2: error: ld returned 1 exit status
> 
> To clarify: it is --disable-system --disable-user build - like,
> tools-and-qga-only.

I can reproduce it with a qga-only build, but building tools already
fixes it again. That is, the following configure line breaks:

    ../configure --disable-system --disable-user --enable-guest-agent

But this one works:

    ../configure --disable-system --disable-user --enable-guest-agent --enable-tools

utils_ss in the meson files is messy. Enabling something that needs the
block layer enables additional files for qga, too. It should probably be
split in two and qga should always only be built with what is present
when you build only qga and nothing else.

But for now, the patch below should fix it.

Kevin

diff --git a/util/meson.build b/util/meson.build
index 33132c04ad6..637f0b82dd0 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -78,7 +78,7 @@ if have_system
 endif

 if have_block or have_ga
-  util_ss.add(files('aiocb.c', 'async.c'))
+  util_ss.add(files('aiocb.c', 'aio-wait.c', 'async.c'))
   util_ss.add(files('base64.c'))
   util_ss.add(files('main-loop.c'))
   util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 'qemu-coroutine-io.c'))
@@ -87,7 +87,6 @@ if have_block or have_ga
   util_ss.add(files('qemu-sockets.c'))
 endif
 if have_block
-  util_ss.add(files('aio-wait.c'))
   util_ss.add(files('buffer.c'))
   util_ss.add(files('bufferiszero.c'))
   util_ss.add(files('hbitmap.c'))



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-04-07 12:04       ` Kevin Wolf
@ 2026-04-07 13:30         ` Michael Tokarev
  2026-04-07 16:46           ` Kevin Wolf
  2026-04-07 16:57         ` Michael Tokarev
  1 sibling, 1 reply; 18+ messages in thread
From: Michael Tokarev @ 2026-04-07 13:30 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-block, qemu-devel, peter.maydell, pbonzini

On 07.04.2026 15:04, Kevin Wolf wrote:

> utils_ss in the meson files is messy. Enabling something that needs the
> block layer enables additional files for qga, too. It should probably be
> split in two and qga should always only be built with what is present
> when you build only qga and nothing else.
> 
> But for now, the patch below should fix it.

> diff --git a/util/meson.build b/util/meson.build>   if have_block or have_ga
> -  util_ss.add(files('aiocb.c', 'async.c'))
> +  util_ss.add(files('aiocb.c', 'aio-wait.c', 'async.c'))>   endif
>   if have_block
> -  util_ss.add(files('aio-wait.c'))
Yes this seems to fix the link failure.  However, the whole thing is
messy indeed.

I *guess* qapi/qmp-dispatch.c should not be used by qga.  That to say, -
instead of adding more stuff to qga link line, it looks like we should
remove stuff from it :)  And maybe split some source files in two, if
needed.  But this is for 11.1, not for 11.0.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-04-07 13:30         ` Michael Tokarev
@ 2026-04-07 16:46           ` Kevin Wolf
  2026-04-07 17:12             ` Michael Tokarev
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Wolf @ 2026-04-07 16:46 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: qemu-block, qemu-devel, peter.maydell, pbonzini

Am 07.04.2026 um 15:30 hat Michael Tokarev geschrieben:
> On 07.04.2026 15:04, Kevin Wolf wrote:
> 
> > utils_ss in the meson files is messy. Enabling something that needs the
> > block layer enables additional files for qga, too. It should probably be
> > split in two and qga should always only be built with what is present
> > when you build only qga and nothing else.
> > 
> > But for now, the patch below should fix it.
> 
> > diff --git a/util/meson.build b/util/meson.build>   if have_block or have_ga
> > -  util_ss.add(files('aiocb.c', 'async.c'))
> > +  util_ss.add(files('aiocb.c', 'aio-wait.c', 'async.c'))>   endif
> >   if have_block
> > -  util_ss.add(files('aio-wait.c'))
> 
> Yes this seems to fix the link failure.  However, the whole thing is
> messy indeed.
> 
> I *guess* qapi/qmp-dispatch.c should not be used by qga.  That to say, -
> instead of adding more stuff to qga link line, it looks like we should
> remove stuff from it :)

I don't think you can have QMP without qapi/qmp-dispatch.c, so this does
look like the right fix to me. There is no real reason why aio-wait.c
should be considered specific to the block layer.

The part that I don't like about the situation is just that the build
didn't fail without disabling everything else in configure. It should
have failed. But yes, that's for 11.1.

Kevin



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-04-07 12:04       ` Kevin Wolf
  2026-04-07 13:30         ` Michael Tokarev
@ 2026-04-07 16:57         ` Michael Tokarev
  1 sibling, 0 replies; 18+ messages in thread
From: Michael Tokarev @ 2026-04-07 16:57 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-block, qemu-devel, peter.maydell, pbonzini

On 07.04.2026 15:04, Kevin Wolf wrote:
...
> But for now, the patch below should fix it.
> 
> Kevin
> 
> diff --git a/util/meson.build b/util/meson.build
> index 33132c04ad6..637f0b82dd0 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -78,7 +78,7 @@ if have_system
>   endif
> 
>   if have_block or have_ga
> -  util_ss.add(files('aiocb.c', 'async.c'))
> +  util_ss.add(files('aiocb.c', 'aio-wait.c', 'async.c'))
>     util_ss.add(files('base64.c'))
>     util_ss.add(files('main-loop.c'))
>     util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 'qemu-coroutine-io.c'))
> @@ -87,7 +87,6 @@ if have_block or have_ga
>     util_ss.add(files('qemu-sockets.c'))
>   endif
>   if have_block
> -  util_ss.add(files('aio-wait.c'))
>     util_ss.add(files('buffer.c'))
>     util_ss.add(files('bufferiszero.c'))
>     util_ss.add(files('hbitmap.c'))
> 

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
FWIW.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-04-07 16:46           ` Kevin Wolf
@ 2026-04-07 17:12             ` Michael Tokarev
  2026-04-08  9:05               ` Kevin Wolf
  0 siblings, 1 reply; 18+ messages in thread
From: Michael Tokarev @ 2026-04-07 17:12 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-block, qemu-devel, peter.maydell, pbonzini

On 07.04.2026 19:46, Kevin Wolf wrote:
> Am 07.04.2026 um 15:30 hat Michael Tokarev geschrieben:

>> I *guess* qapi/qmp-dispatch.c should not be used by qga.  That to say, -
>> instead of adding more stuff to qga link line, it looks like we should
>> remove stuff from it :)
> 
> I don't think you can have QMP without qapi/qmp-dispatch.c, so this does
> look like the right fix to me. There is no real reason why aio-wait.c
> should be considered specific to the block layer.

Well, or at least, qga can provide stub for aio_wait_kick().

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PULL 4/6] monitor: Fix deadlock in monitor_cleanup
  2026-04-07 17:12             ` Michael Tokarev
@ 2026-04-08  9:05               ` Kevin Wolf
  0 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2026-04-08  9:05 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: qemu-block, qemu-devel, peter.maydell, pbonzini

Am 07.04.2026 um 19:12 hat Michael Tokarev geschrieben:
> On 07.04.2026 19:46, Kevin Wolf wrote:
> > Am 07.04.2026 um 15:30 hat Michael Tokarev geschrieben:
> 
> > > I *guess* qapi/qmp-dispatch.c should not be used by qga.  That to say, -
> > > instead of adding more stuff to qga link line, it looks like we should
> > > remove stuff from it :)
> > 
> > I don't think you can have QMP without qapi/qmp-dispatch.c, so this does
> > look like the right fix to me. There is no real reason why aio-wait.c
> > should be considered specific to the block layer.
> 
> Well, or at least, qga can provide stub for aio_wait_kick().

As far as I can tell, qga has the same monitor setup as qemu, i.e. there
is qemu_aio_context as the default, iohandler_ctx where the monitor I/O
is processed and a monitor iothread.

This means that you need the real aio_wait_kick(), not just a stub, to
avoid deadlocks.

Kevin



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-04-08 19:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31 15:03 [PULL 0/6] Block layer patches Kevin Wolf
2026-03-31 15:03 ` [PULL 1/6] ide: Fix potential assertion failure on VM stop for PIO read error Kevin Wolf
2026-03-31 15:03 ` [PULL 2/6] scsi: Don't consider LOGICAL UNIT NOT SUPPORTED guest recoverable Kevin Wolf
2026-03-31 15:03 ` [PULL 3/6] block: Fix references in bdrv_bsc_*() function comments Kevin Wolf
2026-03-31 15:03 ` [PULL 4/6] monitor: Fix deadlock in monitor_cleanup Kevin Wolf
2026-04-04  6:02   ` Michael Tokarev
2026-04-04  6:08     ` Michael Tokarev
2026-04-07 12:04       ` Kevin Wolf
2026-04-07 13:30         ` Michael Tokarev
2026-04-07 16:46           ` Kevin Wolf
2026-04-07 17:12             ` Michael Tokarev
2026-04-08  9:05               ` Kevin Wolf
2026-04-07 16:57         ` Michael Tokarev
2026-03-31 15:03 ` [PULL 5/6] vhost-user-blk-server: fix opt_io_size=1 causing severe Windows I/O degradation Kevin Wolf
2026-03-31 15:03 ` [PULL 6/6] block: Fix crash after setting latency historygram with single bin Kevin Wolf
2026-03-31 18:04 ` [PULL 0/6] Block layer patches Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2024-03-26 13:54 Kevin Wolf
2024-03-26 19:46 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.