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

[Kevin Wolf]

Am 08.02.2013 18:43, schrieb Stefan Weil:
> Am 08.02.2013 13:14, schrieb Jeff Cody:
>> On Fri, Feb 08, 2013 at 09:38:47AM +0100, Kevin Wolf wrote:
>>> 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"?
> 
> Linux is a modern OS per definition :-)
> 
> I made an additional test with Knoppix (from December 2006),
> and it worked as expected.

So I'm afraid that the reason isn't the guest OS but the Virtual PC
version. I guess supporting newer versions of it is more important, so
if Jeff's testing passes, I'm not against taking the patch, even though
I don't feel completely comfortable about it. But I don't see how we
could support both ways at the same time.

Kevin