Re: [PATCH v7 35/47] commit: Deal with filters

[Andrey Shinkevich]

On 23.07.2020 20:15, Andrey Shinkevich wrote:
> On 25.06.2020 18:22, Max Reitz wrote:
> > This includes some permission limiting (for example, we only need to
> > take the RESIZE permission if the base is smaller than the top).
> >
> > Signed-off-by: Max Reitz <mreitz@redhat.com>
> > ---
> >   block/block-backend.c          |  9 +++-
> >   block/commit.c                 | 96 +++++++++++++++++++++++++---------
> >   block/monitor/block-hmp-cmds.c |  2 +-
> >   blockdev.c                     |  4 +-
> >   4 files changed, 81 insertions(+), 30 deletions(-)
> ...
> > +    /*
> > +     * Block all nodes between top and base, because they will
> > +     * disappear from the chain after this operation.
> > +     * Note that this assumes that the user is fine with removing all
> > +     * nodes (including R/W filters) between top and base. Assuring
> > +     * this is the responsibility of the interface (i.e. whoever calls
> > +     * commit_start()).
> > +     */
> > +    s->base_overlay = bdrv_find_overlay(top, base);
> > +    assert(s->base_overlay);
> > +
> > +    /*
> > +     * The topmost node with
> > +     * bdrv_skip_filters(filtered_base) == bdrv_skip_filters(base)
> > +     */
> > +    filtered_base = bdrv_cow_bs(s->base_overlay);
> > +    assert(bdrv_skip_filters(filtered_base) == 
> > bdrv_skip_filters(base));
> > +
> > +    /*
> > +     * XXX BLK_PERM_WRITE needs to be allowed so we don't block 
> > ourselves
> > +     * at s->base (if writes are blocked for a node, they are also 
> > blocked
> > +     * for its backing file). The other options would be a second 
> > filter
> > +     * driver above s->base.
> > +     */
> > +    iter_shared_perms = BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE;
> > +
> > +    for (iter = top; iter != base; iter = 
> > bdrv_filter_or_cow_bs(iter)) {
> > +        if (iter == filtered_base) {
>
>
> The question same to mirroring:
>
> in case of multiple filters, one above another, the permission is 
> extended for the filtered_base only.
>
> Andrey

The question has been answered already.

Andrey


> > +            /*
> > +             * From here on, all nodes are filters on the base.  This
> > +             * allows us to share BLK_PERM_CONSISTENT_READ.
> > +             */
> > +            iter_shared_perms |= BLK_PERM_CONSISTENT_READ;
> > +        }
> > +
> >           ret = block_job_add_bdrv(&s->common, "intermediate node", 
> > iter, 0,
> > -                                 BLK_PERM_WRITE_UNCHANGED | 
> > BLK_PERM_WRITE,
> > -                                 errp);
> > +                                 iter_shared_perms, errp);
> >           if (ret < 0) {
> >               goto fail;
> >           }
>
> ...
>
> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>