Re: [Qemu-devel] [PATCH 06/27] block/parallels: provide _co_readv routine for parallels format driver

[Denis V. Lunev]

On 22/04/15 15:41, Stefan Hajnoczi wrote:

On Wed, Mar 11, 2015 at 01:28:00PM +0300, Denis V. Lunev wrote:

Main approach is taken from qcow2_co_readv.

The patch drops coroutine lock for the duration of IO operation and
peforms normal scatter-gather IO using standard QEMU backend.

Signed-off-by: Denis V. Lunev <d...@openvz.org>
Reviewed-by: Roman Kagan <rka...@parallels.com>
CC: Kevin Wolf <kw...@redhat.com>
CC: Stefan Hajnoczi <stefa...@redhat.com>
---
  block/parallels.c | 46 +++++++++++++++++++++++++++-------------------
  1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index b469984..64b169b 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -186,37 +186,45 @@ static int64_t coroutine_fn 
parallels_co_get_block_status(BlockDriverState *bs,
          BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
  }

-static int parallels_read(BlockDriverState *bs, int64_t sector_num,

-                    uint8_t *buf, int nb_sectors)
+static coroutine_fn int parallels_co_readv(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
  {
      BDRVParallelsState *s = bs->opaque;
+    uint64_t bytes_done = 0;
+    QEMUIOVector hd_qiov;
+    int ret = 0;

+ qemu_iovec_init(&hd_qiov, qiov->niov);

+
+    qemu_co_mutex_lock(&s->lock);
      while (nb_sectors > 0) {
          int64_t position = seek_to_sector(s, sector_num);
          int n = cluster_remainder(s, sector_num, nb_sectors);
-        if (position >= 0) {
-            int ret = bdrv_read(bs->file, position, buf, n);
+        int nbytes = n << BDRV_SECTOR_BITS;
+
+        if (position < 0) {
+            qemu_iovec_memset(qiov, bytes_done, 0, nbytes);
+        } else {
+            qemu_iovec_reset(&hd_qiov);
+            qemu_iovec_concat(&hd_qiov, qiov, bytes_done, nbytes);
+
+            qemu_co_mutex_unlock(&s->lock);
+            ret = bdrv_co_readv(bs->file, position, n, &hd_qiov);
+            qemu_co_mutex_lock(&s->lock);
+
              if (ret < 0) {
-                return ret;
+                goto fail;

The lock is never unlocked if the goto is taken.

nice catch! We have missed that...

              }
-        } else {
-            memset(buf, 0, n << BDRV_SECTOR_BITS);
          }
+
          nb_sectors -= n;
          sector_num += n;
-        buf += n << BDRV_SECTOR_BITS;
+        bytes_done += nbytes;
      }
-    return 0;
-}
-
-static coroutine_fn int parallels_co_read(BlockDriverState *bs, int64_t 
sector_num,
-                                          uint8_t *buf, int nb_sectors)
-{
-    int ret;
-    BDRVParallelsState *s = bs->opaque;
-    qemu_co_mutex_lock(&s->lock);
-    ret = parallels_read(bs, sector_num, buf, nb_sectors);
      qemu_co_mutex_unlock(&s->lock);
+
+fail:
+    qemu_iovec_destroy(&hd_qiov);
      return ret;
  }