qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH 3/3] block/stream: introduce a bottom node
Date: Tue, 2 Apr 2019 08:59:25 +0000

02.04.2019 11:51, Andrey Shinkevich wrote:
> 
> 
> On 01/04/2019 18:44, Alberto Garcia wrote:
>> 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
>>
> 
> We have got a latent trouble with the base involved into the process.
> For instance, if we encounter a filter while searching for the base,
> we will want to manage it somehow. If the base is identified by the
> node name rather than by file name, it would be easier. So, we would
> assign the node name to the base earliest as possible. Another approach
> suggested by Vladimir supposed to eliminate the base from the interface.
> It is a sensitive change and we all need to come to an agreement.
> 


On the other hand we freeze backing chain in stream_start anyway. So, we'd like
to be sure anyway, that there are no yields before stream_start() call.

-- 
Best regards,
Vladimir

reply via email to

[Prev in Thread] Current Thread [Next in Thread]