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

login
register
mail settings
Submitter Michael Tokarev
Date Jan. 17, 2013, 10:44 a.m.
Message ID <1358419481-22728-1-git-send-email-mjt@msgid.tls.msk.ru>
Download mbox | patch
Permalink /patch/213227/
State New
Headers show

Comments

Michael Tokarev - Jan. 17, 2013, 10:44 a.m.
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(-)
Stefan Hajnoczi - Jan. 17, 2013, 1:06 p.m.
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.
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.
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

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 {