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

[Peter Lieven]

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,
> > right?
> 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);
> >      visit_free(v);
> >      QDECREF(compressopts);
> >      QDECREF(options)
> 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:

    int compress_format = -1;
    uint8_t compress_level = 0;
    if (qemu_opt_find(opts, BLOCK_OPT_COMPRESS_FORMAT) ||
        qemu_opt_find(opts, BLOCK_OPT_COMPRESS_LEVEL)) {
        QDict *options, *compressopts = NULL;
        Qcow2Compress compress;
        Visitor *v;
        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) {
            error_propagate(errp, local_err);
            ret = -EINVAL;
            goto finish;
        }
        visit_type_Qcow2Compress_members(v, &compress, &local_err);
        if (local_err) {
            error_propagate(errp, local_err);
            ret = -EINVAL;
            goto finish;
        }
        visit_end_struct(v, NULL);
        visit_free(v);
        QDECREF(compressopts);
        QDECREF(options);

        compress_format = compress.format;
        compress_level = compress.level;
        if (compress_format == QCOW2_COMPRESS_FORMAT_ZLIB &&
            compress_level > 9) {
            error_setg(errp, "Compress level %" PRIu8 " is not supported for"
                       " format '%s'", compress_level,
                       qemu_opt_get(opts, BLOCK_OPT_COMPRESS_FORMAT));
            ret = -EINVAL;
            goto finish;
        }
    }

Maybe there is a better option?

Peter