[Top][All Lists]

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

Re: [Qemu-block] [PATCH V2 5/8] block/qcow2: read and write the compress

From: Peter Lieven
Subject: Re: [Qemu-block] [PATCH V2 5/8] block/qcow2: read and write the compress format extension
Date: Thu, 13 Jul 2017 15:49:09 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

Am 13.07.2017 um 11:21 schrieb Daniel P. Berrange:
On Thu, Jul 13, 2017 at 10:44:53AM +0200, Peter Lieven wrote:
Am 10.07.2017 um 15:55 schrieb Daniel P. Berrange:
On Mon, Jul 10, 2017 at 03:52:18PM +0200, Kevin Wolf wrote:
Am 10.07.2017 um 15:44 hat Daniel P. Berrange geschrieben:
On Mon, Jul 10, 2017 at 03:34:59PM +0200, Kevin Wolf wrote:
Am 10.07.2017 um 15:29 hat Peter Lieven geschrieben:
Am 10.07.2017 um 15:25 schrieb Kevin Wolf:
Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben:
we now read the extension on open and write it on update, but
do not yet use it.

Signed-off-by: Peter Lieven <address@hidden>
   block/qcow2.c | 100 
   block/qcow2.h |  23 +++++++++++---
   2 files changed, 104 insertions(+), 19 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 308121a..39a8afc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -63,6 +63,7 @@ typedef struct {
   #define  QCOW2_EXT_MAGIC_END 0
   #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
   static int qcow2_probe(const uint8_t *buf, int buf_size, const char 
@@ -76,6 +77,26 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, 
const char *filename)
           return 0;
+static int qcow2_compress_format_from_name(char *fmt)
+    if (!fmt || !fmt[0]) {
+    } else if (g_str_equal(fmt, "zlib")) {
+        return QCOW2_COMPRESS_ZLIB;
+    } else {
+        return -EINVAL;
+    }
It might make sense to allow specifying the old compression format in
the compression header. But I'm not so sure about using the empty string
rather than a proper name for it, and even less about not documenting
The old format is used if and only if the compression header is absent.
It makes no sense to write a header and use the old format. The old
settings are suboptimal and old versions can't open a qcow2 with a
compression header anyway.
Your code allows an empty string in the header extension. If you don't
want it there, you need to reject it.
This is a good reason to use the QAPI schema to parse the create options
instead of doing it using qemu_opts - the QAPI schema will generate correct
code to parse & validate enum values
I'd really like to see QAPI used for bdrv_create(), but here we're in
the bdrv_open() path and parsing qcow2 headers. So either way this one
specifically wouldn't be fixed.
Sorry, yes, I was getting mixed up in the two different patches with
similar bits of code.

We could still replace the qcow2_compress_format_from_name() method
though with a call to the qapi_enum_parse() passing in the
'QcowCompressFormat_lookup' table.
Can you advise how exactly I would do that? Sorry, but this is new stuff
for me.
In your line of code:

           s->compress_format_id =

Instead do

            s->compress_format_id =

The 'QcowCompressFormat_lookup' symbol is a global variable that is an
array of strings, automatically created from the 'enum' you define in
the QAPI schema.

Is it possible to limit the valid integer values for an integer in the QAPI 

I would like to do that for the compress.level stuff.


reply via email to

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