All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc
@ 2014-09-09  3:54 Hu Tao
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector Hu Tao
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Hu Tao @ 2014-09-09  3:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

This series adds two preallocation mode to qcow2 and raw:

Option preallocation=full preallocates disk space for image by writing
zeros to disk, this ensures disk space in any cases.

Option preallocation=falloc preallocates disk space by calling
posix_fallocate(). This is faster than preallocation=full.

Note: there is a false positive reported by checkpatch.pl to patch 1.

changes to v13:

  - rebase (patch 3 in v13 is already in)
  - don't convert file size to sector size in hdev_create(), too (patch 2)
  - reintroduce preallocation=falloc. (patch 3)
  - split the implementation of preallocation=full in v13 into
    preallocation=falloc and preallocation=full (patch 4)

changes to v12:

  - remove dependence on minimal_blob_size() (patch 6)
  - remove preallocation=falloc. (patch 4)
  - preallocation=full tries posix_fallocate() first then writing
    zeros (patch 5)
  - round up file size for all formats (patch 1)
  - avoid converting file size for more formats (patch 2)

changes to v11:

 - fix test case 049 (patch 4)
 - unsigned nl2e -> uint64_t nl2e (patch 6)
 - use >> instead of / (patch 6)

changes to v10:

  - PreallocMode is moved from file qapi-schema.json to qapi/block-core.json
  - introdues preallocation=falloc, no changes to preallocation=metadata
  - using minimal_blob_size() to calculate metadata size for qcow2
  - indentation fix in file blockdev.c

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

changes to v8:

 - round up image file size to nearest sector size
 - dont' blindly lose error info
 - target for 2.1 rather than 2.0
 - and, rebase to latest git tree

changes to v5:

  - add `Since 2.0' to PreallocMode
  - apply total_size change to raw-win32.c as well

changes to v4:

  - fix wrong calculation of qcow2 metadata size in v4
  - remove raw_preallocate2()
  - better error out path in raw_create()
  - fix coding style

changes to v3:

  - remove bdrv_preallocate and make preallocation a
    bdrv_create_file option
  - prealloc_mode -> PreallocMode and add it to QAPI
  - fix return value in raw_preallocate2

changes to v2:

  - Fix comments to v2 by Fam.
  - qcow2: first fallocate disk space, then allocate metadata. This avoids
    the problem in v2 that bdrv_preallocate may clear all information in
    metadata. This does not necessarily map all data clusters sequentially
    but does keep information in metadata. Peter, is this acceptable?


Hu Tao (5):
  block: round up file size to nearest sector
  block: don't convert file size to sector size
  qapi: introduce PreallocMode and new PreallocModes full and falloc.
  raw-posix: Add falloc and full preallocation option
  qcow2: Add falloc and full preallocation option

 block/archipelago.c              |   3 +-
 block/cow.c                      |   3 +-
 block/gluster.c                  |   9 ++--
 block/iscsi.c                    |   4 +-
 block/nfs.c                      |   3 +-
 block/qcow.c                     |   7 +--
 block/qcow2.c                    |  80 ++++++++++++++++++++++++------
 block/qed.c                      |   3 +-
 block/raw-posix.c                | 103 ++++++++++++++++++++++++++++++---------
 block/raw-win32.c                |   6 +--
 block/rbd.c                      |   3 +-
 block/sheepdog.c                 |   3 +-
 block/ssh.c                      |   3 +-
 block/vdi.c                      |   3 +-
 block/vhdx.c                     |   3 +-
 block/vmdk.c                     |   3 +-
 block/vpc.c                      |   3 +-
 qapi/block-core.json             |  17 +++++++
 qemu-doc.texi                    |  17 +++++--
 qemu-img.texi                    |  17 +++++--
 tests/qemu-iotests/049.out       |   2 +-
 tests/qemu-iotests/082.out       |  54 ++++++++++----------
 tests/qemu-iotests/104           |  57 ++++++++++++++++++++++
 tests/qemu-iotests/104.out       |  12 +++++
 tests/qemu-iotests/common.filter |  21 ++++++++
 tests/qemu-iotests/group         |   1 +
 26 files changed, 344 insertions(+), 96 deletions(-)
 create mode 100755 tests/qemu-iotests/104
 create mode 100644 tests/qemu-iotests/104.out

-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector
  2014-09-09  3:54 [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc Hu Tao
@ 2014-09-09  3:54 ` Hu Tao
  2014-09-09 12:12   ` Benoît Canet
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 2/5] block: don't convert file size to sector size Hu Tao
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Hu Tao @ 2014-09-09  3:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/archipelago.c              |  3 ++-
 block/cow.c                      |  3 ++-
 block/gluster.c                  |  4 +--
 block/iscsi.c                    |  4 +--
 block/nfs.c                      |  3 ++-
 block/qcow.c                     |  3 ++-
 block/qcow2.c                    |  3 ++-
 block/qed.c                      |  3 ++-
 block/raw-posix.c                |  8 +++---
 block/raw-win32.c                |  4 +--
 block/rbd.c                      |  3 ++-
 block/sheepdog.c                 |  3 ++-
 block/ssh.c                      |  3 ++-
 block/vdi.c                      |  3 ++-
 block/vhdx.c                     |  3 ++-
 block/vmdk.c                     |  3 ++-
 block/vpc.c                      |  3 ++-
 tests/qemu-iotests/104           | 57 ++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/104.out       | 12 +++++++++
 tests/qemu-iotests/common.filter | 21 +++++++++++++++
 tests/qemu-iotests/group         |  1 +
 21 files changed, 127 insertions(+), 23 deletions(-)
 create mode 100755 tests/qemu-iotests/104
 create mode 100644 tests/qemu-iotests/104.out

diff --git a/block/archipelago.c b/block/archipelago.c
index 22a7daa..06c51f9 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -708,7 +708,8 @@ static int qemu_archipelago_create(const char *filename,
 
     parse_filename_opts(filename, errp, &volname, &segment_name, &mport,
                         &vport);
-    total_size = qemu_opt_get_size_del(options, BLOCK_OPT_SIZE, 0);
+    total_size = ROUND_UP(qemu_opt_get_size_del(options, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
 
     if (segment_name == NULL) {
         segment_name = g_strdup("archipelago");
diff --git a/block/cow.c b/block/cow.c
index 6ee4833..c3769fe 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -335,7 +335,8 @@ static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
     BlockDriverState *cow_bs = NULL;
 
     /* Read out options */
-    image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+    image_sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                                 BDRV_SECTOR_SIZE);
     image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
 
     ret = bdrv_create_file(filename, opts, &local_err);
diff --git a/block/gluster.c b/block/gluster.c
index 1912cf9..65c7a58 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -494,8 +494,8 @@ static int qemu_gluster_create(const char *filename,
         goto out;
     }
 
-    total_size =
-        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
+    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                              BDRV_SECTOR_SIZE);
 
     tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
     if (!tmp || !strcmp(tmp, "off")) {
diff --git a/block/iscsi.c b/block/iscsi.c
index 3e19202..84bcae8 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1531,8 +1531,8 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
     bs = bdrv_new("", &error_abort);
 
     /* Read out options */
-    total_size =
-        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
+    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                              BDRV_SECTOR_SIZE);
     bs->opaque = g_new0(struct IscsiLun, 1);
     iscsilun = bs->opaque;
 
diff --git a/block/nfs.c b/block/nfs.c
index 194f301..c76e368 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -418,7 +418,8 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
     client->aio_context = qemu_get_aio_context();
 
     /* Read out options */
-    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
 
     ret = nfs_client_open(client, url, O_CREAT, errp);
     if (ret < 0) {
diff --git a/block/qcow.c b/block/qcow.c
index 67c237f..041af26 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -725,7 +725,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
     BlockDriverState *qcow_bs;
 
     /* Read out options */
-    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                              BDRV_SECTOR_SIZE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
         flags |= BLOCK_FLAG_ENCRYPT;
diff --git a/block/qcow2.c b/block/qcow2.c
index f9e045f..c8050e5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1921,7 +1921,8 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
     int ret;
 
     /* Read out options */
-    sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+    sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                           BDRV_SECTOR_SIZE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
diff --git a/block/qed.c b/block/qed.c
index ba395af..f8d9e12 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -648,7 +648,8 @@ static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
     char *backing_fmt = NULL;
     int ret;
 
-    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
     cluster_size = qemu_opt_get_size_del(opts,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index d737f3a..9c22e3f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1369,8 +1369,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
     strstart(filename, "file:", &filename);
 
     /* Read out options */
-    total_size =
-        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
+    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                              BDRV_SECTOR_SIZE);
     nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
@@ -1966,8 +1966,8 @@ static int hdev_create(const char *filename, QemuOpts *opts,
     (void)has_prefix;
 
     /* Read out options */
-    total_size =
-        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
+    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                              BDRV_SECTOR_SIZE);
 
     fd = qemu_open(filename, O_WRONLY | O_BINARY);
     if (fd < 0) {
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 902eab6..1e1880d 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -511,8 +511,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
     strstart(filename, "file:", &filename);
 
     /* Read out options */
-    total_size =
-        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                              BDRV_SECTOR_SIZE);
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
                    0644);
diff --git a/block/rbd.c b/block/rbd.c
index ea969e7..b7f7d5f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -314,7 +314,8 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
     }
 
     /* Read out options */
-    bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                     BDRV_SECTOR_SIZE);
     objsize = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, 0);
     if (objsize) {
         if ((objsize - 1) & objsize) {    /* not a power of 2? */
diff --git a/block/sheepdog.c b/block/sheepdog.c
index f91afc3..7da36e1 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1702,7 +1702,8 @@ static int sd_create(const char *filename, QemuOpts *opts,
         goto out;
     }
 
-    s->inode.vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    s->inode.vdi_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                                 BDRV_SECTOR_SIZE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
     if (!buf || !strcmp(buf, "off")) {
diff --git a/block/ssh.c b/block/ssh.c
index cd2fd75..cf43bc0 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -700,7 +700,8 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
     ssh_state_init(&s);
 
     /* Get desired file size. */
-    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
     DPRINTF("total_size=%" PRIi64, total_size);
 
     uri_options = qdict_new();
diff --git a/block/vdi.c b/block/vdi.c
index 4b10aac..cfa08b0 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -700,7 +700,8 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
     logout("\n");
 
     /* Read out options. */
-    bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                     BDRV_SECTOR_SIZE);
 #if defined(CONFIG_VDI_BLOCK_SIZE)
     /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
     block_size = qemu_opt_get_size_del(opts,
diff --git a/block/vhdx.c b/block/vhdx.c
index 87c99fc..796b7bd 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1766,7 +1766,8 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     VHDXImageType image_type;
     Error *local_err = NULL;
 
-    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
     log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
     block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
     type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
diff --git a/block/vmdk.c b/block/vmdk.c
index a1cb911..afdea1a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1807,7 +1807,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
         goto exit;
     }
     /* Read out options */
-    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
     adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
diff --git a/block/vpc.c b/block/vpc.c
index 055efc4..5d8fd8e 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -757,7 +757,8 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
     BlockDriverState *bs = NULL;
 
     /* Read out options */
-    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
     disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
     if (disk_type_param) {
         if (!strcmp(disk_type_param, "dynamic")) {
diff --git a/tests/qemu-iotests/104 b/tests/qemu-iotests/104
new file mode 100755
index 0000000..b471aa5
--- /dev/null
+++ b/tests/qemu-iotests/104
@@ -0,0 +1,57 @@
+#!/bin/bash
+#
+# Test image creation with aligned and unaligned 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 generic
+_supported_proto generic
+_supported_os Linux
+
+echo "=== Check qemu-img info output ==="
+echo
+image_sizes="1024 1234"
+
+for s in $image_sizes; do
+    _make_test_img $s | _filter_img_create
+    _img_info | _filter_img_info
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/104.out b/tests/qemu-iotests/104.out
new file mode 100644
index 0000000..de27852
--- /dev/null
+++ b/tests/qemu-iotests/104.out
@@ -0,0 +1,12 @@
+QA output created by 104
+=== 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)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1234 
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 1.5K (1536 bytes)
+***done
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 51192c8..f69cb6b 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -192,5 +192,26 @@ _filter_img_create()
         -e "s/archipelago:a/TEST_DIR\//g"
 }
 
+_filter_img_info()
+{
+    sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
+        -e "s#$TEST_DIR#TEST_DIR#g" \
+        -e "s#$IMGFMT#IMGFMT#g" \
+        -e "/encrypted: yes/d" \
+        -e "/cluster_size: [0-9]\\+/d" \
+        -e "/table_size: [0-9]\\+/d" \
+        -e "/compat: '[^']*'/d" \
+        -e "/compat6: \\(on\\|off\\)/d" \
+        -e "/static: \\(on\\|off\\)/d" \
+        -e "/zeroed_grain: \\(on\\|off\\)/d" \
+        -e "/subformat: '[^']*'/d" \
+        -e "/adapter_type: '[^']*'/d" \
+        -e "/lazy_refcounts: \\(on\\|off\\)/d" \
+        -e "/block_size: [0-9]\\+/d" \
+        -e "/block_state_zero: \\(on\\|off\\)/d" \
+        -e "/log_size: [0-9]\\+/d" \
+        -e "s/archipelago:a/TEST_DIR\//g"
+}
+
 # make sure this script returns success
 /bin/true
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0920b28..622685e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -104,3 +104,4 @@
 100 rw auto quick
 101 rw auto quick
 103 rw auto quick
+104 rw auto
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v14 2/5] block: don't convert file size to sector size
  2014-09-09  3:54 [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc Hu Tao
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector Hu Tao
@ 2014-09-09  3:54 ` Hu Tao
  2014-09-09 12:26   ` Benoît Canet
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc Hu Tao
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Hu Tao @ 2014-09-09  3:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

and avoid converting it back later.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/gluster.c   |  9 ++++-----
 block/qcow.c      |  8 ++++----
 block/qcow2.c     | 10 +++++-----
 block/raw-posix.c | 12 ++++++------
 block/raw-win32.c |  6 +++---
 5 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 65c7a58..1eb3a8c 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -494,8 +494,8 @@ static int qemu_gluster_create(const char *filename,
         goto out;
     }
 
-    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                              BDRV_SECTOR_SIZE);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
 
     tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
     if (!tmp || !strcmp(tmp, "off")) {
@@ -516,9 +516,8 @@ static int qemu_gluster_create(const char *filename,
     if (!fd) {
         ret = -errno;
     } else {
-        if (!glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE)) {
-            if (prealloc && qemu_gluster_zerofill(fd, 0,
-                    total_size * BDRV_SECTOR_SIZE)) {
+        if (!glfs_ftruncate(fd, total_size)) {
+            if (prealloc && qemu_gluster_zerofill(fd, 0, total_size)) {
                 ret = -errno;
             }
         } else {
diff --git a/block/qcow.c b/block/qcow.c
index 041af26..a87bd69 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -725,8 +725,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
     BlockDriverState *qcow_bs;
 
     /* Read out options */
-    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                              BDRV_SECTOR_SIZE);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
         flags |= BLOCK_FLAG_ENCRYPT;
@@ -754,7 +754,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
     memset(&header, 0, sizeof(header));
     header.magic = cpu_to_be32(QCOW_MAGIC);
     header.version = cpu_to_be32(QCOW_VERSION);
-    header.size = cpu_to_be64(total_size * 512);
+    header.size = cpu_to_be64(total_size);
     header_size = sizeof(header);
     backing_filename_len = 0;
     if (backing_file) {
@@ -776,7 +776,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
     }
     header_size = (header_size + 7) & ~7;
     shift = header.cluster_bits + header.l2_bits;
-    l1_size = ((total_size * 512) + (1LL << shift) - 1) >> shift;
+    l1_size = (total_size + (1LL << shift) - 1) >> shift;
 
     header.l1_table_offset = cpu_to_be64(header_size);
     if (flags & BLOCK_FLAG_ENCRYPT) {
diff --git a/block/qcow2.c b/block/qcow2.c
index c8050e5..cf27c3f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1859,7 +1859,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;
@@ -1912,7 +1912,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
     char *backing_file = NULL;
     char *backing_fmt = NULL;
     char *buf = NULL;
-    uint64_t sectors = 0;
+    uint64_t size = 0;
     int flags = 0;
     size_t cluster_size = DEFAULT_CLUSTER_SIZE;
     int prealloc = 0;
@@ -1921,8 +1921,8 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
     int ret;
 
     /* Read out options */
-    sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                           BDRV_SECTOR_SIZE);
+    size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                    BDRV_SECTOR_SIZE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
@@ -1972,7 +1972,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
         goto finish;
     }
 
-    ret = qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
+    ret = qcow2_create2(filename, size, backing_file, backing_fmt, flags,
                         cluster_size, prealloc, opts, version, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 9c22e3f..7208c05 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1369,8 +1369,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
     strstart(filename, "file:", &filename);
 
     /* Read out options */
-    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                              BDRV_SECTOR_SIZE);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
     nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
@@ -1394,7 +1394,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 #endif
         }
 
-        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");
         }
@@ -1966,8 +1966,8 @@ static int hdev_create(const char *filename, QemuOpts *opts,
     (void)has_prefix;
 
     /* Read out options */
-    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                              BDRV_SECTOR_SIZE);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
 
     fd = qemu_open(filename, O_WRONLY | O_BINARY);
     if (fd < 0) {
@@ -1983,7 +1983,7 @@ static int hdev_create(const char *filename, QemuOpts *opts,
         error_setg(errp,
                    "The given file is neither a block nor a character device");
         ret = -ENODEV;
-    } else if (lseek(fd, 0, SEEK_END) < total_size * BDRV_SECTOR_SIZE) {
+    } else if (lseek(fd, 0, SEEK_END) < total_size) {
         error_setg(errp, "Device is too small");
         ret = -ENOSPC;
     }
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 1e1880d..9bf8225 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -511,8 +511,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
     strstart(filename, "file:", &filename);
 
     /* Read out options */
-    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                              BDRV_SECTOR_SIZE);
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                          BDRV_SECTOR_SIZE);
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
                    0644);
@@ -521,7 +521,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
         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] 16+ messages in thread

* [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc.
  2014-09-09  3:54 [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc Hu Tao
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector Hu Tao
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 2/5] block: don't convert file size to sector size Hu Tao
@ 2014-09-09  3:54 ` Hu Tao
  2014-09-09 12:42   ` Eric Blake
  2014-09-09 12:45   ` Benoît Canet
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option Hu Tao
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 5/5] qcow2: " Hu Tao
  4 siblings, 2 replies; 16+ messages in thread
