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

[Peter Lieven <pl@kamp.de>]

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 <kwolf@redhat.com> ---
>>>>>>>
>>>>>>> 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.

Peter