Re: [Qemu-block] [PATCH 3/3] block/stream: introduce a bottom node

[Alberto Garcia]

On Fri 29 Mar 2019 05:15:43 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>>> @@ -3237,7 +3238,14 @@ void qmp_block_stream(bool has_job_id, const char 
>>> *job_id, const char *device,
>>>           job_flags |= JOB_MANUAL_DISMISS;
>>>       }
>>>   
>>> -    stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name,
>>> +    /* Find the bottom node that has the base as its backing image */
>>> +    bottom_node = bs;
>>> +    while ((iter = backing_bs(bottom_node)) != base_bs) {
>>> +        bottom_node = iter;
>>> +    }
>>> +    assert(bottom_node);
>>> +
>>> +    stream_start(has_job_id ? job_id : NULL, bs, bottom_node, base_name,
>>>                    job_flags, has_speed ? speed : 0, on_error, &local_err);
>> 
>> Isn't it simpler to pass 'base' to stream_start() and find the bottom
>> node there? (with bdrv_find_overlay()).
>> 
>> I think bottom should be an internal implementation detail of the
>> block-stream driver, callers don't need to know about it, or do they?
>> 
> My idea is that we should get rid of base before any yield, and better
> do it as soon as possible.

But you can do it at the beginning of stream_start() without exposing
'bottom' to the callers which, again, is an implementation detail.

Berto