On 3/12/20 4:22 AM, Denis Plotnikov wrote:
The patch adds some preparation parts for incompatible compression type
feature to qcow2 allowing the use different compression methods for
image clusters (de)compressing.
It is implied that the compression type is set on the image creation and
can be changed only later by image conversion, thus compression type
defines the only compression algorithm used for the image, and thus,
for all image clusters.
The goal of the feature is to add support of other compression methods
to qcow2. For example, ZSTD which is more effective on compression
than ZLIB.
The default compression is ZLIB. Images created with ZLIB compression
type
are backward compatible with older qemu versions.
Adding of the compression type breaks a number of tests because now the
compression type is reported on image creation and there are some
changes
in the qcow2 header in size and offsets.
The tests are fixed in the following ways:
* filter out compression_type for all the tests
Presumably this filter is optional, and we will not use it on the
specific new tests that prove zstd compression works - but that should
be later in the series, so for this patch it is okay.
* fix header size, feature table size and backing file offset
affected tests: 031, 036, 061, 080
header_size +=8: 1 byte compression type
7 bytes padding
feature_table += 48: incompatible feture compression type
feature
backing_file_offset += 56 (8 + 48 -> header_change +
fature_table_change)
feature
(interesting that you have two different changed spellings ;)
* add "compression type" for test output matching when it isn't
filtered
affected tests: 049, 060, 061, 065, 144, 182, 242, 255
Or maybe the comment above should be changed to "many tests" rather
than "all the tests".
Signed-off-by: Denis Plotnikov <dplotni...@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
qapi/block-core.json | 22 +++++-
block/qcow2.h | 20 ++++-
include/block/block_int.h | 1 +
block/qcow2.c | 121 +++++++++++++++++++++++++++++++
tests/qemu-iotests/031.out | 14 ++--
tests/qemu-iotests/036.out | 4 +-
tests/qemu-iotests/049.out | 102 +++++++++++++-------------
tests/qemu-iotests/060.out | 1 +
tests/qemu-iotests/061.out | 34 +++++----
tests/qemu-iotests/065 | 28 ++++---
tests/qemu-iotests/080 | 2 +-
tests/qemu-iotests/144.out | 4 +-
tests/qemu-iotests/182.out | 2 +-
tests/qemu-iotests/242.out | 5 ++
tests/qemu-iotests/255.out | 8 +-
tests/qemu-iotests/common.filter | 3 +-
16 files changed, 275 insertions(+), 96 deletions(-)
+++ b/block/qcow2.h
@@ -146,8 +146,16 @@ typedef struct QCowHeader {
uint32_t refcount_order;
uint32_t header_length;
+
+ /* Additional fields */
+ uint8_t compression_type;
+
+ /* header must be a multiple of 8 */
+ uint8_t padding[7];
Why two spaces after uint8_t (twice)?
@@ -369,6 +380,13 @@ typedef struct BDRVQcow2State {
bool metadata_preallocation_checked;
bool metadata_preallocation;
+ /*
+ * Compression type used for the image. Default: 0 - ZLIB
+ * The image compression type is set on image creation.
+ * The only way to change the compression type is to convert the
image
+ * with the desired compression type set
Missing trailing '.'. Maybe someday we can get 'qemu-img amend' to
also adjust the compression type in-place; if that's something we
think we might do, then this could be better worded as "For now, the
only way to change...".
+++ b/block/qcow2.c
@@ -1242,6 +1242,48 @@ static int
qcow2_update_options(BlockDriverState *bs, QDict *options,
return ret;
}
+static int validate_compression_type(BDRVQcow2State *s, Error **errp)
+
+static int qcow2_compression_type_from_format(const char *ct)
+{
+ if (g_str_equal(ct, "zlib")) {
+ return QCOW2_COMPRESSION_TYPE_ZLIB;
+ } else {
+ return -EINVAL;
+ }
Why are you open-coding this?
qapi_enum_parse(&Qcow2CompressionType_lookup, ct, -1, errp)
should do what you use this for, and automatically updates itself when
you add zstd to the qapi enum later.
@@ -3401,6 +3493,8 @@ qcow2_co_create(BlockdevCreateOptions
*create_options, Error **errp)
.refcount_table_offset = cpu_to_be64(cluster_size),
.refcount_table_clusters = cpu_to_be32(1),
.refcount_order = cpu_to_be32(refcount_order),
+ /* don't deal with endians since compression_type is 1 byte
long */
endianness
+ .compression_type = compression_type,
.header_length = cpu_to_be32(sizeof(*header)),
};
@@ -5516,6 +5631,12 @@ static QemuOptsList qcow2_create_opts = {
.help = "Width of a reference count entry in bits",
.def_value_str = "16"
},
+ {
+ .name = BLOCK_OPT_COMPRESSION_TYPE,
+ .type = QEMU_OPT_STRING,
+ .help = "Compression method used for image clusters
compression",
s/clusters/cluster/
Overall, getting closer. I assume this would go through Max's tree, if
he has time to include it before soft freeze...