qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups
@ 2011-08-23 13:21 Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 01/15] qcow: allocate QCowAIOCB structure using stack Frediano Ziglio
                   ` (15 more replies)
  0 siblings, 16 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio

These patches mostly cleanup some AIO code using coroutines.
Mostly they use stack instead of allocated AIO structure.
Feel free to collapse it too short.

Frediano Ziglio (15):
  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
  qcow2: removed unused fields
  qcow2: removed cur_nr_sectors field in QCowAIOCB
  qcow2: remove l2meta from QCowAIOCB
  qcow2: remove cluster_offset from QCowAIOCB
  qcow2: remove common from QCowAIOCB
  qcow2: reindent and use while before the big jump
  qcow2: removed QCowAIOCB entirely
  qcow2: remove memory leak
  qcow2: small math optimization
  qcow2: small optimization

 block/qcow.c           |  378 ++++++++++++++------------------------------
 block/qcow2-refcount.c |   16 +--
 block/qcow2.c          |  412 +++++++++++++++++++----------------------------
 3 files changed, 294 insertions(+), 512 deletions(-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 01/15] qcow: allocate QCowAIOCB structure using stack
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 02/15] qcow: QCowAIOCB field cleanup Frediano Ziglio
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, 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 e155d3c..b4506b4 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 bfff6cd..bb6c75e 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 02/15] qcow: QCowAIOCB field cleanup
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 01/15] qcow: allocate QCowAIOCB structure using stack Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 03/15] qcow: move some blocks of code to avoid useless variable initialization Frediano Ziglio
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, 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 |  137 +++++++++++++++++++++++++++------------------------------
 1 files changed, 65 insertions(+), 72 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index b4506b4..9754ca9 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,58 @@ 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 +613,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 +626,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 +647,43 @@ 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 = g_malloc0(s->cluster_size);
+        if (!cluster_data) {
+            cluster_data = g_malloc0(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 +698,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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 03/15] qcow: move some blocks of code to avoid useless variable initialization
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 01/15] qcow: allocate QCowAIOCB structure using stack Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 02/15] qcow: QCowAIOCB field cleanup Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 04/15] qcow: embed qcow_aio_read_cb into qcow_co_readv and qcow_aio_write_cb into qcow_co_writev Frediano Ziglio
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, 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 9754ca9..4ede7f3 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);
@@ -573,7 +556,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 ? */
@@ -582,7 +564,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;
@@ -600,6 +581,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;
 }
 
@@ -631,16 +629,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;
@@ -683,6 +677,11 @@ redo:
     if (ret < 0) {
         return ret;
     }
+
+    acb->nb_sectors -= n;
+    acb->sector_num += n;
+    acb->buf += n * 512;
+
     goto redo;
 }
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 04/15] qcow: embed qcow_aio_read_cb into qcow_co_readv and qcow_aio_write_cb into qcow_co_writev
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (2 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 03/15] qcow: move some blocks of code to avoid useless variable initialization Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 05/15] qcow: remove old #undefined code Frediano Ziglio
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


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

diff --git a/block/qcow.c b/block/qcow.c
index 4ede7f3..f28c821 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -486,223 +486,178 @@ 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 = g_malloc0(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;
-}
+    while (nb_sectors != 0) {
 
-static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
-                          int nb_sectors, QEMUIOVector *qiov)
-{
-    BDRVQcowState *s = bs->opaque;
-    QCowAIOCB acb;
-    int ret;
-
-    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 = g_malloc0(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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 05/15] qcow: remove old #undefined code
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (3 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 04/15] qcow: embed qcow_aio_read_cb into qcow_co_readv and qcow_aio_write_cb into qcow_co_writev Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 06/15] qcow2: removed unused fields Frediano Ziglio
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, 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 f28c821..8cbabdd 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 06/15] qcow2: removed unused fields
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (4 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 05/15] qcow: remove old #undefined code Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 07/15] qcow2: removed cur_nr_sectors field in QCowAIOCB Frediano Ziglio
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow2.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index bb6c75e..4171b47 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -381,11 +381,8 @@ typedef struct QCowAIOCB {
     uint64_t bytes_done;
     uint64_t cluster_offset;
     uint8_t *cluster_data;
-    bool is_write;
     QEMUIOVector hd_qiov;
-    QEMUBH *bh;
     QCowL2Meta l2meta;
-    QLIST_ENTRY(QCowAIOCB) next_depend;
 } QCowAIOCB;
 
 /*
@@ -517,13 +514,12 @@ 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, QCowAIOCB *acb)
+                                  void *opaque, QCowAIOCB *acb)
 {
     memset(acb, 0, sizeof(*acb));
     acb->common.bs = bs;
     acb->sector_num = sector_num;
     acb->qiov = qiov;
-    acb->is_write = is_write;
 
     qemu_iovec_init(&acb->hd_qiov, qiov->niov);
 
@@ -543,7 +539,7 @@ static int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
     QCowAIOCB acb;
     int ret;
 
-    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 0, &acb);
+    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, &acb);
 
     qemu_co_mutex_lock(&s->lock);
     do {
@@ -658,7 +654,7 @@ static int qcow2_co_writev(BlockDriverState *bs,
     QCowAIOCB acb;
     int ret;
 
-    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 1, &acb);
+    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, &acb);
     s->cluster_cache_offset = -1; /* disable compressed cache */
 
     qemu_co_mutex_lock(&s->lock);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 07/15] qcow2: removed cur_nr_sectors field in QCowAIOCB
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (5 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 06/15] qcow2: removed unused fields Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 08/15] qcow2: remove l2meta from QCowAIOCB Frediano Ziglio
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow2.c |   98 +++++++++++++++++++++++++--------------------------------
 1 files changed, 43 insertions(+), 55 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 4171b47..f920dbe 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -377,7 +377,6 @@ typedef struct QCowAIOCB {
     int64_t sector_num;
     QEMUIOVector *qiov;
     int remaining_sectors;
-    int cur_nr_sectors;	/* number of sectors in current iteration */
     uint64_t bytes_done;
     uint64_t cluster_offset;
     uint8_t *cluster_data;
@@ -395,42 +394,22 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster, n1;
     int ret;
-
-    /* post process the read buffer */
-    if (!acb->cluster_offset) {
-        /* nothing to do */
-    } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
-        /* nothing to do */
-    } else {
-        if (s->crypt_method) {
-            qcow2_encrypt_sectors(s, acb->sector_num,  acb->cluster_data,
-                acb->cluster_data, acb->cur_nr_sectors, 0, &s->aes_decrypt_key);
-            qemu_iovec_reset(&acb->hd_qiov);
-            qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
-                acb->cur_nr_sectors * 512);
-            qemu_iovec_from_buffer(&acb->hd_qiov, acb->cluster_data,
-                512 * acb->cur_nr_sectors);
-        }
-    }
-
-    acb->remaining_sectors -= acb->cur_nr_sectors;
-    acb->sector_num += acb->cur_nr_sectors;
-    acb->bytes_done += acb->cur_nr_sectors * 512;
+    int cur_nr_sectors; /* number of sectors in current iteration */
 
     if (acb->remaining_sectors == 0) {
         /* request completed */
         return 0;
     }
 
-    /* prepare next AIO request */
-    acb->cur_nr_sectors = acb->remaining_sectors;
+    /* prepare next request */
+    cur_nr_sectors = acb->remaining_sectors;
     if (s->crypt_method) {
-        acb->cur_nr_sectors = MIN(acb->cur_nr_sectors,
+        cur_nr_sectors = MIN(cur_nr_sectors,
             QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
     }
 
     ret = qcow2_get_cluster_offset(bs, acb->sector_num << 9,
-        &acb->cur_nr_sectors, &acb->cluster_offset);
+        &cur_nr_sectors, &acb->cluster_offset);
     if (ret < 0) {
         return ret;
     }
@@ -439,14 +418,14 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
 
     qemu_iovec_reset(&acb->hd_qiov);
     qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
-        acb->cur_nr_sectors * 512);
+        cur_nr_sectors * 512);
 
     if (!acb->cluster_offset) {
 
         if (bs->backing_hd) {
             /* read from the base image */
             n1 = qcow2_backing_read1(bs->backing_hd, &acb->hd_qiov,
-                acb->sector_num, acb->cur_nr_sectors);
+                acb->sector_num, cur_nr_sectors);
             if (n1 > 0) {
                 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
                 qemu_co_mutex_unlock(&s->lock);
@@ -457,11 +436,9 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
                     return ret;
                 }
             }
-            return 1;
         } else {
             /* Note: in this case, no need to wait */
-            qemu_iovec_memset(&acb->hd_qiov, 0, 512 * acb->cur_nr_sectors);
-            return 1;
+            qemu_iovec_memset(&acb->hd_qiov, 0, 512 * cur_nr_sectors);
         }
     } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
         /* add AIO support for compressed blocks ? */
@@ -472,9 +449,7 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
 
         qemu_iovec_from_buffer(&acb->hd_qiov,
             s->cluster_cache + index_in_cluster * 512,
-            512 * acb->cur_nr_sectors);
-
-        return 1;
+            512 * cur_nr_sectors);
     } else {
         if ((acb->cluster_offset & 511) != 0) {
             return -EIO;
@@ -490,24 +465,37 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
                     g_malloc0(QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
             }
 
-            assert(acb->cur_nr_sectors <=
+            assert(cur_nr_sectors <=
                 QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
             qemu_iovec_reset(&acb->hd_qiov);
             qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
-                512 * acb->cur_nr_sectors);
+                512 * cur_nr_sectors);
         }
 
         BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
         qemu_co_mutex_unlock(&s->lock);
         ret = bdrv_co_readv(bs->file,
                             (acb->cluster_offset >> 9) + index_in_cluster,
-                            acb->cur_nr_sectors, &acb->hd_qiov);
+                            cur_nr_sectors, &acb->hd_qiov);
         qemu_co_mutex_lock(&s->lock);
         if (ret < 0) {
             return ret;
         }
+        if (s->crypt_method) {
+            qcow2_encrypt_sectors(s, acb->sector_num,  acb->cluster_data,
+                acb->cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
+            qemu_iovec_reset(&acb->hd_qiov);
+            qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
+                cur_nr_sectors * 512);
+            qemu_iovec_from_buffer(&acb->hd_qiov, acb->cluster_data,
+                512 * cur_nr_sectors);
+        }
     }
 
+    acb->remaining_sectors -= cur_nr_sectors;
+    acb->sector_num += cur_nr_sectors;
+    acb->bytes_done += cur_nr_sectors * 512;
+
     return 1;
 }
 
@@ -525,7 +513,6 @@ static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
 
     acb->bytes_done = 0;
     acb->remaining_sectors = nb_sectors;
-    acb->cur_nr_sectors = 0;
     acb->cluster_offset = 0;
     acb->l2meta.nb_clusters = 0;
     qemu_co_queue_init(&acb->l2meta.dependent_requests);
@@ -578,18 +565,7 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
     int index_in_cluster;
     int n_end;
     int ret;
-
-    ret = qcow2_alloc_cluster_link_l2(bs, &acb->l2meta);
-
-    run_dependent_requests(s, &acb->l2meta);
-
-    if (ret < 0) {
-        return ret;
-    }
-
-    acb->remaining_sectors -= acb->cur_nr_sectors;
-    acb->sector_num += acb->cur_nr_sectors;
-    acb->bytes_done += acb->cur_nr_sectors * 512;
+    int cur_nr_sectors; /* number of sectors in current iteration */
 
     if (acb->remaining_sectors == 0) {
         /* request completed */
@@ -603,7 +579,7 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
         n_end = QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors;
 
     ret = qcow2_alloc_cluster_offset(bs, acb->sector_num << 9,
-        index_in_cluster, n_end, &acb->cur_nr_sectors, &acb->l2meta);
+        index_in_cluster, n_end, &cur_nr_sectors, &acb->l2meta);
     if (ret < 0) {
         return ret;
     }
@@ -613,7 +589,7 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
 
     qemu_iovec_reset(&acb->hd_qiov);
     qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
-        acb->cur_nr_sectors * 512);
+        cur_nr_sectors * 512);
 
     if (s->crypt_method) {
         if (!acb->cluster_data) {
@@ -625,23 +601,35 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
         qemu_iovec_to_buffer(&acb->hd_qiov, acb->cluster_data);
 
         qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data,
-            acb->cluster_data, acb->cur_nr_sectors, 1, &s->aes_encrypt_key);
+            acb->cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key);
 
         qemu_iovec_reset(&acb->hd_qiov);
         qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
-            acb->cur_nr_sectors * 512);
+            cur_nr_sectors * 512);
     }
 
     BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
     qemu_co_mutex_unlock(&s->lock);
     ret = bdrv_co_writev(bs->file,
                          (acb->cluster_offset >> 9) + index_in_cluster,
-                         acb->cur_nr_sectors, &acb->hd_qiov);
+                         cur_nr_sectors, &acb->hd_qiov);
     qemu_co_mutex_lock(&s->lock);
     if (ret < 0) {
         return ret;
     }
 
+    ret = qcow2_alloc_cluster_link_l2(bs, &acb->l2meta);
+
+    run_dependent_requests(s, &acb->l2meta);
+
+    if (ret < 0) {
+        return ret;
+    }
+
+    acb->remaining_sectors -= cur_nr_sectors;
+    acb->sector_num += cur_nr_sectors;
+    acb->bytes_done += cur_nr_sectors * 512;
+
     return 1;
 }
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 08/15] qcow2: remove l2meta from QCowAIOCB
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (6 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 07/15] qcow2: removed cur_nr_sectors field in QCowAIOCB Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 09/15] qcow2: remove cluster_offset " Frediano Ziglio
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow2.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index f920dbe..4b9ec29 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -381,7 +381,6 @@ typedef struct QCowAIOCB {
     uint64_t cluster_offset;
     uint8_t *cluster_data;
     QEMUIOVector hd_qiov;
-    QCowL2Meta l2meta;
 } QCowAIOCB;
 
 /*
@@ -514,8 +513,6 @@ static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
     acb->bytes_done = 0;
     acb->remaining_sectors = nb_sectors;
     acb->cluster_offset = 0;
-    acb->l2meta.nb_clusters = 0;
-    qemu_co_queue_init(&acb->l2meta.dependent_requests);
     return acb;
 }
 
@@ -566,6 +563,10 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
     int n_end;
     int ret;
     int cur_nr_sectors; /* number of sectors in current iteration */
+    QCowL2Meta l2meta;
+
+    l2meta.nb_clusters = 0;
+    qemu_co_queue_init(&l2meta.dependent_requests);
 
     if (acb->remaining_sectors == 0) {
         /* request completed */
@@ -579,12 +580,12 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
         n_end = QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors;
 
     ret = qcow2_alloc_cluster_offset(bs, acb->sector_num << 9,
-        index_in_cluster, n_end, &cur_nr_sectors, &acb->l2meta);
+        index_in_cluster, n_end, &cur_nr_sectors, &l2meta);
     if (ret < 0) {
         return ret;
     }
 
-    acb->cluster_offset = acb->l2meta.cluster_offset;
+    acb->cluster_offset = l2meta.cluster_offset;
     assert((acb->cluster_offset & 511) == 0);
 
     qemu_iovec_reset(&acb->hd_qiov);
@@ -618,9 +619,9 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
         return ret;
     }
 
-    ret = qcow2_alloc_cluster_link_l2(bs, &acb->l2meta);
+    ret = qcow2_alloc_cluster_link_l2(bs, &l2meta);
 
-    run_dependent_requests(s, &acb->l2meta);
+    run_dependent_requests(s, &l2meta);
 
     if (ret < 0) {
         return ret;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 09/15] qcow2: remove cluster_offset from QCowAIOCB
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (7 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 08/15] qcow2: remove l2meta from QCowAIOCB Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 10/15] qcow2: remove common " Frediano Ziglio
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


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

diff --git a/block/qcow2.c b/block/qcow2.c
index 4b9ec29..7e13283 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -378,7 +378,6 @@ typedef struct QCowAIOCB {
     QEMUIOVector *qiov;
     int remaining_sectors;
     uint64_t bytes_done;
-    uint64_t cluster_offset;
     uint8_t *cluster_data;
     QEMUIOVector hd_qiov;
 } QCowAIOCB;
@@ -394,6 +393,7 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
     int index_in_cluster, n1;
     int ret;
     int cur_nr_sectors; /* number of sectors in current iteration */
+    uint64_t cluster_offset = 0;
 
     if (acb->remaining_sectors == 0) {
         /* request completed */
@@ -408,7 +408,7 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
     }
 
     ret = qcow2_get_cluster_offset(bs, acb->sector_num << 9,
-        &cur_nr_sectors, &acb->cluster_offset);
+        &cur_nr_sectors, &cluster_offset);
     if (ret < 0) {
         return ret;
     }
@@ -419,7 +419,7 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
     qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
         cur_nr_sectors * 512);
 
-    if (!acb->cluster_offset) {
+    if (!cluster_offset) {
 
         if (bs->backing_hd) {
             /* read from the base image */
@@ -439,9 +439,9 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
             /* Note: in this case, no need to wait */
             qemu_iovec_memset(&acb->hd_qiov, 0, 512 * cur_nr_sectors);
         }
-    } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
+    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
         /* add AIO support for compressed blocks ? */
-        ret = qcow2_decompress_cluster(bs, acb->cluster_offset);
+        ret = qcow2_decompress_cluster(bs, cluster_offset);
         if (ret < 0) {
             return ret;
         }
@@ -450,7 +450,7 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
             s->cluster_cache + index_in_cluster * 512,
             512 * cur_nr_sectors);
     } else {
-        if ((acb->cluster_offset & 511) != 0) {
+        if ((cluster_offset & 511) != 0) {
             return -EIO;
         }
 
@@ -474,7 +474,7 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
         BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
         qemu_co_mutex_unlock(&s->lock);
         ret = bdrv_co_readv(bs->file,
-                            (acb->cluster_offset >> 9) + index_in_cluster,
+                            (cluster_offset >> 9) + index_in_cluster,
                             cur_nr_sectors, &acb->hd_qiov);
         qemu_co_mutex_lock(&s->lock);
         if (ret < 0) {
@@ -512,7 +512,6 @@ static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
 
     acb->bytes_done = 0;
     acb->remaining_sectors = nb_sectors;
-    acb->cluster_offset = 0;
     return acb;
 }
 
@@ -564,6 +563,7 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
     int ret;
     int cur_nr_sectors; /* number of sectors in current iteration */
     QCowL2Meta l2meta;
+    uint64_t cluster_offset;
 
     l2meta.nb_clusters = 0;
     qemu_co_queue_init(&l2meta.dependent_requests);
@@ -585,8 +585,8 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
         return ret;
     }
 
-    acb->cluster_offset = l2meta.cluster_offset;
-    assert((acb->cluster_offset & 511) == 0);
+    cluster_offset = l2meta.cluster_offset;
+    assert((cluster_offset & 511) == 0);
 
     qemu_iovec_reset(&acb->hd_qiov);
     qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
@@ -612,7 +612,7 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
     BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
     qemu_co_mutex_unlock(&s->lock);
     ret = bdrv_co_writev(bs->file,
-                         (acb->cluster_offset >> 9) + index_in_cluster,
+                         (cluster_offset >> 9) + index_in_cluster,
                          cur_nr_sectors, &acb->hd_qiov);
     qemu_co_mutex_lock(&s->lock);
     if (ret < 0) {
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 10/15] qcow2: remove common from QCowAIOCB
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (8 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 09/15] qcow2: remove cluster_offset " Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 11/15] qcow2: reindent and use while before the big jump Frediano Ziglio
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


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

diff --git a/block/qcow2.c b/block/qcow2.c
index 7e13283..519fc8b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -373,7 +373,7 @@ int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
 }
 
 typedef struct QCowAIOCB {
-    BlockDriverAIOCB common;
+    BlockDriverState *bs;
     int64_t sector_num;
     QEMUIOVector *qiov;
     int remaining_sectors;
@@ -388,7 +388,7 @@ typedef struct QCowAIOCB {
  */
 static int qcow2_aio_read_cb(QCowAIOCB *acb)
 {
-    BlockDriverState *bs = acb->common.bs;
+    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster, n1;
     int ret;
@@ -504,7 +504,7 @@ static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
                                   void *opaque, QCowAIOCB *acb)
 {
     memset(acb, 0, sizeof(*acb));
-    acb->common.bs = bs;
+    acb->bs = bs;
     acb->sector_num = sector_num;
     acb->qiov = qiov;
 
@@ -556,7 +556,7 @@ static void run_dependent_requests(BDRVQcowState *s, QCowL2Meta *m)
  */
 static int qcow2_aio_write_cb(QCowAIOCB *acb)
 {
-    BlockDriverState *bs = acb->common.bs;
+    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
     int n_end;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 11/15] qcow2: reindent and use while before the big jump
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (9 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 10/15] qcow2: remove common " Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 12/15] qcow2: removed QCowAIOCB entirely Frediano Ziglio
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio

prepare to remove read/write callbacks

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

diff --git a/block/qcow2.c b/block/qcow2.c
index 519fc8b..287dd99 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -395,107 +395,105 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
     int cur_nr_sectors; /* number of sectors in current iteration */
     uint64_t cluster_offset = 0;
 
-    if (acb->remaining_sectors == 0) {
-        /* request completed */
-        return 0;
-    }
-
-    /* prepare next request */
-    cur_nr_sectors = acb->remaining_sectors;
-    if (s->crypt_method) {
-        cur_nr_sectors = MIN(cur_nr_sectors,
-            QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
-    }
+    while (acb->remaining_sectors != 0) {
 
-    ret = qcow2_get_cluster_offset(bs, acb->sector_num << 9,
-        &cur_nr_sectors, &cluster_offset);
-    if (ret < 0) {
-        return ret;
-    }
-
-    index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
-
-    qemu_iovec_reset(&acb->hd_qiov);
-    qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
-        cur_nr_sectors * 512);
-
-    if (!cluster_offset) {
-
-        if (bs->backing_hd) {
-            /* read from the base image */
-            n1 = qcow2_backing_read1(bs->backing_hd, &acb->hd_qiov,
-                acb->sector_num, cur_nr_sectors);
-            if (n1 > 0) {
-                BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
-                qemu_co_mutex_unlock(&s->lock);
-                ret = bdrv_co_readv(bs->backing_hd, acb->sector_num,
-                                    n1, &acb->hd_qiov);
-                qemu_co_mutex_lock(&s->lock);
-                if (ret < 0) {
-                    return ret;
-                }
-            }
-        } else {
-            /* Note: in this case, no need to wait */
-            qemu_iovec_memset(&acb->hd_qiov, 0, 512 * cur_nr_sectors);
+        /* prepare next request */
+        cur_nr_sectors = acb->remaining_sectors;
+        if (s->crypt_method) {
+            cur_nr_sectors = MIN(cur_nr_sectors,
+                QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
         }
-    } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
-        /* add AIO support for compressed blocks ? */
-        ret = qcow2_decompress_cluster(bs, cluster_offset);
+
+        ret = qcow2_get_cluster_offset(bs, acb->sector_num << 9,
+            &cur_nr_sectors, &cluster_offset);
         if (ret < 0) {
             return ret;
         }
 
-        qemu_iovec_from_buffer(&acb->hd_qiov,
-            s->cluster_cache + index_in_cluster * 512,
-            512 * cur_nr_sectors);
-    } else {
-        if ((cluster_offset & 511) != 0) {
-            return -EIO;
-        }
+        index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
 
-        if (s->crypt_method) {
-            /*
-             * For encrypted images, read everything into a temporary
-             * contiguous buffer on which the AES functions can work.
-             */
-            if (!acb->cluster_data) {
-                acb->cluster_data =
-                    g_malloc0(QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+        qemu_iovec_reset(&acb->hd_qiov);
+        qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
+            cur_nr_sectors * 512);
+
+        if (!cluster_offset) {
+
+            if (bs->backing_hd) {
+                /* read from the base image */
+                n1 = qcow2_backing_read1(bs->backing_hd, &acb->hd_qiov,
+                    acb->sector_num, cur_nr_sectors);
+                if (n1 > 0) {
+                    BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
+                    qemu_co_mutex_unlock(&s->lock);
+                    ret = bdrv_co_readv(bs->backing_hd, acb->sector_num,
+                                        n1, &acb->hd_qiov);
+                    qemu_co_mutex_lock(&s->lock);
+                    if (ret < 0) {
+                        return ret;
+                    }
+                }
+            } else {
+                /* Note: in this case, no need to wait */
+                qemu_iovec_memset(&acb->hd_qiov, 0, 512 * cur_nr_sectors);
+            }
+        } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
+            /* add AIO support for compressed blocks ? */
+            ret = qcow2_decompress_cluster(bs, cluster_offset);
+            if (ret < 0) {
+                return ret;
             }
 
-            assert(cur_nr_sectors <=
-                QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
-            qemu_iovec_reset(&acb->hd_qiov);
-            qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
+            qemu_iovec_from_buffer(&acb->hd_qiov,
+                s->cluster_cache + index_in_cluster * 512,
                 512 * cur_nr_sectors);
-        }
+        } else {
+            if ((cluster_offset & 511) != 0) {
+                return -EIO;
+            }
 
-        BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-        qemu_co_mutex_unlock(&s->lock);
-        ret = bdrv_co_readv(bs->file,
-                            (cluster_offset >> 9) + index_in_cluster,
-                            cur_nr_sectors, &acb->hd_qiov);
-        qemu_co_mutex_lock(&s->lock);
-        if (ret < 0) {
-            return ret;
-        }
-        if (s->crypt_method) {
-            qcow2_encrypt_sectors(s, acb->sector_num,  acb->cluster_data,
-                acb->cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
-            qemu_iovec_reset(&acb->hd_qiov);
-            qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
-                cur_nr_sectors * 512);
-            qemu_iovec_from_buffer(&acb->hd_qiov, acb->cluster_data,
-                512 * cur_nr_sectors);
+            if (s->crypt_method) {
+                /*
+                 * For encrypted images, read everything into a temporary
+                 * contiguous buffer on which the AES functions can work.
+                 */
+                if (!acb->cluster_data) {
+                    acb->cluster_data =
+                        g_malloc0(QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+                }
+
+                assert(cur_nr_sectors <=
+                    QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
+                qemu_iovec_reset(&acb->hd_qiov);
+                qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
+                    512 * cur_nr_sectors);
+            }
+
+            BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+            qemu_co_mutex_unlock(&s->lock);
+            ret = bdrv_co_readv(bs->file,
+                                (cluster_offset >> 9) + index_in_cluster,
+                                cur_nr_sectors, &acb->hd_qiov);
+            qemu_co_mutex_lock(&s->lock);
+            if (ret < 0) {
+                return ret;
+            }
+            if (s->crypt_method) {
+                qcow2_encrypt_sectors(s, acb->sector_num,  acb->cluster_data,
+                    acb->cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
+                qemu_iovec_reset(&acb->hd_qiov);
+                qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
+                    cur_nr_sectors * 512);
+                qemu_iovec_from_buffer(&acb->hd_qiov, acb->cluster_data,
+                    512 * cur_nr_sectors);
+            }
         }
-    }
 
-    acb->remaining_sectors -= cur_nr_sectors;
-    acb->sector_num += cur_nr_sectors;
-    acb->bytes_done += cur_nr_sectors * 512;
+        acb->remaining_sectors -= cur_nr_sectors;
+        acb->sector_num += cur_nr_sectors;
+        acb->bytes_done += cur_nr_sectors * 512;
+    }
 
-    return 1;
+    return 0;
 }
 
 static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
@@ -568,70 +566,70 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
     l2meta.nb_clusters = 0;
     qemu_co_queue_init(&l2meta.dependent_requests);
 
-    if (acb->remaining_sectors == 0) {
-        /* request completed */
-        return 0;
-    }
+    while (acb->remaining_sectors != 0) {
 
-    index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
-    n_end = index_in_cluster + acb->remaining_sectors;
-    if (s->crypt_method &&
-        n_end > QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors)
-        n_end = QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors;
+        index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
+        n_end = index_in_cluster + acb->remaining_sectors;
+        if (s->crypt_method &&
+            n_end > QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors) {
+            n_end = QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors;
+        }
 
-    ret = qcow2_alloc_cluster_offset(bs, acb->sector_num << 9,
-        index_in_cluster, n_end, &cur_nr_sectors, &l2meta);
-    if (ret < 0) {
-        return ret;
-    }
+        ret = qcow2_alloc_cluster_offset(bs, acb->sector_num << 9,
+            index_in_cluster, n_end, &cur_nr_sectors, &l2meta);
+        if (ret < 0) {
+            return ret;
+        }
 
-    cluster_offset = l2meta.cluster_offset;
-    assert((cluster_offset & 511) == 0);
+        cluster_offset = l2meta.cluster_offset;
+        assert((cluster_offset & 511) == 0);
 
-    qemu_iovec_reset(&acb->hd_qiov);
-    qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
-        cur_nr_sectors * 512);
+        qemu_iovec_reset(&acb->hd_qiov);
+        qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
+            cur_nr_sectors * 512);
 
-    if (s->crypt_method) {
-        if (!acb->cluster_data) {
-            acb->cluster_data = g_malloc0(QCOW_MAX_CRYPT_CLUSTERS *
-                                             s->cluster_size);
-        }
+        if (s->crypt_method) {
+            if (!acb->cluster_data) {
+                acb->cluster_data = g_malloc0(QCOW_MAX_CRYPT_CLUSTERS *
+                                                 s->cluster_size);
+            }
 
-        assert(acb->hd_qiov.size <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
-        qemu_iovec_to_buffer(&acb->hd_qiov, acb->cluster_data);
+            assert(acb->hd_qiov.size <=
+                   QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+            qemu_iovec_to_buffer(&acb->hd_qiov, acb->cluster_data);
 
-        qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data,
-            acb->cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key);
+            qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data,
+                acb->cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key);
 
-        qemu_iovec_reset(&acb->hd_qiov);
-        qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
-            cur_nr_sectors * 512);
-    }
+            qemu_iovec_reset(&acb->hd_qiov);
+            qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
+                cur_nr_sectors * 512);
+        }
 
-    BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
-    qemu_co_mutex_unlock(&s->lock);
-    ret = bdrv_co_writev(bs->file,
-                         (cluster_offset >> 9) + index_in_cluster,
-                         cur_nr_sectors, &acb->hd_qiov);
-    qemu_co_mutex_lock(&s->lock);
-    if (ret < 0) {
-        return ret;
-    }
+        BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
+        qemu_co_mutex_unlock(&s->lock);
+        ret = bdrv_co_writev(bs->file,
+                             (cluster_offset >> 9) + index_in_cluster,
+                             cur_nr_sectors, &acb->hd_qiov);
+        qemu_co_mutex_lock(&s->lock);
+        if (ret < 0) {
+            return ret;
+        }
 
-    ret = qcow2_alloc_cluster_link_l2(bs, &l2meta);
+        ret = qcow2_alloc_cluster_link_l2(bs, &l2meta);
 
-    run_dependent_requests(s, &l2meta);
+        run_dependent_requests(s, &l2meta);
 
-    if (ret < 0) {
-        return ret;
-    }
+        if (ret < 0) {
+            return ret;
+        }
 
-    acb->remaining_sectors -= cur_nr_sectors;
-    acb->sector_num += cur_nr_sectors;
-    acb->bytes_done += cur_nr_sectors * 512;
+        acb->remaining_sectors -= cur_nr_sectors;
+        acb->sector_num += cur_nr_sectors;
+        acb->bytes_done += cur_nr_sectors * 512;
+    }
 
-    return 1;
+    return 0;
 }
 
 static int qcow2_co_writev(BlockDriverState *bs,
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 12/15] qcow2: removed QCowAIOCB entirely
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (10 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 11/15] qcow2: reindent and use while before the big jump Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 13/15] qcow2: remove memory leak Frediano Ziglio
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow2.c |  207 ++++++++++++++++++++++-----------------------------------
 1 files changed, 80 insertions(+), 127 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 287dd99..d34bd72 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -372,83 +372,77 @@ int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
     return n1;
 }
 
-typedef struct QCowAIOCB {
-    BlockDriverState *bs;
-    int64_t sector_num;
-    QEMUIOVector *qiov;
-    int remaining_sectors;
-    uint64_t bytes_done;
-    uint8_t *cluster_data;
-    QEMUIOVector hd_qiov;
-} QCowAIOCB;
-
-/*
- * Returns 0 when the request is completed successfully, 1 when there is still
- * a part left to do and -errno in error cases.
- */
-static int qcow2_aio_read_cb(QCowAIOCB *acb)
+static int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
+                          int remaining_sectors, QEMUIOVector *qiov)
 {
-    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster, n1;
     int ret;
     int cur_nr_sectors; /* number of sectors in current iteration */
     uint64_t cluster_offset = 0;
+    uint64_t bytes_done = 0;
+    QEMUIOVector hd_qiov;
+    uint8_t *cluster_data = NULL;
 
-    while (acb->remaining_sectors != 0) {
+    qemu_iovec_init(&hd_qiov, qiov->niov);
+
+    qemu_co_mutex_lock(&s->lock);
+
+    while (remaining_sectors != 0) {
 
         /* prepare next request */
-        cur_nr_sectors = acb->remaining_sectors;
+        cur_nr_sectors = remaining_sectors;
         if (s->crypt_method) {
             cur_nr_sectors = MIN(cur_nr_sectors,
                 QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
         }
 
-        ret = qcow2_get_cluster_offset(bs, acb->sector_num << 9,
+        ret = qcow2_get_cluster_offset(bs, sector_num << 9,
             &cur_nr_sectors, &cluster_offset);
         if (ret < 0) {
-            return ret;
+            goto fail;
         }
 
-        index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
+        index_in_cluster = sector_num & (s->cluster_sectors - 1);
 
-        qemu_iovec_reset(&acb->hd_qiov);
-        qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
+        qemu_iovec_reset(&hd_qiov);
+        qemu_iovec_copy(&hd_qiov, qiov, bytes_done,
             cur_nr_sectors * 512);
 
         if (!cluster_offset) {
 
             if (bs->backing_hd) {
                 /* read from the base image */
-                n1 = qcow2_backing_read1(bs->backing_hd, &acb->hd_qiov,
-                    acb->sector_num, cur_nr_sectors);
+                n1 = qcow2_backing_read1(bs->backing_hd, &hd_qiov,
+                    sector_num, cur_nr_sectors);
                 if (n1 > 0) {
                     BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
                     qemu_co_mutex_unlock(&s->lock);
-                    ret = bdrv_co_readv(bs->backing_hd, acb->sector_num,
-                                        n1, &acb->hd_qiov);
+                    ret = bdrv_co_readv(bs->backing_hd, sector_num,
+                                        n1, &hd_qiov);
                     qemu_co_mutex_lock(&s->lock);
                     if (ret < 0) {
-                        return ret;
+                        goto fail;
                     }
                 }
             } else {
                 /* Note: in this case, no need to wait */
-                qemu_iovec_memset(&acb->hd_qiov, 0, 512 * cur_nr_sectors);
+                qemu_iovec_memset(&hd_qiov, 0, 512 * cur_nr_sectors);
             }
         } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
             /* add AIO support for compressed blocks ? */
             ret = qcow2_decompress_cluster(bs, cluster_offset);
             if (ret < 0) {
-                return ret;
+                goto fail;
             }
 
-            qemu_iovec_from_buffer(&acb->hd_qiov,
+            qemu_iovec_from_buffer(&hd_qiov,
                 s->cluster_cache + index_in_cluster * 512,
                 512 * cur_nr_sectors);
         } else {
             if ((cluster_offset & 511) != 0) {
-                return -EIO;
+                ret = -EIO;
+                goto fail;
             }
 
             if (s->crypt_method) {
@@ -456,15 +450,15 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
                  * For encrypted images, read everything into a temporary
                  * contiguous buffer on which the AES functions can work.
                  */
-                if (!acb->cluster_data) {
-                    acb->cluster_data =
+                if (!cluster_data) {
+                    cluster_data =
                         g_malloc0(QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
                 }
 
                 assert(cur_nr_sectors <=
                     QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors);
-                qemu_iovec_reset(&acb->hd_qiov);
-                qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
+                qemu_iovec_reset(&hd_qiov);
+                qemu_iovec_add(&hd_qiov, cluster_data,
                     512 * cur_nr_sectors);
             }
 
@@ -472,63 +466,32 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
             qemu_co_mutex_unlock(&s->lock);
             ret = bdrv_co_readv(bs->file,
                                 (cluster_offset >> 9) + index_in_cluster,
-                                cur_nr_sectors, &acb->hd_qiov);
+                                cur_nr_sectors, &hd_qiov);
             qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
-                return ret;
+                goto fail;
             }
             if (s->crypt_method) {
-                qcow2_encrypt_sectors(s, acb->sector_num,  acb->cluster_data,
-                    acb->cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
-                qemu_iovec_reset(&acb->hd_qiov);
-                qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
+                qcow2_encrypt_sectors(s, sector_num,  cluster_data,
+                    cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
+                qemu_iovec_reset(&hd_qiov);
+                qemu_iovec_copy(&hd_qiov, qiov, bytes_done,
                     cur_nr_sectors * 512);
-                qemu_iovec_from_buffer(&acb->hd_qiov, acb->cluster_data,
+                qemu_iovec_from_buffer(&hd_qiov, cluster_data,
                     512 * cur_nr_sectors);
             }
         }
 
-        acb->remaining_sectors -= cur_nr_sectors;
-        acb->sector_num += cur_nr_sectors;
-        acb->bytes_done += cur_nr_sectors * 512;
+        remaining_sectors -= cur_nr_sectors;
+        sector_num += cur_nr_sectors;
+        bytes_done += cur_nr_sectors * 512;
     }
+    ret = 0;
 
-    return 0;
-}
-
-static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
-                                  QEMUIOVector *qiov, int nb_sectors,
-                                  BlockDriverCompletionFunc *cb,
-                                  void *opaque, QCowAIOCB *acb)
-{
-    memset(acb, 0, sizeof(*acb));
-    acb->bs = bs;
-    acb->sector_num = sector_num;
-    acb->qiov = qiov;
-
-    qemu_iovec_init(&acb->hd_qiov, qiov->niov);
-
-    acb->bytes_done = 0;
-    acb->remaining_sectors = nb_sectors;
-    return acb;
-}
-
-static int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
-                          int nb_sectors, QEMUIOVector *qiov)
-{
-    BDRVQcowState *s = bs->opaque;
-    QCowAIOCB acb;
-    int ret;
-
-    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, &acb);
-
-    qemu_co_mutex_lock(&s->lock);
-    do {
-        ret = qcow2_aio_read_cb(&acb);
-    } while (ret > 0);
+fail:
     qemu_co_mutex_unlock(&s->lock);
 
-    qemu_iovec_destroy(&acb.hd_qiov);
+    qemu_iovec_destroy(&hd_qiov);
 
     return ret;
 }
@@ -548,13 +511,11 @@ static void run_dependent_requests(BDRVQcowState *s, QCowL2Meta *m)
     }
 }
 
-/*
- * Returns 0 when the request is completed successfully, 1 when there is still
- * a part left to do and -errno in error cases.
- */
-static int qcow2_aio_write_cb(QCowAIOCB *acb)
+static int qcow2_co_writev(BlockDriverState *bs,
+                           int64_t sector_num,
+                           int remaining_sectors,
+                           QEMUIOVector *qiov)
 {
-    BlockDriverState *bs = acb->bs;
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster;
     int n_end;
@@ -562,47 +523,56 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
     int cur_nr_sectors; /* number of sectors in current iteration */
     QCowL2Meta l2meta;
     uint64_t cluster_offset;
+    QEMUIOVector hd_qiov;
+    uint64_t bytes_done = 0;
+    uint8_t *cluster_data = NULL;
 
     l2meta.nb_clusters = 0;
     qemu_co_queue_init(&l2meta.dependent_requests);
 
-    while (acb->remaining_sectors != 0) {
+    qemu_iovec_init(&hd_qiov, qiov->niov);
+
+    s->cluster_cache_offset = -1; /* disable compressed cache */
+
+    qemu_co_mutex_lock(&s->lock);
 
-        index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
-        n_end = index_in_cluster + acb->remaining_sectors;
+    while (remaining_sectors != 0) {
+
+        index_in_cluster = sector_num & (s->cluster_sectors - 1);
+        n_end = index_in_cluster + remaining_sectors;
         if (s->crypt_method &&
             n_end > QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors) {
             n_end = QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors;
         }
 
-        ret = qcow2_alloc_cluster_offset(bs, acb->sector_num << 9,
+        ret = qcow2_alloc_cluster_offset(bs, sector_num << 9,
             index_in_cluster, n_end, &cur_nr_sectors, &l2meta);
         if (ret < 0) {
-            return ret;
+            goto fail;
         }
 
         cluster_offset = l2meta.cluster_offset;
         assert((cluster_offset & 511) == 0);
 
-        qemu_iovec_reset(&acb->hd_qiov);
-        qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done,
+        qemu_iovec_reset(&hd_qiov);
+        qemu_iovec_copy(&hd_qiov, qiov, bytes_done,
             cur_nr_sectors * 512);
 
         if (s->crypt_method) {
-            if (!acb->cluster_data) {
-                acb->cluster_data = g_malloc0(QCOW_MAX_CRYPT_CLUSTERS *
+            if (!cluster_data) {
+                cluster_data = g_malloc0(QCOW_MAX_CRYPT_CLUSTERS *
                                                  s->cluster_size);
             }
 
-            assert(acb->hd_qiov.size <=
+            assert(hd_qiov.size <=
                    QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
-            qemu_iovec_to_buffer(&acb->hd_qiov, acb->cluster_data);
+            qemu_iovec_to_buffer(&hd_qiov, cluster_data);
 
-            qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data,
-                acb->cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key);
+            qcow2_encrypt_sectors(s, sector_num, cluster_data,
+                cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key);
 
-            qemu_iovec_reset(&acb->hd_qiov);
-            qemu_iovec_add(&acb->hd_qiov, acb->cluster_data,
+            qemu_iovec_reset(&hd_qiov);
+            qemu_iovec_add(&hd_qiov, cluster_data,
                 cur_nr_sectors * 512);
         }
 
@@ -610,10 +580,10 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
         qemu_co_mutex_unlock(&s->lock);
         ret = bdrv_co_writev(bs->file,
                              (cluster_offset >> 9) + index_in_cluster,
-                             cur_nr_sectors, &acb->hd_qiov);
+                             cur_nr_sectors, &hd_qiov);
         qemu_co_mutex_lock(&s->lock);
         if (ret < 0) {
-            return ret;
+            goto fail;
         }
 
         ret = qcow2_alloc_cluster_link_l2(bs, &l2meta);
@@ -621,36 +591,19 @@ static int qcow2_aio_write_cb(QCowAIOCB *acb)
         run_dependent_requests(s, &l2meta);
 
         if (ret < 0) {
-            return ret;
+            goto fail;
         }
 
-        acb->remaining_sectors -= cur_nr_sectors;
-        acb->sector_num += cur_nr_sectors;
-        acb->bytes_done += cur_nr_sectors * 512;
+        remaining_sectors -= cur_nr_sectors;
+        sector_num += cur_nr_sectors;
+        bytes_done += cur_nr_sectors * 512;
     }
+    ret = 0;
 
-    return 0;
-}
-
-static int qcow2_co_writev(BlockDriverState *bs,
-                           int64_t sector_num,
-                           int nb_sectors,
-                           QEMUIOVector *qiov)
-{
-    BDRVQcowState *s = bs->opaque;
-    QCowAIOCB acb;
-    int ret;
-
-    qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, &acb);
-    s->cluster_cache_offset = -1; /* disable compressed cache */
-
-    qemu_co_mutex_lock(&s->lock);
-    do {
-        ret = qcow2_aio_write_cb(&acb);
-    } while (ret > 0);
+fail:
     qemu_co_mutex_unlock(&s->lock);
 
-    qemu_iovec_destroy(&acb.hd_qiov);
+    qemu_iovec_destroy(&hd_qiov);
 
     return ret;
 }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 13/15] qcow2: remove memory leak
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (11 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 12/15] qcow2: removed QCowAIOCB entirely Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 14/15] qcow2: small math optimization Frediano Ziglio
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


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

diff --git a/block/qcow2.c b/block/qcow2.c
index d34bd72..07529b1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -492,6 +492,7 @@ fail:
     qemu_co_mutex_unlock(&s->lock);
 
     qemu_iovec_destroy(&hd_qiov);
+    g_free(cluster_data);
 
     return ret;
 }
@@ -604,6 +605,7 @@ fail:
     qemu_co_mutex_unlock(&s->lock);
 
     qemu_iovec_destroy(&hd_qiov);
+    g_free(cluster_data);
 
     return ret;
 }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 14/15] qcow2: small math optimization
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (12 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 13/15] qcow2: remove memory leak Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 15:34   ` Kevin Wolf
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 15/15] qcow2: small optimization Frediano Ziglio
  2011-08-23 15:41 ` [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Kevin Wolf
  15 siblings, 1 reply; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow2-refcount.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 2a915be..0f9a64a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -140,10 +140,7 @@ static unsigned int next_refcount_table_size(BDRVQcowState *s,
 static int in_same_refcount_block(BDRVQcowState *s, uint64_t offset_a,
     uint64_t offset_b)
 {
-    uint64_t block_a = offset_a >> (2 * s->cluster_bits - REFCOUNT_SHIFT);
-    uint64_t block_b = offset_b >> (2 * s->cluster_bits - REFCOUNT_SHIFT);
-
-    return (block_a == block_b);
+    return ((offset_a ^ offset_b) >> (2 * s->cluster_bits - REFCOUNT_SHIFT)) == 0;
 }
 
 /*
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v3 15/15] qcow2: small optimization
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (13 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 14/15] qcow2: small math optimization Frediano Ziglio
@ 2011-08-23 13:21 ` Frediano Ziglio
  2011-08-23 15:35   ` Kevin Wolf
  2011-08-23 15:41 ` [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Kevin Wolf
  15 siblings, 1 reply; 19+ messages in thread
From: Frediano Ziglio @ 2011-08-23 13:21 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, Frediano Ziglio


Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
 block/qcow2-refcount.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 0f9a64a..243c24b 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -681,14 +681,13 @@ void qcow2_create_refcount_update(QCowCreateState *s, int64_t offset,
     int64_t size)
 {
     int refcount;
-    int64_t start, last, cluster_offset;
+    int64_t start, last, cluster;
     uint16_t *p;
 
-    start = offset & ~(s->cluster_size - 1);
-    last = (offset + size - 1)  & ~(s->cluster_size - 1);
-    for(cluster_offset = start; cluster_offset <= last;
-        cluster_offset += s->cluster_size) {
-        p = &s->refcount_block[cluster_offset >> s->cluster_bits];
+    start = offset >> s->cluster_bits;
+    last = (offset + size - 1)  >> s->cluster_bits;
+    for(cluster = start; cluster <= last; ++cluster) {
+        p = &s->refcount_block[cluster];
         refcount = be16_to_cpu(*p);
         refcount++;
         *p = cpu_to_be16(refcount);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v3 14/15] qcow2: small math optimization
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 14/15] qcow2: small math optimization Frediano Ziglio
@ 2011-08-23 15:34   ` Kevin Wolf
  0 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2011-08-23 15:34 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: qemu-devel

Am 23.08.2011 15:21, schrieb Frediano Ziglio:
> Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
> ---
>  block/qcow2-refcount.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 2a915be..0f9a64a 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -140,10 +140,7 @@ static unsigned int next_refcount_table_size(BDRVQcowState *s,
>  static int in_same_refcount_block(BDRVQcowState *s, uint64_t offset_a,
>      uint64_t offset_b)
>  {
> -    uint64_t block_a = offset_a >> (2 * s->cluster_bits - REFCOUNT_SHIFT);
> -    uint64_t block_b = offset_b >> (2 * s->cluster_bits - REFCOUNT_SHIFT);
> -
> -    return (block_a == block_b);
> +    return ((offset_a ^ offset_b) >> (2 * s->cluster_bits - REFCOUNT_SHIFT)) == 0;
>  }

Depending on whether the compiler is smart enough this will or will not
change performance. However, even if we assume that it's a slight
improvement, this is in a function that is hardly ever run and the
optimisation comes with a high cost in terms of readability.

I wouldn't do this.

Kevin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v3 15/15] qcow2: small optimization
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 15/15] qcow2: small optimization Frediano Ziglio
@ 2011-08-23 15:35   ` Kevin Wolf
  0 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2011-08-23 15:35 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: qemu-devel

Am 23.08.2011 15:21, schrieb Frediano Ziglio:
> Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
> ---
>  block/qcow2-refcount.c |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 0f9a64a..243c24b 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -681,14 +681,13 @@ void qcow2_create_refcount_update(QCowCreateState *s, int64_t offset,
>      int64_t size)
>  {
>      int refcount;
> -    int64_t start, last, cluster_offset;
> +    int64_t start, last, cluster;
>      uint16_t *p;
>  
> -    start = offset & ~(s->cluster_size - 1);
> -    last = (offset + size - 1)  & ~(s->cluster_size - 1);
> -    for(cluster_offset = start; cluster_offset <= last;
> -        cluster_offset += s->cluster_size) {
> -        p = &s->refcount_block[cluster_offset >> s->cluster_bits];
> +    start = offset >> s->cluster_bits;
> +    last = (offset + size - 1)  >> s->cluster_bits;
> +    for(cluster = start; cluster <= last; ++cluster) {
> +        p = &s->refcount_block[cluster];
>          refcount = be16_to_cpu(*p);
>          refcount++;
>          *p = cpu_to_be16(refcount);

This function is unused nowadays. I'd prefer a patch that removes it
altogether. :-)

Kevin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups
  2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
                   ` (14 preceding siblings ...)
  2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 15/15] qcow2: small optimization Frediano Ziglio
@ 2011-08-23 15:41 ` Kevin Wolf
  15 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2011-08-23 15:41 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: qemu-devel

Am 23.08.2011 15:21, schrieb Frediano Ziglio:
> These patches mostly cleanup some AIO code using coroutines.
> Mostly they use stack instead of allocated AIO structure.
> Feel free to collapse it too short.
> 
> Frediano Ziglio (15):
>   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
>   qcow2: removed unused fields
>   qcow2: removed cur_nr_sectors field in QCowAIOCB
>   qcow2: remove l2meta from QCowAIOCB
>   qcow2: remove cluster_offset from QCowAIOCB
>   qcow2: remove common from QCowAIOCB
>   qcow2: reindent and use while before the big jump
>   qcow2: removed QCowAIOCB entirely
>   qcow2: remove memory leak
>   qcow2: small math optimization
>   qcow2: small optimization
> 
>  block/qcow.c           |  378 ++++++++++++++------------------------------
>  block/qcow2-refcount.c |   16 +--
>  block/qcow2.c          |  412 +++++++++++++++++++----------------------------
>  3 files changed, 294 insertions(+), 512 deletions(-)
> 

Thanks, applied patches 1-13 to the block branch.

Kevin

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2011-08-23 15:38 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-23 13:21 [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 01/15] qcow: allocate QCowAIOCB structure using stack Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 02/15] qcow: QCowAIOCB field cleanup Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 03/15] qcow: move some blocks of code to avoid useless variable initialization Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 04/15] qcow: embed qcow_aio_read_cb into qcow_co_readv and qcow_aio_write_cb into qcow_co_writev Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 05/15] qcow: remove old #undefined code Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 06/15] qcow2: removed unused fields Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 07/15] qcow2: removed cur_nr_sectors field in QCowAIOCB Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 08/15] qcow2: remove l2meta from QCowAIOCB Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 09/15] qcow2: remove cluster_offset " Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 10/15] qcow2: remove common " Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 11/15] qcow2: reindent and use while before the big jump Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 12/15] qcow2: removed QCowAIOCB entirely Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 13/15] qcow2: remove memory leak Frediano Ziglio
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 14/15] qcow2: small math optimization Frediano Ziglio
2011-08-23 15:34   ` Kevin Wolf
2011-08-23 13:21 ` [Qemu-devel] [PATCH v3 15/15] qcow2: small optimization Frediano Ziglio
2011-08-23 15:35   ` Kevin Wolf
2011-08-23 15:41 ` [Qemu-devel] [PATCH v3 00/15] qcow/qcow2 cleanups Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).