Re: [PATCH v6 2/4] copy-on-read: add filter append/drop functions

[Andrey Shinkevich]

On 23.08.2020 22:35, Andrey Shinkevich wrote:
> On 19.08.2020 13:21, Vladimir Sementsov-Ogievskiy wrote:
> > 19.08.2020 00:24, Andrey Shinkevich wrote:
> > > Provide API for the COR-filter insertion/removal.
> > > Also, drop the filter child permissions for an inactive state when the
> > > filter node is being removed.
> > >
> > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> > > ---
> > >   block/copy-on-read.c | 103 
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >   block/copy-on-read.h |  36 ++++++++++++++++++
> > >   2 files changed, 139 insertions(+)
> > >   create mode 100644 block/copy-on-read.h
> > >
> > > diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> > > index cb03e0f..150d9b7 100644
> > > --- a/block/copy-on-read.c
> > > +++ b/block/copy-on-read.c
> > > @@ -23,11 +23,21 @@
...
> > > +BlockDriverState *bdrv_cor_filter_append(BlockDriverState *bs,
> > > +                                         const char *filter_node_name,
> > > +                                         Error **errp)
> > > +{
> > > +    BlockDriverState *cor_filter_bs;
> > > +    BDRVStateCOR *state;
> > > +    Error *local_err = NULL;
> > > +
> > > +    cor_filter_bs = create_filter_node(bs, filter_node_name, errp);
> > > +    if (cor_filter_bs == NULL) {
> > > +        error_prepend(errp, "Could not create filter node: ");
> > > +        return NULL;
> > > +    }
> > > +
> > > +    if (!filter_node_name) {
> > > +        cor_filter_bs->implicit = true;
> > > +    }
> > > +
> > > +    bdrv_drained_begin(bs);
> > > +    bdrv_replace_node(bs, cor_filter_bs, &local_err);
> > > +    bdrv_drained_end(bs);
> > > +
> > > +    if (local_err) {
> > > +        bdrv_unref(cor_filter_bs);
> > > +        error_propagate(errp, local_err);
> > > +        return NULL;
> > > +    }
> > > +
> > > +    state = cor_filter_bs->opaque;
> > > +    state->active = true;
> >
> > hmm stop. it's already active, after create_filter_node, so you don't 
> > need to set it again, isn't it?
>
>
> I will remove the extra assignment from the cor_open() for consistancy.
>
> Andrey

No, I won't. It is wrong to do that. COR-operations wouldn't work.

Andrey


> > > +
> > > +    return cor_filter_bs;
> > > +}
> > > +
...