diff mbox

[v13,1/6] block: round up file size to nearest sector

Message ID 92103ac95fc7ebc94ecf3e30b146c0cf19273447.1409299732.git.hutao@cn.fujitsu.com
State New
Headers show

Commit Message

Hu Tao Aug. 29, 2014, 8:33 a.m. UTC
Signed-off-by: Hu Tao <hutao@cn.fujitsu.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

Comments

Eric Blake Aug. 29, 2014, 12:50 p.m. UTC | #1
On 08/29/2014 02:33 AM, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.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

Code looks correct.  However, I have a possible design concern:

> +++ 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);

At least the qcow2 file format describes virtual file size in bytes
(offsets 24-31 in the header).  It's nice that we are guaranteeing at
least the size requested by the user, but by doing the division, we are
unable to write the user's actual requested size into the file.

I'm okay if all files _we_ create are aligned in size.  But do we have
any lurking bugs if a qcow2 file created by some other means has a
non-aligned size?  The recent work on the image fuzzer should be able to
easily create such images.  Meanwhile, I don't know of any physical
hardware that has an unaligned size.  Should we tighten the qcow2 spec
to forbid a size that is not a multiple of the sector size, and teach
qemu to forcefully reject such images as invalid?  Does the qcow2 format
need a way to flag whether an image has 512 vs. 4096-byte sectors (that
is, what happens with an image that is 512-aligned but not 4096-aligned
when used in an emulation that only exposes 4096-byte alignment to the
guest)?

On the other hand, if we _do_ have code that gracefully handles
unaligned size from externally-created sources, would it be worth
allowing qemu-img to also create unaligned images directly, instead of
having to resort to the image fuzzer to create such images, all so that
it is easier to test our code and ensure it doesn't bit-rot?  It might
be lower-maintenance to just require that such images be rejected as broken.

> +++ 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);

Again, I'm okay with the idea of always treating guest images as aligned
multiples of sectors.  But it's very easy to create an unaligned raw
file, and pass that to qemu.  So even though we don't create such files
via qemu-img, it's still worth ensuring that the code behaves correctly
when such an image is used directly or as a backing file (as a backing
file of a larger qcow2 image, bytes beyond the actual size must read as
padded to 0; as a direct file, I'm not sure that the guest OS can do a
partial sector read, so I'd lean more towards an I/O error when trying
to read a full sector when only a partial sector is available the same
as when trying to read a sector completely beyond the disk bounds, but I
could also live with a successful read with the tail being padded to 0;
similarly, we'd have to ensure that writes either error out, or
guarantee that the tail is ignored and only the valid portion of the
sector is written).

Meanwhile, although we own the qcow2 spec, and can therefore choose to
reject unaligned size in qcow2 files as invalid format, we are not in
charge of the spec for several other file formats, but probably ought to
handle those the same way as other vendors.  For example, is it possible
to create a VMDK file with unaligned size, and if so, how does it behave?

>  
> +_filter_img_info()
> +{
> +    sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \

This works, so I'm not asking for a change; but I prefer | over # when
writing a sed expression that includes a filename expansion, as in:

sed -e "s|$IMGPROTO:$TEST_DIR|TEST_DIR|g"

Why? Because it's easy (even if uncommon) to create file names with #
embedded in them (if $TEST_DIR is absolute, I just have to have
/path/to/my#odd/directory with no shell quoting required as the place
where I unpack and configure qemu), but file names with | can only be
created with shell quoting, so they are less common.  (The complaint
gets more relevant for people doing s,,, with file names, because such
files are more common [anyone remember CVS?])

At any rate, if there are still design issues to resolve (such as
tightening the qcow2 spec), they can be separate patches.  So I'm okay with:

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz Sept. 2, 2014, 9:21 p.m. UTC | #2
On 29.08.2014 10:33, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.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

Reviewed-by: Max Reitz <mreitz@redhat.com>
Kevin Wolf Sept. 4, 2014, 9:33 a.m. UTC | #3
Am 29.08.2014 um 14:50 hat Eric Blake geschrieben:
> On 08/29/2014 02:33 AM, Hu Tao wrote:
> > +++ 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);
> 
> Again, I'm okay with the idea of always treating guest images as aligned
> multiples of sectors.  But it's very easy to create an unaligned raw
> file, and pass that to qemu.  So even though we don't create such files
> via qemu-img, it's still worth ensuring that the code behaves correctly
> when such an image is used directly or as a backing file

The whole qemu block layer doesn't work well with unaligned images (and
the required alignment is always 512 because someone thought that using
512-byte units was a good interface). I want to fix that by converting
everything to byte granularity, but it's not the top priority.

Kevin
Kevin Wolf Sept. 4, 2014, 9:43 a.m. UTC | #4
Am 29.08.2014 um 10:33 hat Hu Tao geschrieben:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>

> diff --git a/tests/qemu-iotests/104 b/tests/qemu-iotests/104
> new file mode 100755
> index 0000000..0c1d4fb
> --- /dev/null
> +++ b/tests/qemu-iotests/104
> @@ -0,0 +1,57 @@
> +#!/bin/bash
> +#
> +# Test qcow2 creation with aligned and unaligned sizes

This test case is not only for qcow2.

With this fixed, you can add:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
diff mbox

Patch

diff --git a/block/archipelago.c b/block/archipelago.c
index 34f72dc..a8d06aa 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -707,7 +707,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 93d87f3..d3ba09e 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -414,7 +414,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 12cbd9d..8293d47 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1695,7 +1695,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 07cb62c..2e126c5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1805,7 +1805,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..0c1d4fb
--- /dev/null
+++ b/tests/qemu-iotests/104
@@ -0,0 +1,57 @@ 
+#!/bin/bash
+#
+# Test qcow2 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 2803d68..e6c23b4 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -103,3 +103,4 @@ 
 099 rw auto quick
 101 rw auto quick
 103 rw auto quick
+104 rw auto