Xen devel - Feb 2008 - [PATCH] blktap: qcow2 image format support

This patch adds support for the qcow2 image format to blktap. It
consists mostly of qemu code, adapted to the blktap interfaces.
Snapshots and compressed images are supported.

The qcow2 driver may be used by either specifying tap:qcow2 or by using
tap:qcow which will detect that you have a version 2 image and will call
the qcow2 driver.

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


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

On 21/2/08 08:49, "Kevin Wolf" <kwolf@suse.de> wrote:
> This patch adds support for the qcow2 image format to blktap. It
> consists mostly of qemu code, adapted to the blktap interfaces.
> Snapshots and compressed images are supported.
> 
> The qcow2 driver may be used by either specifying tap:qcow2 or by using
> tap:qcow which will detect that you have a version 2 image and will call
> the qcow2 driver.
Is qcow2 so different from qcow1 that it really needs its own backend
driver?

 -- Keir



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

On Thursday 21 February 2008 09:57:48 Keir Fraser wrote:
> On 21/2/08 08:49, "Kevin Wolf" <kwolf@suse.de> wrote:
> > This patch adds support for the qcow2 image format to blktap. It
> > consists mostly of qemu code, adapted to the blktap interfaces.
> > Snapshots and compressed images are supported.
> >
> > The qcow2 driver may be used by either specifying tap:qcow2 or by
using
> > tap:qcow which will detect that you have a version 2 image and will
call
> > the qcow2 driver.
>
> Is qcow2 so different from qcow1 that it really needs its own backend
> driver?
IMO, _all_ blktap drivers need some code refactoring. There is a lot of
duplicated code across all drivers
that can be moved into a blktap/drivers/blk-common.c.

Christoph

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy


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

On 21/2/08 09:28, "Kevin Wolf" <kwolf@suse.de> wrote:
> That''s a good question. I am not really familiar with the qcow
code but
> I think I saw quite some differences. If you put enough effort on it,
> you could probably get a single driver for both, but I can''t
really
> estimate if it was a sensible thing to do.
> 
> The main reason why I did it this way is that it is the way qemu has
> done it, so I just had to take and adapt their code. I''m not sure
if it
> is a good idea to diverge too much from the qemu code base.
Okay that seems reasonable.

 -- Keir



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

Keir Fraser schrieb:
> Is qcow2 so different from qcow1 that it really needs its own backend
> driver?
That''s a good question. I am not really familiar with the qcow code but
I think I saw quite some differences. If you put enough effort on it,
you could probably get a single driver for both, but I can''t really
estimate if it was a sensible thing to do.

The main reason why I did it this way is that it is the way qemu has
done it, so I just had to take and adapt their code. I''m not sure if it
is a good idea to diverge too much from the qemu code base.

Kevin

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

Christoph Egger schrieb:
> On Thursday 21 February 2008 09:57:48 Keir Fraser wrote:
> IMO, _all_ blktap drivers need some code refactoring. There is a lot of
> duplicated code across all drivers
> that can be moved into a blktap/drivers/blk-common.c.
Yes, I think you''re right on that. I noticed this especially with the
aio function which are duplicated in each single driver. I decided not
to fix this with the qcow2 patch as it is a different problem, but it
could be a good next step.

Kevin

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

On 21/2/08 08:49, "Kevin Wolf" <kwolf@suse.de> wrote:
> This patch adds support for the qcow2 image format to blktap. It
> consists mostly of qemu code, adapted to the blktap interfaces.
> Snapshots and compressed images are supported.
> 
> The qcow2 driver may be used by either specifying tap:qcow2 or by using
> tap:qcow which will detect that you have a version 2 image and will call
> the qcow2 driver.
Applied. I had to convert lx to PRIx64 in a number of places.

 -- Ekir



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

On Thu, Feb 21, 2008 at 10:07:14AM +0100, Christoph Egger
wrote:
> On Thursday 21 February 2008 09:57:48 Keir Fraser wrote:
> > On 21/2/08 08:49, "Kevin Wolf" <kwolf@suse.de> wrote:
> > > This patch adds support for the qcow2 image format to blktap. It
> > > consists mostly of qemu code, adapted to the blktap interfaces.
> > > Snapshots and compressed images are supported.
> > >
> > > The qcow2 driver may be used by either specifying tap:qcow2 or by
using
> > > tap:qcow which will detect that you have a version 2 image and
will call
> > > the qcow2 driver.
> >
> > Is qcow2 so different from qcow1 that it really needs its own backend
> > driver?
> 
> IMO, _all_ blktap drivers need some code refactoring. There is a lot of
> duplicated code across all drivers
> that can be moved into a blktap/drivers/blk-common.c.
Well if you''re concerned about code duplication - we''ve 2
entirely
separate impls of every disk format - one in blktap and one in QEMU
itself. The original motivation for the duplication was that QEMU''s
block driver API did not support Async-IO operations, but that was
addressed in 0.9.0 - if you look at the internal QEMU block driver
API for disk formats vs blktaps internal driver API they''re near
identical. I think it''d be an interesting project to actually make
QEMU userspace talk directly to blktap kernel driver, thus eliminating
the entire blktap users & all code duplication. Then we''d have
parity
of disk support across PV & HVM guests too. Certainly not an easy or
quick project, but if someone''s looking for something interesting to
hack on....

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   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

