From: "Benoît Canet" <benoit.canet@irqsave.net>
To: Hu Tao <hutao@cn.fujitsu.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <famz@redhat.com>,
"Richard W.M. Jones" <rjones@redhat.com>,
qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
Yasunori Goto <y-goto@jp.fujitsu.com>
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 [thread overview]
Message-ID: <20140909121217.GA22473@irqsave.net> (raw)
In-Reply-To: <53d5d8a8f377692ab284afff6693c02fdb1dea9a.1410232831.git.hutao@cn.fujitsu.com>
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 ?
>
>
next prev parent reply other threads:[~2014-09-09 12:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140909121217.GA22473@irqsave.net \
--to=benoit.canet@irqsave.net \
--cc=famz@redhat.com \
--cc=hutao@cn.fujitsu.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rjones@redhat.com \
--cc=stefanha@redhat.com \
--cc=y-goto@jp.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.