[4 Patches] New blktap implementation, 2nd try

[4 Patches] New blktap implementation, 2nd try

[Dutch Meyer]

The list filters didn't like the first 2 patches, so I'm reposting the 
whole set from my site. I've also removed the hastily written and 
apparently misleading "less open source" line from the explanation below, 
so that people will stop teasing me.  Rest assured, these patches are 100% 
pure open source.

http://www.cs.ubc.ca/~dmeyer/blktapPatches/deprecate-blktap
http://www.cs.ubc.ca/~dmeyer/blktapPatches/restore-blktap
http://www.cs.ubc.ca/~dmeyer/blktapPatches/fixqcow
http://www.cs.ubc.ca/~dmeyer/blktapPatches/blktap2module

Signed-off-by: Jake Wires <jake.wires@citrix.com>, Dutch Meyer 
<dmeyer@cs.ubc.ca>

This is a new and rewritten version of blktap that we have developed at
Citrix.  The current version of blktap is left functionally unmodified.
The change set consists of four patches.

   1) deprecate-blktap. A patch to deprecate the open source blktap, by 
moving it and issuing a warning whenever it is used.  No functionality is 
modified in this patch, it is just housekeeping.

   2) restore-blktap.  A patch to add a new blktap implementation that is 
feature equivalent to (or better than) the current blktap.  This will 
eventually replace the current blktap implementation.

   3) fixqcow.  Fix several bugs in the qcow tools.

   4) blktap2module.  A kernel patch to add a new unified blktap2 module 
that will eventually replace blktap.

The new blktap implementation has several improvements.

   * Isolation from xenstore - Blktap devices can now be created in dom0 as
virtual block devices without coordination from xen and have few
dependencies on xenstore in normal operation.

   * Improved development environment for tapdisks, simpler request
forwarding, new request scheduler.

   * Pause scripts updated to support live qcow snapshot (see xmsnap
script)

   * New tapdisk type: Block Mason disks allow a set of tapdisks to be
flexibly arranged into graph structure and modified on-the-fly.  Several
example modules for Block Mason are included.  Block Mason disks are
constructed and modified with a declarative configuration language.
These capabilities are discussed in more depth in an upcoming paper in the
First Workshop on IO Virtualization, available HERE:
http://www.cs.ubc.ca/~dmeyer/blockmason-wiov-final.pdf

--Dutch

Re: [4 Patches] New blktap implementation, 2nd try

[Kevin Wolf]

Dutch Meyer schrieb:
>   1) deprecate-blktap. A patch to deprecate the open source blktap, by
> moving it and issuing a warning whenever it is used.  No functionality
> is modified in this patch, it is just housekeeping.

Why would you want to keep the old version around if the new one is so
much better?

>   2) restore-blktap.  A patch to add a new blktap implementation that is
> feature equivalent to (or better than) the current blktap.  This will
> eventually replace the current blktap implementation.

Now this is a huge patch, nearly impossible to review. At a first glance
I noticed that it shares large parts with the old implementation. So
while I believe you that the first patch is only moving things around, I
really can't tell what this patch is actually doing.

It would be really useful to have the real diff, i.e. keep the old files
in patch 1 and modify them here. Best you also split this patch into a
patch for the core and one patch for the changes to each driver.

I didn't really look into the patch but merely scrolled through it and
looked at the diffstat, so maybe I'm wrong, but it seems to lack
implementations for tap:qcow2, tap:vmdk and tap:ioemu. Why that?

Kevin

Re: [4 Patches] New blktap implementation, 2nd try

[Dutch Meyer]

> Why would you want to keep the old version around if the new one is so
> much better?

We're very open to removing the old version, but there is a backwards 
compatability concern.  See below.

> Now this is a huge patch, nearly impossible to review. At a first glance
> I noticed that it shares large parts with the old implementation. So
> while I believe you that the first patch is only moving things around, I
> really can't tell what this patch is actually doing.

I think relative to the changes, there isn't that much that is shared with 
the old implementation.  Taking a diff against the two directories will 
show that the core of blktap has been redone, much of it from scratch. 
That's why we did it this way.

You can diff versus the two blktap directories to see this.  If you still 
think this would be very helpful, I can provide this diff.  The drivers 
are, at their core, unchanged so perhaps separate patches would make sense 
in that case.

> I didn't really look into the patch but merely scrolled through it and
> looked at the diffstat, so maybe I'm wrong, but it seems to lack
> implementations for tap:qcow2, tap:vmdk and tap:ioemu. Why that?

You're right about that - we decided to post this without qcow2, but can 
port that one as well.  VMDK is slow, it's written to be purely 
synchronous, and we don't think anyone actually uses it.

ioemu is a different matter - This is a very difficult piece of code to 
work with, because large portions of blktap have been replicated to make 
it work.  On top of that, the wire protocol changed to support this. 
Porting this driver would be quite difficult.  This ioemu code is the 
primary reason we provided backwards compatability with what is now 
"blktap_old".


I really appreciate the conversation here, and look forward to keeping it 
going, but i may be away from email for the weekend, so please excuse any 
slowness in my reply.
--Dutch