[Qemu-devel] [PATCH 0/5] qcow: coroutines cleanup

[Qemu-devel] [PATCH 0/5] qcow: coroutines cleanup

[Frediano Ziglio]

These patches mostly cleanup some AIO code using coroutines.
These patches apply to Kevin's repository, branch coroutine-block.
Mostly they use stack instead of allocated AIO structure.

Frediano Ziglio (5):
  qcow: allocate QCowAIOCB structure using stack
  qcow: QCowAIOCB field cleanup
  qcow: move some blocks of code to avoid useless variable
    initialization
  qcow: embed qcow_aio_read_cb into qcow_co_readv and qcow_aio_write_cb
    into qcow_co_writev
  qcow: remove old #undefined code

 block/qcow.c  |  374 ++++++++++++++++++---------------------------------------
 block/qcow2.c |   38 ++----
 2 files changed, 129 insertions(+), 283 deletions(-)

[Qemu-devel] [PATCH 1/5] qcow: allocate QCowAIOCB structure using stack

[Frediano Ziglio]

instead of calling qemi_aio_get use stack

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow.c  |   52 ++++++++++++++++------------------------------------
 block/qcow2.c |   38 +++++++++++---------------------------
 2 files changed, 27 insertions(+), 63 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 6447c2a..d19ef04 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -503,28 +503,12 @@ typedef struct QCowAIOCB {
     BlockDriverAIOCB *hd_aiocb;
 } QCowAIOCB;
 
-static void qcow_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    QCowAIOCB *acb = container_of(blockacb, QCowAIOCB, common);
-    if (acb->hd_aiocb)
-        bdrv_aio_cancel(acb->hd_aiocb);
-    qemu_aio_release(acb);
-}
-
-static AIOPool qcow_aio_pool = {
-    .aiocb_size         = sizeof(QCowAIOCB),
-    .cancel             = qcow_aio_cancel,
-};
-
 static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-        int is_write)
+        int is_write, QCowAIOCB *acb)
 {
-    QCowAIOCB *acb;
-
-    acb = qemu_aio_get(&qcow_aio_pool, bs, NULL, NULL);
-    if (!acb)
-        return NULL;
+    memset(acb, 0, sizeof(*acb));
+    acb->common.bs = bs;
     acb->hd_aiocb = NULL;
     acb->sector_num = sector_num;
     acb->qiov = qiov;
@@ -543,9 +527,8 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
     return acb;
 }
 
-static int qcow_aio_read_cb(void *opaque)
+static int qcow_aio_read_cb(QCowAIOCB *acb)
 {
-    QCowAIOCB *acb = opaque;
     BlockDriverState *bs = acb->common.bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
@@ -634,29 +617,27 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
                          int nb_sectors, QEMUIOVector *qiov)
 {
     BDRVQcowState *s = bs->opaque;
-    QCowAIOCB *acb;
+    QCowAIOCB acb;
     int ret;
 
-    acb = qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 0);
+    qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 0, &acb);
 
     qemu_co_mutex_lock(&s->lock);
     do {
-        ret = qcow_aio_read_cb(acb);
+        ret = qcow_aio_read_cb(&acb);
     } while (ret > 0);
     qemu_co_mutex_unlock(&s->lock);
 
-    if (acb->qiov->niov > 1) {
-        qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size);
-        qemu_vfree(acb->orig_buf);
+    if (acb.qiov->niov > 1) {
+        qemu_iovec_from_buffer(acb.qiov, acb.orig_buf, acb.qiov->size);
+        qemu_vfree(acb.orig_buf);
     }
-    qemu_aio_release(acb);
 
     return ret;
 }
 
-static int qcow_aio_write_cb(void *opaque)
+static int qcow_aio_write_cb(QCowAIOCB *acb)
 {
-    QCowAIOCB *acb = opaque;
     BlockDriverState *bs = acb->common.bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
@@ -714,23 +695,22 @@ static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
                           int nb_sectors, QEMUIOVector *qiov)
 {
     BDRVQcowState *s = bs->opaque;
-    QCowAIOCB *acb;
+    QCowAIOCB acb;
     int ret;
 
     s->cluster_cache_offset = -1; /* disable compressed cache */
 
-    acb = qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 1);
+    qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 1, &acb);
 
     qemu_co_mutex_lock(&s->lock);
     do {
-        ret = qcow_aio_write_cb(acb);
+        ret = qcow_aio_write_cb(&acb);
     } while (ret > 0);
     qemu_co_mutex_unlock(&s->lock);
 
