Xen devel - Mar 2008 - The two image formats called qcow

Hi,

the idea of the recently submitted tap:ioemu block driver was that it
would be a drop-in replacement for all the other tap:* so that in the
long term tapdisk could be abandoned. This should work quite well
because we''re having a block driver for each format supported by
tapdisk
in ioemu as well.

So far for the theory. In practice this doesn''t prove to be true: There
is a blktap driver claiming to implement a format called qcow and there
is an ioemu driver for qcow - and both formats are not the same, of
course. (The reason is that the original qcow uses a big endian L1 table
whereas blktap uses little endian - I honestly cannot imagine how one
could change this unintentionally, but OTOH why would you want to break
compatibility for no clear benefit?)

This does not only mean that you cannot use qcow images created for
blktap with qemu, but also that PV and HVM qcow have always been
incompatible. Am I really the first one to notice this?

Now if we''re going to use ioemu as the one and only block driver code,
this will be a problem. How should this be handled best? We could add
code to ioemu to deal with the broken blktap images using some
heuristics (and maybe convert endianess whenever you open such a broken
file). This would mean that we have to carry a fix for a bug in older
versions forever. The other possibility would be to let the user convert
the old broken image files manually (with a new tool) and keep ioemu clean.

Which solution would you prefer? Or maybe you have completely different,
better ideas how to deal with it?

Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Kevin Wolf <kwolf@suse.de> writes:
> Which solution would you prefer? Or maybe you have completely different,
> better ideas how to deal with it?
If it was up to me, I''d choose to convert the image and keep the code
clean. Keeping workarounds forever shouldn''t be an option.

-- 
Otavio Salvador                  O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854         http://projetos.ossystems.com.br

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 25/3/08 18:58, "Otavio Salvador" <otavio@ossystems.com.br>
wrote:
> Kevin Wolf <kwolf@suse.de> writes:
> 
>> Which solution would you prefer? Or maybe you have completely
different,
>> better ideas how to deal with it?
> 
> If it was up to me, I''d choose to convert the image and keep the
code
> clean. Keeping workarounds forever shouldn''t be an option.
It''s tricky where users'' non-volatile storage is concerned
though. Other
than that I would say the bug should be fixed immediately. Is there an easy
way to detect with reasonable reliability whether we have an old or new
image? Failing that we may have to provide a tool to convert old images to
new format.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser schrieb:
> It''s tricky where users'' non-volatile storage is
concerned though. Other
> than that I would say the bug should be fixed immediately. Is there an easy
> way to detect with reasonable reliability whether we have an old or new
> image? Failing that we may have to provide a tool to convert old images to
> new format.
Something like "that number looks too big, it be must little endian"
could easily turn into "that harddisk looks big, let''s break the
image",
I suspect.

However, I just noticed that the tapdisk qcow driver writes an extended
Xen-specific header to the image file. This should be reliable enough to
detect tapdisk images.

Is it an option to convert broken images to big endian when it is opened
for the first time in ioemu? In this case, the fix for older versions
could be in one place at least instead of being scattered over the whole
file. Then you wouldn''t be able to open such a file with tapdisk again,
though.

Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Kevin Wolf <kwolf@suse.de> writes:
> Is it an option to convert broken images to big endian when it is opened
> for the first time in ioemu? In this case, the fix for older versions
> could be in one place at least instead of being scattered over the whole
> file. Then you wouldn''t be able to open such a file with tapdisk
again,
> though.
If it could be done, a note in release notes would be required to warn
users that when it has been opened in a newer version using ioemu it
wouldn''t be possible to be used again by tap driver anymore.

But yes, this would be an easy to use option if it''s not too messy and
takes too long to convert the image (otherwise user can think that the
system has freeze).