From: Hu Tao @ 2014-09-09  3:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

This patch prepares for the subsequent patches.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c              | 23 +++++++++++++++--------
 qapi/block-core.json       | 17 +++++++++++++++++
 tests/qemu-iotests/049.out |  2 +-
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index cf27c3f..94d1225 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"
 #include "qemu/option_int.h"
 
@@ -1738,7 +1739,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,
                          QemuOpts *opts, int version,
                          Error **errp)
 {
@@ -1915,7 +1916,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
     uint64_t size = 0;
     int flags = 0;
     size_t cluster_size = DEFAULT_CLUSTER_SIZE;
-    int prealloc = 0;
+    PreallocMode prealloc;
     int version = 3;
     Error *local_err = NULL;
     int ret;
@@ -1931,12 +1932,11 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
     cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
                                          DEFAULT_CLUSTER_SIZE);
     buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
-    if (!buf || !strcmp(buf, "off")) {
-        prealloc = 0;
-    } else if (!strcmp(buf, "metadata")) {
-        prealloc = 1;
-    } else {
-        error_setg(errp, "Invalid preallocation mode: '%s'", buf);
+    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
+                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
+                               &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         ret = -EINVAL;
         goto finish;
     }
@@ -1958,6 +1958,13 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
         flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
     }
 
+    if (prealloc && prealloc != PREALLOC_MODE_METADATA) {
+        ret = -EINVAL;
+        error_setg(errp, "Unsupported preallocate mode: %s",
+                   PreallocMode_lookup[prealloc]);
+        goto finish;
+    }
+
     if (backing_file && prealloc) {
         error_setg(errp, "Backing file and preallocation cannot be used at "
                    "the same time");
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a685d02..a29dbe1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1697,3 +1697,20 @@
             'len'   : 'int',
             'offset': 'int',
             'speed' : 'int' } }
