[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 07/13] block: return errors from bdrv_register_buf()
From: |
Stefan Hajnoczi |
Subject: |
[PULL 07/13] block: return errors from bdrv_register_buf() |
Date: |
Wed, 26 Oct 2022 14:58:40 -0400 |
Registering an I/O buffer is only a performance optimization hint but it
is still necessary to return errors when it fails.
Later patches will need to detect errors when registering buffers but an
immediate advantage is that error_report() calls are no longer needed in
block driver .bdrv_register_buf() functions.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20221013185908.1297568-8-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/block/block-global-state.h | 5 ++-
include/block/block_int-common.h | 5 ++-
include/sysemu/block-backend-global-state.h | 2 +-
block/block-backend.c | 4 +--
block/io.c | 34 +++++++++++++++++++--
block/nvme.c | 18 +++++------
qemu-img.c | 2 +-
7 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/include/block/block-global-state.h
b/include/block/block-global-state.h
index 7901f35863..eba4ed23b4 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -246,8 +246,11 @@ void bdrv_del_child(BlockDriverState *parent, BdrvChild
*child, Error **errp);
*
* Buffers must not overlap and they must be unregistered with the same <host,
* size> values that they were registered with.
+ *
+ * Returns: true on success, false on failure
*/
-void bdrv_register_buf(BlockDriverState *bs, void *host, size_t size);
+bool bdrv_register_buf(BlockDriverState *bs, void *host, size_t size,
+ Error **errp);
void bdrv_unregister_buf(BlockDriverState *bs, void *host, size_t size);
void bdrv_cancel_in_flight(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 19798d0e77..9c569be162 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -433,8 +433,11 @@ struct BlockDriver {
* that it can do IOMMU mapping with VFIO etc., in order to get better
* performance. In the case of VFIO drivers, this callback is used to do
* DMA mapping for hot buffers.
+ *
+ * Returns: true on success, false on failure
*/
- void (*bdrv_register_buf)(BlockDriverState *bs, void *host, size_t size);
+ bool (*bdrv_register_buf)(BlockDriverState *bs, void *host, size_t size,
+ Error **errp);
void (*bdrv_unregister_buf)(BlockDriverState *bs, void *host, size_t size);
/*
diff --git a/include/sysemu/block-backend-global-state.h
b/include/sysemu/block-backend-global-state.h
index 97f7dad2c3..6858e39cb6 100644
--- a/include/sysemu/block-backend-global-state.h
+++ b/include/sysemu/block-backend-global-state.h
@@ -106,7 +106,7 @@ void blk_io_limits_enable(BlockBackend *blk, const char
*group);
void blk_io_limits_update_group(BlockBackend *blk, const char *group);
void blk_set_force_allow_inactivate(BlockBackend *blk);
-void blk_register_buf(BlockBackend *blk, void *host, size_t size);
+bool blk_register_buf(BlockBackend *blk, void *host, size_t size, Error
**errp);
void blk_unregister_buf(BlockBackend *blk, void *host, size_t size);
const BdrvChild *blk_root(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index ae42474891..4f59664397 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2545,10 +2545,10 @@ static void blk_root_drained_end(BdrvChild *child, int
*drained_end_counter)
}
}
-void blk_register_buf(BlockBackend *blk, void *host, size_t size)
+bool blk_register_buf(BlockBackend *blk, void *host, size_t size, Error **errp)
{
GLOBAL_STATE_CODE();
- bdrv_register_buf(blk_bs(blk), host, size);
+ return bdrv_register_buf(blk_bs(blk), host, size, errp);
}
void blk_unregister_buf(BlockBackend *blk, void *host, size_t size)
diff --git a/block/io.c b/block/io.c
index 4207648db6..a9673465dd 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3277,17 +3277,45 @@ void bdrv_io_unplug(BlockDriverState *bs)
}
}
-void bdrv_register_buf(BlockDriverState *bs, void *host, size_t size)
+/* Helper that undoes bdrv_register_buf() when it fails partway through */
+static void bdrv_register_buf_rollback(BlockDriverState *bs,
+ void *host,
+ size_t size,
+ BdrvChild *final_child)
+{
+ BdrvChild *child;
+
+ QLIST_FOREACH(child, &bs->children, next) {
+ if (child == final_child) {
+ break;
+ }
+
+ bdrv_unregister_buf(child->bs, host, size);
+ }
+
+ if (bs->drv && bs->drv->bdrv_unregister_buf) {
+ bs->drv->bdrv_unregister_buf(bs, host, size);
+ }
+}
+
+bool bdrv_register_buf(BlockDriverState *bs, void *host, size_t size,
+ Error **errp)
{
BdrvChild *child;
GLOBAL_STATE_CODE();
if (bs->drv && bs->drv->bdrv_register_buf) {
- bs->drv->bdrv_register_buf(bs, host, size);
+ if (!bs->drv->bdrv_register_buf(bs, host, size, errp)) {
+ return false;
+ }
}
QLIST_FOREACH(child, &bs->children, next) {
- bdrv_register_buf(child->bs, host, size);
+ if (!bdrv_register_buf(child->bs, host, size, errp)) {
+ bdrv_register_buf_rollback(bs, host, size, child);
+ return false;
+ }
}
+ return true;
}
void bdrv_unregister_buf(BlockDriverState *bs, void *host, size_t size)
diff --git a/block/nvme.c b/block/nvme.c
index 94b76b16f2..656624c585 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1587,19 +1587,19 @@ static void nvme_aio_unplug(BlockDriverState *bs)
}
}
-static void nvme_register_buf(BlockDriverState *bs, void *host, size_t size)
+static bool nvme_register_buf(BlockDriverState *bs, void *host, size_t size,
+ Error **errp)
{
int ret;
- Error *local_err = NULL;
BDRVNVMeState *s = bs->opaque;
- ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL, &local_err);
- if (ret) {
- /* FIXME: we may run out of IOVA addresses after repeated
- * bdrv_register_buf/bdrv_unregister_buf, because nvme_vfio_dma_unmap
- * doesn't reclaim addresses for fixed mappings. */
- error_reportf_err(local_err, "nvme_register_buf failed: ");
- }
+ /*
+ * FIXME: we may run out of IOVA addresses after repeated
+ * bdrv_register_buf/bdrv_unregister_buf, because nvme_vfio_dma_unmap
+ * doesn't reclaim addresses for fixed mappings.
+ */
+ ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL, errp);
+ return ret == 0;
}
static void nvme_unregister_buf(BlockDriverState *bs, void *host, size_t size)
diff --git a/qemu-img.c b/qemu-img.c
index 9fe94df650..a3b64c88af 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4570,7 +4570,7 @@ static int img_bench(int argc, char **argv)
data.buf = blk_blockalign(blk, buf_size);
memset(data.buf, pattern, data.nrreq * data.bufsize);
- blk_register_buf(blk, data.buf, buf_size);
+ blk_register_buf(blk, data.buf, buf_size, &error_fatal);
data.qiov = g_new(QEMUIOVector, data.nrreq);
for (i = 0; i < data.nrreq; i++) {
--
2.37.3
- [PULL 00/13] Block patches, Stefan Hajnoczi, 2022/10/26
- [PULL 01/13] coroutine: add flag to re-queue at front of CoQueue, Stefan Hajnoczi, 2022/10/26
- [PULL 02/13] blkio: add libblkio block driver, Stefan Hajnoczi, 2022/10/26
- [PULL 03/13] numa: call ->ram_block_removed() in ram_block_notifer_remove(), Stefan Hajnoczi, 2022/10/26
- [PULL 05/13] block: use BdrvRequestFlags type for supported flag fields, Stefan Hajnoczi, 2022/10/26
- [PULL 06/13] block: add BDRV_REQ_REGISTERED_BUF request flag, Stefan Hajnoczi, 2022/10/26
- [PULL 07/13] block: return errors from bdrv_register_buf(),
Stefan Hajnoczi <=
- [PULL 08/13] numa: use QLIST_FOREACH_SAFE() for RAM block notifiers, Stefan Hajnoczi, 2022/10/26
- [PULL 04/13] block: pass size to bdrv_unregister_buf(), Stefan Hajnoczi, 2022/10/26
- [PULL 10/13] exec/cpu-common: add qemu_ram_get_fd(), Stefan Hajnoczi, 2022/10/26
- [PULL 11/13] stubs: add qemu_ram_block_from_host() and qemu_ram_get_fd(), Stefan Hajnoczi, 2022/10/26
- [PULL 12/13] blkio: implement BDRV_REQ_REGISTERED_BUF optimization, Stefan Hajnoczi, 2022/10/26
- [PULL 13/13] virtio-blk: use BDRV_REQ_REGISTERED_BUF optimization hint, Stefan Hajnoczi, 2022/10/26
- [PULL 09/13] block: add BlockRAMRegistrar, Stefan Hajnoczi, 2022/10/26
- Re: [PULL 00/13] Block patches, Stefan Hajnoczi, 2022/10/31