Re: [Qemu-devel] [Qemu-block] [PATCH v8 00/11] Support streaming to an intermediate layer

[Alberto Garcia]

On Tue 23 Jun 2015 06:48:39 PM CEST, Stefan Hajnoczi wrote:

>> It seems that self.vm.qmp('block-stream', ...) is returning None in
>> your case. Is that the only test that is failing?
>
> Yes, only this test fails.  I have pushed my tree here:
> https://github.com/stefanha/qemu/commits/berto-intermediate-streaming
>
> I wonder if you see this failure on your machine with my tree.

I was debugging the problem. So we have this scenario:

[A] <- [B] <- [C] <- [D] <- [E] <- [F] <- [G]

The test that is failing is test_stream_parallel(), that creates
several concurrent streaming operations, in this order:

 a) [B] => [C]
 b) [D] => [E]
 c) [F] => [G]

The way it works is that stream_start() reopens the destination image
in read-write mode (if needed), creates the stream_run() coroutine to
copy the data, and finally stream_complete() puts it back in read-only
mode and closes the images that are removed from the chain.

This was not causing problems earlier because each operation works on a
different set of images, but it seems that things have changed since the
reopen overhaul patches:

   https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01587.html
   https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01592.html

Here's what happens:

 - We launch block job a).
 - While a) is ongoing, b) enters and tries to reopen [E] in
   read-write mode.
 - With the reopen overhaul patches, the BlockReopenQueue no longer
   contains just [E] but also all its backing chain up to [A].
 - bdrv_reopen_multiple() calls bdrv_reopen_prepare() and flushes all
   images.
 - At this point job a) takes over and finishes, reopening [C] again
   in read-only mode and closing [B], removing it from memory.
 - When b) continues (still in bdrv_reopen_multiple) the queue still
   contains [B], which is no longer a valid pointer.

My first reaction was to keep a reference to each BDS in the
BlockReopenQueue during the operation, but I don't know why that queue
needs to contain all the backing chain in the first place.

Berto