Re: [Qemu-devel] [RFC PATCH] vpc: Ignore geometry for large images

[Peter Lieven]

Am 12.02.2015 um 11:06 schrieb Kevin Wolf:
> Am 12.02.2015 um 11:02 hat Peter Lieven geschrieben:
> > Am 12.02.2015 um 10:58 schrieb Kevin Wolf:
> > > Am 12.02.2015 um 10:23 hat Peter Lieven geschrieben:
> > > > Am 10.02.2015 um 15:53 schrieb Kevin Wolf:
> > > > > Am 10.02.2015 um 15:00 hat Peter Lieven geschrieben:
> > > > > > Am 10.02.2015 um 14:54 schrieb Kevin Wolf:
> > > > > > > Am 10.02.2015 um 14:42 hat Jeff Cody geschrieben:
> > > > > > > > On Tue, Feb 10, 2015 at 02:34:14PM +0100, Kevin Wolf wrote:
> > > > > > > > > Am 10.02.2015 um 12:41 hat Peter Lieven geschrieben:
> > > > > > > > > > Am 09.02.2015 um 17:09 schrieb Kevin Wolf:
> > > > > > > > > > > The CHS calculation as done per the VHD spec imposes a maximum
> > > > > > > > > > > image size of ~127 GB. Real VHD images exist that are larger than
> > > > > > > > > > > that.
> > > > > > > > > > >
> > > > > > > > > > > Apparently there are two separate non-standard ways to achieve
> > > > > > > > > > > this: You could use more heads than the spec does - this is the
> > > > > > > > > > > option that qemu-img create chooses.
> > > > > > > > > > >
> > > > > > > > > > > However, other images exist where the geometry is set to the
> > > > > > > > > > > maximum (65536/16/255), but the actual image size is larger.
> > > > > > > > > > > Until now, such images are truncated at 127 GB when opening them
> > > > > > > > > > > with qemu.
> > > > > > > > > > >
> > > > > > > > > > > This patch changes the vpc driver to ignore geometry in this case
> > > > > > > > > > > and only trust the size field in the header.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Kevin Wolf <address@hidden> ---
> > > > > > > > > > >
> > > > > > > > > > > Peter, I'm replacing some of your code in the hope that the new
> > > > > > > > > > > approach is more generally valid. Of course, I haven't tested if
> > > > > > > > > > > your case with disk2vhd is still covered. Could you check this,
> > > > > > > > > > > please?
> > > > > > > > > > I checked this and found that disk2vhd always sets CHS to 65535ULL
> > > > > > > > > > * 16 * 255 independed of the real size.
> > > > > > > > > >
> > > > > > > > > > But, as the conversion to CHS may have an error its maybe the best
> > > > > > > > > > solution to ignore CHS completely and always derive total_sectors
> > > > > > > > > >from footer->size unconditionally.
> > > > > > > > > > I had a look at what virtualbox does and they only rely on
> > > > > > > > > > footer->size. If they alter the size or create an image the write
> > > > > > > > > > the new size into the footer and recalculate CHS by the formula
> > > > > > > > > > found in the appendix of the original spec.
> > > > > > > > > >
> > > > > > > > > > Check vhdCreateImage, vhdOpen in
> > > > > > > > > > http://www.virtualbox.org/svn/vbox/trunk/src/VBox/Storage/VHD.cpp
> > > > > > > > > >
> > > > > > > > > > The original spec also says that CHS values purpose is the use in
> > > > > > > > > > an ATA controller only.
> > > > > > > > > The problem with just using footer->size back then when I
> > > > > > > > > implemented this was that from the perspective of a VirtualPC guest
> > > > > > > > > run in qemu, the size of its hard disk would change, which you don't
> > > > > > > > > want either. Going from VPC to qemu would be ugly, but mostly
> > > > > > > > > harmless as the disk only grows. But if you use an image in qemu
> > > > > > > > > where the disk looks larger and then go back to VPC which respects
> > > > > > > > > geometry, your data may be truncated.
> > > > > > > > I believe the vpc "creator" field is different if the image was
> > > > > > > > created by Virtual PC, versus created by Hyper-V ("vpc" and "win",
> > > > > > > > respectively, I think).  Perhaps we could use that to infer a guest
> > > > > > > > image came from VirtualPC, and thus not use footer->size in that
> > > > > > > > scenario?
> > > > > > > Right, I think we discussed that before. Do you remember the outcome of
> > > > > > > that discussion? I seem to remember that we had a conclusion, but
> > > > > > > apparently it was never actually implemented.
> > > > > > >
> > > > > > > Would your proposal be to special-case "vpc" to apply the geometry, and
> > > > > > > everything else (including "win", "d2v" and "qemu") would use the footer
> > > > > > > field?
> > > > > > That sounds reasonable. In any case we have to fix qemu-img create
> > > > > > to do not create out of spec geometry for images larger than 127G.
> > > > > > It should set the correct footer->size and then calculate the geometry.
> > > > > Do I understand correctly that you just volunteered to fix up that whole
> > > > > thing? ;-)
> > > > I knew that this would happen ;-)
> > > >
> > > > Regarding the C/H/S calculation. I was just wondering if we should
> > > > not set this to maximum (=invalid?) for all newly created images.
> > > > That is what disk2vhd does.
> > > CHS is what Virtual PC relies on. So I guess if you did that, you
> > > would render images unusable by it. Are you sure that disk2vhd does this
> > > always? I would have thought that it only does it for large images.
> > At least 2.0.1 (latest available version) does this as well as the version
> > that I used when I added the hack for d2v creator.
> >
> > Virtual PC would not be able to use images we create with qemu-img create
> > if we use footer->size (which I suppose to reanme to footer->cur_size, btw)
> > to calculate bs->total_sectors because we might write data to the end of
> > the image which gets truncated in CHS format.
> These kinds of problems are why I'd like to keep CHS and size always
> consistent when creating an image with qemu-img.

Okay, then I would vote for your RFC patch + fixing qemu-img create
to not generate out of spec CHS values and just set maximum which
then would make vpc_open use footer->size.

Peter


> Kevin


--

Mit freundlichen Grüßen

Peter Lieven

...........................................................

  KAMP Netzwerkdienste GmbH
  Vestische Str. 89-91 | 46117 Oberhausen
  Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
  address@hidden | http://www.kamp.de

  Geschäftsführer: Heiner Lante | Michael Lante
  Amtsgericht Duisburg | HRB Nr. 12154
  USt-Id-Nr.: DE 120607556

...........................................................