Re: [PATCH v7 10/47] mirror-top: Support compressed writes

[Kevin Wolf <kwolf@redhat.com>]

Am 25.06.2020 um 17:21 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/mirror.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index e8e8844afc..469acf4600 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1480,6 +1480,15 @@ static int coroutine_fn bdrv_mirror_top_pdiscard(BlockDriverState *bs,
>                                      NULL, 0);
>  }
>  
> +static int coroutine_fn bdrv_mirror_top_pwritev_compressed(BlockDriverState *bs,
> +                                                           uint64_t offset,
> +                                                           uint64_t bytes,
> +                                                           QEMUIOVector *qiov)
> +{
> +    return bdrv_mirror_top_pwritev(bs, offset, bytes, qiov,
> +                                   BDRV_REQ_WRITE_COMPRESSED);
> +}

Hm, not sure if it's a problem, but bdrv_supports_compressed_writes()
will now return true for mirror-top. However, with an active mirror to a
target that doesn't support compression, trying to actually do a
compressed write will always return -ENOTSUP.

So I guess the set of nodes patch 7 looks at still isn't quite complete.
However, it's not obvious how to make it more complete without
delegating to the driver.

Maybe we need to use bs->supported_write_flags, which is set by the
driver, instead of looking at the presence of callbacks.

Of course, in the general case, we also should make sure that graph
changes will be reflected in bs->supported_write_flags, but we already
fail to do this in raw-format, so I guess ignoring it for now is good
enough here, too...

Kevin