[Qemu-devel] [PATCH v7 for 2.0 0/4] qemu-img: add preallocation=full

[Qemu-devel] [PATCH v7 for 2.0 0/4] qemu-img: add preallocation=full

[Hu Tao]

This series implements full image preallocation to create a non-sparse
image file at creation time, both for raw and qcow2 format. The purpose
is to avoid performance deterioration of the guest cause by sparse image.

v7:
  - fix qemu-iotests fail case 082.


Hu Tao (4):
  qapi: introduce PreallocMode and a new PreallocMode full.
  raw, qcow2: don't convert file size to sector size
  raw-posix: Add full image preallocation option
  qcow2: Add full image preallocation option

 block/qcow2.c              | 91 ++++++++++++++++++++++++++++++++++++++++------
 block/raw-posix.c          | 45 ++++++++++++++++++-----
 block/raw-win32.c          |  4 +-
 qapi-schema.json           | 14 +++++++
 tests/qemu-iotests/082.out | 54 +++++++++++++--------------
 5 files changed, 159 insertions(+), 49 deletions(-)

-- 
1.8.5.2.229.g4448466

[Qemu-devel] [PATCH v7 for 2.0 1/4] qapi: introduce PreallocMode and a new PreallocMode full.

[Hu Tao]

This patch prepares for the subsequent patches.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 block/qcow2.c    |  8 ++++----
 qapi-schema.json | 14 ++++++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 945c9d6..154c40c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1455,7 +1455,7 @@ static int preallocate(BlockDriverState *bs)
 
 static int qcow2_create2(const char *filename, int64_t total_size,
                          const char *backing_file, const char *backing_format,
-                         int flags, size_t cluster_size, int prealloc,
+                         int flags, size_t cluster_size, PreallocMode prealloc,
                          QEMUOptionParameter *options, int version,
                          Error **errp)
 {
@@ -1631,7 +1631,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
     uint64_t sectors = 0;
     int flags = 0;
     size_t cluster_size = DEFAULT_CLUSTER_SIZE;
-    int prealloc = 0;
+    PreallocMode prealloc = PREALLOC_MODE_OFF;
     int version = 3;
     Error *local_err = NULL;
     int ret;
@@ -1652,9 +1652,9 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
             }
         } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
             if (!options->value.s || !strcmp(options->value.s, "off")) {
-                prealloc = 0;
+                prealloc = PREALLOC_MODE_OFF;
             } else if (!strcmp(options->value.s, "metadata")) {
-                prealloc = 1;
+                prealloc = PREALLOC_MODE_METADATA;
             } else {
                 error_setg(errp, "Invalid preallocation mode: '%s'",
                            options->value.s);
diff --git a/qapi-schema.json b/qapi-schema.json
index b68cd44..e798a5c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4686,3 +4686,17 @@
               'btn'     : 'InputBtnEvent',
               'rel'     : 'InputMoveEvent',
               'abs'     : 'InputMoveEvent' } }
+
+##
+# @PreallocMode
+#
+# Preallocation mode of QEMU image file
+#
+# @off: no preallocation
+# @metadata: preallocate only for metadata
+# @full: preallocate all data, including metadata
+#
+# Since 2.0
+##
+{ 'enum': 'PreallocMode',
+  'data': [ 'off', 'metadata', 'full' ] }
-- 
1.8.5.2.229.g4448466

[Qemu-devel] [PATCH v7 for 2.0 2/4] raw, qcow2: don't convert file size to sector size

[Hu Tao]

and avoid converting it back later.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 block/qcow2.c     | 8 ++++----
 block/raw-posix.c | 4 ++--
 block/raw-win32.c | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 154c40c..33ecb8a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1575,7 +1575,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     }
 
     /* Okay, now that we have a valid image, let's give it the right size */
-    ret = bdrv_truncate(bs, total_size * BDRV_SECTOR_SIZE);
+    ret = bdrv_truncate(bs, total_size);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not resize image");
         goto out;
