Re: [PATCH v5 1/4] block: support compressed write at generic layer

[Max Reitz <mreitz@redhat.com>]

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

On 20.10.19 22:37, Andrey Shinkevich wrote:
> To inform the block layer about writing all the data compressed, we
> introduce the 'compress' command line option. Based on that option, the
> written data will be aligned by the cluster size at the generic layer.
> 
> Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block.c                   | 20 +++++++++++++++++++-
>  block/io.c                | 13 +++++++++----
>  block/qcow2.c             |  4 ++++
>  blockdev.c                |  9 ++++++++-
>  include/block/block.h     |  1 +
>  include/block/block_int.h |  2 ++
>  qapi/block-core.json      |  5 ++++-
>  qemu-options.hx           |  6 ++++--
>  8 files changed, 51 insertions(+), 9 deletions(-)

The problem with compression is that there are such tight constraints on
it that it can really only work for very defined use cases.  Those
constraints are:

- Only write whole clusters,
- Clusters can be written to only once.

The first point is addressed in this patch by setting request_alignment.
 But I don’t see how the second one can be addressed.  Well, maybe by
allowing it in all drivers that support compression.  But if I just look
at qcow2, that isn’t going to be trivial: You need to allocate a
completely new cluster where you write the data (in case it grows), and
thus you leave behind a hole, which kind of defeats the purpose of
compression.

(For demonstration:

$ ./qemu-img create -f qcow2 test.qcow2 64M
Formatting 'test.qcow2', fmt=qcow2 size=67108864 cluster_size=65536
lazy_refcounts=off refcount_bits=16
$ x86_64-softmmu/qemu-system-x86_64 \
    -blockdev "{'node-name': 'drv0', 'driver': 'qcow2',
                'compress': true,
                'file': {'driver': 'file', 'filename': 'test.qcow2'}}" \
    -monitor stdio
QEMU 4.1.50 monitor - type 'help' for more information
(qemu) qemu-io drv0 "write -P 42 0 64k"
wrote 65536/65536 bytes at offset 0
64 KiB, 1 ops; 00.02 sec (4.055 MiB/sec and 64.8793 ops/sec)
(qemu) qemu-io drv0 "write -P 23 0 64k"
write failed: Input/output error

)

Compression really only works when you fully write all of an image
exactly once; i.e. as the qemu-img convert or as a backup target.  For
both cases we already have a compression option.  So I’m wondering where
this new option is really useful.

(You do add a test for stream, but I don’t know whether that’s really a
good example, see my response there.)

Max


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