[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest se
|
From: |
Benoît Canet |
|
Subject: |
Re: [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector |
|
Date: |
Tue, 9 Sep 2014 14:12:18 +0200 |
|
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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 <address@hidden>
> Reviewed-by: Kevin Wolf <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> ---
> 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
> address@hidden
> +
> +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 ?
>
>