[Qemu-devel] [PATCH v2] block: Change bdrv_commit to handle multiple sectors at once

[Qemu-devel] [PATCH v2] block: Change bdrv_commit to handle multiple sectors at once

[Kevin Wolf]

bdrv_commit copies the image to its backing file sector by sector, which
is (surprise!) relatively slow. Let's take a larger buffer and handle more
sectors at once if possible.

With a 1G qcow2 file, this brought the time bdrv_commit takes down from
5:06 min to 1:14 min for me.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |   39 ++++++++++++++++++++-------------------
 1 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/block.c b/block.c
index 65cf4dc..6e3fc49 100644
--- a/block.c
+++ b/block.c
@@ -724,14 +724,16 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res)
     return bs->drv->bdrv_check(bs, res);
 }
 
+#define COMMIT_BUF_SECTORS 2048
+
 /* commit COW file into the raw image */
 int bdrv_commit(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
-    int64_t i, total_sectors;
-    int n, j, ro, open_flags;
+    int64_t sector, total_sectors;
+    int n, ro, open_flags;
     int ret = 0, rw_ret = 0;
-    unsigned char sector[BDRV_SECTOR_SIZE];
+    uint8_t *buf;
     char filename[1024];
     BlockDriverState *bs_rw, *bs_ro;
 
@@ -774,22 +776,20 @@ int bdrv_commit(BlockDriverState *bs)
     }
 
     total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
-    for (i = 0; i < total_sectors;) {
-        if (drv->bdrv_is_allocated(bs, i, 65536, &n)) {
-            for(j = 0; j < n; j++) {
-                if (bdrv_read(bs, i, sector, 1) != 0) {
-                    ret = -EIO;
-                    goto ro_cleanup;
-                }
+    buf = qemu_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
 
-                if (bdrv_write(bs->backing_hd, i, sector, 1) != 0) {
-                    ret = -EIO;
-                    goto ro_cleanup;
-                }
-                i++;
-	    }
-	} else {
-            i += n;
+    for (sector = 0; sector < total_sectors; sector += n) {
+        if (drv->bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n)) {
+
+            if (bdrv_read(bs, sector, buf, n) != 0) {
+                ret = -EIO;
+                goto ro_cleanup;
+            }
+
+            if (bdrv_write(bs->backing_hd, sector, buf, n) != 0) {
+                ret = -EIO;
+                goto ro_cleanup;
+            }
         }
     }
 
@@ -806,6 +806,7 @@ int bdrv_commit(BlockDriverState *bs)
         bdrv_flush(bs->backing_hd);
 
 ro_cleanup:
+    qemu_free(buf);
 
     if (ro) {
         /* re-open as RO */
-- 
1.7.1.1

Re: [Qemu-devel] [PATCH v2] block: Change bdrv_commit to handle multiple sectors at once

[Stefan Hajnoczi]

On Mon, Jul 19, 2010 at 10:59 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> bdrv_commit copies the image to its backing file sector by sector, which
> is (surprise!) relatively slow. Let's take a larger buffer and handle more
> sectors at once if possible.
>
> With a 1G qcow2 file, this brought the time bdrv_commit takes down from
> 5:06 min to 1:14 min for me.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---

Looks good.

Stefan