Re: [Qemu-devel] [PATCH for 1.4] block/vpc: Fix size calculation

[Kevin Wolf]

Am 07.02.2013 20:26, schrieb Stefan Weil:
> From: Stefan Weil <address@hidden(none)>
> 
> The size calculated from the CHS values is not the real image (disk) size,
> but usually a smaller value. This is caused by rounding effects.
> 
> Only older operating systems use CHS. Such guests won't be able to use
> the whole disk. All modern operating systems use the real size.
> 
> This patch fixes https://bugs.launchpad.net/qemu/+bug/1105670/.
> 
> Signed-off-by: Stefan Weil <address@hidden>
> ---
> 
> This is a rebased extract from my patch series for block/vpc.c.
> It's the minimum needed to fix the open bug for QEMU 1.4.
> 
> The rest of the series can be discussed and applied after 1.4.
> 
> Regards
> 
> Stefan W.
> 
> PS. Please excuse a previous personal mail which I had sent
> with a wrong signature and without addressing qemu-devel.
> 
>  block/vpc.c |   14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vpc.c b/block/vpc.c
> index 82229ef..b4ff564 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -34,6 +34,8 @@
>  
>  #define HEADER_SIZE 512
>  
> +#define VHD_SECTOR_SIZE 512
> +
>  //#define CACHE
>  
>  enum vhd_type {
> @@ -204,11 +206,13 @@ static int vpc_open(BlockDriverState *bs, int flags)
>      /* Write 'checksum' back to footer, or else will leave it with zero. */
>      footer->checksum = be32_to_cpu(checksum);
>  
> -    // The visible size of a image in Virtual PC depends on the geometry
> -    // rather than on the size stored in the footer (the size in the footer
> -    // is too large usually)
> -    bs->total_sectors = (int64_t)
> -        be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
> +    /* The visible size of a image in Virtual PC depends on the guest:
> +     * QEMU and other emulators report the real size (here in sectors).
> +     * All modern operating systems use this real size.
> +     * Very old operating systems use CHS values to calculate the total size.
> +     * This calculated size is usually smaller than the real size.
> +     */
> +    bs->total_sectors = be64_to_cpu(footer->size) / VHD_SECTOR_SIZE;

It's unfortunate that I don't have my old Virtual PC installation around
any more so I could prove that you're wrong for at least some versions.
Or does a Linux of 2009 already count as "very old"?

If we want to commit this - and I still feel uncomfortable about it -
then maybe it's best to remove the comment altogether instead of making
such claims. The new code is the intuitively expected one anyway.

Kevin