From: Stefano Garzarella <sgarzare@redhat.com>

Until libblkio 1.3.0, virtio-blk drivers had completion eventfd

notifications enabled from the start, but from the next releases

this is no longer the case, so we have to explicitly enable them.

In fact, the libblkio documentation says they could be disabled,

so we should always enable them at the start if we want to be

sure to get completion eventfd notifications:

By default, the driver might not generate completion events for

requests so it is necessary to explicitly enable the completion

file descriptor before use:

void blkioq_set_completion_fd_enabled(struct blkioq *q, bool enable);

I discovered this while trying a development version of libblkio:

the guest kernel hangs during boot, while probing the device.

Fixes: fd66dbd424f5 ("blkio: add libblkio block driver")

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

Message-id: 20230725103744.77343-1-sgarzare@redhat.com

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

---

block/blkio.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/block/blkio.c b/block/blkio.c

index XXXXXXX..XXXXXXX 100644

--- a/block/blkio.c

+++ b/block/blkio.c

@@ -XXX,XX +XXX,XX @@ static int blkio_file_open(BlockDriverState *bs, QDict *options, int flags,

QLIST_INIT(&s->bounce_bufs);

s->blkioq = blkio_get_queue(s->blkio, 0);

s->completion_fd = blkioq_get_completion_fd(s->blkioq);

+ blkioq_set_completion_fd_enabled(s->blkioq, true);

blkio_attach_aio_context(bs, bdrv_get_aio_context(bs));

return 0;

--

2.41.0

#endif

From: Hanna Czenczek <hreitz@redhat.com>

bdrv_pad_request() relies on requests' lengths not to exceed SIZE_MAX,

which bdrv_check_qiov_request() does not guarantee.

bdrv_check_request32() however will guarantee this, and both of

bdrv_pad_request()'s callers (bdrv_co_preadv_part() and

bdrv_co_pwritev_part()) already run it before calling

bdrv_pad_request(). Therefore, bdrv_pad_request() can safely call

bdrv_check_request32() without expecting error, too.

In effect, this patch will not change guest-visible behavior. It is a

clean-up to tighten a condition to match what is guaranteed by our

callers, and which exists purely to show clearly why the subsequent

assertion (`assert(*bytes <= SIZE_MAX)`) is always true.

Note there is a difference between the interfaces of

bdrv_check_qiov_request() and bdrv_check_request32(): The former takes

an errp, the latter does not, so we can no longer just pass

&error_abort. Instead, we need to check the returned value. While we

do expect success (because the callers have already run this function),

an assert(ret == 0) is not much simpler than just to return an error if

it occurs, so let us handle errors by returning them up the stack now.

Reported-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>

Message-id: 20230714085938.202730-1-hreitz@redhat.com

Fixes: 18743311b829cafc1737a5f20bc3248d5f91ee2a

("block: Collapse padded I/O vecs exceeding IOV_MAX")

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>

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

---

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

1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/io.c b/block/io.c

index XXXXXXX..XXXXXXX 100644

--- a/block/io.c

+++ b/block/io.c

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

int sliced_niov;

size_t sliced_head, sliced_tail;

- bdrv_check_qiov_request(*offset, *bytes, *qiov, *qiov_offset, &error_abort);

+ /* Should have been checked by the caller already */

+ ret = bdrv_check_request32(*offset, *bytes, *qiov, *qiov_offset);

+ if (ret < 0) {

+ return ret;

+ }

if (!bdrv_init_padding(bs, *offset, *bytes, write, pad)) {

if (padded) {

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

&sliced_head, &sliced_tail,

&sliced_niov);

- /* Guaranteed by bdrv_check_qiov_request() */

+ /* Guaranteed by bdrv_check_request32() */

assert(*bytes <= SIZE_MAX);

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

sliced_head, *bytes);

--

2.41.0

From: Stefano Garzarella <sgarzare@redhat.com>

libblkio 1.3.0 added support of "fd" property for virtio-blk-vhost-vdpa

