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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1665 bytes --]

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 <vsementsov@virtuozzo.com>
> ---
> 
> 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).

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

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]