Re: [Qemu-devel] [PATCH for-2.1] dma-helpers: Fix too long qiov

[Paolo Bonzini <pbonzini@redhat.com>]

Il 09/07/2014 19:23, Kevin Wolf ha scritto:
> If the size of the scatter/gather list isn't a multiple of 512, the
> number of sectors for the block layer request is rounded down, resulting
> in a qiov that doesn't match the request length. Truncate the qiov to the
> new length of the request.
> 
> This fixes the IDE qtest case /x86_64/ide/bmdma/short_prdt.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  dma-helpers.c         |  4 ++++
>  include/qemu-common.h |  1 +
>  util/iov.c            | 13 +++++++++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index 53cbe92..499b52b 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -170,6 +170,10 @@ static void dma_bdrv_cb(void *opaque, int ret)
>          return;
>      }
>  
> +    if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
> +        qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK);
> +    }

This is right for read/write, but not for discard.

Also, it is wrong if you got a misaligned request that straddles a page
boundary, and the second half is from a MMIO device.

Do you think this works:

- add an alignment argument to dma_bdrv_io, and use it instead of 0 in
the "if (dbs->iov.size == 0)" conditional

- only do the qemu_iovec_discard_back if the SG list has been processed
entirely.

Paolo

>      dbs->acb = dbs->io_func(dbs->bs, dbs->sector_num, &dbs->iov,
>                              dbs->iov.size / 512, dma_bdrv_cb, dbs);
>      assert(dbs->acb);
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index ae76197..6ef8282 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -329,6 +329,7 @@ size_t qemu_iovec_memset(QEMUIOVector *qiov, size_t offset,
>                           int fillc, size_t bytes);
>  ssize_t qemu_iovec_compare(QEMUIOVector *a, QEMUIOVector *b);
>  void qemu_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, void *buf);
> +void qemu_iovec_discard_back(QEMUIOVector *qiov, size_t bytes);
>  
>  bool buffer_is_zero(const void *buf, size_t len);
>  
> diff --git a/util/iov.c b/util/iov.c
> index 2b4f46d..24566c8 100644
> --- a/util/iov.c
> +++ b/util/iov.c
> @@ -550,3 +550,16 @@ size_t iov_discard_back(struct iovec *iov, unsigned int *iov_cnt,
>  
>      return total;
>  }
> +
> +void qemu_iovec_discard_back(QEMUIOVector *qiov, size_t bytes)
> +{
> +    size_t total;
> +    unsigned int niov = qiov->niov;
> +
> +    assert(qiov->size >= bytes);
> +    total = iov_discard_back(qiov->iov, &niov, bytes);
> +    assert(total == bytes);
> +
> +    qiov->niov = niov;
> +    qiov->size -= bytes;
> +}
>