-    if (acb->qiov->niov > 1) {
-        qemu_vfree(acb->orig_buf);
+    if (acb.qiov->niov > 1) {
+        qemu_vfree(acb.orig_buf);
     }
-    qemu_aio_release(acb);
 
     return ret;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index f07d550..edc068e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -388,17 +388,6 @@ typedef struct QCowAIOCB {
     QLIST_ENTRY(QCowAIOCB) next_depend;
 } QCowAIOCB;
 
-static void qcow2_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    QCowAIOCB *acb = container_of(blockacb, QCowAIOCB, common);
-    qemu_aio_release(acb);
-}
-
-static AIOPool qcow2_aio_pool = {
-    .aiocb_size         = sizeof(QCowAIOCB),
-    .cancel             = qcow2_aio_cancel,
-};
-
 /*
  * Returns 0 when the request is completed successfully, 1 when there is still
  * a part left to do and -errno in error cases.
@@ -528,13 +517,10 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
 static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
                                   QEMUIOVector *qiov, int nb_sectors,
                                   BlockDriverCompletionFunc *cb,
-                                  void *opaque, int is_write)
+                                  void *opaque, int is_write, QCowAIOCB *acb)
 {
-    QCowAIOCB *acb;
-
-    acb = qemu_aio_get(&qcow2_aio_pool, bs, cb, opaque);
-    if (!acb)
-        return NULL;
+    memset(acb, 0, sizeof(*acb));
+    acb->common.bs = bs;
     acb->sector_num = sector_num;
     acb->qiov = qiov;
     acb->is_write = is_write;
@@ -554,19 +540,18 @@ static int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
                           int nb_sectors, QEMUIOVector *qiov)
 {
     BDRVQcowState *s = bs->opaque;
-    QCowAIOCB *acb;
+    QCowAIOCB acb;
     int ret;
 
-    acb = qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 0);
+    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 0, &acb);
 
     qemu_co_mutex_lock(&s->lock);
     do {
-        ret = qcow2_aio_read_cb(acb);
+        ret = qcow2_aio_read_cb(&acb);
     } while (ret > 0);
     qemu_co_mutex_unlock(&s->lock);
 
-    qemu_iovec_destroy(&acb->hd_qiov);
-    qemu_aio_release(acb);
+    qemu_iovec_destroy(&acb.hd_qiov);
 
     return ret;
 }
@@ -670,20 +655,19 @@ static int qcow2_co_writev(BlockDriverState *bs,
                            QEMUIOVector *qiov)
 {
     BDRVQcowState *s = bs->opaque;
-    QCowAIOCB *acb;
+    QCowAIOCB acb;
     int ret;
 
-    acb = qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 1);
+    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 1, &acb);
     s->cluster_cache_offset = -1; /* disable compressed cache */
 
     qemu_co_mutex_lock(&s->lock);
     do {
-        ret = qcow2_aio_write_cb(acb);
+        ret = qcow2_aio_write_cb(&acb);
     } while (ret > 0);
     qemu_co_mutex_unlock(&s->lock);
 
-    qemu_iovec_destroy(&acb->hd_qiov);
-    qemu_aio_release(acb);
+    qemu_iovec_destroy(&acb.hd_qiov);
 
     return ret;
 }
-- 
1.7.1

[Qemu-devel] [PATCH 2/5] qcow: QCowAIOCB field cleanup

[Frediano Ziglio]

