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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

23.01.2019 14:54, Andrey Shinkevich wrote:
> The copy-on-read filter driver is applied to block-stream operations.
> The 'test_stream_parallel' in the file tests/qemu-iotests/030 runs
> jobs that use nodes for streaming in parallel through the backing chain.
> We've got filters being inserted to and removed from the backing chain
> while jobs are running. As a result, a filter node may be passed as the
> 'base' parameter to the stream_start() function when the base node name
> is not specified (the base node is identified by its file name which is
> the same to the related filter node).
> Another issue is that a function keeps the pointer to the filter BDS
> object that can be replaced and deleted after the co-routine switch.
> For example, the function backing_bs() returns the pointer to the
> backing BDS and the BDS reference counter is not incremented.
> A solution (or workaround) made with the given patch for block-stream
> job helps to pass all the iotests in the file tests/qemu-iotests/030.
> Any piece of advice how to amend the solution will be appreciated.
> I am looking forward to hearing from you.


So, in short, when filters comes to node-graph, we have two problems with stream job:

1. Searching node by filename works bad. It may (and I think, should) be workarounded
by using node-names and only node-names to select top and base node..
But should we, and how to support old scenarios with selecting nodes by filenames?

2. "base" works bad. Actually, job don't own base node, so it's illegal to keep pointer
to it. Base may change during the job.
So the best option, would be to use "bottom-node" concept instead of "base". If we don't want to
change qmp interface, we should calculate bottom-node from base at block-job creation time,
before any context switch, and keep pointer to bottom-node, instead of base.
Also, we should rewrite bdrv_block_status to support bottom_node instead of base, as again,
if caller owns only top and intermediate nodes, base may change during block_status calculation.

-- 
Best regards,
Vladimir