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

Vladimir Sementsov-Ogievskiy posted 17 patches 6 years, 9 months ago
Maintainers: John Snow <jsnow@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Juan Quintela <quintela@redhat.com>, "Denis V. Lunev" <den@openvz.org>, Max Reitz <mreitz@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>, Jeff Cody <jcody@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v3 16/17] hw/ide: drop iov field from IDEBufferedRequest
Posted by Vladimir Sementsov-Ogievskiy 6 years, 9 months ago
@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(-)

diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index fa99486d7a..1b02bb9151 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -346,7 +346,6 @@ extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
 
 typedef struct IDEBufferedRequest {
     QLIST_ENTRY(IDEBufferedRequest) list;
-    struct iovec iov;
     QEMUIOVector qiov;
     QEMUIOVector *original_qiov;
     BlockCompletionFunc *original_cb;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e94ba8c488..1cf87fdefe 100644
--- a/hw/ide/core.c
+++ 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));
     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);
 
     aioreq = blk_aio_preadv(s->blk, sector_num << BDRV_SECTOR_BITS,
                             &req->qiov, 0, ide_buffered_readv_cb, req);
-- 
2.18.0


Re: [Qemu-devel] [PATCH v3 16/17] hw/ide: drop iov field from IDEBufferedRequest
Posted by Eric Blake 6 years, 9 months ago
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