+
+# @PreallocMode
+#
+# Preallocation mode of QEMU image file
+#
+# @off: no preallocation
+# @metadata: preallocate only for metadata
+# @falloc: like @full preallocation but allocate disk space by
+#          posix_fallocate() rather than writing zeros.
+# @full: preallocate all data by writing zeros to device to ensure disk
+#        space is really available. @full preallocation also sets up
+#        metadata correctly.
+#
+# Since 2.2
+##
+{ 'enum': 'PreallocMode',
+  'data': [ 'off', 'metadata', 'falloc', 'full' ] }
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index 71ca44d..09ca0ae 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -179,7 +179,7 @@ qemu-img create -f qcow2 -o preallocation=metadata TEST_DIR/t.qcow2 64M
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='metadata' lazy_refcounts=off 
 
 qemu-img create -f qcow2 -o preallocation=1234 TEST_DIR/t.qcow2 64M
-qemu-img: TEST_DIR/t.qcow2: Invalid preallocation mode: '1234'
+qemu-img: TEST_DIR/t.qcow2: invalid parameter value: 1234
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='1234' lazy_refcounts=off 
 
 == Check encryption option ==
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option
  2014-09-09  3:54 [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc Hu Tao
                   ` (2 preceding siblings ...)
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc Hu Tao
@ 2014-09-09  3:54 ` Hu Tao
  2014-09-09 13:21   ` Benoît Canet
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 5/5] qcow2: " Hu Tao
  4 siblings, 1 reply; 16+ messages in thread
From: Hu Tao @ 2014-09-09  3:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

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

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-posix.c | 93 +++++++++++++++++++++++++++++++++++++++++++------------
 qemu-doc.texi     |  9 ++++++
 qemu-img.texi     |  9 ++++++
 3 files changed, 92 insertions(+), 19 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 7208c05..27c854c 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>
@@ -1365,6 +1366,9 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
     int result = 0;
     int64_t total_size = 0;
     bool nocow = false;
+    PreallocMode prealloc;
+    char *buf = NULL;
+    Error *local_err = NULL;
 
     strstart(filename, "file:", &filename);
 
@@ -1372,37 +1376,83 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
     total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
                           BDRV_SECTOR_SIZE);
     nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
+    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
+                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
+                               &local_err);
+    g_free(buf);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        result = -EINVAL;
+        goto out;
+    }
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
                    0644);
     if (fd < 0) {
         result = -errno;
         error_setg_errno(errp, -result, "Could not create file");
-    } else {
-        if (nocow) {
+        goto out;
+    }
+
+    if (nocow) {
 #ifdef __linux__
-            /* Set NOCOW flag to solve performance issue on fs like btrfs.
-             * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
-             * will be ignored since any failure of this operation should not
-             * block the left work.
-             */
-            int attr;
-            if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
-                attr |= FS_NOCOW_FL;
-                ioctl(fd, FS_IOC_SETFLAGS, &attr);
-            }
-#endif
+        /* Set NOCOW flag to solve performance issue on fs like btrfs.
+         * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
+         * will be ignored since any failure of this operation should not
+         * block the left work.
+         */
+        int attr;
+        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
+            attr |= FS_NOCOW_FL;
+            ioctl(fd, FS_IOC_SETFLAGS, &attr);
         }
+#endif
+    }
 
-        if (ftruncate(fd, total_size) != 0) {
-            result = -errno;
-            error_setg_errno(errp, -result, "Could not resize file");
+    if (ftruncate(fd, total_size) != 0) {
+        result = -errno;
+        error_setg_errno(errp, -result, "Could not resize file");
+        goto out_close;
+    }
+
+    if (prealloc == PREALLOC_MODE_FALLOC) {
+        /* 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) {
+        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);
+    } else if (prealloc != PREALLOC_MODE_OFF) {
+        result = -EINVAL;
+        error_setg(errp, "Unsupported preallocation mode: %s",
+                   PreallocMode_lookup[prealloc]);
     }
+
+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;
 }
 
@@ -1585,6 +1635,11 @@ static QemuOptsList raw_create_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Turn off copy-on-write (valid only on btrfs)"
         },
+        {
+            .name = BLOCK_OPT_PREALLOC,
+            .type = QEMU_OPT_STRING,
+            .help = "Preallocation mode (allowed values: off, falloc, full)"
+        },
         { /* end of list */ }
     }
 };
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 2b232ae..1f289d6 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -527,6 +527,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
 space. Use @code{qemu-img info} to know the real size used by the
 image or @code{ls -ls} on Unix/Linux.
 
+Supported options:
+@table @code
+@item preallocation
+Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
+@code{falloc} mode preallocates space for image by calling posix_fallocate().
+@code{full} mode preallocates space for image by writing zeros to underlying
+storage.
+@end table
+
 @item qcow2
 QEMU image format, the most versatile format. Use it to have smaller
 images (useful if your filesystem does not supports holes, for example
diff --git a/qemu-img.texi b/qemu-img.texi
index cc4668e..d64d05e 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -419,6 +419,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
 space. Use @code{qemu-img info} to know the real size used by the
 image or @code{ls -ls} on Unix/Linux.
 
+Supported options:
+@table @code
+@item preallocation
+Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
+@code{falloc} mode preallocates space for image by calling posix_fallocate().
+@code{full} mode preallocates space for image by writing zeros to underlying
+storage.
+@end table
+
 @item qcow2
 QEMU image format, the most versatile format. Use it to have smaller
 images (useful if your filesystem does not supports holes, for example
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v14 5/5] qcow2: Add falloc and full preallocation option
  2014-09-09  3:54 [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc Hu Tao
                   ` (3 preceding siblings ...)
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option Hu Tao
@ 2014-09-09  3:54 ` Hu Tao
  4 siblings, 0 replies; 16+ messages in thread
From: Hu Tao @ 2014-09-09  3:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

preallocation=falloc allocates disk space by posix_fallocate(),
preallocation=full allocates disk space by writing zeros to disk.
Both modes imply preallocation=metadata.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c              | 62 +++++++++++++++++++++++++++++++++++++++-------
 qemu-doc.texi              |  8 +++---
 qemu-img.texi              |  8 +++---
 tests/qemu-iotests/082.out | 54 ++++++++++++++++++++--------------------
 4 files changed, 90 insertions(+), 42 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 94d1225..375c6a6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1772,6 +1772,56 @@ 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_FALLOC) {
+        int64_t meta_size = 0;
+        uint64_t nreftablee, nrefblocke, nl1e, nl2e;
+        int64_t aligned_total_size = align_offset(total_size, cluster_size);
+
+        /* header: 1 cluster */
+        meta_size += cluster_size;
+
+        /* total size of L2 tables */
+        nl2e = aligned_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 = (aligned_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);
+
+        qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
+                            aligned_total_size + meta_size);
+        qemu_opt_set(opts, BLOCK_OPT_PREALLOC, PreallocMode_lookup[prealloc]);
+    }
+
     ret = bdrv_create_file(filename, opts, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
@@ -1877,7 +1927,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     }
 
     /* And if we're supposed to preallocate metadata, do that now */
-    if (prealloc) {
+    if (prealloc != PREALLOC_MODE_OFF) {
         BDRVQcowState *s = bs->opaque;
         qemu_co_mutex_lock(&s->lock);
         ret = preallocate(bs);
@@ -1958,13 +2008,6 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
         flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
     }
 
-    if (prealloc && prealloc != PREALLOC_MODE_METADATA) {
-        ret = -EINVAL;
-        error_setg(errp, "Unsupported preallocate mode: %s",
-                   PreallocMode_lookup[prealloc]);
-        goto finish;
-    }
-
     if (backing_file && prealloc) {
         error_setg(errp, "Backing file and preallocation cannot be used at "
                    "the same time");
@@ -2525,7 +2568,8 @@ static QemuOptsList qcow2_create_opts = {
         {
             .name = BLOCK_OPT_PREALLOC,
             .type = QEMU_OPT_STRING,
-            .help = "Preallocation mode (allowed values: off, metadata)"
+            .help = "Preallocation mode (allowed values: off, metadata, "
+                    "falloc, full)"
         },
         {
             .name = BLOCK_OPT_LAZY_REFCOUNTS,
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 1f289d6..ef3be72 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -584,9 +584,11 @@ sizes can improve the image file size whereas larger cluster sizes generally
 provide better performance.
 
 @item preallocation
-Preallocation mode (allowed values: off, metadata). An image with preallocated
-metadata is initially larger but can improve performance when the image needs
-to grow.
+Preallocation mode (allowed values: @code{off}, @code{metadata}, @code{falloc},
+@code{full}). An image with preallocated metadata is initially larger but can
+improve performance when the image needs to grow. @code{falloc} and @code{full}
+preallocations are like the same options of @code{raw} format, but sets up
+metadata also.
 
 @item lazy_refcounts
 If this option is set to @code{on}, reference count updates are postponed with
diff --git a/qemu-img.texi b/qemu-img.texi
index d64d05e..50d2cca 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -476,9 +476,11 @@ sizes can improve the image file size whereas larger cluster sizes generally
 provide better performance.
 
 @item preallocation
-Preallocation mode (allowed values: off, metadata). An image with preallocated
-metadata is initially larger but can improve performance when the image needs
-to grow.
+Preallocation mode (allowed values: @code{off}, @code{metadata}, @code{falloc},
+@code{full}). An image with preallocated metadata is initially larger but can
+improve performance when the image needs to grow. @code{falloc} and @code{full}
+preallocations are like the same options of @code{raw} format, but sets up
+metadata also.
 
 @item lazy_refcounts
 If this option is set to @code{on}, reference count updates are postponed with
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 413e7ef..90c21c8 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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -76,7 +76,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -88,7 +88,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -100,7 +100,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -112,7 +112,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -124,7 +124,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -136,7 +136,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -148,7 +148,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -175,7 +175,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: create -o help
@@ -253,7 +253,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -265,7 +265,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -277,7 +277,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -301,7 +301,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -313,7 +313,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -325,7 +325,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -337,7 +337,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -364,7 +364,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -o help
@@ -431,7 +431,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -443,7 +443,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -455,7 +455,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -467,7 +467,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -479,7 +479,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -491,7 +491,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -503,7 +503,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -515,7 +515,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 nocow            Turn off copy-on-write (valid only on btrfs)
 
@@ -544,7 +544,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, falloc, full)
 lazy_refcounts   Postpone refcount updates
 
 Testing: convert -o help
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector Hu Tao
@ 2014-09-09 12:12   ` Benoît Canet
  2014-09-10  1:50     ` Hu Tao
  0 siblings, 1 reply; 16+ messages in thread
From: Benoît Canet @ 2014-09-09 12:12 UTC (permalink / raw)
  To: Hu Tao
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

The Tuesday 09 Sep 2014 à 11:54:27 (+0800), Hu Tao wrote :

Taking the time to systematically write a nice sentence in
the commit message body about why your commit exists and
explaining the long term purpose of the patch will:

-improve the quality of your commit
-please everyone involved in the review
-ultimately help your to get the patch merged faster

> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/archipelago.c              |  3 ++-
>  block/cow.c                      |  3 ++-
>  block/gluster.c                  |  4 +--
>  block/iscsi.c                    |  4 +--
>  block/nfs.c                      |  3 ++-
>  block/qcow.c                     |  3 ++-
>  block/qcow2.c                    |  3 ++-
>  block/qed.c                      |  3 ++-
>  block/raw-posix.c                |  8 +++---
>  block/raw-win32.c                |  4 +--
>  block/rbd.c                      |  3 ++-
>  block/sheepdog.c                 |  3 ++-
>  block/ssh.c                      |  3 ++-
>  block/vdi.c                      |  3 ++-
>  block/vhdx.c                     |  3 ++-
>  block/vmdk.c                     |  3 ++-
>  block/vpc.c                      |  3 ++-
>  tests/qemu-iotests/104           | 57 ++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/104.out       | 12 +++++++++
>  tests/qemu-iotests/common.filter | 21 +++++++++++++++
>  tests/qemu-iotests/group         |  1 +
>  21 files changed, 127 insertions(+), 23 deletions(-)
>  create mode 100755 tests/qemu-iotests/104
>  create mode 100644 tests/qemu-iotests/104.out
> 
> diff --git a/block/archipelago.c b/block/archipelago.c
> index 22a7daa..06c51f9 100644
> --- a/block/archipelago.c
> +++ b/block/archipelago.c
> @@ -708,7 +708,8 @@ static int qemu_archipelago_create(const char *filename,
>  
>      parse_filename_opts(filename, errp, &volname, &segment_name, &mport,
>                          &vport);
> -    total_size = qemu_opt_get_size_del(options, BLOCK_OPT_SIZE, 0);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(options, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>  
>      if (segment_name == NULL) {
>          segment_name = g_strdup("archipelago");
> diff --git a/block/cow.c b/block/cow.c
> index 6ee4833..c3769fe 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -335,7 +335,8 @@ static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
>      BlockDriverState *cow_bs = NULL;
>  
>      /* Read out options */
> -    image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> +    image_sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                                 BDRV_SECTOR_SIZE);
>      image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>  
>      ret = bdrv_create_file(filename, opts, &local_err);
> diff --git a/block/gluster.c b/block/gluster.c
> index 1912cf9..65c7a58 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -494,8 +494,8 @@ static int qemu_gluster_create(const char *filename,
>          goto out;
>      }
>  
> -    total_size =
> -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                              BDRV_SECTOR_SIZE);
>  
>      tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
>      if (!tmp || !strcmp(tmp, "off")) {
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 3e19202..84bcae8 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1531,8 +1531,8 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
>      bs = bdrv_new("", &error_abort);
>  
>      /* Read out options */
> -    total_size =
> -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                              BDRV_SECTOR_SIZE);
>      bs->opaque = g_new0(struct IscsiLun, 1);
>      iscsilun = bs->opaque;
>  
> diff --git a/block/nfs.c b/block/nfs.c
> index 194f301..c76e368 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -418,7 +418,8 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
>      client->aio_context = qemu_get_aio_context();
>  
>      /* Read out options */
> -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>  
>      ret = nfs_client_open(client, url, O_CREAT, errp);
>      if (ret < 0) {
> diff --git a/block/qcow.c b/block/qcow.c
> index 67c237f..041af26 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -725,7 +725,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
>      BlockDriverState *qcow_bs;
>  
>      /* Read out options */
> -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                              BDRV_SECTOR_SIZE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
>          flags |= BLOCK_FLAG_ENCRYPT;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f9e045f..c8050e5 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1921,7 +1921,8 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>      int ret;
>  
>      /* Read out options */
> -    sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> +    sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                           BDRV_SECTOR_SIZE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> diff --git a/block/qed.c b/block/qed.c
> index ba395af..f8d9e12 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -648,7 +648,8 @@ static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
>      char *backing_fmt = NULL;
>      int ret;
>  
> -    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
>      cluster_size = qemu_opt_get_size_del(opts,
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index d737f3a..9c22e3f 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1369,8 +1369,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      strstart(filename, "file:", &filename);
>  
>      /* Read out options */
> -    total_size =
> -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                              BDRV_SECTOR_SIZE);
>      nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
>  
>      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> @@ -1966,8 +1966,8 @@ static int hdev_create(const char *filename, QemuOpts *opts,
>      (void)has_prefix;
>  
>      /* Read out options */
> -    total_size =
> -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                              BDRV_SECTOR_SIZE);
>  
>      fd = qemu_open(filename, O_WRONLY | O_BINARY);
>      if (fd < 0) {
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 902eab6..1e1880d 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -511,8 +511,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      strstart(filename, "file:", &filename);
>  
>      /* Read out options */
> -    total_size =
> -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                              BDRV_SECTOR_SIZE);
>  
>      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
>                     0644);
> diff --git a/block/rbd.c b/block/rbd.c
> index ea969e7..b7f7d5f 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -314,7 +314,8 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
>      }
>  
>      /* Read out options */
> -    bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                     BDRV_SECTOR_SIZE);
>      objsize = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, 0);
>      if (objsize) {
>          if ((objsize - 1) & objsize) {    /* not a power of 2? */
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index f91afc3..7da36e1 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1702,7 +1702,8 @@ static int sd_create(const char *filename, QemuOpts *opts,
>          goto out;
>      }
>  
> -    s->inode.vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    s->inode.vdi_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                                 BDRV_SECTOR_SIZE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
>      if (!buf || !strcmp(buf, "off")) {
> diff --git a/block/ssh.c b/block/ssh.c
> index cd2fd75..cf43bc0 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -700,7 +700,8 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
>      ssh_state_init(&s);
>  
>      /* Get desired file size. */
> -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>      DPRINTF("total_size=%" PRIi64, total_size);
>  
>      uri_options = qdict_new();
> diff --git a/block/vdi.c b/block/vdi.c
> index 4b10aac..cfa08b0 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -700,7 +700,8 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
>      logout("\n");
>  
>      /* Read out options. */
> -    bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                     BDRV_SECTOR_SIZE);
>  #if defined(CONFIG_VDI_BLOCK_SIZE)
>      /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
>      block_size = qemu_opt_get_size_del(opts,
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 87c99fc..796b7bd 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1766,7 +1766,8 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
>      VHDXImageType image_type;
>      Error *local_err = NULL;
>  
> -    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>      log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
>      block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
>      type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> diff --git a/block/vmdk.c b/block/vmdk.c
> index a1cb911..afdea1a 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1807,7 +1807,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>          goto exit;
>      }
>      /* Read out options */
> -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>      adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> diff --git a/block/vpc.c b/block/vpc.c
> index 055efc4..5d8fd8e 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -757,7 +757,8 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
>      BlockDriverState *bs = NULL;
>  
>      /* Read out options */
> -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>      disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
>      if (disk_type_param) {
>          if (!strcmp(disk_type_param, "dynamic")) {
> diff --git a/tests/qemu-iotests/104 b/tests/qemu-iotests/104
> new file mode 100755
> index 0000000..b471aa5
> --- /dev/null
> +++ b/tests/qemu-iotests/104
> @@ -0,0 +1,57 @@
> +#!/bin/bash
> +#
> +# Test image creation with aligned and unaligned 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 generic
> +_supported_proto generic
> +_supported_os Linux
> +
> +echo "=== Check qemu-img info output ==="
> +echo
> +image_sizes="1024 1234"
> +
> +for s in $image_sizes; do
> +    _make_test_img $s | _filter_img_create
> +    _img_info | _filter_img_info
> +done
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/104.out b/tests/qemu-iotests/104.out
> new file mode 100644
> index 0000000..de27852
> --- /dev/null
> +++ b/tests/qemu-iotests/104.out
> @@ -0,0 +1,12 @@
> +QA output created by 104
> +=== 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)
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1234 
> +image: TEST_DIR/t.IMGFMT
> +file format: IMGFMT
> +virtual size: 1.5K (1536 bytes)
> +***done
> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
> index 51192c8..f69cb6b 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -192,5 +192,26 @@ _filter_img_create()
>          -e "s/archipelago:a/TEST_DIR\//g"
>  }
>  
> +_filter_img_info()
> +{
> +    sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
> +        -e "s#$TEST_DIR#TEST_DIR#g" \
> +        -e "s#$IMGFMT#IMGFMT#g" \
> +        -e "/encrypted: yes/d" \
> +        -e "/cluster_size: [0-9]\\+/d" \
> +        -e "/table_size: [0-9]\\+/d" \
> +        -e "/compat: '[^']*'/d" \
> +        -e "/compat6: \\(on\\|off\\)/d" \
> +        -e "/static: \\(on\\|off\\)/d" \
> +        -e "/zeroed_grain: \\(on\\|off\\)/d" \
> +        -e "/subformat: '[^']*'/d" \
> +        -e "/adapter_type: '[^']*'/d" \
> +        -e "/lazy_refcounts: \\(on\\|off\\)/d" \
> +        -e "/block_size: [0-9]\\+/d" \
> +        -e "/block_state_zero: \\(on\\|off\\)/d" \
> +        -e "/log_size: [0-9]\\+/d" \
> +        -e "s/archipelago:a/TEST_DIR\//g"
> +}
> +
>  # make sure this script returns success
>  /bin/true
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 0920b28..622685e 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -104,3 +104,4 @@
>  100 rw auto quick
>  101 rw auto quick
>  103 rw auto quick
> +104 rw auto
> -- 
> 1.9.3

seems correct but the patch does not apply anymore on latest Kevin/block
branch neither with git am nor with git apply..
Could you rebase ?
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 2/5] block: don't convert file size to sector size
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 2/5] block: don't convert file size to sector size Hu Tao
@ 2014-09-09 12:26   ` Benoît Canet
  0 siblings, 0 replies; 16+ messages in thread
From: Benoît Canet @ 2014-09-09 12:26 UTC (permalink / raw)
  To: Hu Tao
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

The Tuesday 09 Sep 2014 à 11:54:28 (+0800), Hu Tao wrote :
> and avoid converting it back later.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/gluster.c   |  9 ++++-----
>  block/qcow.c      |  8 ++++----
>  block/qcow2.c     | 10 +++++-----
>  block/raw-posix.c | 12 ++++++------
>  block/raw-win32.c |  6 +++---
>  5 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index 65c7a58..1eb3a8c 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -494,8 +494,8 @@ static int qemu_gluster_create(const char *filename,
>          goto out;
>      }
>  
> -    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> -                              BDRV_SECTOR_SIZE);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>  
>      tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
>      if (!tmp || !strcmp(tmp, "off")) {
> @@ -516,9 +516,8 @@ static int qemu_gluster_create(const char *filename,
>      if (!fd) {
>          ret = -errno;
>      } else {
> -        if (!glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE)) {
> -            if (prealloc && qemu_gluster_zerofill(fd, 0,
> -                    total_size * BDRV_SECTOR_SIZE)) {
> +        if (!glfs_ftruncate(fd, total_size)) {
> +            if (prealloc && qemu_gluster_zerofill(fd, 0, total_size)) {
>                  ret = -errno;
>              }
>          } else {
> diff --git a/block/qcow.c b/block/qcow.c
> index 041af26..a87bd69 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -725,8 +725,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
>      BlockDriverState *qcow_bs;
>  
>      /* Read out options */
> -    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> -                              BDRV_SECTOR_SIZE);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
>          flags |= BLOCK_FLAG_ENCRYPT;
> @@ -754,7 +754,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
>      memset(&header, 0, sizeof(header));
>      header.magic = cpu_to_be32(QCOW_MAGIC);
>      header.version = cpu_to_be32(QCOW_VERSION);
> -    header.size = cpu_to_be64(total_size * 512);
> +    header.size = cpu_to_be64(total_size);
>      header_size = sizeof(header);
>      backing_filename_len = 0;
>      if (backing_file) {
> @@ -776,7 +776,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
>      }
>      header_size = (header_size + 7) & ~7;
>      shift = header.cluster_bits + header.l2_bits;
> -    l1_size = ((total_size * 512) + (1LL << shift) - 1) >> shift;
> +    l1_size = (total_size + (1LL << shift) - 1) >> shift;
>  
>      header.l1_table_offset = cpu_to_be64(header_size);
>      if (flags & BLOCK_FLAG_ENCRYPT) {
> diff --git a/block/qcow2.c b/block/qcow2.c
> index c8050e5..cf27c3f 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1859,7 +1859,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;
> @@ -1912,7 +1912,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>      char *backing_file = NULL;
>      char *backing_fmt = NULL;
>      char *buf = NULL;
> -    uint64_t sectors = 0;
> +    uint64_t size = 0;
>      int flags = 0;
>      size_t cluster_size = DEFAULT_CLUSTER_SIZE;
>      int prealloc = 0;
> @@ -1921,8 +1921,8 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>      int ret;
>  
>      /* Read out options */
> -    sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> -                           BDRV_SECTOR_SIZE);
> +    size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                    BDRV_SECTOR_SIZE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> @@ -1972,7 +1972,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>          goto finish;
>      }
>  
> -    ret = qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
> +    ret = qcow2_create2(filename, size, backing_file, backing_fmt, flags,
>                          cluster_size, prealloc, opts, version, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 9c22e3f..7208c05 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1369,8 +1369,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      strstart(filename, "file:", &filename);
>  
>      /* Read out options */
> -    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> -                              BDRV_SECTOR_SIZE);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>      nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
>  
>      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> @@ -1394,7 +1394,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>  #endif
>          }
>  
> -        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");
>          }
> @@ -1966,8 +1966,8 @@ static int hdev_create(const char *filename, QemuOpts *opts,
>      (void)has_prefix;
>  
>      /* Read out options */
> -    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> -                              BDRV_SECTOR_SIZE);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>  
>      fd = qemu_open(filename, O_WRONLY | O_BINARY);
>      if (fd < 0) {
> @@ -1983,7 +1983,7 @@ static int hdev_create(const char *filename, QemuOpts *opts,
>          error_setg(errp,
>                     "The given file is neither a block nor a character device");
>          ret = -ENODEV;
> -    } else if (lseek(fd, 0, SEEK_END) < total_size * BDRV_SECTOR_SIZE) {
> +    } else if (lseek(fd, 0, SEEK_END) < total_size) {
>          error_setg(errp, "Device is too small");
>          ret = -ENOSPC;
>      }
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 1e1880d..9bf8225 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -511,8 +511,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      strstart(filename, "file:", &filename);
>  
>      /* Read out options */
> -    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> -                              BDRV_SECTOR_SIZE);
> +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> +                          BDRV_SECTOR_SIZE);
>  
>      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
>                     0644);
> @@ -521,7 +521,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>          return -EIO;
>      }
>      set_sparse(fd);
> -    ftruncate(fd, total_size * 512);
> +    ftruncate(fd, total_size);
>      qemu_close(fd);
>      return 0;
>  }
> -- 
> 1.9.3
> 
> 

Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc.
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc Hu Tao
@ 2014-09-09 12:42   ` Eric Blake
  2014-09-10  1:44     ` Hu Tao
  2014-09-09 12:45   ` Benoît Canet
  1 sibling, 1 reply; 16+ messages in thread
From: Eric Blake @ 2014-09-09 12:42 UTC (permalink / raw)
  To: Hu Tao, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

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

On 09/08/2014 09:54 PM, Hu Tao wrote:
> This patch prepares for the subsequent patches.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c              | 23 +++++++++++++++--------
>  qapi/block-core.json       | 17 +++++++++++++++++
>  tests/qemu-iotests/049.out |  2 +-
>  3 files changed, 33 insertions(+), 9 deletions(-)
> 

>      buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> -    if (!buf || !strcmp(buf, "off")) {
> -        prealloc = 0;
> -    } else if (!strcmp(buf, "metadata")) {
> -        prealloc = 1;
> -    } else {
> -        error_setg(errp, "Invalid preallocation mode: '%s'", buf);
> +    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> +                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> +                               &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
>          ret = -EINVAL;
>          goto finish;
>      }
> @@ -1958,6 +1958,13 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>          flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
>      }
>  
> +    if (prealloc && prealloc != PREALLOC_MODE_METADATA) {
> +        ret = -EINVAL;
> +        error_setg(errp, "Unsupported preallocate mode: %s",
> +                   PreallocMode_lookup[prealloc]);
> +        goto finish;
> +    }

I _still_ think this looks weird, and would be better as either:

if (prealloc != PREALLOC_MODE_NONE &&
    prealloc != PREALLOC_MODE_METADATA) {

to make it obvious that you are filtering for two acceptable modes, or as:

if (prealloc == PREALLOC_MODE_FALLOC ||
    prealloc == PREALLOC_MODE_FULL) {

to make it obvious the modes that you do not support.  But my complaint
is not strong enough to prevent this patch, especially if later in the
series revisits this code.

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

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


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc.
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc Hu Tao
  2014-09-09 12:42   ` Eric Blake
