Re: [Qemu-block] [PATCH RFC 1/1] Stream block job involves copy-on-read filter

[Vladimir Sementsov-Ogievskiy]

11.02.2019 18:52, Alberto Garcia wrote:
> On Mon 11 Feb 2019 03:51:33 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>>>>> I think this was already discussed in the previous version of this
>>>>> patch: if you insert a copy-on-read filter here then all guest
>>>>> reads will copy the data from the backing chain, but you don't want
>>>>> to copy anything below the 'base' node, so the copy-on-read filter
>>>>> needs a 'base' parameter.
>>>>>
>>>> Before that, we still have the unresolved issue with getting the
>>>> filter as the 'base' input parameter. That happens sometimes because
>>>> the base is being searched by the file name, which is the same to
>>>> the one of the filter, rather than by the node name (!)
>>>
>>> I'm not sure if I understand. block-stream has 'base' and 'base-node'
>>> parameters, and in the first case you would convert the file name to
>>> a node name.
>>>
>>> stream_start() gets the BlockDriverState, not the file name, so why
>>> can't you get the node name from there and pass it to the
>>> copy-on-read filter?
>>
>> The problem is in the concept of "base" node. The code written in
>> manner that base is not changed during block job. However, job don't
>> own base and there is no guarantee that it will not change during the
>> job.
> 
> But if that's the case then we have a problem already, because 'base' is
> a member of StreamBlockJob and is used in the existing stream_run()
> code.

I think it should be possible to reproduce, using block-commit (which already 
has
filter) with block-stream in parallel, we'll try.

> 
> So if there's a way to make 'base' disappear during the job (how?) then
> we could protect it with block_job_add_bdrv().

I'm not sure this is correct. What is the reason for stream to own base? It's 
not
really interested in it.

At least it'll break iotest test_stream_parallel which creates parallel stream 
jobs,
which shares border-nodes (ones top is others base). However this scenario looks
not real-life.

So, it's questionable, should stream block base or not. Not blocking is anyway 
more
flexible, and my suggestion tries to keep base not blocked.

Hm, I think it would be better to continue discussion over working reproducer,
I hope we'll make it soon.


-- 
Best regards,
Vladimir