Re: [Qemu-block] [PATCH v6 11/42] block: Add bdrv_supports_compressed_writes()
On 05.09.19 15:11, Kevin Wolf wrote: > Am 09.08.2019 um 18:13 hat Max Reitz geschrieben: >> 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). >> >> Signed-off-by: Max Reitz >> Reviewed-by: Vladimir Sementsov-Ogievskiy > > Should patches 2 and 3 that add the .bdrv_co_pwritev_compressed() > callback to filter drivers come only after this one? Why not. > And should we also > support it in the mirror filter? Hm. AFAIU, compressed writes have very limited use. You can basically only use them when writing to a new image (where you’d never write anywhere you’ve already written something to), i.e. for qemu-img convert or the backup target. It makes sense to blockdev-backup to throttle, so that’s why it should be implemented there. I don’t really see how it would make sense for mirror. OTOH, it doesn’t make sense for COR either. And it isn’t that hard. Now I don’t have a strong preference for either dropping the COR patch or adding it to mirror as well... Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v6 11/42] block: Add bdrv_supports_compressed_writes()
Am 09.08.2019 um 18:13 hat Max Reitz geschrieben: > 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). > > Signed-off-by: Max Reitz > Reviewed-by: Vladimir Sementsov-Ogievskiy Should patches 2 and 3 that add the .bdrv_co_pwritev_compressed() callback to filter drivers come only after this one? And should we also support it in the mirror filter? Kevin
[Qemu-block] [PATCH v6 11/42] block: Add bdrv_supports_compressed_writes()
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). Signed-off-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- 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 9cfe77abaf..6ba853fb90 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -487,6 +487,7 @@ BlockDriverState *bdrv_next(BdrvNextIterator *it); void bdrv_next_cleanup(BdrvNextIterator *it); BlockDriverState *bdrv_next_monitor_owned(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 415c555bf5..029c809a8e 100644 --- a/block.c +++ b/block.c @@ -4696,6 +4696,28 @@ bool bdrv_is_sg(BlockDriverState *bs) return bs->sg; } +/** + * 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; -- 2.21.0