[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 17:21:13 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

Am 13.07.2017 um 17:17 schrieb Daniel P. Berrange:
On Thu, Jul 13, 2017 at 05:13:23PM +0200, Peter Lieven wrote:
Am 13.07.2017 um 17:06 schrieb Daniel P. Berrange:
On Thu, Jul 13, 2017 at 05:02:51PM +0200, Peter Lieven wrote:
Am 13.07.2017 um 17:01 schrieb Daniel P. Berrange:
On Thu, Jul 13, 2017 at 05:00:39PM +0200, Peter Lieven wrote:
Am 13.07.2017 um 16:58 schrieb Daniel P. Berrange:
On Thu, Jul 13, 2017 at 04:18:13PM +0200, Peter Lieven wrote:
Okay, so it has to be a mix of QAPI parsing and manual parameter checking,
Yeah. It does feel like a valid RFE for QAPI to add a permitted range to
'int' types though, which would simplify the code in future.

I currently have the following:

        options = qemu_opts_to_qdict(opts, NULL);
        qdict_extract_subqdict(options, &compressopts, "compress.");
        v = qobject_input_visitor_new_keyval(QOBJECT(compressopts));
        visit_start_struct(v, NULL, NULL, 0, &local_err);
        if (local_err) {
            ret= -EINVAL;
            goto finish;
        visit_type_Qcow2Compress_members(v, &compress, &local_err);
        if (local_err) {
            ret= -EINVAL;
            goto finish;
        visit_end_struct(v, NULL);
Looks good.

And I have the following 2 questions:
     a) I have to specifiy compress.format and compress.level otherwise I will 
get an error. How can I fix that the settings are optional?
Put an '*' as the first character of any field name if it should be optional.

     b) If I just specify a compress.format can I default the compress.level to 
0 without an error?
I believe you'd get compress.level as 0 automatically for an 'int' type.
I still face the issue that I now always have to specify a compress.format.
I tried to solve it like this:

that's not needed if you name the parameter '*level' in the QAPI schema
its not needed for the *level, but I still get the error that the format
parameter is missing otherwise. And I can't make format optional.
Surely specifying compress.format is how you turn on compression in the
first place, so making that optional should not be necessary. eg the
simple case becomes:

    qemu-img create -f qcow2 -o compress.format=zlib  foo.qcow2 1G

Yes, there is no notion of a "default" format, but IMHO that's a good
thing - same way with encryption - it requires an explict encrypt.format=luks
to turn on encryption
Yes, but visit_type_Qcow2Compress_members always returns an error if 
is missing from the options. So I should not call it an error out if 
compress.format is not in the
options. Thus the check if either compress.format or compress.level is 
specified at all.
Oh yes, I see what you mean now. Yes, just wrapping the whole qapi block
in an    if(qemu_opt_get(opts, "compress.format"))  is the right way to
handle that.

qemu_opt_get doesn't work, because it seems to return an empty string. I now 
also always see
compress.format and compress.level in the qemu-img output even if I don't 
specify it. Have
to figure out why this is...

~/git/qemu$ ./qemu-img create -f qcow2 /tmp/1G.qcow2 1G
Formatting '/tmp/1G.qcow2', fmt=qcow2 size=1073741824 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16 compress.format= 

I hope I have the new patchset ready tomorrow. I would appreciate if you could 
have a look at it.

Thank you,

reply via email to

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