| Message ID | 25565d3a53f29829dc2d97480cb19ba34be49895.1388112645.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Dec 27, 2013 at 11:05:54AM +0800, Hu Tao wrote: This approach seems okay but the calculation isn't quite right yet. On Windows an error would be raised since we don't have preallocate=full support. That's okay. > @@ -1477,16 +1478,53 @@ 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 nrefe, nl2e; > + BlockDriver *drv; > + > + 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 refblocks */ > + nrefe = (total_size / cluster_size); > + nrefe = align_offset(nrefe, cluster_size / sizeof(uint16_t)); > + meta_size += nrefe * sizeof(uint16_t); Every host cluster is reference-counted, including metadata (even refcount blocks are recursively included). This calculation is wrong because it only considers data clusters. > + /* total size of reftables */ > + meta_size += nrefe * sizeof(uint16_t) * sizeof(uint16_t) / cluster_size; I don't understand this calculation. The refcount table consists of contiguous clusters where each element is a 64-bit offset to a refcount block. > + /* 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 */ > + meta_size += nl2e * sizeof(uint64_t) * sizeof(uint64_t) / cluster_size; Another strange calculation. The L1 table consists of contiguous clusters where each element is a 64-bit offset to an L1 table.
Stefan, On Fri, Jan 17, 2014 at 04:48:16PM +0800, Stefan Hajnoczi wrote: > On Fri, Dec 27, 2013 at 11:05:54AM +0800, Hu Tao wrote: > > This approach seems okay but the calculation isn't quite right yet. > > On Windows an error would be raised since we don't have preallocate=full > support. That's okay. > > > @@ -1477,16 +1478,53 @@ 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 nrefe, nl2e; > > + BlockDriver *drv; > > + > > + 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 refblocks */ > > + nrefe = (total_size / cluster_size); > > + nrefe = align_offset(nrefe, cluster_size / sizeof(uint16_t)); > > + meta_size += nrefe * sizeof(uint16_t); > > Every host cluster is reference-counted, including metadata (even > refcount blocks are recursively included). This calculation is wrong > because it only considers data clusters. Right. I'll fix this. > > > + /* total size of reftables */ > > + meta_size += nrefe * sizeof(uint16_t) * sizeof(uint16_t) / cluster_size; > > I don't understand this calculation. The refcount table consists of > contiguous clusters where each element is a 64-bit offset to a refcount > block. > > > + /* 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 */ > > + meta_size += nl2e * sizeof(uint64_t) * sizeof(uint64_t) / cluster_size; > > Another strange calculation. The L1 table consists of contiguous > clusters where each element is a 64-bit offset to an L1 table. I guest you mean "...offset to an L2 table" here. Given the number of total L2 table entries nl2e, the number of L2 tables is: nl2table = nl2e * sizeof(l2e) / cluster_size because each L1 table entry points to a L2 table, so this is also the number of L1 table entries. So the total size of L1 tables is: l1table_size = nl2table * sizeof(l1e) = nl2e * sizeof(l2e) * sizeof(l1e) / cluster_size = nl2e * sizeof(uint64_t) * sizeof(uint64_t) / cluster_size I still can't see what's wrong here. But you're welcome to fix me. The caculation of reftable size above is wrong because of the two problem you've pointed out. Thanks!
On Mon, Jan 20, 2014 at 10:16:23AM +0800, Hu Tao wrote: > Stefan, > > On Fri, Jan 17, 2014 at 04:48:16PM +0800, Stefan Hajnoczi wrote: > > On Fri, Dec 27, 2013 at 11:05:54AM +0800, Hu Tao wrote: > > > > This approach seems okay but the calculation isn't quite right yet. > > > > On Windows an error would be raised since we don't have preallocate=full > > support. That's okay. > > > > > @@ -1477,16 +1478,53 @@ 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 nrefe, nl2e; > > > + BlockDriver *drv; > > > + > > > + 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 refblocks */ > > > + nrefe = (total_size / cluster_size); > > > + nrefe = align_offset(nrefe, cluster_size / sizeof(uint16_t)); > > > + meta_size += nrefe * sizeof(uint16_t); > > > > Every host cluster is reference-counted, including metadata (even > > refcount blocks are recursively included). This calculation is wrong > > because it only considers data clusters. > > Right. I'll fix this. > > > > > > + /* total size of reftables */ > > > + meta_size += nrefe * sizeof(uint16_t) * sizeof(uint16_t) / cluster_size; > > > > I don't understand this calculation. The refcount table consists of > > contiguous clusters where each element is a 64-bit offset to a refcount > > block. > > > > > + /* 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 */ > > > + meta_size += nl2e * sizeof(uint64_t) * sizeof(uint64_t) / cluster_size; > > > > Another strange calculation. The L1 table consists of contiguous > > clusters where each element is a 64-bit offset to an L1 table. > > I guest you mean "...offset to an L2 table" here. > > Given the number of total L2 table entries nl2e, the number of L2 tables > is: > > nl2table = nl2e * sizeof(l2e) / cluster_size > > because each L1 table entry points to a L2 table, so this is also the > number of L1 table entries. So the total size of L1 tables is: > > l1table_size = nl2table * sizeof(l1e) > = nl2e * sizeof(l2e) * sizeof(l1e) / cluster_size > = nl2e * sizeof(uint64_t) * sizeof(uint64_t) / cluster_size > > I still can't see what's wrong here. But you're welcome to fix me. Hi, Any comments?
diff --git a/block/qcow2.c b/block/qcow2.c index f0a8d87..f601089 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1448,6 +1448,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; @@ -1477,16 +1478,53 @@ 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 nrefe, nl2e; + BlockDriver *drv; + + 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 refblocks */ + nrefe = (total_size / cluster_size); + nrefe = align_offset(nrefe, cluster_size / sizeof(uint16_t)); + meta_size += nrefe * sizeof(uint16_t); + /* total size of reftables */ + meta_size += nrefe * sizeof(uint16_t) * sizeof(uint16_t) / 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 */ + meta_size += nl2e * sizeof(uint64_t) * sizeof(uint64_t) / cluster_size; + + 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; } ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err); if (ret < 0) { error_propagate(errp, local_err); - return ret; + goto out_options; } /* Write the header */ @@ -1603,6 +1641,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, ret = 0; out: bdrv_unref(bs); +out_options: + free_option_parameters(alloc_options); return ret; } @@ -1638,6 +1678,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); @@ -2223,7 +2265,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,
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 | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-)