qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation opt


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation option
Date: Fri, 20 Jun 2014 20:37:51 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0

On 20.06.2014 10:25, Hu Tao wrote:
On Sat, Jun 14, 2014 at 10:37:33PM +0200, Max Reitz wrote:
On 12.06.2014 05:54, Hu Tao wrote:
This adds a preallocation=full mode to qcow2 image creation, which
creates a non-sparse image file.

Signed-off-by: Hu Tao <address@hidden>
---
  block/qcow2.c              | 90 ++++++++++++++++++++++++++++++++++++++++------
  tests/qemu-iotests/082.out | 54 ++++++++++++++--------------
  2 files changed, 107 insertions(+), 37 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 2e0b83c..8c87c1a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -30,6 +30,7 @@
  #include "qemu/error-report.h"
  #include "qapi/qmp/qerror.h"
  #include "qapi/qmp/qbool.h"
+#include "qapi/util.h"
  #include "trace.h"
  /*
@@ -1597,6 +1598,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;
@@ -1626,10 +1628,78 @@ static int qcow2_create2(const char *filename, int64_t 
total_size,
      Error *local_err = NULL;
      int ret;
+    if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_METADATA) {
+        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);
Hm, you could use the function minimal_blob_size() from my
"qemu-img: Implement commit like QMP" series. It calculates the
minimal size required for an L1 table, the refcount table and all
refcount blocks for an image with a given number of guest sectors
and a given number of "overhead", which are clusters that should be
covered by the refcount structures in addition to
L1+Reftable+Refblocks. To use it here, you would have to use it
probably like this:

meta_size = (1 + minimal_blob_size(total_size / BDRV_SECTOR_SIZE,
cluster_bits, cluster_bits - BDRV_SECTOR_BITS, 1 + l2_clusters +
(total_size >> cluster_bits)) + l2_clusters) << cluster_bits;

The first parameter is the number of guest sectors; the second is
cluster_bits; the third is parameter x, so that (1 << x) == number
of sectors per cluster; and the fourth is the number of clusters
which should be covered by the refcount structures in addition to
L1+Reftable+Refblocks. These are the image header (1 cluster), the
L2 tables (l2_clusters, which you'd have to calculate yourself (as
you are already doing in this patch)) and the clusters for guest
data (total_size >> cluster_bits).

The function then returns the number of clusters required from
L1+Reftable+Refblocks to which the image header cluster and the L2
clusters have to be added to get the full metadata size.

I'm suggesting you use this function (even though it's not merged
yet, but fully reviewed), because I guess it's always likely to have
some kind of small mistake in the size calculation for the refcount
structures; and therefore it's better to have a single place where
these mistakes can occur.
Thanks! I used minimal_blob_size() from your patch. But tests show that
the calculated meta size (and the image file size) is larger than the
number calculated from my patch and without patch(file size).

test command:
qemu-img create -f qcow2 -o preallocation=metadata delme.img 8588490000

result:

without patch:

Formatting 'delme.img', fmt=qcow2 size=8588490000 encryption=off 
cluster_size=65536 preallocation='metadata' lazy_refcounts=off
-rw-r--r-- 1 root root 8590065664 Jun 20 15:02 delme.img


my patch:

Formatting 'delme.img', fmt=qcow2 size=8588490000 encryption=off 
cluster_size=65536 preallocation='metadata' lazy_refcounts=off
meta_size: 1507328, 1507328
total_size: 8588492800, 8588492800
-rw-r--r-- 1 root root 8590065664 Jun 20 15:01 delme.img


using minimal_blob_size:

Formatting 'delme.img', fmt=qcow2 size=8588490000 encryption=off 
cluster_size=65536 preallocation='metadata' lazy_refcounts=off
-rw-r--r-- 1 root root 8590131200 Jun 20 15:03 delme.img

Well, then I'll need to fix minimal_blob_size() anyway. ;-)

But, well, it's just one cluster too much, so I guess it's still fine (I mostly made sure to allocate enough space in any case without using (potentially imprecise) floating point operations; so it may indeed sometimes require too much space, but as long as it's rather insignificant, I guess we can live with it).

I need to review minimal_blob_size() further.

In http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg04820.html I attached a document which explains how I ended up with my formula, maybe it'll prove helpful to you.

Max



As for your own calculation code: You should round all structure
sizes separately to full clusters (the L1 table, the L2 tables, the
refcount table and the refcount blocks). Especially this makes the
refcount structure size so "hard" to calculate (or at least
error-prone), as this has to be considered for the recursive
calculation you demonstrated in your comment.

+
+        set_option_parameter_int(alloc_options, BLOCK_OPT_SIZE,
+                                 total_size + meta_size);
Wouldn't it suffice for preallocation=metadata to only allocate meta_size?

If we allocate total_size + meta_size for metadata preallocation as
well, both metadata and full preallocation are the same (as far as I
can see). But if we only allocate meta_size, we have to rework
preallocate(): This function really "allocates" the data clusters
from qcow2's point of view, which means they are holes in the raw
file whereas all metadata should ideally be contained in the blob
sized meta_size at the start of the file and the rest should be
completely unused.

With the current preallocate(), we either have to allocate
everything (including data), or can't guarantee (and it will in fact
never happen) that all metadata clusters fit into the meta_size
sized blob, because it creates a mixture of interleaved data and
metadata. The only way I can think of to really fix this is to
rework preallocate() so that it doesn't use the regular
qcow2_alloc_cluster_offset() function anymore but fills all metadata
tables by itself (which should not be too hard for a simple linear
preallocation of everything) and places all data clusters behind the
metadata blob.

+        if (prealloc == PREALLOC_MODE_FULL) {
+            set_option_parameter(alloc_options, BLOCK_OPT_PREALLOC, "full");
+        } else if (prealloc == PREALLOC_MODE_METADATA) {
+            set_option_parameter(alloc_options, BLOCK_OPT_PREALLOC, 
"metadata");
+        }
How about set_option_parameter(alloc_options, BLOCK_OPT_PREALLOC,
PreallocMode_lookup[prealloc]); ?

Max

+
+        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;
@@ -1637,7 +1707,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 */
@@ -1759,6 +1829,8 @@ out:
      if (bs) {
          bdrv_unref(bs);
      }
+out_options:
+    free_option_parameters(alloc_options);
      return ret;
  }
@@ -1790,13 +1862,11 @@ static int qcow2_create(const char *filename, 
QEMUOptionParameter *options,
                  cluster_size = options->value.n;
              }
          } 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, "metadata")) {
-                prealloc = PREALLOC_MODE_METADATA;
-            } else {
-                error_setg(errp, "Invalid preallocation mode: '%s'",
-                           options->value.s);
+            prealloc = qapi_enum_parse(PreallocMode_lookup, options->value.s,
+                                       PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
+                                       &local_err);
+            if (local_err) {
+                error_propagate(errp, local_err);
                  return -EINVAL;
              }
          } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) {
@@ -2359,7 +2429,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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]