From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53712) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLz5E-0006sr-C7 for qemu-devel@nongnu.org; Thu, 12 Feb 2015 14:05:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLz59-00030k-8P for qemu-devel@nongnu.org; Thu, 12 Feb 2015 14:05:52 -0500 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:51292 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLz58-0002zl-Dw for qemu-devel@nongnu.org; Thu, 12 Feb 2015 14:05:46 -0500 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) From: Peter Lieven In-Reply-To: <54DC7DFB02000091000EE3A0@prv-mh.provo.novell.com> Date: Thu, 12 Feb 2015 20:05:35 +0100 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20150210133414.GE5202@noname.str.redhat.com> <20150210134242.GB19775@localhost.localdomain> <20150210135439.GF5202@noname.str.redhat.com> <54DA0EEA.7050908@kamp.de> <20150210145329.GG5202@noname.str.redhat.com> <54DC7112.30809@kamp.de> <20150212095821.GE4189@noname.str.redhat.com> <54DC7A1E.6070405@kamp.de> <20150212100638.GF4189@noname.str.redhat.com> <54DC7BC4.8080709@kamp.de> <20150212102307.GG4189@noname.str.redhat.com> <54DC7DFB02000091000EE3A0@prv-mh.provo.novell.com> Subject: Re: [Qemu-devel] [RFC PATCH] vpc: Ignore geometry for large images List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Charles Arnold Cc: Kevin Wolf , Jeff Cody , "" , "" Am 12.02.2015 um 18:18 schrieb Charles Arnold : >>>> On 2/12/2015 at 03:23 AM, Kevin Wolf wrote: >> Am 12.02.2015 um 11:09 hat Peter Lieven geschrieben: >>>> 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 maximu= m >>>>>>>>>>>>>> image size of ~127 GB. Real VHD images exist that are larger t= han >>>>>>>>>>>>>> that. >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> Apparently there are two separate non-standard ways to achiev= e >>>>>>>>>>>>>> this: You could use more heads than the spec does - this is t= he >>>>>>>>>>>>>> option that qemu-img create chooses. >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> 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 t= hem >>>>>>>>>>>>>> with qemu. >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> This patch changes the vpc driver to ignore geometry in this c= ase >>>>>>>>>>>>>> and only trust the size field in the header. >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> Signed-off-by: Kevin Wolf --- >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> Peter, I'm replacing some of your code in the hope that the n= ew >>>>>>>>>>>>>> approach is more generally valid. Of course, I haven't tested= if >>>>>>>>>>>>>> your case with disk2vhd is still covered. Could you check thi= s, >>>>>>>>>>>>>> please? >>>>>>>>>>>>> I checked this and found that disk2vhd always sets CHS to 6553= 5ULL >>>>>>>>>>>>> * 16 * 255 independed of the real size. >>>>>>>>>>>>>=20 >>>>>>>>>>>>> But, as the conversion to CHS may have an error its maybe the b= est >>>>>>>>>>>>> solution to ignore CHS completely and always derive total_sect= ors >>>>>>>>>>>>> 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 wr= ite >>>>>>>>>>>>> the new size into the footer and recalculate CHS by the formul= a >>>>>>>>>>>>> found in the appendix of the original spec. >>>>>>>>>>>>>=20 >>>>>>>>>>>>> Check vhdCreateImage, vhdOpen in >>>>>>>>>>>>> http://www.virtualbox.org/svn/vbox/trunk/src/VBox/Storage/VHD.= cpp >>>>>>>>>>>>>=20 >>>>>>>>>>>>> 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 g= uest >>>>>>>>>>>> run in qemu, the size of its hard disk would change, which you d= on'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 qem= u >>>>>>>>>>>> where the disk looks larger and then go back to VPC which respe= cts >>>>>>>>>>>> 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 gu= est >>>>>>>>>>> image came from VirtualPC, and thus not use footer->size in that= >>>>>>>>>>> scenario? >>>>>>>>>> Right, I think we discussed that before. Do you remember the outc= ome of >>>>>>>>>> that discussion? I seem to remember that we had a conclusion, but= >>>>>>>>>> apparently it was never actually implemented. >>>>>>>>>>=20 >>>>>>>>>> Would your proposal be to special-case "vpc" to apply the geometr= y, 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 geom= etry. >>>>>>>> Do I understand correctly that you just volunteered to fix up that w= hole >>>>>>>> thing? ;-) >>>>>>> I knew that this would happen ;-) >>>>>>>=20 >>>>>>> Regarding the C/H/S calculation. I was just wondering if we should >>>>>>> not set this to maximum (=3Dinvalid?) 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 t= his >>>>>> 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 ver= sion >>>>> that I used when I added the hack for d2v creator. >>>>>=20 >>>>> Virtual PC would not be able to use images we create with qemu-img cre= ate >>>>> 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 o= f >>>>> 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. >>>=20 >>> 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. >>=20 >> Really the RFC patch or what we discussed above ("vpc" creator =3D CHS, >> everything else =3D footer->size)? Once I know what we prefer, I'll send >> the real patch. >>=20 >> As for heads > 16, that would essentially mean reverting 258d2edb. >> Should be easy to do, the harder part is probably the commit message >> explaining why it's helpful and safe. Note that the commit message of >> 258d2edb claims that it's not out of spec. I _think_ we can do the >> revert with a good explanation, but I'll leave that to you. >>=20 >> (CCed Charles who wrote that commit) >=20 > IIUC, the plan is to revert my old commit and use the footer->size field t= o > describe images greater than 127 GB. This change would break other tools > from Virtual PC, Xens vhd-util and maybe others from reading images greate= r > than 127 GB because the head field would be forced back to using 16 and > these tools won't know to check the footer->size field. Is there any=20 > reason not to keep the original commit and still use the footer->size fiel= d? do you have a Pointer to a spec that is newer than 2006? the one i have desc= ribes CHS calculation up to 65535 x 16 x 255 sectors. that is set as Maximum= if total sectors is higher. I would do the same when writing a footer. in v= hd_open I would derive total_sectors from C x H x S except for the case that= it is exactly 65535 x 16 x 255. In this case I would take footer->size / 51= 2. Virtualbox does it that way and at the comment from Stefan in the commit mes= sage for your Patch suggest that you observed a similar behaviour for HyperV= . Peter >=20 > A purist would argue that heads must be 16 for true ATA emulation but allo= wing > up to 255 doesn't seem to matter and the VHD spec does support up to 2 TB.= >=20 > - Charles >=20