Re: [libvirt] [Qemu-devel] QEMU interfaces for image streaming and post-copy block migration
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/d44ea43be084cc879cd1a33e...
>
>>> 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