Re: [Qemu-block] [PATCH v3 1/3] block: include base when checking image chain for block allocation

[Andrey Shinkevich]

On 08/04/2019 18:22, Alberto Garcia wrote:
> On Mon 08 Apr 2019 05:16:22 PM CEST, Andrey Shinkevich wrote:
>> On 08/04/2019 18:03, Alberto Garcia wrote:
>>> On Fri 05 Apr 2019 06:56:17 PM CEST, Andrey Shinkevich wrote:
>>>> +int bdrv_is_allocated_above(BlockDriverState *top,
>>>> +                            BlockDriverState *base,
>>>> +                            int64_t offset, int64_t bytes, int64_t *pnum)
>>>> +{
>>>> +    return bdrv_do_is_allocated_above(top, base, false, offset, bytes, 
>>>> pnum);
>>>> +}
>>>> +
>>>> +int bdrv_is_allocated_above_inclusive(BlockDriverState *top,
>>>> +                                      BlockDriverState *base,
>>>> +                                      int64_t offset, int64_t bytes,
>>>> +                                      int64_t *pnum)
>>>> +{
>>>> +    return bdrv_do_is_allocated_above(top, base, true, offset, bytes, 
>>>> pnum);
>>>> +}
>>>
>>> Instead of having these two, isn't it simpler to add an 'include_base'
>>> parameter to the original function?
>>>
>>> Another alternative (I haven't checked this one so it could be more
>>> cumbersome): change the semantics of the function to always include the
>>> base and modify the callers.
>>
>> The idea behind those two functions was to keep the rest of the code
>> unmodified. Currently, we have the issue with the block-stream
>> parallel jobs. What if we manage this case first and then, when proved
>> to be robust, take care of the rest?
> 
> Sure, that makes sense if you need to do significant changes to the
> other callers, but if you just need to pass an extra 'false'
> parameter...
> 
> It's not a big deal, I just think you'd have a simpler patch.
> 
> Berto
> 

OK, I'll do that with the next v4.
-- 
With the best regards,
Andrey Shinkevich