From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44553) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLqi5-0004JU-3T for qemu-devel@nongnu.org; Thu, 12 Feb 2015 05:09:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLqi0-0001ZF-JJ for qemu-devel@nongnu.org; Thu, 12 Feb 2015 05:09:25 -0500 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:53146 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLqi0-0001Z1-8F for qemu-devel@nongnu.org; Thu, 12 Feb 2015 05:09:20 -0500 Message-ID: <54DC7BC4.8080709@kamp.de> Date: Thu, 12 Feb 2015 11:09:08 +0100 From: Peter Lieven MIME-Version: 1.0 References: <1423498163-2001-1-git-send-email-kwolf@redhat.com> <54D9EE7A.9050802@kamp.de> <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> In-Reply-To: <20150212100638.GF4189@noname.str.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit 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: Kevin Wolf Cc: Jeff Cody , qemu-devel@nongnu.org, stefanha@redhat.com 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 --- >>>>>>>>>>> >>>>>>>>>>> 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 pl@kamp.de | http://www.kamp.de Geschäftsführer: Heiner Lante | Michael Lante Amtsgericht Duisburg | HRB Nr. 12154 USt-Id-Nr.: DE 120607556 ...........................................................