remove unused field from this structure and put some of them in qcow_aio_read_cb and qcow_aio_write_cb

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow.c |  135 +++++++++++++++++++++++++++-------------------------------
 1 files changed, 63 insertions(+), 72 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index d19ef04..e52df91 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -487,72 +487,61 @@ static int qcow_read(BlockDriverState *bs, int64_t sector_num,
 #endif
 
 typedef struct QCowAIOCB {
-    BlockDriverAIOCB common;
+    BlockDriverState *bs;
     int64_t sector_num;
     QEMUIOVector *qiov;
     uint8_t *buf;
     void *orig_buf;
     int nb_sectors;
-    int n;
-    uint64_t cluster_offset;
-    uint8_t *cluster_data;
-    struct iovec hd_iov;
-    bool is_write;
-    QEMUBH *bh;
-    QEMUIOVector hd_qiov;
-    BlockDriverAIOCB *hd_aiocb;
 } QCowAIOCB;
 
 static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         int is_write, QCowAIOCB *acb)
 {
-    memset(acb, 0, sizeof(*acb));
-    acb->common.bs = bs;
-    acb->hd_aiocb = NULL;
+    acb->bs = bs;
     acb->sector_num = sector_num;
     acb->qiov = qiov;
-    acb->is_write = is_write;
 
     if (qiov->niov > 1) {
         acb->buf = acb->orig_buf = qemu_blockalign(bs, qiov->size);
         if (is_write)
             qemu_iovec_to_buffer(qiov, acb->buf);
     } else {
+        acb->orig_buf = NULL;
         acb->buf = (uint8_t *)qiov->iov->iov_base;
     }
     acb->nb_sectors = nb_sectors;
-    acb->n = 0;
-    acb->cluster_offset = 0;
     return acb;
 }
 
 static int qcow_aio_read_cb(QCowAIOCB *acb)
 {
-    BlockDriverState *bs = acb->common.bs;
+    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
-    int ret;
-
-    acb->hd_aiocb = NULL;
+    int ret, n = 0;
+    uint64_t cluster_offset = 0;
+    struct iovec hd_iov;
+    QEMUIOVector hd_qiov;
 
  redo:
     /* post process the read buffer */
-    if (!acb->cluster_offset) {
+    if (!cluster_offset) {
         /* nothing to do */
-    } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
+    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
         /* nothing to do */
     } else {
         if (s->crypt_method) {
             encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
-                            acb->n, 0,
+                            n, 0,
                             &s->aes_decrypt_key);
         }
     }
 
-    acb->nb_sectors -= acb->n;
-    acb->sector_num += acb->n;
-    acb->buf += acb->n * 512;
+    acb->nb_sectors -= n;
+    acb->sector_num += n;
+    acb->buf += n * 512;
 
     if (acb->nb_sectors == 0) {
         /* request completed */
@@ -560,57 +549,57 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
     }
 
     /* prepare next AIO request */
-    acb->cluster_offset = get_cluster_offset(bs, acb->sector_num << 9,
+    cluster_offset = get_cluster_offset(bs, acb->sector_num << 9,
                                              0, 0, 0, 0);
     index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
-    acb->n = s->cluster_sectors - index_in_cluster;
-    if (acb->n > acb->nb_sectors)
-        acb->n = acb->nb_sectors;
+    n = s->cluster_sectors - index_in_cluster;
+    if (n > acb->nb_sectors)
+        n = acb->nb_sectors;
 
-    if (!acb->cluster_offset) {
+    if (!cluster_offset) {
         if (bs->backing_hd) {
             /* read from the base image */
-            acb->hd_iov.iov_base = (void *)acb->buf;
-            acb->hd_iov.iov_len = acb->n * 512;
-            qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+            hd_iov.iov_base = (void *)acb->buf;
+            hd_iov.iov_len = n * 512;
+            qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
             qemu_co_mutex_unlock(&s->lock);
             ret = bdrv_co_readv(bs->backing_hd, acb->sector_num,
-                                acb->n, &acb->hd_qiov);
+                                n, &hd_qiov);
             qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
                 return -EIO;
             }
         } else {
             /* Note: in this case, no need to wait */
-            memset(acb->buf, 0, 512 * acb->n);
+            memset(acb->buf, 0, 512 * n);
             goto redo;
         }
-    } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
+    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
         /* add AIO support for compressed blocks ? */
-        if (decompress_cluster(bs, acb->cluster_offset) < 0) {
+        if (decompress_cluster(bs, cluster_offset) < 0) {
             return -EIO;
         }
         memcpy(acb->buf,
-               s->cluster_cache + index_in_cluster * 512, 512 * acb->n);
+               s->cluster_cache + index_in_cluster * 512, 512 * n);
         goto redo;
     } else {
-        if ((acb->cluster_offset & 511) != 0) {
+        if ((cluster_offset & 511) != 0) {
             return -EIO;
         }
-        acb->hd_iov.iov_base = (void *)acb->buf;
-        acb->hd_iov.iov_len = acb->n * 512;
-        qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+        hd_iov.iov_base = (void *)acb->buf;
+        hd_iov.iov_len = n * 512;
+        qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
         qemu_co_mutex_unlock(&s->lock);
         ret = bdrv_co_readv(bs->file,
-                            (acb->cluster_offset >> 9) + index_in_cluster,
-                            acb->n, &acb->hd_qiov);
+                            (cluster_offset >> 9) + index_in_cluster,
+                            n, &hd_qiov);
         qemu_co_mutex_lock(&s->lock);
         if (ret < 0) {
             return ret;
         }
     }
 
