* [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full
@ 2014-06-12 3:54 Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector Hu Tao
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Hu Tao @ 2014-06-12 3:54 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Max Reitz,
Stefan Hajnoczi, y-goto
The purpose of this series is to use posix_fallocate() when creating
img file to ensure there are disk space for it which is way fast than
acturally writing to disk. But this only works in file system level.
For cases like thin provisioning, an option full preallocation is
added to write zeros to storage to ensure disk space.
note: there are two false positives of checkpatch.pl to patch 01.
changes to v9:
- use ROUND_UP to do round up
- split the round up into its own patch and add test case
- new patch rename parse_enum_option to qapi_enum_parse and make it public
- reuse qapi_enum_parse
Hu Tao (6):
block: round up file size to nearest sector
raw, qcow2: don't convert file size to sector size
rename parse_enum_option to qapi_enum_parse and make it public
qapi: introduce PreallocMode and a new PreallocMode full.
raw-posix: Add full image preallocation option
qcow2: Add full image preallocation option
block/qcow2.c | 102 ++++++++++++++++++++++++++++++++++++++-------
block/raw-posix.c | 61 +++++++++++++++++++++++----
block/raw-win32.c | 4 +-
blockdev.c | 22 +---------
include/qapi/util.h | 17 ++++++++
qapi-schema.json | 14 +++++++
qapi/Makefile.objs | 2 +-
qapi/qapi-util.c | 32 ++++++++++++++
tests/qemu-iotests/082.out | 54 ++++++++++++------------
tests/qemu-iotests/093 | 64 ++++++++++++++++++++++++++++
tests/qemu-iotests/093.out | 14 +++++++
tests/qemu-iotests/group | 1 +
12 files changed, 313 insertions(+), 74 deletions(-)
create mode 100644 include/qapi/util.h
create mode 100644 qapi/qapi-util.c
create mode 100755 tests/qemu-iotests/093
create mode 100644 tests/qemu-iotests/093.out
--
1.9.3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector
2014-06-12 3:54 [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full Hu Tao
@ 2014-06-12 3:54 ` Hu Tao
2014-06-14 18:51 ` Max Reitz
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 2/6] raw, qcow2: don't convert file size to sector size Hu Tao
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Hu Tao @ 2014-06-12 3:54 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Max Reitz,
Stefan Hajnoczi, y-goto
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
block/qcow2.c | 2 +-
block/raw-posix.c | 2 +-
block/raw-win32.c | 2 +-
tests/qemu-iotests/093 | 64 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/093.out | 14 ++++++++++
tests/qemu-iotests/group | 1 +
6 files changed, 82 insertions(+), 3 deletions(-)
create mode 100755 tests/qemu-iotests/093
create mode 100644 tests/qemu-iotests/093.out
diff --git a/block/qcow2.c b/block/qcow2.c
index a54d2ba..75b28cd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1778,7 +1778,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;
+ sectors = DIV_ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
backing_file = options->value.s;
} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
diff --git a/block/raw-posix.c b/block/raw-posix.c
index c2b30be..c5b81e1 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1285,7 +1285,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 = DIV_ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
}
options++;
}
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 324e818..8f83fab 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -514,7 +514,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 = DIV_ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
}
options++;
}
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
new file mode 100755
index 0000000..02316d1
--- /dev/null
+++ b/tests/qemu-iotests/093
@@ -0,0 +1,64 @@
+#!/bin/bash
+#
+# Test qcow2 preallocation with different cluster_sizes
+#
+# Copyright (C) 2014 Fujitsu.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=hutao@cn.fujitsu.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+function test_qemu_img()
+{
+ echo qemu-img "$@" | _filter_testdir
+ $QEMU_IMG "$@" 2>&1 | _filter_testdir
+ echo
+}
+
+echo "=== Check qemu-img info output ==="
+echo
+image_sizes="1024 1234"
+
+for s in $image_sizes; do
+ _make_test_img $s
+ _img_info
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
new file mode 100644
index 0000000..766b512
--- /dev/null
+++ b/tests/qemu-iotests/093.out
@@ -0,0 +1,14 @@
+QA output created by 093
+=== Check qemu-img info output ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 1.0K (1024 bytes)
+cluster_size: 65536
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1234
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 1.5K (1536 bytes)
+cluster_size: 65536
+***done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0f07440..304b596 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -99,3 +99,4 @@
090 rw auto quick
091 rw auto
092 rw auto quick
+093 rw auto
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v10 2/6] raw, qcow2: don't convert file size to sector size
2014-06-12 3:54 [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector Hu Tao
@ 2014-06-12 3:54 ` Hu Tao
2014-06-14 19:00 ` Max Reitz
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public Hu Tao
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Hu Tao @ 2014-06-12 3:54 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Max Reitz,
Stefan Hajnoczi, y-goto
and avoid converting it back later. And round up file size to nearest
sector.
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 75b28cd..6732e7c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1714,7 +1714,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;
@@ -1767,7 +1767,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;
int prealloc = 0;
@@ -1778,7 +1778,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 = DIV_ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
+ size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
backing_file = options->value.s;
} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
@@ -1829,7 +1829,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 c5b81e1..35057f0 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1285,7 +1285,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 = DIV_ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
+ total_size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
}
options++;
}
@@ -1296,7 +1296,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 8f83fab..f681648 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -514,7 +514,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 = DIV_ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
+ total_size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
}
options++;
}
@@ -526,7 +526,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.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public
2014-06-12 3:54 [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 2/6] raw, qcow2: don't convert file size to sector size Hu Tao
@ 2014-06-12 3:54 ` Hu Tao
2014-06-14 19:07 ` Max Reitz
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full Hu Tao
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Hu Tao @ 2014-06-12 3:54 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Max Reitz,
Stefan Hajnoczi, y-goto
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Suggested-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 22 ++--------------------
include/qapi/util.h | 17 +++++++++++++++++
qapi/Makefile.objs | 2 +-
qapi/qapi-util.c | 32 ++++++++++++++++++++++++++++++++
4 files changed, 52 insertions(+), 21 deletions(-)
create mode 100644 include/qapi/util.h
create mode 100644 qapi/qapi-util.c
diff --git a/blockdev.c b/blockdev.c
index 4cbcc56..9adfdbb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -39,6 +39,7 @@
#include "qapi/qmp/types.h"
#include "qapi-visit.h"
#include "qapi/qmp-output-visitor.h"
+#include "qapi/util.h"
#include "sysemu/sysemu.h"
#include "block/block_int.h"
#include "qmp-commands.h"
@@ -287,25 +288,6 @@ static int parse_block_error_action(const char *buf, bool is_read, Error **errp)
}
}
-static inline int parse_enum_option(const char *lookup[], const char *buf,
- int max, int def, Error **errp)
-{
- int i;
-
- if (!buf) {
- return def;
- }
-
- for (i = 0; i < max; i++) {
- if (!strcmp(buf, lookup[i])) {
- return i;
- }
- }
-
- error_setg(errp, "invalid parameter value: %s", buf);
- return def;
-}
-
static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
{
if (throttle_conflicting(cfg)) {
@@ -472,7 +454,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
}
detect_zeroes =
- parse_enum_option(BlockdevDetectZeroesOptions_lookup,
+ qapi_enum_parse(BlockdevDetectZeroesOptions_lookup,
qemu_opt_get(opts, "detect-zeroes"),
BLOCKDEV_DETECT_ZEROES_OPTIONS_MAX,
BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
diff --git a/include/qapi/util.h b/include/qapi/util.h
new file mode 100644
index 0000000..de9238b
--- /dev/null
+++ b/include/qapi/util.h
@@ -0,0 +1,17 @@
+/*
+ * QAPI util functions
+ *
+ * Copyright Fujitsu, Inc. 2014
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QAPI_UTIL_H
+#define QAPI_UTIL_H
+
+int qapi_enum_parse(const char *lookup[], const char *buf,
+ int max, int def, Error **errp);
+
+#endif
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 1f9c973..97c98c5 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -2,4 +2,4 @@ util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
util-obj-y += string-input-visitor.o string-output-visitor.o
-util-obj-y += opts-visitor.o
+util-obj-y += opts-visitor.o qapi-util.o
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
new file mode 100644
index 0000000..3e4e07c
--- /dev/null
+++ b/qapi/qapi-util.c
@@ -0,0 +1,32 @@
+/*
+ * QAPI util functions
+ *
+ * Copyright Fujitsu, Inc. 2014
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qapi/util.h"
+
+int qapi_enum_parse(const char *lookup[], const char *buf,
+ int max, int def, Error **errp)
+{
+ int i;
+
+ if (!buf) {
+ return def;
+ }
+
+ for (i = 0; i < max; i++) {
+ if (!strcmp(buf, lookup[i])) {
+ return i;
+ }
+ }
+
+ error_setg(errp, "invalid parameter value: %s", buf);
+ return def;
+}
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full.
2014-06-12 3:54 [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full Hu Tao
` (2 preceding siblings ...)
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public Hu Tao
@ 2014-06-12 3:54 ` Hu Tao
2014-06-14 19:17 ` Max Reitz
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 6/6] qcow2: " Hu Tao
5 siblings, 1 reply; 19+ messages in thread
From: Hu Tao @ 2014-06-12 3:54 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Max Reitz,
Stefan Hajnoczi, y-goto
This patch prepares for the subsequent patches.
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@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 6732e7c..2e0b83c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1593,7 +1593,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)
{
@@ -1770,7 +1770,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
uint64_t size = 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;
@@ -1791,9 +1791,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 14b498b..f5ccb31 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3077,3 +3077,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.1
+##
+{ 'enum': 'PreallocMode',
+ 'data': [ 'off', 'metadata', 'full' ] }
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option
2014-06-12 3:54 [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full Hu Tao
` (3 preceding siblings ...)
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full Hu Tao
@ 2014-06-12 3:54 ` Hu Tao
2014-06-14 19:38 ` Max Reitz
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 6/6] qcow2: " Hu Tao
5 siblings, 1 reply; 19+ messages in thread
From: Hu Tao @ 2014-06-12 3:54 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Max Reitz,
Stefan Hajnoczi, y-goto
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 | 59 ++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 52 insertions(+), 7 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 35057f0..73b10cd 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -30,6 +30,7 @@
#include "block/thread-pool.h"
#include "qemu/iov.h"
#include "raw-aio.h"
+#include "qapi/util.h"
#if defined(__APPLE__) && (__MACH__)
#include <paths.h>
@@ -1279,6 +1280,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
int fd;
int result = 0;
int64_t total_size = 0;
+ PreallocMode prealloc = PREALLOC_MODE_OFF;
+ Error *error = NULL;
strstart(filename, "file:", &filename);
@@ -1286,6 +1289,14 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
while (options && options->name) {
if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
total_size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
+ } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
+ prealloc = qapi_enum_parse(PreallocMode_lookup, options->value.s,
+ PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
+ &error);
+ if (error) {
+ error_propagate(errp, error);
+ return -EINVAL;
+ }
}
options++;
}
@@ -1295,16 +1306,45 @@ 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");
+ 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_METADATA) {
+ /* 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");
}
- if (qemu_close(fd) != 0) {
- result = -errno;
- error_setg_errno(errp, -result, "Could not close the new file");
+ } else if (prealloc == PREALLOC_MODE_FULL) {
+ char *buf = g_malloc0(65536);
+ int64_t num = 0, left = total_size;
+
+ while (left > 0) {
+ num = MIN(left, 65536);
+ result = write(fd, buf, num);
+ if (result < 0) {
+ result = -errno;
+ error_setg_errno(errp, -result,
+ "Could not write to the new file");
+ g_free(buf);
+ goto out_close;
+ }
+ left -= num;
}
+ fsync(fd);
+ g_free(buf);
}
+out_close:
+ if (qemu_close(fd) != 0 && result == 0) {
+ result = -errno;
+ error_setg_errno(errp, -result, "Could not close the new file");
+ }
+out:
return result;
}
@@ -1479,6 +1519,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, metadata, full)"
+ },
{ NULL }
};
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation option
2014-06-12 3:54 [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full Hu Tao
` (4 preceding siblings ...)
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option Hu Tao
@ 2014-06-12 3:54 ` Hu Tao
2014-06-14 20:37 ` Max Reitz
5 siblings, 1 reply; 19+ messages in thread
From: Hu Tao @ 2014-06-12 3:54 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Max Reitz,
Stefan Hajnoczi, y-goto
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 | 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);
+
+ set_option_parameter_int(alloc_options, BLOCK_OPT_SIZE,
+ total_size + meta_size);
+ 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");
+ }
+
+ 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
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector Hu Tao
@ 2014-06-14 18:51 ` Max Reitz
0 siblings, 0 replies; 19+ messages in thread
From: Max Reitz @ 2014-06-14 18:51 UTC (permalink / raw)
To: Hu Tao, qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Stefan Hajnoczi, y-goto
On 12.06.2014 05:54, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> block/qcow2.c | 2 +-
> block/raw-posix.c | 2 +-
> block/raw-win32.c | 2 +-
> tests/qemu-iotests/093 | 64 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/093.out | 14 ++++++++++
> tests/qemu-iotests/group | 1 +
> 6 files changed, 82 insertions(+), 3 deletions(-)
> create mode 100755 tests/qemu-iotests/093
> create mode 100644 tests/qemu-iotests/093.out
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 2/6] raw, qcow2: don't convert file size to sector size
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 2/6] raw, qcow2: don't convert file size to sector size Hu Tao
@ 2014-06-14 19:00 ` Max Reitz
0 siblings, 0 replies; 19+ messages in thread
From: Max Reitz @ 2014-06-14 19:00 UTC (permalink / raw)
To: Hu Tao, qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Stefan Hajnoczi, y-goto
On 12.06.2014 05:54, Hu Tao wrote:
> and avoid converting it back later. And round up file size to nearest
> sector.
>
> 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 75b28cd..6732e7c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1714,7 +1714,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;
> @@ -1767,7 +1767,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;
> int prealloc = 0;
> @@ -1778,7 +1778,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 = DIV_ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
> + size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
I'm not even sure whether the ROUND_UP() is needed anymore, as qcow2
should only care about clusters. However, we still have that more or
less arbitrary "sector" unit in the block layer (although I think Markus
is trying to get rid of it), so it's probably for the better to align
the size here.
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public Hu Tao
@ 2014-06-14 19:07 ` Max Reitz
2014-06-17 2:36 ` Hu Tao
0 siblings, 1 reply; 19+ messages in thread
From: Max Reitz @ 2014-06-14 19:07 UTC (permalink / raw)
To: Hu Tao, qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Stefan Hajnoczi, y-goto
On 12.06.2014 05:54, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> ---
> blockdev.c | 22 ++--------------------
> include/qapi/util.h | 17 +++++++++++++++++
> qapi/Makefile.objs | 2 +-
> qapi/qapi-util.c | 32 ++++++++++++++++++++++++++++++++
> 4 files changed, 52 insertions(+), 21 deletions(-)
> create mode 100644 include/qapi/util.h
> create mode 100644 qapi/qapi-util.c
>
> diff --git a/blockdev.c b/blockdev.c
> index 4cbcc56..9adfdbb 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -39,6 +39,7 @@
> #include "qapi/qmp/types.h"
> #include "qapi-visit.h"
> #include "qapi/qmp-output-visitor.h"
> +#include "qapi/util.h"
> #include "sysemu/sysemu.h"
> #include "block/block_int.h"
> #include "qmp-commands.h"
> @@ -287,25 +288,6 @@ static int parse_block_error_action(const char *buf, bool is_read, Error **errp)
> }
> }
>
> -static inline int parse_enum_option(const char *lookup[], const char *buf,
> - int max, int def, Error **errp)
> -{
> - int i;
> -
> - if (!buf) {
> - return def;
> - }
> -
> - for (i = 0; i < max; i++) {
> - if (!strcmp(buf, lookup[i])) {
> - return i;
> - }
> - }
> -
> - error_setg(errp, "invalid parameter value: %s", buf);
> - return def;
> -}
> -
> static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
> {
> if (throttle_conflicting(cfg)) {
> @@ -472,7 +454,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
> }
>
> detect_zeroes =
> - parse_enum_option(BlockdevDetectZeroesOptions_lookup,
> + qapi_enum_parse(BlockdevDetectZeroesOptions_lookup,
> qemu_opt_get(opts, "detect-zeroes"),
> BLOCKDEV_DETECT_ZEROES_OPTIONS_MAX,
> BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
Please adapt the indentation of the other parameters.
Other than that:
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full.
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full Hu Tao
@ 2014-06-14 19:17 ` Max Reitz
2014-06-25 5:46 ` Hu Tao
0 siblings, 1 reply; 19+ messages in thread
From: Max Reitz @ 2014-06-14 19:17 UTC (permalink / raw)
To: Hu Tao, qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Stefan Hajnoczi, y-goto
On 12.06.2014 05:54, Hu Tao wrote:
> This patch prepares for the subsequent patches.
>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Eric Blake <eblake@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 6732e7c..2e0b83c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1593,7 +1593,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)
> {
> @@ -1770,7 +1770,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
> uint64_t size = 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;
> @@ -1791,9 +1791,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 14b498b..f5ccb31 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
As of commit 5db15096 (Benoît's "Extract QAPI block commands" series),
block-related QAPI definitions should go into qapi/block.json or
qapi/block-core.json. I think the following definition should now go
into qapi/block-core.json instead of qapi-schema.json.
Apart from that, this patch looks good.
Max
> @@ -3077,3 +3077,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.1
> +##
> +{ 'enum': 'PreallocMode',
> + 'data': [ 'off', 'metadata', 'full' ] }
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option Hu Tao
@ 2014-06-14 19:38 ` Max Reitz
2014-06-25 6:04 ` Hu Tao
0 siblings, 1 reply; 19+ messages in thread
From: Max Reitz @ 2014-06-14 19:38 UTC (permalink / raw)
To: Hu Tao, qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Stefan Hajnoczi, y-goto
On 12.06.2014 05:54, Hu Tao wrote:
> 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 | 59 ++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 35057f0..73b10cd 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -30,6 +30,7 @@
> #include "block/thread-pool.h"
> #include "qemu/iov.h"
> #include "raw-aio.h"
> +#include "qapi/util.h"
>
> #if defined(__APPLE__) && (__MACH__)
> #include <paths.h>
> @@ -1279,6 +1280,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> int fd;
> int result = 0;
> int64_t total_size = 0;
> + PreallocMode prealloc = PREALLOC_MODE_OFF;
> + Error *error = NULL;
>
> strstart(filename, "file:", &filename);
>
> @@ -1286,6 +1289,14 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> while (options && options->name) {
> if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> total_size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
> + } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> + prealloc = qapi_enum_parse(PreallocMode_lookup, options->value.s,
> + PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> + &error);
> + if (error) {
> + error_propagate(errp, error);
> + return -EINVAL;
Could be "result = -EINVAL; goto out;" instead, but definitely isn't
wrong this way.
> + }
> }
> options++;
> }
> @@ -1295,16 +1306,45 @@ 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");
> + 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_METADATA) {
> + /* 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");
> }
> - if (qemu_close(fd) != 0) {
> - result = -errno;
> - error_setg_errno(errp, -result, "Could not close the new file");
> + } else if (prealloc == PREALLOC_MODE_FULL) {
> + char *buf = g_malloc0(65536);
> + int64_t num = 0, left = total_size;
> +
> + while (left > 0) {
> + num = MIN(left, 65536);
> + result = write(fd, buf, num);
> + if (result < 0) {
> + result = -errno;
> + error_setg_errno(errp, -result,
> + "Could not write to the new file");
> + g_free(buf);
> + goto out_close;
> + }
> + left -= num;
> }
Technically, a raw file does not have any metadata, therefore
preallocation=metadata is a bit ambiguous. I'd accept both allocating
nothing and allocating everything; you chose the latter, which is fine.
However, why do you implement the preallocation differently for
preallocation=full, then? posix_fallocate() does not seem to change the
contents of the areas which were newly allocated; and the ftruncate()
before made sure they are read back as zeroes. Therefore, using
ftruncate() and then posix_fallocate() seems to be functionally
equivalent to writing zeroes.
Max
> + fsync(fd);
> + g_free(buf);
> }
> +out_close:
> + if (qemu_close(fd) != 0 && result == 0) {
> + result = -errno;
> + error_setg_errno(errp, -result, "Could not close the new file");
> + }
> +out:
> return result;
> }
>
> @@ -1479,6 +1519,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, metadata, full)"
> + },
> { NULL }
> };
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation option
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 6/6] qcow2: " Hu Tao
@ 2014-06-14 20:37 ` Max Reitz
2014-06-20 8:25 ` Hu Tao
0 siblings, 1 reply; 19+ messages in thread
From: Max Reitz @ 2014-06-14 20:37 UTC (permalink / raw)
To: Hu Tao, qemu-devel
Cc: kwolf, Peter Lieven, Markus Armbruster, Stefan Hajnoczi, y-goto
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 <hutao@cn.fujitsu.com>
> ---
> 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.
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public
2014-06-14 19:07 ` Max Reitz
@ 2014-06-17 2:36 ` Hu Tao
0 siblings, 0 replies; 19+ messages in thread
From: Hu Tao @ 2014-06-17 2:36 UTC (permalink / raw)
To: Max Reitz
Cc: kwolf, Peter Lieven, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, y-goto
On Sat, Jun 14, 2014 at 09:07:19PM +0200, Max Reitz wrote:
> On 12.06.2014 05:54, Hu Tao wrote:
> >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >Suggested-by: Markus Armbruster <armbru@redhat.com>
> >---
> > blockdev.c | 22 ++--------------------
> > include/qapi/util.h | 17 +++++++++++++++++
> > qapi/Makefile.objs | 2 +-
> > qapi/qapi-util.c | 32 ++++++++++++++++++++++++++++++++
> > 4 files changed, 52 insertions(+), 21 deletions(-)
> > create mode 100644 include/qapi/util.h
> > create mode 100644 qapi/qapi-util.c
> >
> >diff --git a/blockdev.c b/blockdev.c
> >index 4cbcc56..9adfdbb 100644
> >--- a/blockdev.c
> >+++ b/blockdev.c
> >@@ -39,6 +39,7 @@
> > #include "qapi/qmp/types.h"
> > #include "qapi-visit.h"
> > #include "qapi/qmp-output-visitor.h"
> >+#include "qapi/util.h"
> > #include "sysemu/sysemu.h"
> > #include "block/block_int.h"
> > #include "qmp-commands.h"
> >@@ -287,25 +288,6 @@ static int parse_block_error_action(const char *buf, bool is_read, Error **errp)
> > }
> > }
> >-static inline int parse_enum_option(const char *lookup[], const char *buf,
> >- int max, int def, Error **errp)
> >-{
> >- int i;
> >-
> >- if (!buf) {
> >- return def;
> >- }
> >-
> >- for (i = 0; i < max; i++) {
> >- if (!strcmp(buf, lookup[i])) {
> >- return i;
> >- }
> >- }
> >-
> >- error_setg(errp, "invalid parameter value: %s", buf);
> >- return def;
> >-}
> >-
> > static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
> > {
> > if (throttle_conflicting(cfg)) {
> >@@ -472,7 +454,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
> > }
> > detect_zeroes =
> >- parse_enum_option(BlockdevDetectZeroesOptions_lookup,
> >+ qapi_enum_parse(BlockdevDetectZeroesOptions_lookup,
> > qemu_opt_get(opts, "detect-zeroes"),
> > BLOCKDEV_DETECT_ZEROES_OPTIONS_MAX,
> > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
>
> Please adapt the indentation of the other parameters.
Sure.
>
> Other than that:
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation option
2014-06-14 20:37 ` Max Reitz
@ 2014-06-20 8:25 ` Hu Tao
2014-06-20 18:37 ` Max Reitz
0 siblings, 1 reply; 19+ messages in thread
From: Hu Tao @ 2014-06-20 8:25 UTC (permalink / raw)
To: Max Reitz
Cc: kwolf, Peter Lieven, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, y-goto
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 <hutao@cn.fujitsu.com>
> >---
> > 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
I need to review minimal_blob_size() further.
>
>
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation option
2014-06-20 8:25 ` Hu Tao
@ 2014-06-20 18:37 ` Max Reitz
2014-06-25 5:41 ` Hu Tao
0 siblings, 1 reply; 19+ messages in thread
From: Max Reitz @ 2014-06-20 18:37 UTC (permalink / raw)
To: Hu Tao
Cc: kwolf, Peter Lieven, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, y-goto
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 <hutao@cn.fujitsu.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation option
2014-06-20 18:37 ` Max Reitz
@ 2014-06-25 5:41 ` Hu Tao
0 siblings, 0 replies; 19+ messages in thread
From: Hu Tao @ 2014-06-25 5:41 UTC (permalink / raw)
To: Max Reitz
Cc: kwolf, Peter Lieven, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, y-goto
On Fri, Jun 20, 2014 at 08:37:51PM +0200, Max Reitz wrote:
> 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 <hutao@cn.fujitsu.com>
> >>>---
> >>> 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'm fine with it. The only problem I'm not sure about is whether it will
bring any compatibility issues that the same cmd line gets two different
image sizes.
>
> >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.
Thanks. looks OK to me.
>
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full.
2014-06-14 19:17 ` Max Reitz
@ 2014-06-25 5:46 ` Hu Tao
0 siblings, 0 replies; 19+ messages in thread
From: Hu Tao @ 2014-06-25 5:46 UTC (permalink / raw)
To: Max Reitz
Cc: kwolf, Peter Lieven, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, y-goto
On Sat, Jun 14, 2014 at 09:17:02PM +0200, Max Reitz wrote:
> On 12.06.2014 05:54, Hu Tao wrote:
> >This patch prepares for the subsequent patches.
> >
> >Reviewed-by: Fam Zheng <famz@redhat.com>
> >Reviewed-by: Eric Blake <eblake@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 6732e7c..2e0b83c 100644
> >--- a/block/qcow2.c
> >+++ b/block/qcow2.c
> >@@ -1593,7 +1593,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)
> > {
> >@@ -1770,7 +1770,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
> > uint64_t size = 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;
> >@@ -1791,9 +1791,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 14b498b..f5ccb31 100644
> >--- a/qapi-schema.json
> >+++ b/qapi-schema.json
>
> As of commit 5db15096 (Benoît's "Extract QAPI block commands"
> series), block-related QAPI definitions should go into
> qapi/block.json or qapi/block-core.json. I think the following
> definition should now go into qapi/block-core.json instead of
> qapi-schema.json.
>
> Apart from that, this patch looks good.
>
> Max
Thanks, I'll fix in next version.
Hu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option
2014-06-14 19:38 ` Max Reitz
@ 2014-06-25 6:04 ` Hu Tao
0 siblings, 0 replies; 19+ messages in thread
From: Hu Tao @ 2014-06-25 6:04 UTC (permalink / raw)
To: Max Reitz
Cc: kwolf, Peter Lieven, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, y-goto
On Sat, Jun 14, 2014 at 09:38:30PM +0200, Max Reitz wrote:
> On 12.06.2014 05:54, Hu Tao wrote:
> >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 | 59 ++++++++++++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 52 insertions(+), 7 deletions(-)
> >
> >diff --git a/block/raw-posix.c b/block/raw-posix.c
> >index 35057f0..73b10cd 100644
> >--- a/block/raw-posix.c
> >+++ b/block/raw-posix.c
> >@@ -30,6 +30,7 @@
> > #include "block/thread-pool.h"
> > #include "qemu/iov.h"
> > #include "raw-aio.h"
> >+#include "qapi/util.h"
> > #if defined(__APPLE__) && (__MACH__)
> > #include <paths.h>
> >@@ -1279,6 +1280,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> > int fd;
> > int result = 0;
> > int64_t total_size = 0;
> >+ PreallocMode prealloc = PREALLOC_MODE_OFF;
> >+ Error *error = NULL;
> > strstart(filename, "file:", &filename);
> >@@ -1286,6 +1289,14 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> > while (options && options->name) {
> > if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> > total_size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
> >+ } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> >+ prealloc = qapi_enum_parse(PreallocMode_lookup, options->value.s,
> >+ PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> >+ &error);
> >+ if (error) {
> >+ error_propagate(errp, error);
> >+ return -EINVAL;
>
> Could be "result = -EINVAL; goto out;" instead, but definitely isn't
> wrong this way.
>
> >+ }
> > }
> > options++;
> > }
> >@@ -1295,16 +1306,45 @@ 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");
> >+ 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_METADATA) {
> >+ /* 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");
> > }
> >- if (qemu_close(fd) != 0) {
> >- result = -errno;
> >- error_setg_errno(errp, -result, "Could not close the new file");
> >+ } else if (prealloc == PREALLOC_MODE_FULL) {
> >+ char *buf = g_malloc0(65536);
> >+ int64_t num = 0, left = total_size;
> >+
> >+ while (left > 0) {
> >+ num = MIN(left, 65536);
> >+ result = write(fd, buf, num);
> >+ if (result < 0) {
> >+ result = -errno;
> >+ error_setg_errno(errp, -result,
> >+ "Could not write to the new file");
> >+ g_free(buf);
> >+ goto out_close;
> >+ }
> >+ left -= num;
> > }
>
> Technically, a raw file does not have any metadata, therefore
> preallocation=metadata is a bit ambiguous. I'd accept both
> allocating nothing and allocating everything; you chose the latter,
> which is fine.
qcow2's behaviour of metadata preallocation depends on raw's, this is
why I chose the latter here.
>
> However, why do you implement the preallocation differently for
> preallocation=full, then? posix_fallocate() does not seem to change
> the contents of the areas which were newly allocated; and the
> ftruncate() before made sure they are read back as zeroes.
> Therefore, using ftruncate() and then posix_fallocate() seems to be
> functionally equivalent to writing zeroes.
The difference is posix_fallocate() reserves space but ftruncate()
doesn't, as said in man page:
After a successful call to posix_fallocate(), subsequent writes
to bytes in the specified range are guaranteed not to fail because
of lack of disk space.
however, posix_fallocate() doesn't guarantee this in other cases like
thin provisioning, this is why preallocation=metadata is different than
preallocation=full.
Hu
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2014-06-25 6:06 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-12 3:54 [Qemu-devel] [PATCH v10 0/6] qemu-img: add preallocation=full Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector Hu Tao
2014-06-14 18:51 ` Max Reitz
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 2/6] raw, qcow2: don't convert file size to sector size Hu Tao
2014-06-14 19:00 ` Max Reitz
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public Hu Tao
2014-06-14 19:07 ` Max Reitz
2014-06-17 2:36 ` Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full Hu Tao
2014-06-14 19:17 ` Max Reitz
2014-06-25 5:46 ` Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option Hu Tao
2014-06-14 19:38 ` Max Reitz
2014-06-25 6:04 ` Hu Tao
2014-06-12 3:54 ` [Qemu-devel] [PATCH v10 6/6] qcow2: " Hu Tao
2014-06-14 20:37 ` Max Reitz
2014-06-20 8:25 ` Hu Tao
2014-06-20 18:37 ` Max Reitz
2014-06-25 5:41 ` Hu Tao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.