The following changes since commit 66547f416a61e0cb711dc76821890242432ba193:

block/nvme: invoke blk_io_plug_call() outside q->lock (2023-07-17 09:17:41 -0400)

for you to fetch changes up to 1c38fe69e2b8a05c1762b122292fa7e3662f06fd:

block/blkio: use blkio_set_int("fd") to check fd support (2023-07-27 15:51:46 -0400)

Hanna Czenczek (1):

block: Fix pad_request's request restriction

Sam Li (1):

block/file-posix: fix g_file_get_contents return path

Stefano Garzarella (6):

block/blkio: enable the completion eventfd

block/blkio: do not use open flags in qemu_open()

block/blkio: move blkio_connect() in the drivers functions

block/blkio: retry blkio_connect() if it fails using `fd`

block/blkio: fall back on using `path` when `fd` setting fails

block/blkio: use blkio_set_int("fd") to check fd support

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

block/file-posix.c | 6 +--

block/io.c | 8 ++-

3 files changed, 97 insertions(+), 49 deletions(-)

2.41.0

From: Sam Li <faithilikerun@gmail.com>

The g_file_get_contents() function returns a g_boolean. If it fails, the

returned value will be 0 instead of -1. Solve the issue by skipping

assigning ret value.

This issue was found by Matthew Rosato using virtio-blk-{pci,ccw} backed

by an NVMe partition e.g. /dev/nvme0n1p1 on s390x.

Signed-off-by: Sam Li <faithilikerun@gmail.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

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

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Message-id: 20230727115844.8480-1-faithilikerun@gmail.com

block/file-posix.c | 6 ++----

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

@@ -XXX,XX +XXX,XX @@ static int hdev_get_max_hw_transfer(int fd, struct stat *st)

static int get_sysfs_str_val(struct stat *st, const char *attribute,

char **val) {

g_autofree char *sysfspath = NULL;

- int ret;

size_t len;

if (!S_ISBLK(st->st_mode)) {

@@ -XXX,XX +XXX,XX @@ static int get_sysfs_str_val(struct stat *st, const char *attribute,

sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/%s",

major(st->st_rdev), minor(st->st_rdev),

attribute);

- ret = g_file_get_contents(sysfspath, val, &len, NULL);

- if (ret == -1) {

+ if (!g_file_get_contents(sysfspath, val, &len, NULL)) {

return -ENOENT;

@@ -XXX,XX +XXX,XX @@ static int get_sysfs_str_val(struct stat *st, const char *attribute,

if (*(p + len - 1) == '\n') {

*(p + len - 1) = '\0';

2.41.0

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>

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

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

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

This is in preparation for the next patch, where for virtio-blk

drivers we need to handle the failure of blkio_connect().

Let's also rename the *_open() functions to *_connect() to make

the code reflect the changes applied.

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

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

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

1 file changed, 42 insertions(+), 29 deletions(-)

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

--- a/block/blkio.c

+++ b/block/blkio.c

@@ -XXX,XX +XXX,XX @@ static void blkio_unregister_buf(BlockDriverState *bs, void *host, size_t size)

}

-static int blkio_io_uring_open(BlockDriverState *bs, QDict *options, int flags,

- Error **errp)

+static int blkio_io_uring_connect(BlockDriverState *bs, QDict *options,

+ int flags, Error **errp)

const char *filename = qdict_get_str(options, "filename");

BDRVBlkioState *s = bs->opaque;

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

+ ret = blkio_connect(s->blkio);

+ if (ret < 0) {

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

+ blkio_get_error_msg());

+ return ret;

+ }

+

return 0;

}

-static int blkio_nvme_io_uring(BlockDriverState *bs, QDict *options, int flags,

- Error **errp)

+static int blkio_nvme_io_uring_connect(BlockDriverState *bs, QDict *options,

+ int flags, Error **errp)

{

const char *path = qdict_get_try_str(options, "path");

BDRVBlkioState *s = bs->opaque;

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

return -EINVAL;

}

+ ret = blkio_connect(s->blkio);

+ if (ret < 0) {

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

+ blkio_get_error_msg());

+ return ret;

+ }

+

return 0;

}

-static int blkio_virtio_blk_common_open(BlockDriverState *bs,

- QDict *options, int flags, Error **errp)

+static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,

+ int flags, Error **errp)

{

const char *path = qdict_get_try_str(options, "path");

BDRVBlkioState *s = bs->opaque;

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

+ ret = blkio_connect(s->blkio);

+ if (ret < 0) {

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

+ blkio_get_error_msg());

+ return ret;

+ }

+

qdict_del(options, "path");

return 0;

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

return ret;

}

+ if (!(flags & BDRV_O_RDWR)) {

+ ret = blkio_set_bool(s->blkio, "read-only", true);

+ if (ret < 0) {

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

+ blkio_get_error_msg());

+ blkio_destroy(&s->blkio);

+ return ret;

+

if (strcmp(blkio_driver, "io_uring") == 0) {

- ret = blkio_io_uring_open(bs, options, flags, errp);

+ ret = blkio_io_uring_connect(bs, options, flags, errp);

} else if (strcmp(blkio_driver, "nvme-io_uring") == 0) {

- ret = blkio_nvme_io_uring(bs, options, flags, errp);

+ ret = blkio_nvme_io_uring_connect(bs, options, flags, errp);

} else if (strcmp(blkio_driver, "virtio-blk-vfio-pci") == 0) {

- ret = blkio_virtio_blk_common_open(bs, options, flags, errp);

+ ret = blkio_virtio_blk_connect(bs, options, flags, errp);

} else if (strcmp(blkio_driver, "virtio-blk-vhost-user") == 0) {

- ret = blkio_virtio_blk_common_open(bs, options, flags, errp);

+ ret = blkio_virtio_blk_connect(bs, options, flags, errp);

} else if (strcmp(blkio_driver, "virtio-blk-vhost-vdpa") == 0) {

- ret = blkio_virtio_blk_common_open(bs, options, flags, errp);

+ ret = blkio_virtio_blk_connect(bs, options, flags, errp);

} else {

g_assert_not_reached();

}

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

return ret;

}

- if (!(flags & BDRV_O_RDWR)) {

- ret = blkio_set_bool(s->blkio, "read-only", true);

- if (ret < 0) {

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

- blkio_get_error_msg());

- blkio_destroy(&s->blkio);

- return ret;

- }

- }

-

- ret = blkio_connect(s->blkio);

- if (ret < 0) {

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

- blkio_get_error_msg());

- blkio_destroy(&s->blkio);

- return ret;

- }

-

ret = blkio_get_bool(s->blkio,

"needs-mem-regions",

&s->needs_mem_regions);

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

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

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

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

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

From: Stefano Garzarella <sgarzare@redhat.com>

Setting the `fd` property fails with virtio-blk-* libblkio drivers

that do not support fd passing since

https://gitlab.com/libblkio/libblkio/-/merge_requests/208.

Getting the `fd` property, on the other hand, always succeeds for

virtio-blk-* libblkio drivers even when they don't support fd passing.

block/blkio.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

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

--- a/block/blkio.c

+++ b/block/blkio.c

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

return -EINVAL;

}

- if (blkio_get_int(s->blkio, "fd", &fd) == 0) {

+ if (blkio_set_int(s->blkio, "fd", -1) == 0) {

fd_supported = true;

}

2.41.0