On Mon, 05/23 14:54, Paolo Bonzini wrote:
> Callers of dma_blk_io have no way to pass extra data to the DMAIOFunc,
> because the original callback and opaque are gone by the time DMAIOFunc
> is called. On the other hand, the BlockBackend is usually derived
> from those extra data that you could pass to the DMAIOFunc (in the
> next patch, that would be the SCSIRequest).
>
> So change DMAIOFunc's prototype, decoupling it from blk_aio_readv
> and blk_aio_writev's. The new prototype loses the BlockBackend
> and gains an extra opaque value which, in the case of dma_blk_readv
> and dma_blk_writev, is of course used for the BlockBackend.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> dma-helpers.c | 48 +++++++++++++++++++++++++++++++++++-------------
> hw/ide/core.c | 14 ++++++++------
> hw/ide/internal.h | 6 +++---
> hw/ide/macio.c | 2 +-
> include/sysemu/dma.h | 12 ++++++------
> 5 files changed, 53 insertions(+), 29 deletions(-)
>
> diff --git a/dma-helpers.c b/dma-helpers.c
> index af07aab..92c5d55 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -70,7 +70,7 @@ void qemu_sglist_destroy(QEMUSGList *qsg)
>
> typedef struct {
> BlockAIOCB common;
> - BlockBackend *blk;
> + AioContext *ctx;
> BlockAIOCB *acb;
> QEMUSGList *sg;
> uint64_t offset;
> @@ -80,6 +80,7 @@ typedef struct {
> QEMUIOVector iov;
> QEMUBH *bh;
> DMAIOFunc *io_func;
> + void *io_func_opaque;
> } DMAAIOCB;
>
> static void dma_blk_cb(void *opaque, int ret);
> @@ -154,8 +155,7 @@ static void dma_blk_cb(void *opaque, int ret)
>
> if (dbs->iov.size == 0) {
> trace_dma_map_wait(dbs);
> - dbs->bh = aio_bh_new(blk_get_aio_context(dbs->blk),
> - reschedule_dma, dbs);
> + dbs->bh = aio_bh_new(dbs->ctx, reschedule_dma, dbs);
> cpu_register_map_client(dbs->bh);
> return;
> }
> @@ -164,8 +164,8 @@ static void dma_blk_cb(void *opaque, int ret)
> qemu_iovec_discard_back(&dbs->iov, dbs->iov.size &
> ~BDRV_SECTOR_MASK);
> }
>
> - dbs->acb = dbs->io_func(dbs->blk, dbs->offset, &dbs->iov, 0,
> - dma_blk_cb, dbs);
> + dbs->acb = dbs->io_func(dbs->offset, &dbs->iov,
> + dma_blk_cb, dbs, dbs->io_func_opaque);
> assert(dbs->acb);
> }
>
> @@ -191,23 +191,25 @@ static const AIOCBInfo dma_aiocb_info = {
> .cancel_async = dma_aio_cancel,
> };
>
> -BlockAIOCB *dma_blk_io(
> - BlockBackend *blk, QEMUSGList *sg, uint64_t offset,
> - DMAIOFunc *io_func, BlockCompletionFunc *cb,
> +BlockAIOCB *dma_blk_io(AioContext *ctx,
> + QEMUSGList *sg, uint64_t opaque,
As pointed out by Mark, s/opaque/offset/
> + DMAIOFunc *io_func, void *io_func_opaque,
> + BlockCompletionFunc *cb,
> void *opaque, DMADirection dir)
> {
> - DMAAIOCB *dbs = blk_aio_get(&dma_aiocb_info, blk, cb, opaque);
> + DMAAIOCB *dbs = qemu_aio_get(&dma_aiocb_info, NULL, cb, opaque);
>
> - trace_dma_blk_io(dbs, blk, offset, (dir == DMA_DIRECTION_TO_DEVICE));
> + trace_dma_blk_io(dbs, io_func_opaque, offset, (dir ==
> DMA_DIRECTION_TO_DEVICE));
>
> dbs->acb = NULL;
> - dbs->blk = blk;
> dbs->sg = sg;
> + dbs->ctx = ctx;
> dbs->offset = offset;
> dbs->sg_cur_index = 0;
> dbs->sg_cur_byte = 0;
> dbs->dir = dir;
> dbs->io_func = io_func;
> + dbs->io_func_opaque = io_func_opaque;
> dbs->bh = NULL;
> qemu_iovec_init(&dbs->iov, sg->nsg);
> dma_blk_cb(dbs, 0);
> @@ -215,19 +217,39 @@ BlockAIOCB *dma_blk_io(
> }
>
>
> +static
> +BlockAIOCB *dma_blk_read_io_func(int64_t offset, QEMUIOVector *iov,
> + BlockCompletionFunc *cb, void *cb_opaque,
> + void *opaque)
> +{
> + BlockBackend *blk = opaque;
> + return blk_aio_preadv(blk, offset, iov, 0, cb, cb_opaque);
> +}
> +
> BlockAIOCB *dma_blk_read(BlockBackend *blk,
> QEMUSGList *sg, uint64_t offset,
> void (*cb)(void *opaque, int ret), void *opaque)
> {
> - return dma_blk_io(blk, sg, offset, blk_aio_preadv, cb, opaque,
> + return dma_blk_io(blk_get_aio_context(blk),
> + sg, offset, dma_blk_read_io_func, blk, cb, opaque,
> DMA_DIRECTION_FROM_DEVICE);
> }
>
> +static
> +BlockAIOCB *dma_blk_write_io_func(int64_t offset, QEMUIOVector *iov,
> + BlockCompletionFunc *cb, void *cb_opaque,
> + void *opaque)
> +{
> + BlockBackend *blk = opaque;
> + return blk_aio_pwritev(blk, offset, iov, 0, cb, cb_opaque);
> +}
> +
> BlockAIOCB *dma_blk_write(BlockBackend *blk,
> QEMUSGList *sg, uint64_t offset,
> void (*cb)(void *opaque, int ret), void *opaque)
> {
> - return dma_blk_io(blk, sg, offset, blk_aio_pwritev, cb, opaque,
> + return dma_blk_io(blk_get_aio_context(blk),
> + sg, offset, dma_blk_write_io_func, blk, cb, opaque,
> DMA_DIRECTION_TO_DEVICE);
> }
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 7326189..029f6b9 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -441,13 +441,14 @@ static void ide_issue_trim_cb(void *opaque, int ret)
> }
> }
>
> -BlockAIOCB *ide_issue_trim(BlockBackend *blk,
> - int64_t offset, QEMUIOVector *qiov, BdrvRequestFlags flags,
> - BlockCompletionFunc *cb, void *opaque)
> +BlockAIOCB *ide_issue_trim(
> + int64_t offset, QEMUIOVector *qiov,
> + BlockCompletionFunc *cb, void *cb_opaque, void *opaque)
> {
> + BlockBackend *blk = opaque;
> TrimAIOCB *iocb;
>
> - iocb = blk_aio_get(&trim_aiocb_info, blk, cb, opaque);
> + iocb = blk_aio_get(&trim_aiocb_info, blk, cb, cb_opaque);
> iocb->blk = blk;
> iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
> iocb->ret = 0;
> @@ -871,8 +872,9 @@ static void ide_dma_cb(void *opaque, int ret)
> ide_dma_cb, s);
> break;
> case IDE_DMA_TRIM:
> - s->bus->dma->aiocb = dma_blk_io(s->blk, &s->sg, offset,
> - ide_issue_trim, ide_dma_cb, s,
> + s->bus->dma->aiocb = dma_blk_io(blk_get_aio_context(s->blk),
> + &s->sg, offset,
> + ide_issue_trim, s->blk, ide_dma_cb,
> s,
> DMA_DIRECTION_TO_DEVICE);
> break;
> default:
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index ceb9e59..773928a 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -613,9 +613,9 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int
> size,
> EndTransferFunc *end_transfer_func);
> void ide_transfer_stop(IDEState *s);
> void ide_set_inactive(IDEState *s, bool more);
> -BlockAIOCB *ide_issue_trim(BlockBackend *blk,
> - int64_t offset, QEMUIOVector *qiov, BdrvRequestFlags flags,
> - BlockCompletionFunc *cb, void *opaque);
> +BlockAIOCB *ide_issue_trim(
> + int64_t offset, QEMUIOVector *qiov,
> + BlockCompletionFunc *cb, void *cb_opaque, void *opaque);
> BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
> QEMUIOVector *iov, int nb_sectors,
> BlockCompletionFunc *cb, void *opaque);
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index d7d9c0f..42ad68a 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -230,7 +230,7 @@ static void pmac_dma_trim(BlockBackend *blk,
> s->io_buffer_index += io->len;
> io->len = 0;
>
> - s->bus->dma->aiocb = ide_issue_trim(blk, offset, &io->iov, 0, cb, io);
> + s->bus->dma->aiocb = ide_issue_trim(offset, &io->iov, cb, io, blk);
> }
>
> static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
> diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
> index f0e0c30..34c8eaf 100644
> --- a/include/sysemu/dma.h
> +++ b/include/sysemu/dma.h
> @@ -194,14 +194,14 @@ void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base,
> dma_addr_t len);
> void qemu_sglist_destroy(QEMUSGList *qsg);
> #endif
>
> -typedef BlockAIOCB *DMAIOFunc(BlockBackend *blk, int64_t offset,
> - QEMUIOVector *iov, BdrvRequestFlags flags,
> - BlockCompletionFunc *cb, void *opaque);
Wasn't flags parameter added for a reason in d4f510eb3f? Would it be useful for
things like offloading FUA?
Fam
> +typedef BlockAIOCB *DMAIOFunc(int64_t offset, QEMUIOVector *iov,
> + BlockCompletionFunc *cb, void *cb_opaque,
> + void *opaque);
>
> -BlockAIOCB *dma_blk_io(BlockBackend *blk,
> +BlockAIOCB *dma_blk_io(AioContext *ctx,
> QEMUSGList *sg, uint64_t offset,
> - DMAIOFunc *io_func, BlockCompletionFunc *cb,
> - void *opaque, DMADirection dir);
> + DMAIOFunc *io_func, void *io_func_opaque,
> + BlockCompletionFunc *cb, void *opaque, DMADirection
> dir);
> BlockAIOCB *dma_blk_read(BlockBackend *blk,
> QEMUSGList *sg, uint64_t offset,
> BlockCompletionFunc *cb, void *opaque);
> --
> 2.5.5
>
>
>
