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

[Andrey Shinkevich]

On 29/03/2019 19:07, Alberto Garcia wrote:
> On Fri 29 Mar 2019 02:29:14 PM CET, Andrey Shinkevich 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 am going to move the bottom = bdrv_find_overlay() into the
stream_start() in v3, which is coming soon.

> 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?
> 
> Berto
> 

-- 
With the best regards,
Andrey Shinkevich