driver. In QEMU, starting from commit cad2ccc395 ("block/blkio: use

qemu_open() to support fd passing for virtio-blk") we are using

`blkio_get_int(..., "fd")` to check if the "fd" property is supported

for all the virtio-blk-* driver.

Unfortunately that property is also available for those driver that do

not support it, such as virtio-blk-vhost-user.

So, `blkio_get_int()` is not enough to check whether the driver supports

the `fd` property or not. This is because the virito-blk common libblkio

driver only checks whether or not `fd` is set during `blkio_connect()`

and fails with -EINVAL for those transports that do not support it

(all except vhost-vdpa for now).

So let's handle the `blkio_connect()` failure, retrying it using `path`

directly.

Fixes: cad2ccc395 ("block/blkio: use qemu_open() to support fd passing for virtio-blk")

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

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

Message-id: 20230727161020.84213-3-sgarzare@redhat.com

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

---

block/blkio.c | 29 +++++++++++++++++++++++++++++

1 file changed, 29 insertions(+)

diff --git a/block/blkio.c b/block/blkio.c

index XXXXXXX..XXXXXXX 100644

--- a/block/blkio.c

+++ b/block/blkio.c

@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,

}

ret = blkio_connect(s->blkio);

+ /*

+ * If the libblkio driver doesn't support the `fd` property, blkio_connect()

+ * will fail with -EINVAL. So let's try calling blkio_connect() again by

+ * directly setting `path`.

+ */

+ if (fd_supported && ret == -EINVAL) {

+ qemu_close(fd);

+

+ /*

+ * We need to clear the `fd` property we set previously by setting

+ * it to -1.

+ */

+ ret = blkio_set_int(s->blkio, "fd", -1);

+ if (ret < 0) {

+ error_setg_errno(errp, -ret, "failed to set fd: %s",

+ blkio_get_error_msg());

+ return ret;

+ }

+

+ ret = blkio_set_str(s->blkio, "path", path);

+ if (ret < 0) {

+ error_setg_errno(errp, -ret, "failed to set path: %s",

+ blkio_get_error_msg());

+ return ret;

+ }

+

+ ret = blkio_connect(s->blkio);

+ }

+

if (ret < 0) {

error_setg_errno(errp, -ret, "blkio_connect failed: %s",

blkio_get_error_msg());

--

2.41.0

From: Stefano Garzarella <sgarzare@redhat.com>

qemu_open() fails if called with an unix domain socket in this way:

-blockdev node-name=drive0,driver=virtio-blk-vhost-user,path=vhost-user-blk.sock,cache.direct=on: Could not open 'vhost-user-blk.sock': No such device or address

Since virtio-blk-vhost-user does not support fd passing, let`s always fall back

on using `path` if we fail the fd passing.

Fixes: cad2ccc395 ("block/blkio: use qemu_open() to support fd passing for virtio-blk")

Reported-by: Qing Wang <qinwang@redhat.com>

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

Message-id: 20230727161020.84213-4-sgarzare@redhat.com

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

---

block/blkio.c | 20 ++++++++++----------

1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c

index XXXXXXX..XXXXXXX 100644

--- a/block/blkio.c

+++ b/block/blkio.c

@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,

* In order to open the device read-only, we are using the `read-only`

* property of the libblkio driver in blkio_file_open().

*/

- fd = qemu_open(path, O_RDWR, errp);

+ fd = qemu_open(path, O_RDWR, NULL);

if (fd < 0) {

- return -EINVAL;

+ fd_supported = false;

+ } else {

+ ret = blkio_set_int(s->blkio, "fd", fd);

+ if (ret < 0) {

+ fd_supported = false;

+ qemu_close(fd);

+ }

}

+ }

- ret = blkio_set_int(s->blkio, "fd", fd);

- if (ret < 0) {

- error_setg_errno(errp, -ret, "failed to set fd: %s",

- blkio_get_error_msg());

- qemu_close(fd);

- return ret;

- }

- } else {

+ if (!fd_supported) {

ret = blkio_set_str(s->blkio, "path", path);

if (ret < 0) {

error_setg_errno(errp, -ret, "failed to set path: %s",

--

2.41.0