-    return 1;
+    goto redo;
 }
 
 static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -623,9 +612,7 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
     qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 0, &acb);
 
     qemu_co_mutex_lock(&s->lock);
-    do {
-        ret = qcow_aio_read_cb(&acb);
-    } while (ret > 0);
+    ret = qcow_aio_read_cb(&acb);
     qemu_co_mutex_unlock(&s->lock);
 
     if (acb.qiov->niov > 1) {
@@ -638,18 +625,20 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
 
 static int qcow_aio_write_cb(QCowAIOCB *acb)
 {
-    BlockDriverState *bs = acb->common.bs;
+    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
     uint64_t cluster_offset;
     const uint8_t *src_buf;
-    int ret;
-
-    acb->hd_aiocb = NULL;
+    int ret, n = 0;
+    uint8_t *cluster_data = NULL;
+    struct iovec hd_iov;
+    QEMUIOVector hd_qiov;
 
-    acb->nb_sectors -= acb->n;
-    acb->sector_num += acb->n;
-    acb->buf += acb->n * 512;
+  redo:
+    acb->nb_sectors -= n;
+    acb->sector_num += n;
+    acb->buf += n * 512;
 
     if (acb->nb_sectors == 0) {
         /* request completed */
@@ -657,38 +646,42 @@ static int qcow_aio_write_cb(QCowAIOCB *acb)
     }
 
     index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
-    acb->n = s->cluster_sectors - index_in_cluster;
-    if (acb->n > acb->nb_sectors)
-        acb->n = acb->nb_sectors;
+    n = s->cluster_sectors - index_in_cluster;
+    if (n > acb->nb_sectors)
+        n = acb->nb_sectors;
     cluster_offset = get_cluster_offset(bs, acb->sector_num << 9, 1, 0,
                                         index_in_cluster,
-                                        index_in_cluster + acb->n);
+                                        index_in_cluster + n);
     if (!cluster_offset || (cluster_offset & 511) != 0) {
         return -EIO;
     }
     if (s->crypt_method) {
-        if (!acb->cluster_data) {
-            acb->cluster_data = qemu_mallocz(s->cluster_size);
+        if (!cluster_data) {
+            cluster_data = qemu_mallocz(s->cluster_size);
         }
-        encrypt_sectors(s, acb->sector_num, acb->cluster_data, acb->buf,
-                        acb->n, 1, &s->aes_encrypt_key);
-        src_buf = acb->cluster_data;
+        encrypt_sectors(s, acb->sector_num, cluster_data, acb->buf,
+                        n, 1, &s->aes_encrypt_key);
+        src_buf = cluster_data;
     } else {
         src_buf = acb->buf;
     }
 
-    acb->hd_iov.iov_base = (void *)src_buf;
-    acb->hd_iov.iov_len = acb->n * 512;
-    qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+    hd_iov.iov_base = (void *)src_buf;
+    hd_iov.iov_len = n * 512;
+    qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
     qemu_co_mutex_unlock(&s->lock);
     ret = bdrv_co_writev(bs->file,
                          (cluster_offset >> 9) + index_in_cluster,
-                         acb->n, &acb->hd_qiov);
+                         n, &hd_qiov);
+    if (cluster_data) {
+        free(cluster_data);
+        cluster_data = NULL;
+    }
     qemu_co_mutex_lock(&s->lock);
     if (ret < 0) {
         return ret;
     }
-    return 1;
+    goto redo;
 }
 
 static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
@@ -703,9 +696,7 @@ static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
     qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 1, &acb);
 
     qemu_co_mutex_lock(&s->lock);
