qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 1/4] block/qcow2: add compression_algorithm crea


From: Peter Lieven
Subject: Re: [Qemu-block] [PATCH 1/4] block/qcow2: add compression_algorithm create option
Date: Tue, 27 Jun 2017 17:13:18 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

Am 27.06.2017 um 17:11 schrieb Peter Lieven:
Am 27.06.2017 um 17:04 schrieb Eric Blake:
On 06/27/2017 09:49 AM, Peter Lieven wrote:

Before I continue, can you please give feedback on the following spec
change:

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index 80cdfd0..f1428e9 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -85,7 +85,11 @@ in the description of a field.
                                  be written to (unless for regaining
                                  consistency).

-                    Bits 2-63:  Reserved (set to 0)
+                    Bit 2:      Compression format bit.  Iff this bit
I know what this means, but spelling it "If and only if" or "When" might
make more sense to other readers, as "Iff" is not common in English.

is set then
+                                the compression format extension MUST
be present
+                                and MUST be parsed and checked for
compatibility.
+
+                    Bits 3-63:  Reserved (set to 0)

           80 -  87:  compatible_features
                      Bitmask of compatible features. An implementation can
@@ -135,6 +139,7 @@ be stored. Each extension has a structure like the
following:
                          0xE2792ACA - Backing file format name
                          0x6803f857 - Feature name table
                          0x23852875 - Bitmaps extension
+                        0xC0318300 - Compression format extension
Now that you aren't burning 256 magic numbers, it may make sense to have
the last two hex digits be non-zero.


+== Compression format extension ==
+
+The compression format extension is an optional header extension. It
provides
Inline pasting created interesting wrapping, but the actual patch will
obviously read better.

+the ability to specify the compression algorithm and compression
parameters
+that are used for compressed clusters. This new header MUST be present if
+the incompatible-feature bit "compression format bit" is set and MUST
be absent
+otherwise.
+
+The fields of the compression format extension are:
+
+    Byte  0 - 15:  compression_format_name (padded with zeros, but not
+                   necessarily null terminated if it has full length)
Do we really want arbitrary names of formats, or do we want to specify
specific algorithms (gzip, lzo, zstd) as an enum?  Which way gives us
maximum likelihood of interoperability?

At some point I have to parse the name and convert it into a number.
This could be the same routine for the parsing of the compress.format
parameter and the compression_format_name in the header field.
The idea was that an accidently change in the enum cannot break
anything as it is local to the qemu binary. But I am ok to have an enum
if the values for each format a fixed. In this case it might be sufficient
to have an uint8_t for the compression format + an uint8_t for the compression
level. Then 2 bytes reserved + an uint32_t for the extra data size. So 
currently (the extra
data is not necessary at the moment for any format) the header would
be limited to 8 bytes.

Anyway if we glue this into a QAPI scheme, I would appreciate to get
some help with it as I am not that familiar with it yet.

Forgot to mention, the idea to store the name here was to be able to display
an appropiate error message if a format is not supported (like compression 
format xyz is not supported).

Peter



reply via email to

[Prev in Thread] Current Thread [Next in Thread]