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 <hu...@cn.fujitsu.com>
> > Reviewed-by: Kevin Wolf <kw...@redhat.com>
> > Reviewed-by: Eric Blake <ebl...@redhat.com>
> > Reviewed-by: Max Reitz <mre...@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=hu...@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