@ 2014-09-09 12:45   ` Benoît Canet
  2014-09-10  1:41     ` Hu Tao
  1 sibling, 1 reply; 16+ messages in thread
From: Benoît Canet @ 2014-09-09 12:45 UTC (permalink / raw)
  To: Hu Tao
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

The Tuesday 09 Sep 2014 à 11:54:29 (+0800), Hu Tao wrote :
> This patch prepares for the subsequent patches.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c              | 23 +++++++++++++++--------
>  qapi/block-core.json       | 17 +++++++++++++++++
>  tests/qemu-iotests/049.out |  2 +-
>  3 files changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index cf27c3f..94d1225 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"
>  #include "qemu/option_int.h"
>  
> @@ -1738,7 +1739,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,
>                           QemuOpts *opts, int version,
>                           Error **errp)

below that.

Carefull study of the code tell us that here prealloc will be 0 or 1
but i think you could prepare a bit sooner the next patch by doing:

-    if (prealloc) {                                                             
+    if (prealloc == PREALLOC_MODE_METADATA) {
            BDRVQcowState *s = bs->opaque;

in the same qcow2_create2 function.


If you do so someone who start reading the code at this precise commit will
not have to lookup the declaration order of PreallocMode in the QAPI file.

>  {
> @@ -1915,7 +1916,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>      uint64_t size = 0;
>      int flags = 0;
>      size_t cluster_size = DEFAULT_CLUSTER_SIZE;
> -    int prealloc = 0;
> +    PreallocMode prealloc;
>      int version = 3;
>      Error *local_err = NULL;
>      int ret;
> @@ -1931,12 +1932,11 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>      cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
>                                           DEFAULT_CLUSTER_SIZE);
>      buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> -    if (!buf || !strcmp(buf, "off")) {
> -        prealloc = 0;
> -    } else if (!strcmp(buf, "metadata")) {
> -        prealloc = 1;
> -    } else {
> -        error_setg(errp, "Invalid preallocation mode: '%s'", buf);
> +    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> +                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> +                               &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
>          ret = -EINVAL;
>          goto finish;
>      }
> @@ -1958,6 +1958,13 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>          flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
>      }
>  
> +    if (prealloc && prealloc != PREALLOC_MODE_METADATA) {
> +        ret = -EINVAL;
> +        error_setg(errp, "Unsupported preallocate mode: %s",
> +                   PreallocMode_lookup[prealloc]);
> +        goto finish;
> +    }
> +
>      if (backing_file && prealloc) {
>          error_setg(errp, "Backing file and preallocation cannot be used at "
>                     "the same time");
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index a685d02..a29dbe1 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1697,3 +1697,20 @@
>              'len'   : 'int',
>              'offset': 'int',
>              'speed' : 'int' } }
> +
> +# @PreallocMode
> +#
> +# Preallocation mode of QEMU image file
> +#
> +# @off: no preallocation
> +# @metadata: preallocate only for metadata
> +# @falloc: like @full preallocation but allocate disk space by
> +#          posix_fallocate() rather than writing zeros.
> +# @full: preallocate all data by writing zeros to device to ensure disk
> +#        space is really available. @full preallocation also sets up
> +#        metadata correctly.
> +#
> +# Since 2.2
> +##
> +{ 'enum': 'PreallocMode',
> +  'data': [ 'off', 'metadata', 'falloc', 'full' ] }
> diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
> index 71ca44d..09ca0ae 100644
> --- a/tests/qemu-iotests/049.out
> +++ b/tests/qemu-iotests/049.out
> @@ -179,7 +179,7 @@ qemu-img create -f qcow2 -o preallocation=metadata TEST_DIR/t.qcow2 64M
>  Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='metadata' lazy_refcounts=off 
>  
>  qemu-img create -f qcow2 -o preallocation=1234 TEST_DIR/t.qcow2 64M
> -qemu-img: TEST_DIR/t.qcow2: Invalid preallocation mode: '1234'
> +qemu-img: TEST_DIR/t.qcow2: invalid parameter value: 1234
>  Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='1234' lazy_refcounts=off 
>  
>  == Check encryption option ==
> -- 
> 1.9.3
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option
  2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option Hu Tao
