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)

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

From: Thomas Huth <thuth@redhat.com>

Tests 157 and 227 use the virtio-blk device, so we have to mark these

tests accordingly to be skipped if this devices is not available (e.g.

when running the tests with qemu-system-avr only).

Signed-off-by: Thomas Huth <thuth@redhat.com>

Message-ID: <20240325154737.1305063-1-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

tests/qemu-iotests/157 | 2 ++

tests/qemu-iotests/227 | 2 ++

2 files changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/157 b/tests/qemu-iotests/157

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/157

+++ b/tests/qemu-iotests/157

@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15

_supported_fmt generic

_supported_proto file

+_require_devices virtio-blk

+

do_run_qemu()

(

diff --git a/tests/qemu-iotests/227 b/tests/qemu-iotests/227

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/227

+++ b/tests/qemu-iotests/227

@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15

_supported_fmt generic

_supported_proto file

+_require_devices virtio-blk

+

do_run_qemu()

{

echo Testing: "$@"

2.44.0

VDUSE requires that virtqueues are first enabled before the DRIVER_OK

status flag is set; with the current API of the kernel module, it is

impossible to enable the opposite order in our block export code because

userspace is not notified when a virtqueue is enabled.

This requirement also mathces the normal initialisation order as done by

the generic vhost code in QEMU. However, commit 6c482547 accidentally

changed the order for vdpa-dev and broke access to VDUSE devices with

this.

This changes vdpa-dev to use the normal order again and use the standard

vhost callback .vhost_set_vring_enable for this. VDUSE devices can be

used with vdpa-dev again after this fix.

vhost_net intentionally avoided enabling the vrings for vdpa and does

this manually later while it does enable them for other vhost backends.

Reflect this in the vhost_net code and return early for vdpa, so that

the behaviour doesn't change for this device.

Cc: qemu-stable@nongnu.org

Fixes: 6c4825476a43 ('vdpa: move vhost_vdpa_set_vring_ready to the caller')

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-ID: <20240315155949.86066-1-kwolf@redhat.com>

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

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 +-

5 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c

--- a/hw/net/vhost_net.c

+++ b/hw/net/vhost_net.c

@@ -XXX,XX +XXX,XX @@ int vhost_set_vring_enable(NetClientState *nc, int enable)

VHostNetState *net = get_vhost_net(nc);

const VhostOps *vhost_ops = net->dev.vhost_ops;

+ /*

+ * vhost-vdpa network devices need to enable dataplane virtqueues after

+ * DRIVER_OK, so they can recover device state before starting dataplane.

+ * Because of that, we don't enable virtqueues here and leave it to

+ * net/vhost-vdpa.c.

+ */

+ if (nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {

+ return 0;

nc->vring_enable = enable;

if (vhost_ops && vhost_ops->vhost_set_vring_enable) {

diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vdpa-dev.c

+++ b/hw/virtio/vdpa-dev.c

@@ -XXX,XX +XXX,XX @@ static int vhost_vdpa_device_start(VirtIODevice *vdev, Error **errp)

s->dev.acked_features = vdev->guest_features;

- ret = vhost_dev_start(&s->dev, vdev, false);

+ ret = vhost_dev_start(&s->dev, vdev, true);

if (ret < 0) {

error_setg_errno(errp, -ret, "Error starting vhost");

goto err_guest_notifiers;

}

- for (i = 0; i < s->dev.nvqs; ++i) {

- vhost_vdpa_set_vring_ready(&s->vdpa, i);

- }

s->started = true;

/*

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost-vdpa.c

+++ b/hw/virtio/vhost-vdpa.c

@@ -XXX,XX +XXX,XX @@ static int vhost_vdpa_get_vq_index(struct vhost_dev *dev, int idx)

return idx;

}

-int vhost_vdpa_set_vring_ready(struct vhost_vdpa *v, unsigned idx)

+static int vhost_vdpa_set_vring_enable_one(struct vhost_vdpa *v, unsigned idx,

+ int enable)

{

struct vhost_dev *dev = v->dev;

struct vhost_vring_state state = {

.index = idx,

- .num = 1,

+ .num = enable,

};

int r = vhost_vdpa_call(dev, VHOST_VDPA_SET_VRING_ENABLE, &state);

- trace_vhost_vdpa_set_vring_ready(dev, idx, r);

+ trace_vhost_vdpa_set_vring_enable_one(dev, idx, enable, r);

return r;

}

+static int vhost_vdpa_set_vring_enable(struct vhost_dev *dev, int enable)

+{

+ struct vhost_vdpa *v = dev->opaque;

+ unsigned int i;

+ int ret;

+ for (i = 0; i < dev->nvqs; ++i) {

+ ret = vhost_vdpa_set_vring_enable_one(v, i, enable);

+ if (ret < 0) {

+ return ret;

+ }

+ }

+ return 0;

+}

+int vhost_vdpa_set_vring_ready(struct vhost_vdpa *v, unsigned idx)

+{

+ return vhost_vdpa_set_vring_enable_one(v, idx, 1);

+}

static int vhost_vdpa_set_config_call(struct vhost_dev *dev,

int fd)

{

@@ -XXX,XX +XXX,XX @@ const VhostOps vdpa_ops = {

.vhost_set_features = vhost_vdpa_set_features,

.vhost_reset_device = vhost_vdpa_reset_device,

.vhost_get_vq_index = vhost_vdpa_get_vq_index,

+ .vhost_set_vring_enable = vhost_vdpa_set_vring_enable,

.vhost_get_config = vhost_vdpa_get_config,

.vhost_set_config = vhost_vdpa_set_config,

.vhost_requires_shm_log = NULL,

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost.c

+++ b/hw/virtio/vhost.c

@@ -XXX,XX +XXX,XX @@ static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)

return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);

-/* Host notifiers must be enabled at this point. */

+/*

+ * Host notifiers must be enabled at this point.

+ *

+ * If @vrings is true, this function will enable all vrings before starting the

+ * device. If it is false, the vring initialization is left to be done by the

+ * caller.

+ */

int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)

{

int i, r;

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events

--- a/hw/virtio/trace-events

+++ b/hw/virtio/trace-events

@@ -XXX,XX +XXX,XX @@ vhost_vdpa_set_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRI

vhost_vdpa_get_device_id(void *dev, uint32_t device_id) "dev: %p device_id %"PRIu32

vhost_vdpa_reset_device(void *dev) "dev: %p"

vhost_vdpa_get_vq_index(void *dev, int idx, int vq_idx) "dev: %p idx: %d vq idx: %d"

-vhost_vdpa_set_vring_ready(void *dev, unsigned i, int r) "dev: %p, idx: %u, r: %d"

+vhost_vdpa_set_vring_enable_one(void *dev, unsigned i, int enable, int r) "dev: %p, idx: %u, enable: %u, r: %d"

vhost_vdpa_dump_config(void *dev, const char *line) "dev: %p %s"

vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size, uint32_t flags) "dev: %p offset: %"PRIu32" size: %"PRIu32" flags: 0x%"PRIx32

vhost_vdpa_get_config(void *dev, void *config, uint32_t config_len) "dev: %p config: %p config_len: %"PRIu32

2.44.0

From: Stefan Reiter <s.reiter@proxmox.com>

Some operations, e.g. block-stream, perform reads while discarding the

results (only copy-on-read matters). In this case, they will pass NULL

as the target QEMUIOVector, which will however trip bdrv_pad_request,

since it wants to extend its passed vector. In particular, this is the

case for the blk_co_preadv() call in stream_populate().

If there is no qiov, no operation can be done with it, but the bytes

and offset still need to be updated, so the subsequent aligned read

will actually be aligned and not run into an assertion failure.

In particular, this can happen when the request alignment of the top

node is larger than the allocated part of the bottom node, in which

case padding becomes necessary. For example:

> ./qemu-img create /tmp/backing.qcow2 -f qcow2 64M -o cluster_size=32768

> ./qemu-io -c "write -P42 0x0 0x1" /tmp/backing.qcow2

> ./qemu-img create /tmp/top.qcow2 -f qcow2 64M -b /tmp/backing.qcow2 -F qcow2

> ./qemu-system-x86_64 --qmp stdio \

> --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/top.qcow2 \

> <<EOF

> {"execute": "qmp_capabilities"}

> {"execute": "blockdev-add", "arguments": { "driver": "compress", "file": "node0", "node-name": "node1" } }

> {"execute": "block-stream", "arguments": { "job-id": "stream0", "device": "node1" } }

> EOF

Originally-by: Stefan Reiter <s.reiter@proxmox.com>

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

[FE: do update bytes and offset in any case

add reproducer to commit message]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>

Message-ID: <20240322095009.346989-2-f.ebner@proxmox.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

block/io.c | 33 ++++++++++++++++++++-------------

1 file changed, 20 insertions(+), 13 deletions(-)

@@ -XXX,XX +XXX,XX @@ static int bdrv_pad_request(BlockDriverState *bs,

return 0;

+ * For prefetching in stream_populate(), no qiov is passed along, because

+ * only copy-on-read matters.

+ if (qiov && *qiov) {

+ sliced_iov = qemu_iovec_slice(*qiov, *qiov_offset, *bytes,

+ &sliced_head, &sliced_tail,

+ &sliced_niov);

+ /* Guaranteed by bdrv_check_request32() */

+ assert(*bytes <= SIZE_MAX);

+ ret = bdrv_create_padded_qiov(bs, pad, sliced_iov, sliced_niov,

+ sliced_head, *bytes);

+ if (ret < 0) {

+ bdrv_padding_finalize(pad);

+ return ret;

*bytes += pad->head + pad->tail;

*offset -= pad->head;

- *qiov = &pad->local_qiov;

- *qiov_offset = 0;

if (padded) {

*padded = true;

}

2.44.0

From: Fiona Ebner <f.ebner@proxmox.com>

The old_bs variable in bdrv_next() is currently determined by looking

at the old block backend. However, if the block graph changes before

the next bdrv_next() call, it might be that the associated BDS is not

the same that was referenced previously. In that case, the wrong BDS

is unreferenced, leading to an assertion failure later:

> bdrv_unref: Assertion `bs->refcnt > 0' failed.

In particular, this can happen in the context of bdrv_flush_all(),

when polling for bdrv_co_flush() in the generated co-wrapper leads to

a graph change (for example with a stream block job [0]).

A racy reproducer:

> #!/bin/bash

> rm -f /tmp/backing.qcow2

> rm -f /tmp/top.qcow2

> ./qemu-img create /tmp/backing.qcow2 -f qcow2 64M

> ./qemu-io -c "write -P42 0x0 0x1" /tmp/backing.qcow2

> ./qemu-img create /tmp/top.qcow2 -f qcow2 64M -b /tmp/backing.qcow2 -F qcow2

> ./qemu-system-x86_64 --qmp stdio \

> --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/top.qcow2 \

> <<EOF

> {"execute": "qmp_capabilities"}

> {"execute": "block-stream", "arguments": { "job-id": "stream0", "device": "node0" } }

> {"execute": "quit"}

> EOF

[0]:

> #0 bdrv_replace_child_tran (child=..., new_bs=..., tran=...)

> #1 bdrv_replace_node_noperm (from=..., to=..., auto_skip=..., tran=..., errp=...)

> #2 bdrv_replace_node_common (from=..., to=..., auto_skip=..., detach_subchain=..., errp=...)

> #3 bdrv_drop_filter (bs=..., errp=...)

> #4 bdrv_cor_filter_drop (cor_filter_bs=...)

> #5 stream_prepare (job=...)

> #6 job_prepare_locked (job=...)

> #7 job_txn_apply_locked (fn=..., job=...)

> #8 job_do_finalize_locked (job=...)

> #9 job_exit (opaque=...)

> #10 aio_bh_poll (ctx=...)

> #11 aio_poll (ctx=..., blocking=...)

> #12 bdrv_poll_co (s=...)

> #13 bdrv_flush (bs=...)

> #14 bdrv_flush_all ()

> #15 do_vm_stop (state=..., send_stop=...)

> #16 vm_shutdown ()