Re: [Qemu-devel] block/vpc: Fix vhd extra sectors issue

[Kevin Wolf <kwolf@redhat.com>]

Am 13.11.2015 um 16:32 hat Lucian Petrut geschrieben:
> 
> At the moment, qemu-img extends new image virtual sizes based
> on the CHS algorithm provided by the VHD specs in order to
> ensure that the disk geometry (and payload as seen by some
> guests which use the CHS value) can fit in the requested disk.
> 
> This patch drops this behavior, as it breaks compatibility with
> Azure, which requires the MB alignment to be preserved.
> 
> Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
> ---
> Proposed fix for https://bugs.launchpad.net/qemu/+bug/1490611

This may fix one scenario, but it's sure to break others which are
currently working. The problem has been discussed more than once and
it's essentially a problem with MS using their own file format
inconsistently.

I think we once came to the conclusion that looking at the creator
string might be a working heuristics. Apparently this was never
implemented - I don't remember whether that was because we noticed a
problem with it, or just because noone got to it.

Jeff and Peter, I seem to remember that you were involved the last time
we discussed this, so does one of you remember why we didn't implement
this heuristics in the end?

Kevin

> diff --git a/block/vpc.c b/block/vpc.c
> index 299d373..77c0a28 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -762,7 +762,6 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
>      uint8_t buf[1024];
>      VHDFooter *footer = (VHDFooter *) buf;
>      char *disk_type_param;
> -    int i;
>      uint16_t cyls = 0;
>      uint8_t heads = 0;
>      uint8_t secs_per_cyl = 0;
> @@ -802,31 +801,16 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
>          goto out;
>      }
>  
> -    /*
> -     * Calculate matching total_size and geometry. Increase the number of
> -     * sectors requested until we get enough (or fail). This ensures that
> -     * qemu-img convert doesn't truncate images, but rather rounds up.
> -     *
> -     * If the image size can't be represented by a spec conform CHS geometry,
> -     * we set the geometry to 65535 x 16 x 255 (CxHxS) sectors and use
> -     * the image size from the VHD footer to calculate total_sectors.
> -     */
> -    total_sectors = MIN(VHD_MAX_GEOMETRY, total_size / BDRV_SECTOR_SIZE);
> -    for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl; i++) {
> -        calculate_geometry(total_sectors + i, &cyls, &heads, &secs_per_cyl);
> -    }
> -
> -    if ((int64_t)cyls * heads * secs_per_cyl == VHD_MAX_GEOMETRY) {
> -        total_sectors = total_size / BDRV_SECTOR_SIZE;
> +    total_sectors = total_size / BDRV_SECTOR_SIZE;
> +    if (total_sectors > VHD_MAX_SECTORS) {
>          /* Allow a maximum disk size of approximately 2 TB */
> -        if (total_sectors > VHD_MAX_SECTORS) {
> -            ret = -EFBIG;
> -            goto out;
> -        }
> -    } else {
> -        total_sectors = (int64_t)cyls * heads * secs_per_cyl;
> -        total_size = total_sectors * BDRV_SECTOR_SIZE;
> +        ret = -EFBIG;
> +        goto out;
>      }
> +    /*
> +     * Calculate geometry.
> +     */
> +    calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl);
>  
>      /* Prepare the Hard Disk Footer */
>      memset(buf, 0, 1024);