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

[Jeff Cody]

On Tue, Feb 05, 2013 at 12:35:49PM +0100, Kevin Wolf wrote:
> 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.
>

Hi Kevin,

I can verify the issue he is seeing (see my comments on Bug 1105670 
(https://bugs.launchpad.net/qemu/+bug/1105670/comments/3), at least
using Hyper-V.  I haven't tried it against VPC from Win7.

Jeff