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

[Andrey Shinkevich]

On 11/02/2019 17:07, Alberto Garcia wrote:
> On Fri 08 Feb 2019 04:29:48 PM CET, Andrey Shinkevich wrote:
>> On 08/02/2019 16:13, Alberto Garcia wrote:
>>> On Wed 23 Jan 2019 12:54:24 PM CET, Andrey Shinkevich wrote:
>>>> +static BlockDriverState *insert_filter(BlockDriverState *bs, Error **errp)
>>>> +{
>>>> +    BlockDriverState *cor_filter_bs;
>>>> +    Error *local_err = NULL;
>>>> +
>>>> +    cor_filter_bs = create_filter_node(bs, errp);
>>>> +    if (cor_filter_bs == NULL) {
>>>> +        error_prepend(errp, "Could not create filter node: ");
>>>> +        return NULL;
>>>> +    }
>>>> +
>>>> +    bdrv_set_aio_context(cor_filter_bs, bdrv_get_aio_context(bs));
>>>> +
>>>> +    bdrv_drained_begin(bs);
>>>> +    bdrv_replace_node(bs, cor_filter_bs, &local_err);
>>>> +    bdrv_drained_end(bs);
>>>
>>> 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?
> 
> Berto
> 
When the block-stream QMP command is run with a base file path name,
qmp_block_stream() invokes bdrv_lookup_bs() to get the
BlockDriverState by the file name and to pass it further to
stream_start(), while has_base_node == false.
It works the same way for the copy-on-read filter inserted above the
QCOW2 node because they both have the same and only file path name.
So, the BlockDriverState of the filter is passed to the
stream_start() as the parameter instead of the one of the QCOW2 base
node. I emailed the workaround patch that skips the filter BS passed
to the stream_start() as the parameter, if any.
There is an idea of Vladimir to pass an intermediate node that has
the base node as its backing one instead. If you can suggest another
idea, it will be appreciated.

-- 
With the best regards,
Andrey Shinkevich