Daniel P. Berrange writes ("Re: [Xen-devel] [PATCH] blktap: qcow2 image
format support"):
> Well if you''re concerned about code duplication - we''ve 2
entirely
> separate impls of every disk format - one in blktap and one in QEMU
> itself. The original motivation for the duplication was that
QEMU''s
> block driver API did not support Async-IO operations, but that was
> addressed in 0.9.0 - if you look at the internal QEMU block driver
> API for disk formats vs blktaps internal driver API they''re near
> identical.
Yes.  This is pretty poor and although I don''t want to stand in the
way of useful stuff I''m disappointed to see more of this duplication
going on.
>  I think it''d be an interesting project to actually make
> QEMU userspace talk directly to blktap kernel driver, thus eliminating
> the entire blktap users & all code duplication. Then we''d have
parity
> of disk support across PV & HVM guests too. Certainly not an easy or
> quick project, but if someone''s looking for something interesting
to
> hack on....
I''m currently trying to merge our qemu with upstream''s, at
least
somewhat, and this is definitely something I''m keeping an eye on.

Ian.

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

Hi,

On Thu, Feb 21, 2008 at 10:31:35AM +0000, Keir Fraser
wrote:
> On 21/2/08 08:49, "Kevin Wolf" <kwolf@suse.de> wrote:
> 
> > This patch adds support for the qcow2 image format to blktap. It
> > consists mostly of qemu code, adapted to the blktap interfaces.
> > Snapshots and compressed images are supported.
> > 
> > The qcow2 driver may be used by either specifying tap:qcow2 or by
using
> > tap:qcow which will detect that you have a version 2 image and will
call
> > the qcow2 driver.
> 
> Applied. I had to convert lx to PRIx64 in a number of places.
> 
>  -- Ekir
Don''t know if this build breakage has been reported yet:

gcc  -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -m32 -march=i686 -fno-strict-aliasing -std=gnu99
-Wall -Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement  -m32
-march=i686 -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
-Wno-unused-value -Wdeclaration-after-statement  -m32 -march=i686
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-value
-Wdeclaration-after-statement  -D__XEN_TOOLS__ -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-mno-tls-direct-seg-refs -m32 -march=i686 -fno-strict-aliasing -std=gnu99 -Wall
-Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement 
-D__XEN_TOOLS__ -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs -m32 -march=i686
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-value
-Wdeclaration-after-statement  -D__XEN_TOOLS__ -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-mno-tls-direct-seg-refs -m32 -march=i686 -fno-strict-aliasing -std=gnu99 -Wall
-Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement 
-D__XEN_TOOLS__ -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs -Werror -Wno-unused
-I../../../tools/libxc -I../../../tools/include -I ../../libaio/src -I.. 
-I../lib -I../../../tools/include -I. -I../../xenstore  -D_GNU_SOURCE
-Wp,-MD,.block-qcow2.o.d -c -o block-qcow2.o block-qcow2.c
cc1: warnings being treated as errors
block-qcow2.c: In function ''qcow2_create'':
block-qcow2.c:1512: warning: ignoring return value of ''write'',
declared with attribute warn_unused_result
block-qcow2.c:1514: warning: ignoring return value of ''write'',
declared with attribute warn_unused_result
block-qcow2.c:1519: warning: ignoring return value of ''write'',
declared with attribute warn_unused_result
block-qcow2.c:1522: warning: ignoring return value of ''write'',
declared with attribute warn_unused_result
block-qcow2.c:1525: warning: ignoring return value of ''write'',
declared with attribute warn_unused_result
block-qcow2.c: In function ''qcow_write_compressed'':
block-qcow2.c:1554: warning: ignoring return value of
''ftruncate'',
declared with attribute warn_unused_result
make[4]: *** [block-qcow2.o] Error 1
make[4]: Leaving directory
`/builddir/build/BUILD/xen-unstable/tools/blktap/drivers''


Need to deal with return value of writes in qcow2_create() and ftruncate
in qcow_write_compressed().  Probably need to deal with errno too.

Thanks
kurt

-- 

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