Re: [PATCH v2] block/backup-top: fix flags handling

[Vladimir Sementsov-Ogievskiy]

06.02.2020 19:31, Max Reitz wrote:
> On 03.02.20 14:42, Vladimir Sementsov-Ogievskiy wrote:
> > backup-top "supports" write-unchanged, by skipping CBW operation in
> > backup_top_co_pwritev. But it forgets to do the same in
> > backup_top_co_pwrite_zeroes, as well as declare support for
> > BDRV_REQ_WRITE_UNCHANGED.
> >
> > Fix this, and, while being here, declare also support for flags
> > supported by source child.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---
> >
> > v2: restrict flags propagation like it is done in other filters [Eric]
> >      move state variable initialization to the top
> >
> >   block/backup-top.c | 32 ++++++++++++++++++++------------
> >   1 file changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/block/backup-top.c b/block/backup-top.c
> > index 9aed2eb4c0..a4cec60859 100644
> > --- a/block/backup-top.c
> > +++ b/block/backup-top.c
>
>
> [...]
>
> > @@ -186,17 +190,21 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState 
> > *source,
> >                                            Error **errp)
> >   {
> >       Error *local_err = NULL;
> > -    BDRVBackupTopState *state;
> >       BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
> >                                                    filter_node_name,
> >                                                    BDRV_O_RDWR, errp);
> > +    BDRVBackupTopState *state = top->opaque;
> >   
> >       if (!top) {
> >           return NULL;
> >       }
>
> If top can be NULL, then we shouldn’t dereference it before (with
> state = top->opaque).

Oh yes.

> (Pulling up the initialization of @state is also unrelated to this
> patch, I don’t exactly know why you’re doing it here.)

I just though, that it is small good style enhancement, but it isn't :)

Let's keep state variable handling as it was.

--
Best regards,
Vladimir