-    do {
-        ret = qcow_aio_write_cb(&acb);
-    } while (ret > 0);
+    ret = qcow_aio_write_cb(&acb);
     qemu_co_mutex_unlock(&s->lock);
 
     if (acb.qiov->niov > 1) {
-- 
1.7.1

[Qemu-devel] [PATCH 3/5] qcow: move some blocks of code to avoid useless variable initialization

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow.c |   53 ++++++++++++++++++++++++++---------------------------
 1 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index e52df91..015a472 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -520,35 +520,18 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
     BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
-    int ret, n = 0;
-    uint64_t cluster_offset = 0;
+    int ret, n;
+    uint64_t cluster_offset;
     struct iovec hd_iov;
     QEMUIOVector hd_qiov;
 
  redo:
-    /* post process the read buffer */
-    if (!cluster_offset) {
-        /* nothing to do */
-    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
-        /* nothing to do */
-    } else {
-        if (s->crypt_method) {
-            encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
-                            n, 0,
-                            &s->aes_decrypt_key);
-        }
-    }
-
-    acb->nb_sectors -= n;
-    acb->sector_num += n;
-    acb->buf += n * 512;
-
     if (acb->nb_sectors == 0) {
         /* request completed */
         return 0;
     }
 
-    /* prepare next AIO request */
+    /* prepare next request */
     cluster_offset = get_cluster_offset(bs, acb->sector_num << 9,
                                              0, 0, 0, 0);
     index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
@@ -572,7 +555,6 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
         } else {
             /* Note: in this case, no need to wait */
             memset(acb->buf, 0, 512 * n);
-            goto redo;
         }
     } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
         /* add AIO support for compressed blocks ? */
@@ -581,7 +563,6 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
         }
         memcpy(acb->buf,
                s->cluster_cache + index_in_cluster * 512, 512 * n);
-        goto redo;
     } else {
         if ((cluster_offset & 511) != 0) {
             return -EIO;
@@ -599,6 +580,23 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
         }
     }
 
+    /* post process the read buffer */
+    if (!cluster_offset) {
+        /* nothing to do */
+    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
+        /* nothing to do */
+    } else {
+        if (s->crypt_method) {
+            encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
+                            n, 0,
+                            &s->aes_decrypt_key);
+        }
+    }
+
+    acb->nb_sectors -= n;
+    acb->sector_num += n;
+    acb->buf += n * 512;
+
     goto redo;
 }
 
@@ -630,16 +628,12 @@ static int qcow_aio_write_cb(QCowAIOCB *acb)
     int index_in_cluster;
     uint64_t cluster_offset;
     const uint8_t *src_buf;
-    int ret, n = 0;
+    int ret, n;
     uint8_t *cluster_data = NULL;
     struct iovec hd_iov;
     QEMUIOVector hd_qiov;
 
   redo:
-    acb->nb_sectors -= n;
-    acb->sector_num += n;
-    acb->buf += n * 512;
-
     if (acb->nb_sectors == 0) {
         /* request completed */
         return 0;
@@ -681,6 +675,11 @@ static int qcow_aio_write_cb(QCowAIOCB *acb)
     if (ret < 0) {
         return ret;
     }
+
+    acb->nb_sectors -= n;
+    acb->sector_num += n;
+    acb->buf += n * 512;
+
     goto redo;
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 4/5] qcow: embed qcow_aio_read_cb into qcow_co_readv and qcow_aio_write_cb into qcow_co_writev

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow.c |  287 ++++++++++++++++++++++++---------------------------------
 1 files changed, 121 insertions(+), 166 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 015a472..2feb7d7 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -486,221 +486,176 @@ static int qcow_read(BlockDriverState *bs, int64_t sector_num,
 }
 #endif
 
