On Tue 09 Apr 2019 04:43:12 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>> - while (intermediate && intermediate != base) {
>>> + while (include_base || intermediate != base) {
>>> int64_t pnum_inter;
>>> int64_t size_inter;
>>>
>>> @@ -2360,6 +2364,10 @@ int bdrv_is_allocated_above(BlockDriverState *top,
>>> n = pnum_inter;
>>> }
>>>
>>> + if (intermediate == base) {
>>> + break;
>>> + }
>>> +
>>> intermediate = backing_bs(intermediate);
>>
>> I find that the new condition + the break make things a bit less
>> readable. I think it would be simpler with something like this:
>>
>> BlockDriverState *stop_at = include_base ? backing_bs(base) : base;
>>
>> while (intermediate != stop_at) {
>> ...
>> }
>>
>
> But in this way you return back dependence on base, which we don't
> freeze and which may disappear on some iteration. We should not touch
> backing_bs(base) in any way.
Ok, I see.
Reviewed-by: Alberto Garcia <[email protected]>
(feel free to edit the comment with my suggestion, or leave it as it is
if you prefer)
Berto
