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

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

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

 block/vpc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 46803b1..00409a6 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -215,12 +215,10 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     bs->total_sectors = (int64_t)
         be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
 
-    /* images created with disk2vhd report a far higher virtual size
-     * than expected with the cyls * heads * sectors_per_cyl formula.
-     * use the footer->size instead if the image was created with
-     * disk2vhd.
-     */
-    if (!strncmp(footer->creator_app, "d2v", 4)) {
+    /* Images that have exactly the maximum geometry are probably bigger and
+     * would be truncated if we adhered to the geometry for them. Rely on
+     * footer->size for them. */
+    if (bs->total_sectors == 65535ULL * 16 * 255) {
         bs->total_sectors = be64_to_cpu(footer->size) / BDRV_SECTOR_SIZE;
     }
 
-- 
1.8.3.1

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

[Peter Lieven]

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.

Peter


>
>   block/vpc.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/block/vpc.c b/block/vpc.c
> index 46803b1..00409a6 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -215,12 +215,10 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
>       bs->total_sectors = (int64_t)
>           be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
>   
> -    /* images created with disk2vhd report a far higher virtual size
> -     * than expected with the cyls * heads * sectors_per_cyl formula.
> -     * use the footer->size instead if the image was created with
> -     * disk2vhd.
> -     */
> -    if (!strncmp(footer->creator_app, "d2v", 4)) {
> +    /* Images that have exactly the maximum geometry are probably bigger and
> +     * would be truncated if we adhered to the geometry for them. Rely on
> +     * footer->size for them. */
> +    if (bs->total_sectors == 65535ULL * 16 * 255) {
>           bs->total_sectors = be64_to_cpu(footer->size) / BDRV_SECTOR_SIZE;
>       }
>   


-- 

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

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

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

[Kevin Wolf]

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.

Kevin

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

[Jeff Cody]

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?

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

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

Kevin

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

[Peter Lieven]

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.

Peter

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

[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? ;-)

Kevin

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

[Peter Lieven]

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

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

[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 <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.

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.

Kevin

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

[Peter Lieven]

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 <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.
> 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.

Peter

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

[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 <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.
> >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.

Kevin

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 <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.
>>> 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

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

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

[Kevin Wolf]

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 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.
> >>>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.

Really the RFC patch or what we discussed above ("vpc" creator = CHS,
everything else = footer->size)? Once I know what we prefer, I'll send
the real patch.

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.

(CCed Charles who wrote that commit)

Kevin

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

[Peter Lieven]

[-- Attachment #1: Type: text/plain, Size: 6437 bytes --]

Am 12.02.2015 um 11:23 schrieb Kevin Wolf:
> 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 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.
>>>>> 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.
> Really the RFC patch or what we discussed above ("vpc" creator = CHS,
> everything else = footer->size)? Once I know what we prefer, I'll send
> the real patch.

The RFC patch as is.

>
> 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.

The only spec I have access to is [1].
If you check the CHS calculation code in the appendix, you will find:

if (totalSectors > 65535 * 16 * 255)

{

totalSectors = 65535 * 16 * 255;

}



[1] http://download.microsoft.com/download/f/f/e/ffef50a5-07dd-4cf8-aaa3-442c0673a029/Virtual%20Hard%20Disk%20Format%20Spec_10_18_06.doc

Peter


[-- Attachment #2: Type: text/html, Size: 10277 bytes --]

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

[Charles Arnold]

>>> On 2/12/2015 at 03:23 AM, Kevin Wolf <kwolf@redhat.com> 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 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.
>> >>>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.
> 
> Really the RFC patch or what we discussed above ("vpc" creator = CHS,
> everything else = footer->size)? Once I know what we prefer, I'll send
> the real patch.
> 
> 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.
> 
> (CCed Charles who wrote that commit)

IIUC, the plan is to revert my old commit and use the footer->size field to
describe images greater than 127 GB.  This change would break other tools
from Virtual PC, Xens vhd-util and maybe others from reading images greater
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 
reason not to keep the original commit and still use the footer->size field?

A purist would argue that heads must be 16 for true ATA emulation but allowing
up to 255 doesn't seem to matter and the VHD spec does support up to 2 TB.

- Charles

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

[Peter Lieven]

Am 12.02.2015 um 18:18 schrieb Charles Arnold <carnold@suse.com>:

>>>> On 2/12/2015 at 03:23 AM, Kevin Wolf <kwolf@redhat.com> 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 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.
>>>>>> 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.
>> 
>> Really the RFC patch or what we discussed above ("vpc" creator = CHS,
>> everything else = footer->size)? Once I know what we prefer, I'll send
>> the real patch.
>> 
>> 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.
>> 
>> (CCed Charles who wrote that commit)
> 
> IIUC, the plan is to revert my old commit and use the footer->size field to
> describe images greater than 127 GB.  This change would break other tools
> from Virtual PC, Xens vhd-util and maybe others from reading images greater
> 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 
> reason not to keep the original commit and still use the footer->size field?

do you have a Pointer to a spec that is newer than 2006? the one i have describes 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 vhd_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 / 512.
Virtualbox does it that way and at the comment from Stefan in the commit message for your Patch suggest that you observed a similar behaviour for HyperV.

Peter

> 
> A purist would argue that heads must be 16 for true ATA emulation but allowing
> up to 255 doesn't seem to matter and the VHD spec does support up to 2 TB.
> 
> - Charles
> 

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

[Charles Arnold]

>>> On 2/12/2015 at 12:05 PM, Peter Lieven <pl@kamp.de> wrote: 

> Am 12.02.2015 um 18:18 schrieb Charles Arnold <carnold@suse.com>:
> 
>>>>> On 2/12/2015 at 03:23 AM, Kevin Wolf <kwolf@redhat.com> 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 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.
>>>>>>> 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.
>>> 
>>> Really the RFC patch or what we discussed above ("vpc" creator = CHS,
>>> everything else = footer->size)? Once I know what we prefer, I'll send
>>> the real patch.
>>> 
>>> 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.
>>> 
>>> (CCed Charles who wrote that commit)
>> 
>> IIUC, the plan is to revert my old commit and use the footer->size field to
>> describe images greater than 127 GB.  This change would break other tools
>> from Virtual PC, Xens vhd-util and maybe others from reading images greater
>> 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 
>> reason not to keep the original commit and still use the footer->size field?
> 
> do you have a Pointer to a spec that is newer than 2006?

No, that is the most recent.

> the one i have 
> describes 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 vhd_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 / 512.
> Virtualbox does it that way and at the comment from Stefan in the commit 
> message for your Patch suggest that you observed a similar behaviour for 
> HyperV.

Right, although this was a long time ago for me to remember the specifics :)
In the end I think supporting at least the 2 TB size allowed by the spec is what
we need while not breaking existing images.

- Charles

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

[Peter Lieven]

Am 12.02.2015 um 22:01 schrieb Charles Arnold:
>>>> On 2/12/2015 at 12:05 PM, Peter Lieven <pl@kamp.de> wrote: 
>> Am 12.02.2015 um 18:18 schrieb Charles Arnold <carnold@suse.com>:
>>
>>>>>> On 2/12/2015 at 03:23 AM, Kevin Wolf <kwolf@redhat.com> 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 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.
>>>>>>>> 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.
>>>> Really the RFC patch or what we discussed above ("vpc" creator = CHS,
>>>> everything else = footer->size)? Once I know what we prefer, I'll send
>>>> the real patch.
>>>>
>>>> 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.
>>>>
>>>> (CCed Charles who wrote that commit)
>>> IIUC, the plan is to revert my old commit and use the footer->size field to
>>> describe images greater than 127 GB.  This change would break other tools
>>> from Virtual PC, Xens vhd-util and maybe others from reading images greater
>>> 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 
>>> reason not to keep the original commit and still use the footer->size field?
>> do you have a Pointer to a spec that is newer than 2006?
> No, that is the most recent.
>
>> the one i have 
>> describes 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 vhd_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 / 512.
>> Virtualbox does it that way and at the comment from Stefan in the commit 
>> message for your Patch suggest that you observed a similar behaviour for 
>> HyperV.
> Right, although this was a long time ago for me to remember the specifics :)
> In the end I think supporting at least the 2 TB size allowed by the spec is what
> we need while not breaking existing images.

Thats clear, the absolute limit is 2 TB. We still refuse large images with -EFBIG.
Before your patch every image above 127GB was refused with that error.

Would you agree to implement the following:

a) Use footer->size / 512 for bs->total_sectors iff CxHxS === 65535 x 16 x 255 (Kevins RFC Patch).
    Setting these values seems to be the inofficial way to say look at the footer->size.
    Normally I would argue to use footer->size iff CxHxS >== 65535 x 16 x 255, but
    this would break opening of our old out of spec images created with qemu-img.
b) change calculate_geometry() to do exactly what is in the old specs of 2006. In
    particular this will set CxHxS to 65535 x 16 x 255 for anything >127G and thus tells
    vpc_open to look at the footer->size.

Cheers,
Peter

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

[meta-comment]

On 02/18/2015 06:38 AM, Peter Lieven wrote:

>>>>>>>>>>>>>>>> Am 09.02.2015 um 17:09 schrieb Kevin Wolf:
>>>>>>>>>>>>>>>>> The CHS calculation as done per the VHD spec imposes a maximum

That's a LOT of nested quoting.  It's okay to trim your reply to cut out
the portion of the thread that people have already seen 17 times. :)

>> Right, although this was a long time ago for me to remember the specifics :)
>> In the end I think supporting at least the 2 TB size allowed by the spec is what
>> we need while not breaking existing images.
> 
> Thats clear, the absolute limit is 2 TB. We still refuse large images with -EFBIG.
> Before your patch every image above 127GB was refused with that error.

For example, trimming to just this much means that the reply occurs on
the first screenful, rather than having to scroll through several pages
of quotes to find new content.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Jeff Cody]

On Tue, Feb 10, 2015 at 02:54:39PM +0100, Kevin Wolf wrote:
> 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?
>

Yes, exactly.  I don't know if that will catch all the edge use case
combinations (an image may have been created by Virtual PC, but
formatted and used as a guest under Hyper-V, etc..).  But it seems
like a reasonable approach that improves the current situation.