On 09/12/2010 08:40 AM, Avi Kivity wrote:
Why would it serialize all I/O operations? It's just like another
vcpu issuing reads.
Because the block layer isn't re-entrant.
What you basically do is:
stream_step_three():
complete()
stream_step_two(offset, length):
bdrv_aio_readv(offset, length, buffer, stream_step_three)
bdrv_aio_stream():
bdrv_aio_find_free_cluster(stream_step_two)
Isn't there a write() missing somewhere?
Streaming relies on copy-on-read to do the writing.
And that's exactly what the current code looks like. The only change
to the patch that this does is make some of qed's internals be block
layer interfaces.
Why do you need find_free_cluster()? That's a physical offset thing.
Just write to the same logical offset.
IOW:
bdrv_aio_stream():
bdrv_aio_read(offset, stream_2)
It's an optimization. If you've got a fully missing L1 entry, then
you're going to memset() 2GB worth of zeros. That's just wasted work.
With a 1TB image with a 1GB allocation, it's a huge amount of wasted work.
stream_2():
if all zeros:
increment offset
if more:
bdrv_aio_stream()
bdrv_aio_write(offset, stream_3)
stream_3():
bdrv_aio_write(offset, stream_4)
I don't understand why stream_3() is needed.
stream_4():
increment offset
if more:
bdrv_aio_stream()
Of course, need to serialize wrt guest writes, which adds a bit more
complexity. I'll leave it to you to code the state machine for that.
http://repo.or.cz/w/qemu/aliguori.git/commitdiff/d44ea43be084cc879cd1a33e1a04a105f4cb7637?hp=34ed425e7dd39c511bc247d1ab900e19b8c74a5d
Third problem is that streaming really requires being able to do
zero write detection in a meaningful way. You don't want to always
do zero write detection so you need another interface to mark a
specific write as a write that should be checked for zeros.
You can do that in bdrv_stream(), above, before the actual write,
and call bdrv_unmap() if you detect zeros.
My QED branch now does that FWIW. At the moment, it only detects
zero reads to unallocated clusters and writes a special zero cluster
marker. However, the detection code is in the generic path so once
the fsck() logic is working, we can implement a free list in QED.
In QED, the detection code needs to have a lot of knowledge about
cluster boundaries and the format of the device. In principle, this
should be common code but it's not for the same reason copy-on-write
is not common code today.
Parts of it are: commit. Of course, that's horribly synchronous.
If you've got AIO internally, making commit work is pretty easy. Doing
asynchronous commit at a generic layer is not easy though unless you
expose lots of details.
Generally, I think the block layer makes more sense if the interface to
the formats are high level and code sharing is achieved not by mandating
a world view but rather but making libraries of common functionality.
This is more akin to how the FS layer works in Linux.
So IMHO, we ought to add a bdrv_aio_commit function, turn the current
code into a generic_aio_commit, implement a qed_aio_commit, then somehow
do qcow2_aio_commit, and look at what we can refactor into common code.
Regards,
Anthony Liguori
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list