[Qemu-devel] [PATCH] vpc: Fix size in fixed image creation

[Qemu-devel] [PATCH] vpc: Fix size in fixed image creation

[Kevin Wolf]

If total_sectors is rounded to match the geometry, total_size needs to
be changed as well. Otherwise we end up with an image whose geometry
describes a disk larger than the image file, which doesn't end well.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vpc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 379ff4a..46830ee 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -850,6 +850,7 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
     }
 
     total_sectors = (int64_t) cyls * heads * secs_per_cyl;
+    total_size = total_sectors * BDRV_SECTOR_SIZE;
 
     /* Prepare the Hard Disk Footer */
     memset(buf, 0, 1024);
@@ -871,13 +872,8 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
     /* Version of Virtual PC 2007 */
     footer->major = cpu_to_be16(0x0005);
     footer->minor = cpu_to_be16(0x0003);
-    if (disk_type == VHD_DYNAMIC) {
-        footer->orig_size = cpu_to_be64(total_sectors * 512);
-        footer->size = cpu_to_be64(total_sectors * 512);
-    } else {
-        footer->orig_size = cpu_to_be64(total_size);
-        footer->size = cpu_to_be64(total_size);
-    }
+    footer->orig_size = cpu_to_be64(total_size);
+    footer->size = cpu_to_be64(total_size);
     footer->cyls = cpu_to_be16(cyls);
     footer->heads = heads;
     footer->secs_per_cyl = secs_per_cyl;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] vpc: Fix size in fixed image creation

[Max Reitz]

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

On 2015-02-11 at 11:22, Kevin Wolf wrote:
> If total_sectors is rounded to match the geometry, total_size needs to
> be changed as well. Otherwise we end up with an image whose geometry
> describes a disk larger than the image file, which doesn't end well.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/vpc.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)

Looks fine to me, but the spec says: "When the user creates a hard disk 
of a certain size, the size of the hard disk image in the virtual 
machine is smaller than that created by the user. This is because CHS 
value calculated from the hard disk size is rounded down."

So, well, qemu has been violating the specification all along, I guess.

So, with that in mind (that nobody has been screaming so far):

Reviewed-by: Max Reitz <mreitz@redhat.com>

Alternatively we may want to "fix" it, and actually let the CHS size be 
smaller than total_sectors/total_size, if that's possible. But maybe 
that'll be even worse, so I'm fine either way.

[-- Attachment #2: Type: text/html, Size: 2269 bytes --]

Re: [Qemu-devel] [PATCH] vpc: Fix size in fixed image creation

[Kevin Wolf]

Am 11.02.2015 um 23:32 hat Max Reitz geschrieben:
> On 2015-02-11 at 11:22, Kevin Wolf wrote:
> 
>     If total_sectors is rounded to match the geometry, total_size needs to
>     be changed as well. Otherwise we end up with an image whose geometry
>     describes a disk larger than the image file, which doesn't end well.
> 
>     Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>     ---
>      block/vpc.c | 10 +++-------
>      1 file changed, 3 insertions(+), 7 deletions(-)
> 
> 
> Looks fine to me, but the spec says: "When the user creates a hard disk of a
> certain size, the size of the hard disk image in the virtual machine is smaller
> than that created by the user. This is because CHS value calculated from the
> hard disk size is rounded down."
> 
> So, well, qemu has been violating the specification all along, I guess.

Initially we rounded the size down. This ended up truncating images
during qemu-img convert, so it was decided that rounding up is the less
bad solution.

> So, with that in mind (that nobody has been screaming so far):
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks!

> Alternatively we may want to "fix" it, and actually let the CHS size be smaller
> than total_sectors/total_size, if that's possible. But maybe that'll be even
> worse, so I'm fine either way.

That's possible. With VHD, however, I wouldn't make any such changes
without proof that some software needs it and other software isn't
affected. This is all pretty fragile.

Kevin

Re: [Qemu-devel] [PATCH] vpc: Fix size in fixed image creation

[Kevin Wolf]

Am 11.02.2015 um 17:22 hat Kevin Wolf geschrieben:
> If total_sectors is rounded to match the geometry, total_size needs to
> be changed as well. Otherwise we end up with an image whose geometry
> describes a disk larger than the image file, which doesn't end well.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Applied to the block branch.

Kevin