@@ -1628,7 +1628,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
 {
     const char *backing_file = NULL;
     const char *backing_fmt = NULL;
-    uint64_t sectors = 0;
+    uint64_t size = 0;
     int flags = 0;
     size_t cluster_size = DEFAULT_CLUSTER_SIZE;
     PreallocMode prealloc = PREALLOC_MODE_OFF;
@@ -1639,7 +1639,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
     /* Read out options */
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            sectors = options->value.n / 512;
+            size = options->value.n & BDRV_SECTOR_MASK;
         } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
             backing_file = options->value.s;
         } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
@@ -1690,7 +1690,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
         return -EINVAL;
     }
 
-    ret = qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
+    ret = qcow2_create2(filename, size, backing_file, backing_fmt, flags,
                         cluster_size, prealloc, options, version, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 1688e16..a363b71 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1246,7 +1246,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     /* Read out options */
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            total_size = options->value.n / BDRV_SECTOR_SIZE;
+            total_size = options->value.n & BDRV_SECTOR_MASK;
         }
         options++;
     }
@@ -1257,7 +1257,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
         result = -errno;
         error_setg_errno(errp, -result, "Could not create file");
     } else {
-        if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
+        if (ftruncate(fd, total_size) != 0) {
             result = -errno;
             error_setg_errno(errp, -result, "Could not resize file");
         }
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 48cb2c2..9fe60f3 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -486,7 +486,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     /* Read out options */
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            total_size = options->value.n / 512;
+            total_size = options->value.n & BDRV_SECTOR_MASK;
         }
         options++;
     }
@@ -498,7 +498,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
         return -EIO;
     }
     set_sparse(fd);
-    ftruncate(fd, total_size * 512);
+    ftruncate(fd, total_size);
     qemu_close(fd);
     return 0;
 }
-- 
1.8.5.2.229.g4448466

[Qemu-devel] [PATCH v7 for 2.0 3/4] raw-posix: Add full image preallocation option

[Hu Tao]

This patch adds a new option preallocation for raw format, and implements
full preallocation.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 block/raw-posix.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index a363b71..e004ff8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1240,6 +1240,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     int fd;
     int result = 0;
     int64_t total_size = 0;
+    PreallocMode prealloc = PREALLOC_MODE_OFF;
 
     strstart(filename, "file:", &filename);
 
@@ -1247,6 +1248,16 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
             total_size = options->value.n & BDRV_SECTOR_MASK;
+        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
+            if (!options->value.s || !strcmp(options->value.s, "off")) {
+                prealloc = PREALLOC_MODE_OFF;
+            } else if (!strcmp(options->value.s, "full")) {
+                prealloc = PREALLOC_MODE_FULL;
+            } else {
+                error_setg(errp, "Invalid preallocation mode: '%s'",
+                           options->value.s);
+                return -EINVAL;
+            }
         }
         options++;
     }
@@ -1256,16 +1267,27 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     if (fd < 0) {
         result = -errno;
         error_setg_errno(errp, -result, "Could not create file");
-    } else {
-        if (ftruncate(fd, total_size) != 0) {
-            result = -errno;
-            error_setg_errno(errp, -result, "Could not resize file");
-        }
-        if (qemu_close(fd) != 0) {
-            result = -errno;
-            error_setg_errno(errp, -result, "Could not close the new file");
+        goto out;
+    }
+    if (ftruncate(fd, total_size) != 0) {
+        result = -errno;
+        error_setg_errno(errp, -result, "Could not resize file");
+        goto out_close;
+    }
+    if (prealloc == PREALLOC_MODE_FULL) {
+        /* posix_fallocate() doesn't set errno. */
+        result = -posix_fallocate(fd, 0, total_size);
+        if (result != 0) {
+            error_setg_errno(errp, -result,
+                             "Could not preallocate data for the new file");
         }
     }
+out_close:
+    if (qemu_close(fd) != 0) {
+        result = -errno;
+        error_setg_errno(errp, -result, "Could not close the new file");
+    }
+out:
     return result;
 }
 
@@ -1416,6 +1438,11 @@ static QEMUOptionParameter raw_create_options[] = {
         .type = OPT_SIZE,
         .help = "Virtual disk size"
     },
+    {
+        .name = BLOCK_OPT_PREALLOC,
+        .type = OPT_STRING,
+        .help = "Preallocation mode (allowed values: off, full)"
+    },
     { NULL }
 };
 