@ 2014-09-09 13:21   ` Benoît Canet
  2014-09-10  2:02     ` Hu Tao
  0 siblings, 1 reply; 16+ messages in thread
From: Benoît Canet @ 2014-09-09 13:21 UTC (permalink / raw)
  To: Hu Tao
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

The Tuesday 09 Sep 2014 à 11:54:30 (+0800), Hu Tao wrote :
> This patch adds a new option preallocation for raw format, and implements
> falloc and full preallocation.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-posix.c | 93 +++++++++++++++++++++++++++++++++++++++++++------------
>  qemu-doc.texi     |  9 ++++++
>  qemu-img.texi     |  9 ++++++
>  3 files changed, 92 insertions(+), 19 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 7208c05..27c854c 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>
> @@ -1365,6 +1366,9 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      int result = 0;
>      int64_t total_size = 0;
>      bool nocow = false;
> +    PreallocMode prealloc;
> +    char *buf = NULL;
> +    Error *local_err = NULL;
>  
>      strstart(filename, "file:", &filename);
>  
> @@ -1372,37 +1376,83 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
>                            BDRV_SECTOR_SIZE);
>      nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
> +    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> +    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> +                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> +                               &local_err);
> +    g_free(buf);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        result = -EINVAL;
> +        goto out;
> +    }
>  
>      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
>                     0644);
>      if (fd < 0) {
>          result = -errno;
>          error_setg_errno(errp, -result, "Could not create file");
> -    } else {
> -        if (nocow) {
> +        goto out;
> +    }
> +
> +    if (nocow) {
>  #ifdef __linux__
> -            /* Set NOCOW flag to solve performance issue on fs like btrfs.
> -             * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
> -             * will be ignored since any failure of this operation should not
> -             * block the left work.
> -             */
> -            int attr;
> -            if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> -                attr |= FS_NOCOW_FL;
> -                ioctl(fd, FS_IOC_SETFLAGS, &attr);
> -            }
> -#endif
> +        /* Set NOCOW flag to solve performance issue on fs like btrfs.
> +         * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
> +         * will be ignored since any failure of this operation should not
> +         * block the left work.
> +         */
> +        int attr;
> +        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> +            attr |= FS_NOCOW_FL;
> +            ioctl(fd, FS_IOC_SETFLAGS, &attr);
>          }
> +#endif
> +    }
>  
> -        if (ftruncate(fd, total_size) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not resize file");
> +    if (ftruncate(fd, total_size) != 0) {
> +        result = -errno;
> +        error_setg_errno(errp, -result, "Could not resize file");
> +        goto out_close;
> +    }
> +
> +    if (prealloc == PREALLOC_MODE_FALLOC) {
> +        /* 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");

Here you choose not to goto out_close but let the code flow lead to it.

>          }

> -        if (qemu_close(fd) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not close the new file");
> +    } else if (prealloc == PREALLOC_MODE_FULL) {
> +        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;

As a consequence you could replace the two previous line by:

+                break;

The while loop would gently exit left would be decremented and fsync would
be done but we don't care these sides effects and buf would be freed.
Then the code would exit the if branch and reach out_close.

(I heard some people don't like exiting a loop with a goto).


> +            }
> +            left -= num;
>          }
> +        fsync(fd);
> +        g_free(buf);
> +    } else if (prealloc != PREALLOC_MODE_OFF) {
> +        result = -EINVAL;
> +        error_setg(errp, "Unsupported preallocation mode: %s",
> +                   PreallocMode_lookup[prealloc]);
>      }
> +
> +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;
>  }
>  
> @@ -1585,6 +1635,11 @@ static QemuOptsList raw_create_opts = {
>              .type = QEMU_OPT_BOOL,
>              .help = "Turn off copy-on-write (valid only on btrfs)"
>          },
> +        {
> +            .name = BLOCK_OPT_PREALLOC,
> +            .type = QEMU_OPT_STRING,
> +            .help = "Preallocation mode (allowed values: off, falloc, full)"
> +        },
>          { /* end of list */ }
>      }
>  };
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 2b232ae..1f289d6 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -527,6 +527,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
>  space. Use @code{qemu-img info} to know the real size used by the
>  image or @code{ls -ls} on Unix/Linux.
>  
> +Supported options:
> +@table @code
> +@item preallocation
> +Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
> +@code{falloc} mode preallocates space for image by calling posix_fallocate().
> +@code{full} mode preallocates space for image by writing zeros to underlying
> +storage.
> +@end table
> +
>  @item qcow2
>  QEMU image format, the most versatile format. Use it to have smaller
>  images (useful if your filesystem does not supports holes, for example
> diff --git a/qemu-img.texi b/qemu-img.texi
> index cc4668e..d64d05e 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -419,6 +419,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
>  space. Use @code{qemu-img info} to know the real size used by the
>  image or @code{ls -ls} on Unix/Linux.
>  
> +Supported options:
> +@table @code
> +@item preallocation
> +Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
> +@code{falloc} mode preallocates space for image by calling posix_fallocate().
> +@code{full} mode preallocates space for image by writing zeros to underlying
> +storage.
> +@end table
> +
>  @item qcow2
>  QEMU image format, the most versatile format. Use it to have smaller
>  images (useful if your filesystem does not supports holes, for example
> -- 
> 1.9.3
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc.
  2014-09-09 12:45   ` Benoît Canet
@ 2014-09-10  1:41     ` Hu Tao
  0 siblings, 0 replies; 16+ messages in thread
From: Hu Tao @ 2014-09-10  1:41 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

On Tue, Sep 09, 2014 at 02:45:56PM +0200, Benoît Canet wrote:
> The Tuesday 09 Sep 2014 à 11:54:29 (+0800), Hu Tao wrote :
> > This patch prepares for the subsequent patches.
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block/qcow2.c              | 23 +++++++++++++++--------
> >  qapi/block-core.json       | 17 +++++++++++++++++
> >  tests/qemu-iotests/049.out |  2 +-
> >  3 files changed, 33 insertions(+), 9 deletions(-)
> > 
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index cf27c3f..94d1225 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"
> >  #include "qemu/option_int.h"
> >  
> > @@ -1738,7 +1739,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,
> >                           QemuOpts *opts, int version,
> >                           Error **errp)
> 
> below that.
> 
> Carefull study of the code tell us that here prealloc will be 0 or 1
> but i think you could prepare a bit sooner the next patch by doing:
> 
> -    if (prealloc) {                                                             
> +    if (prealloc == PREALLOC_MODE_METADATA) {
>             BDRVQcowState *s = bs->opaque;
> 
> in the same qcow2_create2 function.
> 
> 
> If you do so someone who start reading the code at this precise commit will
> not have to lookup the declaration order of PreallocMode in the QAPI file.

Thanks for your suggestion, I think this makes much sense.

Regards,
Hu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc.
  2014-09-09 12:42   ` Eric Blake
@ 2014-09-10  1:44     ` Hu Tao
  0 siblings, 0 replies; 16+ messages in thread
From: Hu Tao @ 2014-09-10  1:44 UTC (permalink / raw)
  To: Eric Blake
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

On Tue, Sep 09, 2014 at 06:42:13AM -0600, Eric Blake wrote:
> On 09/08/2014 09:54 PM, Hu Tao wrote:
> > This patch prepares for the subsequent patches.
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block/qcow2.c              | 23 +++++++++++++++--------
> >  qapi/block-core.json       | 17 +++++++++++++++++
> >  tests/qemu-iotests/049.out |  2 +-
> >  3 files changed, 33 insertions(+), 9 deletions(-)
> > 
> 
> >      buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> > -    if (!buf || !strcmp(buf, "off")) {
> > -        prealloc = 0;
> > -    } else if (!strcmp(buf, "metadata")) {
> > -        prealloc = 1;
> > -    } else {
> > -        error_setg(errp, "Invalid preallocation mode: '%s'", buf);
> > +    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> > +                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> > +                               &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> >          ret = -EINVAL;
> >          goto finish;
> >      }
> > @@ -1958,6 +1958,13 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
> >          flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
> >      }
> >  
> > +    if (prealloc && prealloc != PREALLOC_MODE_METADATA) {
> > +        ret = -EINVAL;
> > +        error_setg(errp, "Unsupported preallocate mode: %s",
> > +                   PreallocMode_lookup[prealloc]);
> > +        goto finish;
> > +    }
> 
> I _still_ think this looks weird, and would be better as either:
> 
> if (prealloc != PREALLOC_MODE_NONE &&
>     prealloc != PREALLOC_MODE_METADATA) {
> 
> to make it obvious that you are filtering for two acceptable modes, or as:
> 
> if (prealloc == PREALLOC_MODE_FALLOC ||
>     prealloc == PREALLOC_MODE_FULL) {
> 
> to make it obvious the modes that you do not support.  But my complaint
> is not strong enough to prevent this patch, especially if later in the
> series revisits this code.

After reading Benoît's comment, I understand why you think the code
looks weird. I'll change it as you suggested. Thanks! 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector
  2014-09-09 12:12   ` Benoît Canet
@ 2014-09-10  1:50     ` Hu Tao
  2014-09-10  2:36       ` Hu Tao
  0 siblings, 1 reply; 16+ messages in thread
From: Hu Tao @ 2014-09-10  1:50 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

On Tue, Sep 09, 2014 at 02:12:18PM +0200, Benoît Canet wrote:
> The Tuesday 09 Sep 2014 à 11:54:27 (+0800), Hu Tao wrote :
> 
> Taking the time to systematically write a nice sentence in
> the commit message body about why your commit exists and
> explaining the long term purpose of the patch will:
> 
> -improve the quality of your commit
> -please everyone involved in the review
> -ultimately help your to get the patch merged faster

Okay.

> 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > ---
> >  block/archipelago.c              |  3 ++-
> >  block/cow.c                      |  3 ++-
> >  block/gluster.c                  |  4 +--
> >  block/iscsi.c                    |  4 +--
> >  block/nfs.c                      |  3 ++-
> >  block/qcow.c                     |  3 ++-
> >  block/qcow2.c                    |  3 ++-
> >  block/qed.c                      |  3 ++-
> >  block/raw-posix.c                |  8 +++---
> >  block/raw-win32.c                |  4 +--
> >  block/rbd.c                      |  3 ++-
> >  block/sheepdog.c                 |  3 ++-
> >  block/ssh.c                      |  3 ++-
> >  block/vdi.c                      |  3 ++-
> >  block/vhdx.c                     |  3 ++-
> >  block/vmdk.c                     |  3 ++-
> >  block/vpc.c                      |  3 ++-
> >  tests/qemu-iotests/104           | 57 ++++++++++++++++++++++++++++++++++++++++
> >  tests/qemu-iotests/104.out       | 12 +++++++++
> >  tests/qemu-iotests/common.filter | 21 +++++++++++++++
> >  tests/qemu-iotests/group         |  1 +
> >  21 files changed, 127 insertions(+), 23 deletions(-)
> >  create mode 100755 tests/qemu-iotests/104
> >  create mode 100644 tests/qemu-iotests/104.out
> > 
> > diff --git a/block/archipelago.c b/block/archipelago.c
> > index 22a7daa..06c51f9 100644
> > --- a/block/archipelago.c
> > +++ b/block/archipelago.c
> > @@ -708,7 +708,8 @@ static int qemu_archipelago_create(const char *filename,
> >  
> >      parse_filename_opts(filename, errp, &volname, &segment_name, &mport,
> >                          &vport);
> > -    total_size = qemu_opt_get_size_del(options, BLOCK_OPT_SIZE, 0);
> > +    total_size = ROUND_UP(qemu_opt_get_size_del(options, BLOCK_OPT_SIZE, 0),
> > +                          BDRV_SECTOR_SIZE);
> >  
> >      if (segment_name == NULL) {
> >          segment_name = g_strdup("archipelago");
> > diff --git a/block/cow.c b/block/cow.c
> > index 6ee4833..c3769fe 100644
> > --- a/block/cow.c
> > +++ b/block/cow.c
> > @@ -335,7 +335,8 @@ static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
> >      BlockDriverState *cow_bs = NULL;
> >  
> >      /* Read out options */
> > -    image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> > +    image_sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                                 BDRV_SECTOR_SIZE);
> >      image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> >  
> >      ret = bdrv_create_file(filename, opts, &local_err);
> > diff --git a/block/gluster.c b/block/gluster.c
> > index 1912cf9..65c7a58 100644
> > --- a/block/gluster.c
> > +++ b/block/gluster.c
> > @@ -494,8 +494,8 @@ static int qemu_gluster_create(const char *filename,
> >          goto out;
> >      }
> >  
> > -    total_size =
> > -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> > +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                              BDRV_SECTOR_SIZE);
> >  
> >      tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> >      if (!tmp || !strcmp(tmp, "off")) {
> > diff --git a/block/iscsi.c b/block/iscsi.c
> > index 3e19202..84bcae8 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
> > @@ -1531,8 +1531,8 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
> >      bs = bdrv_new("", &error_abort);
> >  
> >      /* Read out options */
> > -    total_size =
> > -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> > +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                              BDRV_SECTOR_SIZE);
> >      bs->opaque = g_new0(struct IscsiLun, 1);
> >      iscsilun = bs->opaque;
> >  
> > diff --git a/block/nfs.c b/block/nfs.c
> > index 194f301..c76e368 100644
> > --- a/block/nfs.c
> > +++ b/block/nfs.c
> > @@ -418,7 +418,8 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
> >      client->aio_context = qemu_get_aio_context();
> >  
> >      /* Read out options */
> > -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                          BDRV_SECTOR_SIZE);
> >  
> >      ret = nfs_client_open(client, url, O_CREAT, errp);
> >      if (ret < 0) {
> > diff --git a/block/qcow.c b/block/qcow.c
> > index 67c237f..041af26 100644
> > --- a/block/qcow.c
> > +++ b/block/qcow.c
> > @@ -725,7 +725,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
> >      BlockDriverState *qcow_bs;
> >  
> >      /* Read out options */
> > -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> > +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                              BDRV_SECTOR_SIZE);
> >      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> >      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> >          flags |= BLOCK_FLAG_ENCRYPT;
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index f9e045f..c8050e5 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -1921,7 +1921,8 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
> >      int ret;
> >  
> >      /* Read out options */
> > -    sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> > +    sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                           BDRV_SECTOR_SIZE);
> >      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> >      backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
> >      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> > diff --git a/block/qed.c b/block/qed.c
> > index ba395af..f8d9e12 100644
> > --- a/block/qed.c
> > +++ b/block/qed.c
> > @@ -648,7 +648,8 @@ static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
> >      char *backing_fmt = NULL;
> >      int ret;
> >  
> > -    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                          BDRV_SECTOR_SIZE);
> >      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> >      backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
> >      cluster_size = qemu_opt_get_size_del(opts,
> > diff --git a/block/raw-posix.c b/block/raw-posix.c
> > index d737f3a..9c22e3f 100644
> > --- a/block/raw-posix.c
> > +++ b/block/raw-posix.c
> > @@ -1369,8 +1369,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
> >      strstart(filename, "file:", &filename);
> >  
> >      /* Read out options */
> > -    total_size =
> > -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> > +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                              BDRV_SECTOR_SIZE);
> >      nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
> >  
> >      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> > @@ -1966,8 +1966,8 @@ static int hdev_create(const char *filename, QemuOpts *opts,
> >      (void)has_prefix;
> >  
> >      /* Read out options */
> > -    total_size =
> > -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
> > +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                              BDRV_SECTOR_SIZE);
> >  
> >      fd = qemu_open(filename, O_WRONLY | O_BINARY);
> >      if (fd < 0) {
> > diff --git a/block/raw-win32.c b/block/raw-win32.c
> > index 902eab6..1e1880d 100644
> > --- a/block/raw-win32.c
> > +++ b/block/raw-win32.c
> > @@ -511,8 +511,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
> >      strstart(filename, "file:", &filename);
> >  
> >      /* Read out options */
> > -    total_size =
> > -        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> > +    total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                              BDRV_SECTOR_SIZE);
> >  
> >      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> >                     0644);
> > diff --git a/block/rbd.c b/block/rbd.c
> > index ea969e7..b7f7d5f 100644
> > --- a/block/rbd.c
> > +++ b/block/rbd.c
> > @@ -314,7 +314,8 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
> >      }
> >  
> >      /* Read out options */
> > -    bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                     BDRV_SECTOR_SIZE);
> >      objsize = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, 0);
> >      if (objsize) {
> >          if ((objsize - 1) & objsize) {    /* not a power of 2? */
> > diff --git a/block/sheepdog.c b/block/sheepdog.c
> > index f91afc3..7da36e1 100644
> > --- a/block/sheepdog.c
> > +++ b/block/sheepdog.c
> > @@ -1702,7 +1702,8 @@ static int sd_create(const char *filename, QemuOpts *opts,
> >          goto out;
> >      }
> >  
> > -    s->inode.vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    s->inode.vdi_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                                 BDRV_SECTOR_SIZE);
> >      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> >      buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> >      if (!buf || !strcmp(buf, "off")) {
> > diff --git a/block/ssh.c b/block/ssh.c
> > index cd2fd75..cf43bc0 100644
> > --- a/block/ssh.c
> > +++ b/block/ssh.c
> > @@ -700,7 +700,8 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
> >      ssh_state_init(&s);
> >  
> >      /* Get desired file size. */
> > -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                          BDRV_SECTOR_SIZE);
> >      DPRINTF("total_size=%" PRIi64, total_size);
> >  
> >      uri_options = qdict_new();
> > diff --git a/block/vdi.c b/block/vdi.c
> > index 4b10aac..cfa08b0 100644
> > --- a/block/vdi.c
> > +++ b/block/vdi.c
> > @@ -700,7 +700,8 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
> >      logout("\n");
> >  
> >      /* Read out options. */
> > -    bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                     BDRV_SECTOR_SIZE);
> >  #if defined(CONFIG_VDI_BLOCK_SIZE)
> >      /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
> >      block_size = qemu_opt_get_size_del(opts,
> > diff --git a/block/vhdx.c b/block/vhdx.c
> > index 87c99fc..796b7bd 100644
> > --- a/block/vhdx.c
> > +++ b/block/vhdx.c
> > @@ -1766,7 +1766,8 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
> >      VHDXImageType image_type;
> >      Error *local_err = NULL;
> >  
> > -    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                          BDRV_SECTOR_SIZE);
> >      log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
> >      block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
> >      type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index a1cb911..afdea1a 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -1807,7 +1807,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
> >          goto exit;
> >      }
> >      /* Read out options */
> > -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                          BDRV_SECTOR_SIZE);
> >      adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
> >      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> >      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> > diff --git a/block/vpc.c b/block/vpc.c
> > index 055efc4..5d8fd8e 100644
> > --- a/block/vpc.c
> > +++ b/block/vpc.c
> > @@ -757,7 +757,8 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
> >      BlockDriverState *bs = NULL;
> >  
> >      /* Read out options */
> > -    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > +    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > +                          BDRV_SECTOR_SIZE);
> >      disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> >      if (disk_type_param) {
> >          if (!strcmp(disk_type_param, "dynamic")) {
> > diff --git a/tests/qemu-iotests/104 b/tests/qemu-iotests/104
> > new file mode 100755
> > index 0000000..b471aa5
> > --- /dev/null
> > +++ b/tests/qemu-iotests/104
> > @@ -0,0 +1,57 @@
> > +#!/bin/bash
> > +#
> > +# Test image creation with aligned and unaligned 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 generic
> > +_supported_proto generic
> > +_supported_os Linux
> > +
> > +echo "=== Check qemu-img info output ==="
> > +echo
> > +image_sizes="1024 1234"
> > +
> > +for s in $image_sizes; do
> > +    _make_test_img $s | _filter_img_create
> > +    _img_info | _filter_img_info
> > +done
> > +
> > +# success, all done
> > +echo "*** done"
> > +rm -f $seq.full
> > +status=0
> > diff --git a/tests/qemu-iotests/104.out b/tests/qemu-iotests/104.out
> > new file mode 100644
> > index 0000000..de27852
> > --- /dev/null
> > +++ b/tests/qemu-iotests/104.out
> > @@ -0,0 +1,12 @@
> > +QA output created by 104
> > +=== 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)
> > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1234 
> > +image: TEST_DIR/t.IMGFMT
> > +file format: IMGFMT
> > +virtual size: 1.5K (1536 bytes)
> > +***done
> > diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
> > index 51192c8..f69cb6b 100644
> > --- a/tests/qemu-iotests/common.filter
> > +++ b/tests/qemu-iotests/common.filter
> > @@ -192,5 +192,26 @@ _filter_img_create()
> >          -e "s/archipelago:a/TEST_DIR\//g"
> >  }
> >  
> > +_filter_img_info()
> > +{
> > +    sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
> > +        -e "s#$TEST_DIR#TEST_DIR#g" \
> > +        -e "s#$IMGFMT#IMGFMT#g" \
> > +        -e "/encrypted: yes/d" \
> > +        -e "/cluster_size: [0-9]\\+/d" \
> > +        -e "/table_size: [0-9]\\+/d" \
> > +        -e "/compat: '[^']*'/d" \
> > +        -e "/compat6: \\(on\\|off\\)/d" \
> > +        -e "/static: \\(on\\|off\\)/d" \
> > +        -e "/zeroed_grain: \\(on\\|off\\)/d" \
> > +        -e "/subformat: '[^']*'/d" \
> > +        -e "/adapter_type: '[^']*'/d" \
> > +        -e "/lazy_refcounts: \\(on\\|off\\)/d" \
> > +        -e "/block_size: [0-9]\\+/d" \
> > +        -e "/block_state_zero: \\(on\\|off\\)/d" \
> > +        -e "/log_size: [0-9]\\+/d" \
> > +        -e "s/archipelago:a/TEST_DIR\//g"
> > +}
> > +
> >  # make sure this script returns success
> >  /bin/true
> > diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> > index 0920b28..622685e 100644
> > --- a/tests/qemu-iotests/group
> > +++ b/tests/qemu-iotests/group
> > @@ -104,3 +104,4 @@
> >  100 rw auto quick
> >  101 rw auto quick
> >  103 rw auto quick
> > +104 rw auto
> > -- 
> > 1.9.3
> 
> seems correct but the patch does not apply anymore on latest Kevin/block
> branch neither with git am nor with git apply..
> Could you rebase ?

I made it on top of master. I'll respin basing on Kevin/block.

Regards,
Hu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option
  2014-09-09 13:21   ` Benoît Canet
@ 2014-09-10  2:02     ` Hu Tao
  0 siblings, 0 replies; 16+ messages in thread
From: Hu Tao @ 2014-09-10  2:02 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Kevin Wolf, Fam Zheng, Richard W.M. Jones, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

On Tue, Sep 09, 2014 at 03:21:49PM +0200, Benoît Canet wrote:
> The Tuesday 09 Sep 2014 à 11:54:30 (+0800), Hu Tao wrote :
> > This patch adds a new option preallocation for raw format, and implements
> > falloc and full preallocation.
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > ---
> >  block/raw-posix.c | 93 +++++++++++++++++++++++++++++++++++++++++++------------
> >  qemu-doc.texi     |  9 ++++++
> >  qemu-img.texi     |  9 ++++++
> >  3 files changed, 92 insertions(+), 19 deletions(-)
> > 
> > diff --git a/block/raw-posix.c b/block/raw-posix.c
> > index 7208c05..27c854c 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>
> > @@ -1365,6 +1366,9 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
> >      int result = 0;
> >      int64_t total_size = 0;
> >      bool nocow = false;
> > +    PreallocMode prealloc;
> > +    char *buf = NULL;
> > +    Error *local_err = NULL;
> >  
> >      strstart(filename, "file:", &filename);
> >  
> > @@ -1372,37 +1376,83 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
> >      total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> >                            BDRV_SECTOR_SIZE);
> >      nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
> > +    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> > +    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> > +                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> > +                               &local_err);
> > +    g_free(buf);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        result = -EINVAL;
> > +        goto out;
> > +    }
> >  
> >      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> >                     0644);
> >      if (fd < 0) {
> >          result = -errno;
> >          error_setg_errno(errp, -result, "Could not create file");
> > -    } else {
> > -        if (nocow) {
> > +        goto out;
> > +    }
> > +
> > +    if (nocow) {
> >  #ifdef __linux__
> > -            /* Set NOCOW flag to solve performance issue on fs like btrfs.
> > -             * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
> > -             * will be ignored since any failure of this operation should not
> > -             * block the left work.
> > -             */
> > -            int attr;
> > -            if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> > -                attr |= FS_NOCOW_FL;
> > -                ioctl(fd, FS_IOC_SETFLAGS, &attr);
> > -            }
> > -#endif
> > +        /* Set NOCOW flag to solve performance issue on fs like btrfs.
> > +         * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
> > +         * will be ignored since any failure of this operation should not
> > +         * block the left work.
> > +         */
> > +        int attr;
> > +        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> > +            attr |= FS_NOCOW_FL;
> > +            ioctl(fd, FS_IOC_SETFLAGS, &attr);
> >          }
> > +#endif
> > +    }
> >  
> > -        if (ftruncate(fd, total_size) != 0) {
> > -            result = -errno;
> > -            error_setg_errno(errp, -result, "Could not resize file");
> > +    if (ftruncate(fd, total_size) != 0) {
> > +        result = -errno;
> > +        error_setg_errno(errp, -result, "Could not resize file");
> > +        goto out_close;
> > +    }
> > +
> > +    if (prealloc == PREALLOC_MODE_FALLOC) {
> > +        /* 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");
> 
> Here you choose not to goto out_close but let the code flow lead to it.
> 
> >          }
> 
> > -        if (qemu_close(fd) != 0) {
> > -            result = -errno;
> > -            error_setg_errno(errp, -result, "Could not close the new file");
> > +    } else if (prealloc == PREALLOC_MODE_FULL) {
> > +        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;
> 
> As a consequence you could replace the two previous line by:
> 
> +                break;
> 
> The while loop would gently exit left would be decremented and fsync would
> be done but we don't care these sides effects and buf would be freed.
> Then the code would exit the if branch and reach out_close.

Looks correct, but left wouldn't be decremented since break breaks out
of while directly. I guess you were looking at the wrong closing
brace. I'll change it providing what you heard.

Regards,
Hu

> 
> (I heard some people don't like exiting a loop with a goto).
> 
> 
> > +            }
> > +            left -= num;
> >          }
> > +        fsync(fd);
> > +        g_free(buf);
> > +    } else if (prealloc != PREALLOC_MODE_OFF) {
> > +        result = -EINVAL;
> > +        error_setg(errp, "Unsupported preallocation mode: %s",
> > +                   PreallocMode_lookup[prealloc]);
> >      }
> > +
> > +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;
> >  }
> >  
> > @@ -1585,6 +1635,11 @@ static QemuOptsList raw_create_opts = {
> >              .type = QEMU_OPT_BOOL,
> >              .help = "Turn off copy-on-write (valid only on btrfs)"
> >          },
> > +        {
> > +            .name = BLOCK_OPT_PREALLOC,
> > +            .type = QEMU_OPT_STRING,
> > +            .help = "Preallocation mode (allowed values: off, falloc, full)"
> > +        },
> >          { /* end of list */ }
> >      }
> >  };
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index 2b232ae..1f289d6 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -527,6 +527,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
> >  space. Use @code{qemu-img info} to know the real size used by the
> >  image or @code{ls -ls} on Unix/Linux.
> >  
> > +Supported options:
> > +@table @code
> > +@item preallocation
> > +Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
> > +@code{falloc} mode preallocates space for image by calling posix_fallocate().
> > +@code{full} mode preallocates space for image by writing zeros to underlying
> > +storage.
> > +@end table
> > +
> >  @item qcow2
> >  QEMU image format, the most versatile format. Use it to have smaller
> >  images (useful if your filesystem does not supports holes, for example
> > diff --git a/qemu-img.texi b/qemu-img.texi
> > index cc4668e..d64d05e 100644
> > --- a/qemu-img.texi
> > +++ b/qemu-img.texi
> > @@ -419,6 +419,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
> >  space. Use @code{qemu-img info} to know the real size used by the
> >  image or @code{ls -ls} on Unix/Linux.
> >  
> > +Supported options:
> > +@table @code
> > +@item preallocation
> > +Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
> > +@code{falloc} mode preallocates space for image by calling posix_fallocate().
> > +@code{full} mode preallocates space for image by writing zeros to underlying
> > +storage.
> > +@end table
> > +
> >  @item qcow2
> >  QEMU image format, the most versatile format. Use it to have smaller
> >  images (useful if your filesystem does not supports holes, for example
> > -- 
> > 1.9.3
> > 
> > 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector
  2014-09-10  1:50     ` Hu Tao
@ 2014-09-10  2:36       ` Hu Tao
  0 siblings, 0 replies; 16+ messages in thread
From: Hu Tao @ 2014-09-10  2:36 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Kevin Wolf, Fam Zheng, qemu-devel, Richard W.M. Jones, Max Reitz,
	Stefan Hajnoczi, Yasunori Goto

On Wed, Sep 10, 2014 at 09:50:02AM +0800, Hu Tao wrote:
> On Tue, Sep 09, 2014 at 02:12:18PM +0200, Benoît Canet wrote:

...

> > 
> > seems correct but the patch does not apply anymore on latest Kevin/block
> > branch neither with git am nor with git apply..
> > Could you rebase ?
> 
> I made it on top of master. I'll respin basing on Kevin/block.

I can apply it basing on http://repo.or.cz/qemu/kevin.git block. Am I
using the wrong block branch?

Regards,
Hu

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-09-10  2:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09  3:54 [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc Hu Tao
2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector Hu Tao
2014-09-09 12:12   ` Benoît Canet
2014-09-10  1:50     ` Hu Tao
2014-09-10  2:36       ` Hu Tao
2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 2/5] block: don't convert file size to sector size Hu Tao
2014-09-09 12:26   ` Benoît Canet
2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc Hu Tao
2014-09-09 12:42   ` Eric Blake
2014-09-10  1:44     ` Hu Tao
2014-09-09 12:45   ` Benoît Canet
2014-09-10  1:41     ` Hu Tao
2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option Hu Tao
2014-09-09 13:21   ` Benoît Canet
2014-09-10  2:02     ` Hu Tao
2014-09-09  3:54 ` [Qemu-devel] [PATCH v14 5/5] qcow2: " 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.