Re: [PATCH v6 2/5] block/io: bdrv_common_block_status_above: support include_base

[Vladimir Sementsov-Ogievskiy]

23.09.2020 19:18, Alberto Garcia wrote:
> On Wed 16 Sep 2020 02:20:05 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> > -    for (p = backing_bs(bs); p != base; p = backing_bs(p)) {
> > +    for (p = backing_bs(bs); include_base || p != base; p = backing_bs(p)) {
> >           ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
> >                                      file);
> >           if (ret < 0) {
> > @@ -2420,6 +2422,11 @@ bdrv_co_common_block_status_above(BlockDriverState *bs,
> >               break;
> >           }
> >   
> > +        if (p == base) {
> > +            assert(include_base);
> > +            break;
> > +        }
> > +
>
> Another option is something like:
>
>     BlockDriverState *last_bs = include_base ? base : backing_bs(base);

hmm, in case when include_base is false, last bs is not backing_bs(base) but 
the parent of base.

> and you get a simpler 'for' loop.
>
> But why do we need include_base at all? Can't the caller just pass
> backing_bs(base) instead? I'm talking also about the existing case of
> bdrv_is_allocated_above().


include_base was introduced for the case when caller doesn't own 
backing_bs(base), and therefore shouldn't do operations that may yield 
(block_status can) dependent on backing_bs(base). In particular, in block 
stream, where link to base is not frozen.


--
Best regards,
Vladimir