-- 
1.8.5.2.229.g4448466

[Qemu-devel] [PATCH v7 for 2.0 4/4] qcow2: Add full image preallocation option

[Hu Tao]

This adds a preallocation=full mode to qcow2 image creation, which
creates a non-sparse image file.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 block/qcow2.c              | 75 ++++++++++++++++++++++++++++++++++++++++++++--
 tests/qemu-iotests/082.out | 54 ++++++++++++++++-----------------
 2 files changed, 99 insertions(+), 30 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 33ecb8a..c4bb9f5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1459,6 +1459,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
                          QEMUOptionParameter *options, int version,
                          Error **errp)
 {
+    QEMUOptionParameter *alloc_options = NULL;
     /* Calculate cluster_bits */
     int cluster_bits;
     cluster_bits = ffs(cluster_size) - 1;
@@ -1488,10 +1489,74 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     Error *local_err = NULL;
     int ret;
 
+    if (prealloc == PREALLOC_MODE_FULL) {
+        int64_t meta_size = 0;
+        unsigned nreftablee, nrefblocke, nl1e, nl2e;
+        BlockDriver *drv;
+
+        total_size = align_offset(total_size, cluster_size);
+
+        drv = bdrv_find_protocol(filename, true);
+        if (drv == NULL) {
+            error_setg(errp, "Could not find protocol for file '%s'", filename);
+            return -ENOENT;
+        }
+
+        alloc_options = append_option_parameters(alloc_options,
+                                                 drv->create_options);
+        alloc_options = append_option_parameters(alloc_options, options);
+
+        /* header: 1 cluster */
+        meta_size += cluster_size;
+
+        /* total size of L2 tables */
+        nl2e = total_size / cluster_size;
+        nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
+        meta_size += nl2e * sizeof(uint64_t);
+
+        /* total size of L1 tables */
+        nl1e = nl2e * sizeof(uint64_t) / cluster_size;
+        nl1e = align_offset(nl1e, cluster_size / sizeof(uint64_t));
+        meta_size += nl1e * sizeof(uint64_t);
+
+        /* total size of refcount blocks
+         *
+         * note: every host cluster is reference-counted, including metadata
+         * (even refcount blocks are recursively included).
+         * Let:
+         *   a = total_size (this is the guest disk size)
+         *   m = meta size not including refcount blocks and refcount tables
+         *   c = cluster size
+         *   y1 = number of refcount blocks entries
+         *   y2 = meta size including everything
+         * then,
+         *   y1 = (y2 + a)/c
+         *   y2 = y1 * sizeof(u16) + y1 * sizeof(u16) * sizeof(u64) / c + m
+         * we can get y1:
+         *   y1 = (a + m) / (c - sizeof(u16) - sizeof(u16) * sizeof(u64) / c)
+         */
+        nrefblocke = (total_size + meta_size + cluster_size) /
+            (cluster_size - sizeof(uint16_t) -
+             1.0 * sizeof(uint16_t) * sizeof(uint64_t) / cluster_size);
+        nrefblocke = align_offset(nrefblocke, cluster_size / sizeof(uint16_t));
+        meta_size += nrefblocke * sizeof(uint16_t);
+
+        /* total size of refcount tables */
+        nreftablee = nrefblocke * sizeof(uint16_t) / cluster_size;
+        nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
+        meta_size += nreftablee * sizeof(uint64_t);
+
+        set_option_parameter_int(alloc_options, BLOCK_OPT_SIZE,
+                                 total_size + meta_size);
+        set_option_parameter(alloc_options, BLOCK_OPT_PREALLOC, "full");
+
+        options = alloc_options;
+    }
+
     ret = bdrv_create_file(filename, options, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
-        return ret;
+        goto out_options;
     }
 
     bs = NULL;
@@ -1499,7 +1564,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
                     NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
-        return ret;
+        goto out_options;
     }
 
     /* Write the header */
@@ -1620,6 +1685,8 @@ out:
     if (bs) {
         bdrv_unref(bs);
     }
+out_options:
+    free_option_parameters(alloc_options);
     return ret;
 }
 