-typedef struct QCowAIOCB {
-    BlockDriverState *bs;
-    int64_t sector_num;
-    QEMUIOVector *qiov;
-    uint8_t *buf;
-    void *orig_buf;
-    int nb_sectors;
-} QCowAIOCB;
-
-static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
-        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-        int is_write, QCowAIOCB *acb)
-{
-    acb->bs = bs;
-    acb->sector_num = sector_num;
-    acb->qiov = qiov;
-
-    if (qiov->niov > 1) {
-        acb->buf = acb->orig_buf = qemu_blockalign(bs, qiov->size);
-        if (is_write)
-            qemu_iovec_to_buffer(qiov, acb->buf);
-    } else {
-        acb->orig_buf = NULL;
-        acb->buf = (uint8_t *)qiov->iov->iov_base;
-    }
-    acb->nb_sectors = nb_sectors;
-    return acb;
-}
-
-static int qcow_aio_read_cb(QCowAIOCB *acb)
+static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
+                         int nb_sectors, QEMUIOVector *qiov)
 {
-    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
-    int ret, n;
+    int ret = 0, n;
     uint64_t cluster_offset;
     struct iovec hd_iov;
     QEMUIOVector hd_qiov;
+    uint8_t *buf;
+    void *orig_buf;
 
- redo:
-    if (acb->nb_sectors == 0) {
-        /* request completed */
-        return 0;
+    if (qiov->niov > 1) {
+        buf = orig_buf = qemu_blockalign(bs, qiov->size);
+    } else {
+        orig_buf = NULL;
+        buf = (uint8_t *)qiov->iov->iov_base;
     }
 
-    /* prepare next request */
-    cluster_offset = get_cluster_offset(bs, acb->sector_num << 9,
-                                             0, 0, 0, 0);
-    index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
-    n = s->cluster_sectors - index_in_cluster;
-    if (n > acb->nb_sectors)
-        n = acb->nb_sectors;
+    qemu_co_mutex_lock(&s->lock);
+
+    while (nb_sectors != 0) {
+        /* prepare next request */
+        cluster_offset = get_cluster_offset(bs, sector_num << 9,
+                                                 0, 0, 0, 0);
+        index_in_cluster = sector_num & (s->cluster_sectors - 1);
+        n = s->cluster_sectors - index_in_cluster;
+        if (n > nb_sectors)
+            n = nb_sectors;
 
-    if (!cluster_offset) {
-        if (bs->backing_hd) {
-            /* read from the base image */
-            hd_iov.iov_base = (void *)acb->buf;
+        if (!cluster_offset) {
+            if (bs->backing_hd) {
+                /* read from the base image */
+                hd_iov.iov_base = (void *)buf;
+                hd_iov.iov_len = n * 512;
+                qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
+                qemu_co_mutex_unlock(&s->lock);
+                ret = bdrv_co_readv(bs->backing_hd, sector_num,
+                                    n, &hd_qiov);
+                qemu_co_mutex_lock(&s->lock);
+                if (ret < 0) {
+                    goto fail;
+                }
+            } else {
+                /* Note: in this case, no need to wait */
+                memset(buf, 0, 512 * n);
+            }
+        } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
+            /* add AIO support for compressed blocks ? */
+            if (decompress_cluster(bs, cluster_offset) < 0) {
+                goto fail;
+            }
+            memcpy(buf,
+                   s->cluster_cache + index_in_cluster * 512, 512 * n);
+        } else {
+            if ((cluster_offset & 511) != 0) {
+                goto fail;
+            }
+            hd_iov.iov_base = (void *)buf;
             hd_iov.iov_len = n * 512;
             qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
             qemu_co_mutex_unlock(&s->lock);
-            ret = bdrv_co_readv(bs->backing_hd, acb->sector_num,
+            ret = bdrv_co_readv(bs->file,
+                                (cluster_offset >> 9) + index_in_cluster,
                                 n, &hd_qiov);
             qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
-                return -EIO;
+                break;
+            }
+            if (s->crypt_method) {
+                encrypt_sectors(s, sector_num, buf, buf,
+                                n, 0,
+                                &s->aes_decrypt_key);
             }
-        } else {
-            /* Note: in this case, no need to wait */
-            memset(acb->buf, 0, 512 * n);
-        }
-    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
-        /* add AIO support for compressed blocks ? */
-        if (decompress_cluster(bs, cluster_offset) < 0) {
-            return -EIO;
-        }
-        memcpy(acb->buf,
-               s->cluster_cache + index_in_cluster * 512, 512 * n);
-    } else {
-        if ((cluster_offset & 511) != 0) {
-            return -EIO;
-        }
-        hd_iov.iov_base = (void *)acb->buf;
-        hd_iov.iov_len = n * 512;
-        qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
-        qemu_co_mutex_unlock(&s->lock);
-        ret = bdrv_co_readv(bs->file,
-                            (cluster_offset >> 9) + index_in_cluster,
-                            n, &hd_qiov);
-        qemu_co_mutex_lock(&s->lock);
-        if (ret < 0) {
-            return ret;
         }
-    }
+        ret = 0;
 
-    /* post process the read buffer */
-    if (!cluster_offset) {
-        /* nothing to do */
-    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
-        /* nothing to do */
-    } else {
-        if (s->crypt_method) {
-            encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
-                            n, 0,
-                            &s->aes_decrypt_key);
-        }
+        nb_sectors -= n;
+        sector_num += n;
+        buf += n * 512;
     }
 
-    acb->nb_sectors -= n;
-    acb->sector_num += n;
-    acb->buf += n * 512;
-
-    goto redo;
-}
-
-static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
-                         int nb_sectors, QEMUIOVector *qiov)
-{
-    BDRVQcowState *s = bs->opaque;
-    QCowAIOCB acb;
-    int ret;
-
-    qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 0, &acb);
-
-    qemu_co_mutex_lock(&s->lock);
-    ret = qcow_aio_read_cb(&acb);
+done:
     qemu_co_mutex_unlock(&s->lock);
 
