Re: [Qemu-devel] [PATCH 5/7] block/vpc: Fix size calculation

[Kevin Wolf]

Am 05.02.2013 12:10, schrieb Stefan Weil:
> Am 05.02.2013 11:54, schrieb Kevin Wolf:
>> Am 01.02.2013 22:51, schrieb Stefan Weil:
>>> 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.
>>>
>>> This patch fixes https://bugs.launchpad.net/qemu/+bug/1105670/.
>>>
>>> Signed-off-by: Stefan Weil <address@hidden>
>>> ---
>>>  block/vpc.c |    9 ++++-----
>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/block/vpc.c b/block/vpc.c
>>> index fff103b..799b1c9 100644
>>> --- a/block/vpc.c
>>> +++ b/block/vpc.c
>>> @@ -198,11 +198,10 @@ 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 an image in Virtual PC may depend on the 
>>> geometry
>>> +     * rather than on the size stored in the footer (the size in the footer
>>> +     * is usually larger). Nevertheless we must use the real size here. */
>>> +    bs->total_sectors = be64_to_cpu(footer->size) / 512;
>>>  
>>>      /* Allow a maximum disk size of approximately 2 TB */
>>>      if (bs->total_sectors >= 65535LL * 255 * 255) {
>> Now VMs created with qemu can't be run in Virtual PC any more because
>> they use more space than is really visible on VPC.
> 
> Now VMs created with QEMU show the size which is also
> reported by MS tools. I don't think this is a problem.
> 
> 
>> The bug report doesn't say anything about practical consequences of the
>> unexpected behaviour, so I'd tend to claim that just the expectation was
>> wrong. Having to deal with two differing sizes is ugly, but we had the
>> discussion before and using the geometry was considered the least bad
>> way, because it gives you the same disk size within the VM as on Virtual PC.
>>
>> If you want to change it, you need to provide a real use case that breaks.
>>
>> Kevin
> 
> Test case:
> 
> * Create a VHD disk image (either with QEMU or MS tools).
> * Run a modern guest OS (which does not use CHS) on this image
>   using Virtual PC or VirtualBox and write into the last disk blocks.
> * Copy the image to another format using qemu-img.
> 
> => The last blocks are missing now.

When I wrote that code it wasn't like this. I doubt that Linux was
relying on the geometry until recently, so is this a change in the
behaviour of newer Virtual PC versions? If yes, that would be very nasty
to deal with.

I'm very sure that back then I needed to use the geometry so that images
could be exchanged between qemu and VPC. If I didn't, they would either
get truncated or appear to be larger on one of both.

Kevin