diff mbox

[v2] win32-aio: use iov utility functions instead of open-coding them

Message ID 1358419481-22728-1-git-send-email-mjt@msgid.tls.msk.ru
State New
Headers show

Commit Message

Michael Tokarev Jan. 17, 2013, 10:44 a.m. UTC 
We have iov_from_buf() and iov_to_buf(), use them instead of
open-coding these in block/win32-aio.c
---
 block/win32-aio.c |   16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

Comments

Stefan Hajnoczi Jan. 17, 2013, 1:06 p.m. UTC | #1 
On Thu, Jan 17, 2013 at 02:44:41PM +0400, Michael Tokarev wrote:
> We have iov_from_buf() and iov_to_buf(), use them instead of
> open-coding these in block/win32-aio.c

Please use qemu_iovec_from_buf() and qemu_iovec_to_buf() since we're
operating on a QEMUIOVector.

Stefan
Michael Tokarev Jan. 17, 2013, 1:51 p.m. UTC | #2 
17.01.2013 17:06, Stefan Hajnoczi wrote:
> On Thu, Jan 17, 2013 at 02:44:41PM +0400, Michael Tokarev wrote:
>> We have iov_from_buf() and iov_to_buf(), use them instead of
>> open-coding these in block/win32-aio.c
>
> Please use qemu_iovec_from_buf() and qemu_iovec_to_buf() since we're
> operating on a QEMUIOVector.

I'd remove qemu_iovec_{from,to}_buf() completely at this point
due to their trivialness and almost no gain in usage as polluting
the namespace (having too many trivial utility functions isn't
good).  Right now these are only used in one place - in
hw/dataplane/virtio-blk.c .

If not, we can at least inline them.

Thanks,

/mjt
Stefan Hajnoczi Jan. 18, 2013, 8:46 a.m. UTC | #3 
On Thu, Jan 17, 2013 at 05:51:05PM +0400, Michael Tokarev wrote:
> 17.01.2013 17:06, Stefan Hajnoczi wrote:
> >On Thu, Jan 17, 2013 at 02:44:41PM +0400, Michael Tokarev wrote:
> >>We have iov_from_buf() and iov_to_buf(), use them instead of
> >>open-coding these in block/win32-aio.c
> >
> >Please use qemu_iovec_from_buf() and qemu_iovec_to_buf() since we're
> >operating on a QEMUIOVector.
> 
> I'd remove qemu_iovec_{from,to}_buf() completely at this point
> due to their trivialness and almost no gain in usage as polluting
> the namespace (having too many trivial utility functions isn't
> good).  Right now these are only used in one place - in
> hw/dataplane/virtio-blk.c .

I like them because it makes the calling code nicer to read.  It saves
us from pulling apart the QEMUIOVector struct.

But it's not a big deal to me.  I can merge this version of the patch.

Stefan
diff mbox

Patch

diff --git a/block/win32-aio.c b/block/win32-aio.c
index b9236ea..b10a0c0 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -80,13 +80,7 @@  static void win32_aio_process_completion(QEMUWin32AIOState *s,
     if (!waiocb->is_linear) {
         if (ret == 0 && waiocb->is_read) {
             QEMUIOVector *qiov = waiocb->qiov;
-            char *p = waiocb->buf;
-            int i;
-
-            for (i = 0; i < qiov->niov; ++i) {
-                memcpy(qiov->iov[i].iov_base, p, qiov->iov[i].iov_len);
-                p += qiov->iov[i].iov_len;
-            }
+            iov_from_buf(qiov->iov, qiov->niov, 0, waiocb->buf, qiov->size);
         }
         qemu_vfree(waiocb->buf);
     }
@@ -153,13 +147,7 @@  BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
     if (qiov->niov > 1) {
         waiocb->buf = qemu_blockalign(bs, qiov->size);
         if (type & QEMU_AIO_WRITE) {
-            char *p = waiocb->buf;
-            int i;
-
-            for (i = 0; i < qiov->niov; ++i) {
-                memcpy(p, qiov->iov[i].iov_base, qiov->iov[i].iov_len);
-                p += qiov->iov[i].iov_len;
-            }
+            iov_to_buf(qiov->iov, qiov->niov, 0, waiocb->buf, qiov->size);
         }
         waiocb->is_linear = false;
     } else {