-    if (acb.qiov->niov > 1) {
-        qemu_iovec_from_buffer(acb.qiov, acb.orig_buf, acb.qiov->size);
-        qemu_vfree(acb.orig_buf);
+    if (qiov->niov > 1) {
+        qemu_iovec_from_buffer(qiov, orig_buf, qiov->size);
+        qemu_vfree(orig_buf);
     }
 
     return ret;
+
+fail:
+    ret = -EIO;
+    goto done;
 }
 
-static int qcow_aio_write_cb(QCowAIOCB *acb)
+static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
+                          int nb_sectors, QEMUIOVector *qiov)
 {
-    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
     uint64_t cluster_offset;
     const uint8_t *src_buf;
-    int ret, n;
+    int ret = 0, n;
     uint8_t *cluster_data = NULL;
     struct iovec hd_iov;
     QEMUIOVector hd_qiov;
+    uint8_t *buf;
+    void *orig_buf;
 
-  redo:
-    if (acb->nb_sectors == 0) {
-        /* request completed */
-        return 0;
-    }
+    s->cluster_cache_offset = -1; /* disable compressed cache */
 
-    index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
-    n = s->cluster_sectors - index_in_cluster;
-    if (n > acb->nb_sectors)
-        n = acb->nb_sectors;
-    cluster_offset = get_cluster_offset(bs, acb->sector_num << 9, 1, 0,
-                                        index_in_cluster,
-                                        index_in_cluster + n);
-    if (!cluster_offset || (cluster_offset & 511) != 0) {
-        return -EIO;
-    }
-    if (s->crypt_method) {
-        if (!cluster_data) {
-            cluster_data = qemu_mallocz(s->cluster_size);
-        }
-        encrypt_sectors(s, acb->sector_num, cluster_data, acb->buf,
-                        n, 1, &s->aes_encrypt_key);
-        src_buf = cluster_data;
+    if (qiov->niov > 1) {
+        buf = orig_buf = qemu_blockalign(bs, qiov->size);
+        qemu_iovec_to_buffer(qiov, buf);
     } else {
-        src_buf = acb->buf;
+        orig_buf = NULL;
+        buf = (uint8_t *)qiov->iov->iov_base;
     }
 
-    hd_iov.iov_base = (void *)src_buf;
-    hd_iov.iov_len = n * 512;
-    qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
-    qemu_co_mutex_unlock(&s->lock);
-    ret = bdrv_co_writev(bs->file,
-                         (cluster_offset >> 9) + index_in_cluster,
-                         n, &hd_qiov);
-    if (cluster_data) {
-        free(cluster_data);
-        cluster_data = NULL;
-    }
     qemu_co_mutex_lock(&s->lock);
-    if (ret < 0) {
-        return ret;
-    }
-
-    acb->nb_sectors -= n;
-    acb->sector_num += n;
-    acb->buf += n * 512;
-
-    goto redo;
-}
 
-static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
-                          int nb_sectors, QEMUIOVector *qiov)
-{
-    BDRVQcowState *s = bs->opaque;
-    QCowAIOCB acb;
-    int ret;
+    while (nb_sectors != 0) {
 
-    s->cluster_cache_offset = -1; /* disable compressed cache */
+        index_in_cluster = sector_num & (s->cluster_sectors - 1);
+        n = s->cluster_sectors - index_in_cluster;
+        if (n > nb_sectors)
+            n = nb_sectors;
+        cluster_offset = get_cluster_offset(bs, sector_num << 9, 1, 0,
+                                            index_in_cluster,
+                                            index_in_cluster + n);
+        if (!cluster_offset || (cluster_offset & 511) != 0) {
+            ret = -EIO;
+            break;
+        }
+        if (s->crypt_method) {
+            if (!cluster_data) {
+                cluster_data = qemu_mallocz(s->cluster_size);
+            }
+            encrypt_sectors(s, sector_num, cluster_data, buf,
+                            n, 1, &s->aes_encrypt_key);
+            src_buf = cluster_data;
+        } else {
+            src_buf = buf;
+        }
 
-    qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 1, &acb);
+        hd_iov.iov_base = (void *)src_buf;
+        hd_iov.iov_len = n * 512;
+        qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
+        qemu_co_mutex_unlock(&s->lock);
+        ret = bdrv_co_writev(bs->file,
+                             (cluster_offset >> 9) + index_in_cluster,
+                             n, &hd_qiov);
+        qemu_co_mutex_lock(&s->lock);
+        if (ret < 0) {
+            break;
+        }
+        ret = 0;
 
-    qemu_co_mutex_lock(&s->lock);
-    ret = qcow_aio_write_cb(&acb);
+        nb_sectors -= n;
+        sector_num += n;
+        buf += n * 512;
+    }
     qemu_co_mutex_unlock(&s->lock);
 