@@ -1655,6 +1722,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
                 prealloc = PREALLOC_MODE_OFF;
             } else if (!strcmp(options->value.s, "metadata")) {
                 prealloc = PREALLOC_MODE_METADATA;
+            } else if (!strcmp(options->value.s, "full")) {
+                prealloc = PREALLOC_MODE_FULL;
             } else {
                 error_setg(errp, "Invalid preallocation mode: '%s'",
                            options->value.s);
@@ -2220,7 +2289,7 @@ static QEMUOptionParameter qcow2_create_options[] = {
     {
         .name = BLOCK_OPT_PREALLOC,
         .type = OPT_STRING,
-        .help = "Preallocation mode (allowed values: off, metadata)"
+        .help = "Preallocation mode (allowed values: off, metadata, full)"
     },
     {
         .name = BLOCK_OPT_LAZY_REFCOUNTS,
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 28309a0..5689802 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -64,7 +64,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o ? TEST_DIR/t.qcow2 128M
@@ -75,7 +75,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o cluster_size=4k,help TEST_DIR/t.qcow2 128M
@@ -86,7 +86,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o cluster_size=4k,? TEST_DIR/t.qcow2 128M
@@ -97,7 +97,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o help,cluster_size=4k TEST_DIR/t.qcow2 128M
@@ -108,7 +108,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o ?,cluster_size=4k TEST_DIR/t.qcow2 128M
@@ -119,7 +119,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o cluster_size=4k -o help TEST_DIR/t.qcow2 128M
@@ -130,7 +130,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o cluster_size=4k -o ? TEST_DIR/t.qcow2 128M
@@ -141,7 +141,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2 128M
@@ -167,7 +167,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -o help
@@ -245,7 +245,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o ? TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -256,7 +256,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o cluster_size=4k,help TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -267,7 +267,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o cluster_size=4k,? TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -278,7 +278,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o help,cluster_size=4k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -289,7 +289,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o ?,cluster_size=4k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -300,7 +300,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o cluster_size=4k -o help TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -311,7 +311,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o cluster_size=4k -o ? TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -322,7 +322,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -O qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -348,7 +348,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -o help
@@ -415,7 +415,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o ? TEST_DIR/t.qcow2
@@ -426,7 +426,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o cluster_size=4k,help TEST_DIR/t.qcow2
@@ -437,7 +437,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o cluster_size=4k,? TEST_DIR/t.qcow2
@@ -448,7 +448,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o help,cluster_size=4k TEST_DIR/t.qcow2
@@ -459,7 +459,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o ?,cluster_size=4k TEST_DIR/t.qcow2
@@ -470,7 +470,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o cluster_size=4k -o help TEST_DIR/t.qcow2
@@ -481,7 +481,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o cluster_size=4k -o ? TEST_DIR/t.qcow2
@@ -492,7 +492,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2
@@ -520,7 +520,7 @@ backing_file     File name of a base image
 backing_fmt      Image format of the base image
 encryption       Encrypt the image
 cluster_size     qcow2 cluster size
-preallocation    Preallocation mode (allowed values: off, metadata)
+preallocation    Preallocation mode (allowed values: off, metadata, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -o help
-- 
1.8.5.2.229.g4448466

Re: [Qemu-devel] [PATCH v7 for 2.0 1/4] qapi: introduce PreallocMode and a new PreallocMode full.

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 714 bytes --]

On 03/17/2014 12:53 AM, Hu Tao wrote:
> This patch prepares for the subsequent patches.
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  block/qcow2.c    |  8 ++++----
>  qapi-schema.json | 14 ++++++++++++++
>  2 files changed, 18 insertions(+), 4 deletions(-)

I'll leave it up to maintainers on whether we are still in time for 2.0
(the feature has been present on list prior to freeze - but at this
point it is a feature addition rather than a bug fix), but assuming the
answer is yes:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v7 for 2.0 1/4] qapi: introduce PreallocMode and a new PreallocMode full.

[Hu Tao]

On Mon, Mar 17, 2014 at 08:58:48AM -0600, Eric Blake wrote:
> On 03/17/2014 12:53 AM, Hu Tao wrote:
> > This patch prepares for the subsequent patches.
> > 
> > Reviewed-by: Fam Zheng <famz@redhat.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  block/qcow2.c    |  8 ++++----
> >  qapi-schema.json | 14 ++++++++++++++
> >  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> I'll leave it up to maintainers on whether we are still in time for 2.0
> (the feature has been present on list prior to freeze - but at this
> point it is a feature addition rather than a bug fix), but assuming the

I'm sorry to post the patchset late after freeze. Stefan, can we have
any chance to get it merged for 2.0?


> answer is yes:
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

Re: [Qemu-devel] [PATCH v7 for 2.0 3/4] raw-posix: Add full image preallocation option

[Kevin Wolf]

Am 17.03.2014 um 07:53 hat Hu Tao geschrieben:
> This patch adds a new option preallocation for raw format, and implements
> full preallocation.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  block/raw-posix.c | 43 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index a363b71..e004ff8 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1240,6 +1240,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>      int fd;
>      int result = 0;
>      int64_t total_size = 0;
> +    PreallocMode prealloc = PREALLOC_MODE_OFF;
>  
>      strstart(filename, "file:", &filename);
>  
> @@ -1247,6 +1248,16 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>      while (options && options->name) {
>          if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>              total_size = options->value.n & BDRV_SECTOR_MASK;
> +        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> +            if (!options->value.s || !strcmp(options->value.s, "off")) {
> +                prealloc = PREALLOC_MODE_OFF;
> +            } else if (!strcmp(options->value.s, "full")) {
> +                prealloc = PREALLOC_MODE_FULL;
> +            } else {
> +                error_setg(errp, "Invalid preallocation mode: '%s'",
> +                           options->value.s);
> +                return -EINVAL;
> +            }
>          }
>          options++;
>      }
> @@ -1256,16 +1267,27 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>      if (fd < 0) {
>          result = -errno;
>          error_setg_errno(errp, -result, "Could not create file");
> -    } else {
> -        if (ftruncate(fd, total_size) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not resize file");
> -        }
> -        if (qemu_close(fd) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not close the new file");
> +        goto out;
> +    }
> +    if (ftruncate(fd, total_size) != 0) {
> +        result = -errno;
> +        error_setg_errno(errp, -result, "Could not resize file");
> +        goto out_close;
> +    }
> +    if (prealloc == PREALLOC_MODE_FULL) {
> +        /* posix_fallocate() doesn't set errno. */
> +        result = -posix_fallocate(fd, 0, total_size);
> +        if (result != 0) {
> +            error_setg_errno(errp, -result,
> +                             "Could not preallocate data for the new file");
>          }

Can you please make that PREALLOC_MODE_METADATA?

You're really only preallocating metadata on a filesystem level here. If
the backing block device were thin provisioned, we wouldn't make any
attempt to preallocate that (e.g. with a loop writing zeroes or
bdrv_write_zeroes() without BDRV_REQ_MAY_UNMAP), so I'd hardly call it
"full" preallocation.

Kevin

Re: [Qemu-devel] [PATCH v7 for 2.0 4/4] qcow2: Add full image preallocation option

[Kevin Wolf]

Am 17.03.2014 um 07:53 hat Hu Tao geschrieben:
> This adds a preallocation=full mode to qcow2 image creation, which
> creates a non-sparse image file.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>

I see that you changed the implementation from bdrv_preallocate() to
bdrv_create() options in v4 after Stefan had asked whether
bdrv_preallocate() was really necessary or just complicating things.

In hindsight, do you really think the new code is simpler? I certainly
think this one is rather adventurous and the formulas to precalculate
the image file size are easy to get wrong.

Kevin

Re: [Qemu-devel] [PATCH v7 for 2.0 3/4] raw-posix: Add full image preallocation option

[hutao]

On Thu, Mar 20, 2014 at 02:22:19PM +0100, Kevin Wolf wrote:
> Am 17.03.2014 um 07:53 hat Hu Tao geschrieben:
> > This patch adds a new option preallocation for raw format, and implements
> > full preallocation.
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  block/raw-posix.c | 43 +++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 35 insertions(+), 8 deletions(-)
> > 
> > diff --git a/block/raw-posix.c b/block/raw-posix.c
> > index a363b71..e004ff8 100644
> > --- a/block/raw-posix.c
> > +++ b/block/raw-posix.c
> > @@ -1240,6 +1240,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> >      int fd;
> >      int result = 0;
> >      int64_t total_size = 0;
> > +    PreallocMode prealloc = PREALLOC_MODE_OFF;
> >  
> >      strstart(filename, "file:", &filename);
> >  
> > @@ -1247,6 +1248,16 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> >      while (options && options->name) {
> >          if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> >              total_size = options->value.n & BDRV_SECTOR_MASK;
> > +        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> > +            if (!options->value.s || !strcmp(options->value.s, "off")) {
> > +                prealloc = PREALLOC_MODE_OFF;
> > +            } else if (!strcmp(options->value.s, "full")) {
> > +                prealloc = PREALLOC_MODE_FULL;
> > +            } else {
> > +                error_setg(errp, "Invalid preallocation mode: '%s'",
> > +                           options->value.s);
> > +                return -EINVAL;
> > +            }
> >          }
> >          options++;
> >      }
> > @@ -1256,16 +1267,27 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> >      if (fd < 0) {
> >          result = -errno;
> >          error_setg_errno(errp, -result, "Could not create file");
> > -    } else {
> > -        if (ftruncate(fd, total_size) != 0) {
> > -            result = -errno;
> > -            error_setg_errno(errp, -result, "Could not resize file");
> > -        }
> > -        if (qemu_close(fd) != 0) {
> > -            result = -errno;
> > -            error_setg_errno(errp, -result, "Could not close the new file");
> > +        goto out;
> > +    }
> > +    if (ftruncate(fd, total_size) != 0) {
> > +        result = -errno;
> > +        error_setg_errno(errp, -result, "Could not resize file");
> > +        goto out_close;
> > +    }
> > +    if (prealloc == PREALLOC_MODE_FULL) {
> > +        /* posix_fallocate() doesn't set errno. */
> > +        result = -posix_fallocate(fd, 0, total_size);
> > +        if (result != 0) {
> > +            error_setg_errno(errp, -result,
> > +                             "Could not preallocate data for the new file");
> >          }
> 
> Can you please make that PREALLOC_MODE_METADATA?
> 
> You're really only preallocating metadata on a filesystem level here. If
> the backing block device were thin provisioned, we wouldn't make any

Thanks for reminding me! I was not aware of thin privisioning case.

> attempt to preallocate that (e.g. with a loop writing zeroes or
> bdrv_write_zeroes() without BDRV_REQ_MAY_UNMAP), so I'd hardly call it
> "full" preallocation.

Just to confirm it, we can only do full preallocation by writing zeros
to the backing device? If yes, I agree that this patch doesn't do full
preallocation.

In the other hand, this patch changes the behaviour of qcow2 metadata
preallocation, I think it is not reasonable to still call it
PREALLOC_MODE_METADATA. What's your opinion?

Regards,
Hu Tao

Re: [Qemu-devel] [PATCH v7 for 2.0 4/4] qcow2: Add full image preallocation option

[Hu Tao]

Hi,

On Thu, Mar 20, 2014 at 02:38:32PM +0100, Kevin Wolf wrote:
> Am 17.03.2014 um 07:53 hat Hu Tao geschrieben:
> > This adds a preallocation=full mode to qcow2 image creation, which
> > creates a non-sparse image file.
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> 
> I see that you changed the implementation from bdrv_preallocate() to
> bdrv_create() options in v4 after Stefan had asked whether
> bdrv_preallocate() was really necessary or just complicating things.
> 
> In hindsight, do you really think the new code is simpler? I certainly
> think this one is rather adventurous and the formulas to precalculate
> the image file size are easy to get wrong.

When I tried to implement the bdrv_preallocate() way, I found that it
was inevitable to precalculate the size. For example, in
qcow2::bdrv_preallcate() we have to call low level
raw::bdrv_preallocate() first, but we have to pass in the actual image
file size which we have to precalculate before qcow2::bdrv_preallocate()
can create any metadata.

Another problem I don't understand about bdrv_preallocate() is that how
it ensures existing data not being corrupted for every call to
bdrv_preallocate().

Regards,
Hu Tao