From f4ec04bae9577eaa55ac35f3971dc3086a4a9192 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 13 Oct 2022 14:59:02 -0400 Subject: [PATCH] block: return errors from bdrv_register_buf() 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 Message-id: 20221013185908.1297568-8-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- block/block-backend.c | 4 +-- block/io.c | 34 +++++++++++++++++++-- block/nvme.c | 18 +++++------ include/block/block-global-state.h | 5 ++- include/block/block_int-common.h | 5 ++- include/sysemu/block-backend-global-state.h | 2 +- qemu-img.c | 2 +- 7 files changed, 52 insertions(+), 18 deletions(-) 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/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 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/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++) {