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

[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"?
> 
> 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

Kevin,

I can test this on Virtual PC on a Win 7 install, as well as
Hyper-V (I've already tested the equivalent of this patch on Hyper-V
on Windows Server 2012).  I'll do some testing today with this - if
you have anything in particular you want me to look at, just let me
know.

Stefan, did you test if this behaves the same for all disk sizes, or
only for larger disk sizes of 127G and up?

Thanks,
Jeff