Re: [Qemu-devel] [PATCH v3 16/17] hw/ide: drop iov field from IDEBufferedRequest

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]

On 2/7/19 4:24 AM, Vladimir Sementsov-Ogievskiy wrote:
> @iov is used only to initialize @qiov. Let's use new
> qemu_iovec_init_buf() instead, which simplifies the code.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/hw/ide/internal.h |  1 -
>  hw/ide/core.c             | 11 ++++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)

> +++ b/hw/ide/core.c
> @@ -629,13 +629,15 @@ static void ide_buffered_readv_cb(void *opaque, int ret)
>      IDEBufferedRequest *req = opaque;
>      if (!req->orphaned) {
>          if (!ret) {
> -            qemu_iovec_from_buf(req->original_qiov, 0, req->iov.iov_base,
> +            assert(req->qiov.size == req->original_qiov->size);
> +            qemu_iovec_from_buf(req->original_qiov, 0,
> +                                req->qiov.local_iov.iov_base,
>                                  req->original_qiov->size);
>          }
>          req->original_cb(req->original_opaque, ret);
>      }
>      QLIST_REMOVE(req, list);
> -    qemu_vfree(req->iov.iov_base);
> +    qemu_vfree(qemu_iovec_get_buf(&req->qiov));

Okay, I can see that freeing this variable in the callback needs some
way to get at the buffer that was allocated earlier from a different
function.  Still, do we need qemu_iovec_get_buf(), or can we just
open-code it as qemu_vfree(req->qiov.local_iov.iov_base), since we
open-coded it in the qemu_iovec_from_buf() a few lines earlier?

>      g_free(req);
>  }
>  
> @@ -660,9 +662,8 @@ BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
>      req->original_qiov = iov;
>      req->original_cb = cb;
>      req->original_opaque = opaque;
> -    req->iov.iov_base = qemu_blockalign(blk_bs(s->blk), iov->size);
> -    req->iov.iov_len = iov->size;
> -    qemu_iovec_init_external(&req->qiov, &req->iov, 1);
> +    qemu_iovec_init_buf(&req->qiov, blk_blockalign(s->blk, iov->size),
> +                        iov->size);
>  

Took me longer to review this one, but looks like a correct conversion.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]