-- 
Otavio Salvador                  O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854         http://projetos.ossystems.com.br

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, Mar 26, 2008 at 09:50:06AM +0100, Kevin Wolf
wrote:
> Keir Fraser schrieb:
> > It''s tricky where users'' non-volatile storage is
concerned though. Other
> > than that I would say the bug should be fixed immediately. Is there an
easy
> > way to detect with reasonable reliability whether we have an old or
new
> > image? Failing that we may have to provide a tool to convert old
images to
> > new format.
> 
> Something like "that number looks too big, it be must little
endian"
> could easily turn into "that harddisk looks big, let''s break
the image",
> I suspect.
> 
> However, I just noticed that the tapdisk qcow driver writes an extended
> Xen-specific header to the image file. This should be reliable enough to
> detect tapdisk images.
> 
> Is it an option to convert broken images to big endian when it is opened
> for the first time in ioemu? In this case, the fix for older versions
> could be in one place at least instead of being scattered over the whole
> file. Then you wouldn''t be able to open such a file with tapdisk
again,
> though.
I don''t like the idea of  secretly migrating them to  the fixed disk
format
without admin interaction / confirmation.

If we can detect the old style disk format, then perhaps we could put a
check into the hotplug scripts. That way, when the user tries to start
a guest with the old broken format, we could prevent the guest starting
and show them a error message. Then can then run the conversion tool
to fix the format & start the guest again. 

Dan
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Otavio Salvador schrieb:
> Kevin Wolf <kwolf@suse.de> writes:
> 
>> Is it an option to convert broken images to big endian when it is
opened
>> for the first time in ioemu? In this case, the fix for older versions
>> could be in one place at least instead of being scattered over the
whole
>> file. Then you wouldn''t be able to open such a file with
tapdisk again,
>> though.
> 
> If it could be done, a note in release notes would be required to warn
> users that when it has been opened in a newer version using ioemu it
> wouldn''t be possible to be used again by tap driver anymore.
I agree, a note wouldn''t be wrong.
> But yes, this would be an easy to use option if it''s not too messy
and
> takes too long to convert the image (otherwise user can think that the
> system has freeze).
Converting the image means byte-swapping the L1 table which is some
kilobytes in size. This is done in no time, the user won''t even notice
the delay. The attached patch implements this conversion.

What should we do with the tapdisk implementation? Leave it broken and
hope that it will disappear soon, add support for big endian L1 tables
or do a conversion the other way round? The latter doesn''t feel right
(in fact it would be intentionally breaking a correct image), but adding
support for big endian is much more critical because we end up with
"mixed endian" if we miss one conversion...

Kevin


ioemu: Fix L1 table endianess of qcow images created by tapdisk

The qemu/ioemu implementation of the qcow format uses a big endian L1
table. tapdisk omits the necessary conversion, so qcow images have the
wrong endianess and cannot be read by correct implementations of qcow.

This patch detects broken tapdisk images and converts their L1 tables to
big endian when the image file is opened in ioemu for the first time.
The fixed image has a new flag EXTHDR_L1_BIG_ENDIAN set in the extended
header.

Note that a converted image cannot be opened by tapdisk again.

Signed-off-by: Kevin Wolf <kwolf@suse.de>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 26/3/08 13:54, "Kevin Wolf" <kwolf@suse.de> wrote:
> ioemu: Fix L1 table endianess of qcow images created by tapdisk
> 
> The qemu/ioemu implementation of the qcow format uses a big endian L1
> table. tapdisk omits the necessary conversion, so qcow images have the
> wrong endianess and cannot be read by correct implementations of qcow.
> 
> This patch detects broken tapdisk images and converts their L1 tables to
> big endian when the image file is opened in ioemu for the first time.
> The fixed image has a new flag EXTHDR_L1_BIG_ENDIAN set in the extended
> header.
> 
> Note that a converted image cannot be opened by tapdisk again.
Can we really tack an ''extended header'' into a public format
like qcow?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser schrieb:
> On 26/3/08 13:54, "Kevin Wolf" <kwolf@suse.de> wrote:
> 
>> ioemu: Fix L1 table endianess of qcow images created by tapdisk
>>
>> The qemu/ioemu implementation of the qcow format uses a big endian L1
>> table. tapdisk omits the necessary conversion, so qcow images have the
>> wrong endianess and cannot be read by correct implementations of qcow.
>>
>> This patch detects broken tapdisk images and converts their L1 tables
to
>> big endian when the image file is opened in ioemu for the first time.
>> The fixed image has a new flag EXTHDR_L1_BIG_ENDIAN set in the extended
>> header.
>>
>> Note that a converted image cannot be opened by tapdisk again.
> 
> Can we really tack an ''extended header'' into a public
format like qcow?
I didn''t introduce this, it was already there in tapdisk. I
don''t see a
problem with it as the start of the L1 table is referenced in the normal
qcow header. qemu-img sets this to something like 0x48 which is
immediately after the header, tapdisk uses 0x1000 and gains some unused
space for things like the extended header. This is compatible with the
qcow implementation of qemu/ioemu.