-    if (acb.qiov->niov > 1) {
-        qemu_vfree(acb.orig_buf);
+    if (qiov->niov > 1) {
+        qemu_vfree(orig_buf);
     }
+    free(cluster_data);
 
     return ret;
 }
-- 
1.7.1

[Qemu-devel] [PATCH 5/5] qcow: remove old #undefined code

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow.c |   63 ----------------------------------------------------------
 1 files changed, 0 insertions(+), 63 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 2feb7d7..42331c7 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -190,24 +190,6 @@ static int qcow_set_key(BlockDriverState *bs, const char *key)
         return -1;
     if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0)
         return -1;
-#if 0
-    /* test */
-    {
-        uint8_t in[16];
-        uint8_t out[16];
-        uint8_t tmp[16];
-        for(i=0;i<16;i++)
-            in[i] = i;
-        AES_encrypt(in, tmp, &s->aes_encrypt_key);
-        AES_decrypt(tmp, out, &s->aes_decrypt_key);
-        for(i = 0; i < 16; i++)
-            printf(" %02x", tmp[i]);
-        printf("\n");
-        for(i = 0; i < 16; i++)
-            printf(" %02x", out[i]);
-        printf("\n");
-    }
-#endif
     return 0;
 }
 
@@ -441,51 +423,6 @@ static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
     return 0;
 }
 
-#if 0
-
-static int qcow_read(BlockDriverState *bs, int64_t sector_num,
-                     uint8_t *buf, int nb_sectors)
-{
-    BDRVQcowState *s = bs->opaque;
-    int ret, index_in_cluster, n;
-    uint64_t cluster_offset;
-
-    while (nb_sectors > 0) {
-        cluster_offset = get_cluster_offset(bs, sector_num << 9, 0, 0, 0, 0);
-        index_in_cluster = sector_num & (s->cluster_sectors - 1);
-        n = s->cluster_sectors - index_in_cluster;
-        if (n > nb_sectors)
-            n = nb_sectors;
-        if (!cluster_offset) {
-            if (bs->backing_hd) {
-                /* read from the base image */
-                ret = bdrv_read(bs->backing_hd, sector_num, buf, n);
-                if (ret < 0)
-                    return -1;
-            } else {
-                memset(buf, 0, 512 * n);
-            }
-        } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
-            if (decompress_cluster(bs, cluster_offset) < 0)
-                return -1;
-            memcpy(buf, s->cluster_cache + index_in_cluster * 512, 512 * n);
-        } else {
-            ret = bdrv_pread(bs->file, cluster_offset + index_in_cluster * 512, buf, n * 512);
-            if (ret != n * 512)
-                return -1;
-            if (s->crypt_method) {
-                encrypt_sectors(s, sector_num, buf, buf, n, 0,
-                                &s->aes_decrypt_key);
-            }
-        }
-        nb_sectors -= n;
-        sector_num += n;
-        buf += n * 512;
-    }
-    return 0;
-}
-#endif
-
 static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
                          int nb_sectors, QEMUIOVector *qiov)
 {
-- 
1.7.1