[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 11/42] block: Add bdrv_supports_compressed_wr
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v5 11/42] block: Add bdrv_supports_compressed_writes() |
Date: |
Thu, 13 Jun 2019 13:29:00 +0000 |
13.06.2019 1:09, Max Reitz wrote:
> Filters cannot compress data themselves but they have to implement
> .bdrv_co_pwritev_compressed() still (or they cannot forward compressed
> writes). Therefore, checking whether
> bs->drv->bdrv_co_pwritev_compressed is non-NULL is not sufficient to
> know whether the node can actually handle compressed writes. This
> function looks down the filter chain to see whether there is a
> non-filter that can actually convert the compressed writes into
> compressed data (and thus normal writes).
Why not to use this function in (as I remember only 2-3 cases) when
we check for bs->drv->bdrv_co_pwritev_compressed? It would be a complete fix
for described problem.
(hmm, ok, other new APIs are added separately too, for some reason they don't
confuse me and this confuses)
On the other hand, (the second time I think about it during review), could
we handle compression through flags completely?
We have supported_write_flags feature, which should be used for all these
checks..
And may be, we may drop .bdrv_co_pwritev_compressed at all.
But if you want to keep it as is, it's OK too:
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> include/block/block.h | 1 +
> block.c | 22 ++++++++++++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 687c03b275..7835c5b370 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -487,6 +487,7 @@ void bdrv_next_cleanup(BdrvNextIterator *it);
>
> BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs);
> bool bdrv_is_encrypted(BlockDriverState *bs);
> +bool bdrv_supports_compressed_writes(BlockDriverState *bs);
> void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
> void *opaque, bool read_only);
> const char *bdrv_get_node_name(const BlockDriverState *bs);
> diff --git a/block.c b/block.c
> index 567a0f82c8..97774b7b06 100644
> --- a/block.c
> +++ b/block.c
> @@ -4584,6 +4584,28 @@ bool bdrv_is_encrypted(BlockDriverState *bs)
> return false;
> }
>
> +/**
> + * Return whether the given node supports compressed writes.
> + */
> +bool bdrv_supports_compressed_writes(BlockDriverState *bs)
> +{
> + BlockDriverState *filtered = bdrv_filtered_rw_bs(bs);
> +
> + if (!bs->drv || !bs->drv->bdrv_co_pwritev_compressed) {
> + return false;
> + }
> +
> + if (filtered) {
> + /*
> + * Filters can only forward compressed writes, so we have to
> + * check the child.
> + */
> + return bdrv_supports_compressed_writes(filtered);
> + }
> +
> + return true;
> +}
> +
> const char *bdrv_get_format_name(BlockDriverState *bs)
> {
> return bs->drv ? bs->drv->format_name : NULL;
>
--
Best regards,
Vladimir
- [Qemu-block] [PATCH v5 06/42] qcow2: Implement .bdrv_storage_child(), (continued)
- [Qemu-block] [PATCH v5 06/42] qcow2: Implement .bdrv_storage_child(), Max Reitz, 2019/06/12
- [Qemu-block] [PATCH v5 08/42] block: bdrv_set_backing_hd() is about bs->backing, Max Reitz, 2019/06/12
- [Qemu-block] [PATCH v5 07/42] block: *filtered_cow_child() for *has_zero_init(), Max Reitz, 2019/06/12
- [Qemu-block] [PATCH v5 10/42] block: Use CAF in bdrv_is_encrypted(), Max Reitz, 2019/06/12
- [Qemu-block] [PATCH v5 11/42] block: Add bdrv_supports_compressed_writes(), Max Reitz, 2019/06/12
- Re: [Qemu-block] [PATCH v5 11/42] block: Add bdrv_supports_compressed_writes(),
Vladimir Sementsov-Ogievskiy <=
- [Qemu-block] [PATCH v5 09/42] block: Include filters when freezing backing chain, Max Reitz, 2019/06/12
- [Qemu-block] [PATCH v5 12/42] block: Use bdrv_filtered_rw* where obvious, Max Reitz, 2019/06/12
- [Qemu-block] [PATCH v5 13/42] block: Use CAFs in block status functions, Max Reitz, 2019/06/12
- [Qemu-block] [PATCH v5 14/42] block: Use CAFs when working with backing chains, Max Reitz, 2019/06/12