On the other hand, I could simply strip that extended header (i.e.
overwrite the magic with 0x0) after having fixed the image. Then it
wouldn''t be detected as broken on the next start as well.

Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 26/3/08 14:07, "Kevin Wolf" <kwolf@suse.de> wrote:
>> Can we really tack an ''extended header'' into a public
format like qcow?
> 
> I didn''t introduce this, it was already there in tapdisk. I
don''t see a
> problem with it as the start of the L1 table is referenced in the normal
> qcow header. qemu-img sets this to something like 0x48 which is
> immediately after the header, tapdisk uses 0x1000 and gains some unused
> space for things like the extended header. This is compatible with the
> qcow implementation of qemu/ioemu.
> 
> On the other hand, I could simply strip that extended header (i.e.
> overwrite the magic with 0x0) after having fixed the image. Then it
> wouldn''t be detected as broken on the next start as well.
Oh, I see. I think it''s fine as it is then. Is there any reason not to
paste
this fixup code into tapdisk too?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser schrieb:
> On 26/3/08 14:07, "Kevin Wolf" <kwolf@suse.de> wrote:
> 
>>> Can we really tack an ''extended header'' into a
public format like qcow?
>> I didn''t introduce this, it was already there in tapdisk. I
don''t see a
>> problem with it as the start of the L1 table is referenced in the
normal
>> qcow header. qemu-img sets this to something like 0x48 which is
>> immediately after the header, tapdisk uses 0x1000 and gains some unused
>> space for things like the extended header. This is compatible with the
>> qcow implementation of qemu/ioemu.
>>
>> On the other hand, I could simply strip that extended header (i.e.
>> overwrite the magic with 0x0) after having fixed the image. Then it
>> wouldn''t be detected as broken on the next start as well.
> 
> Oh, I see. I think it''s fine as it is then. Is there any reason
not to paste
> this fixup code into tapdisk too?
It''s not done with the conversion in tapdisk. You would also need to
change all writes to the L1 table. Additionally, I noticed that this
glorious extended header contains a checksum over the L1 table. And I''m
not sure if there are other traps in that code.

As I explained in the mail containing the patch I really don''t want to
end up with a "mixed endian" image by overlooking a needed change. You
could throw it away then. Better don''t start the VM at all and let the
user specify tap:ioemu...

Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Kevin Wolf schrieb:
> What should we do with the tapdisk implementation? Leave it broken and
> hope that it will disappear soon, add support for big endian L1 tables
> or do a conversion the other way round? The latter doesn''t feel
right
> (in fact it would be intentionally breaking a correct image), but adding
> support for big endian is much more critical because we end up with
> "mixed endian" if we miss one conversion...
And another one for tapdisk. I''m taking the same approach as for ioemu
here, i.e. converting the endianess when the image is opened and
rewriting the tapdisk code to use big endian.

To avoid the mentioned "mixed endian" issue and thus data corruption,
please double check the patch before you check it in. I successfully
installed a VM with this patch, though, so I''m confident that it is
correct.

Kevin


tapdisk: Fix L1 table endianess of qcow images

Fix tapdisk to use big endian L1 tables as used by qemu/ioemu. Old
tapdisk images with native endianess are automagically converted to big
endian when the image file is opened for the first time.

Signed-off-by: Kevin Wolf <kwolf@suse.de>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel