Re: [Qemu-devel] [PATCH V11 3/4] Use QemuOpts support in block layer

[Dong Xu Wang]

于 2013-1-29 1:41, Markus Armbruster 写道:
> Dong Xu Wang <address@hidden> writes:
> 
>> Markus Armbruster <address@hidden> writes:
>>> Dong Xu Wang <address@hidden> writes:
>>>
>>>> This patch will use QemuOpts related functions in block layer, add
>>>> a member bdrv_create_options to BlockDriver struct, it will return
>>>> a QemuOptsList pointer, which includes the image format's create
>>>> options.
>>>>
>>>> And create options's primary consumer is block creating related functions,
>>>> so modify them together.
>>>>
>>>>
>>>> Signed-off-by: Dong Xu Wang <address@hidden>
>>>> ---
>>>> v10->v11)
>>>> 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or
>>>> qemu_opts_print produce un-expanded cluster_size.
>>>> 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> opts,
>>>> or while using protocol, there will be an error.
>>>>
>>>> v8->v9)
>>>> 1) add qemu_ prefix to gluster_create_opts.
>>>> 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be
>>>>      converted.
>>>>
>>>> v7->v8)
>>>> 1) rebase to upstream source tree.
>>>> 2) add gluster.c, raw-win32.c, and rbd.c.
>>>>
>>>> v6->v7:
>>>> 1) use osdep.h:stringify(), not redefining new macro.
>>>> 2) preserve TODO comment.
>>>> 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC.
>>>> 4) initialize disk_type even when opts is NULL.
>>>>
>>>> v5->v6:
>>>> 1) judge if opts == NULL in block layer create functions.
>>>> 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create 
>>>> funtion.
>>>> 3) made more readable while using qemu_opt_get_number.
>>>>
>>>>
>>>>    block.c                   |  91 ++++++++++++------------
>>>>    block/cow.c               |  46 ++++++-------
>>>>    block/gluster.c           |  37 +++++-----
>>>>    block/qcow.c              |  60 ++++++++--------
>>>>    block/qcow2.c             | 171 
>>>> +++++++++++++++++++++++-----------------------
>>>>    block/qed.c               |  86 +++++++++++------------
>>>>    block/qed.h               |   2 +-
>>>>    block/raw-posix.c         |  59 ++++++++--------
>>>>    block/raw-win32.c         |  30 ++++----
>>>>    block/raw.c               |  30 ++++----
>>>>    block/rbd.c               |  62 ++++++++---------
>>>>    block/sheepdog.c          |  75 ++++++++++----------
>>>>    block/vdi.c               |  69 +++++++++----------
>>>>    block/vmdk.c              |  74 ++++++++++----------
>>>>    block/vpc.c               |  67 +++++++++---------
>>>>    block/vvfat.c             |  11 +--
>>>>    include/block/block.h     |   4 +-
>>>>    include/block/block_int.h |   6 +-
>>>>    qemu-img.c                |  61 ++++++++---------
>>>>    19 files changed, 519 insertions(+), 522 deletions(-)
>>>>
>>>> diff --git a/block.c b/block.c
>>>> index 6fa7c90..56e4613 100644
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -357,7 +357,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char 
>>>> *format_name)
>>>>    typedef struct CreateCo {
>>>>        BlockDriver *drv;
>>>>        char *filename;
>>>> -    QEMUOptionParameter *options;
>>>> +    QemuOpts *opts;
>>>>        int ret;
>>>>    } CreateCo;
>>>>    
>>>> @@ -366,11 +366,11 @@ static void coroutine_fn bdrv_create_co_entry(void 
>>>> *opaque)
>>>>        CreateCo *cco = opaque;
>>>>        assert(cco->drv);
>>>>    
>>>> -    cco->ret = cco->drv->bdrv_create(cco->filename, cco->options);
>>>> +    cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts);
>>>>    }
>>>>    
>>>>    int bdrv_create(BlockDriver *drv, const char* filename,
>>>> -    QEMUOptionParameter *options)
>>>> +    QemuOpts *opts)
>>>>    {
>>>>        int ret;
>>>>    
>>>> @@ -378,7 +378,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
>>>>        CreateCo cco = {
>>>>            .drv = drv,
>>>>            .filename = g_strdup(filename),
>>>> -        .options = options,
>>>> +        .opts = opts,
>>>>            .ret = NOT_DONE,
>>>>        };
>>>>    
>>>> @@ -405,7 +405,7 @@ out:
>>>>        return ret;
>>>>    }
>>>>    
>>>> -int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
>>>> +int bdrv_create_file(const char *filename, QemuOpts *opts)
>>>>    {
>>>>        BlockDriver *drv;
>>>>    
>>>> @@ -414,7 +414,7 @@ int bdrv_create_file(const char* filename, 
>>>> QEMUOptionParameter *options)
>>>>            return -ENOENT;
>>>>        }
>>>>    
>>>> -    return bdrv_create(drv, filename, options);
>>>> +    return bdrv_create(drv, filename, opts);
>>>>    }
>>>>    
>>>>    /*
>>>> @@ -794,7 +794,7 @@ int bdrv_open(BlockDriverState *bs, const char 
>>>> *filename, int flags,
>>>>            int64_t total_size;
>>>>            int is_protocol = 0;
>>>>            BlockDriver *bdrv_qcow2;
>>>> -        QEMUOptionParameter *options;
>>>> +        QemuOpts *opts;
>>>>            char backing_filename[PATH_MAX];
>>>>    
>>>>            /* if snapshot, we create a temporary backing file and open it
>>>> @@ -827,17 +827,16 @@ int bdrv_open(BlockDriverState *bs, const char 
>>>> *filename, int flags,
>>>>                return -errno;
>>>>    
>>>>            bdrv_qcow2 = bdrv_find_format("qcow2");
>>>> -        options = parse_option_parameters("", bdrv_qcow2->create_options, 
>>>> NULL);
>>>> +        opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_options);
>>>>    
>>>> -        set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size);
>>>> -        set_option_parameter(options, BLOCK_OPT_BACKING_FILE, 
>>>> backing_filename);
>>>> +        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size);
>>>> +        qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename);
>>>>            if (drv) {
>>>> -            set_option_parameter(options, BLOCK_OPT_BACKING_FMT,
>>>> -                drv->format_name);
>>>> +            qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name);
>>>>            }
>>>>    
>>>> -        ret = bdrv_create(bdrv_qcow2, tmp_filename, options);
>>>> -        free_option_parameters(options);
>>>> +        ret = bdrv_create(bdrv_qcow2, tmp_filename, opts);
>>>> +        qemu_opts_del(opts);
>>>>            if (ret < 0) {
>>>>                return ret;
>>>>            }
>>>> @@ -4493,8 +4492,10 @@ void bdrv_img_create(const char *filename, const 
>>>> char *fmt,
>>>>                         const char *base_filename, const char *base_fmt,
>>>>                         char *options, uint64_t img_size, int flags, Error 
>>>> **errp)
>>>>    {
>>>> -    QEMUOptionParameter *param = NULL, *create_options = NULL;
>>>> -    QEMUOptionParameter *backing_fmt, *backing_file, *size;
>>>> +    QemuOpts *opts = NULL;
>>>> +    QemuOptsList *create_options = NULL;
>>>> +    const char *backing_fmt, *backing_file;
>>>> +    int64_t size;
>>>>        BlockDriverState *bs = NULL;
>>>>        BlockDriver *drv, *proto_drv;
>>>>        BlockDriver *backing_drv = NULL;
>>>> @@ -4512,28 +4513,23 @@ void bdrv_img_create(const char *filename, const 
>>>> char *fmt,
>>>>            error_setg(errp, "Unknown protocol '%s'", filename);
>>>>            return;
>>>>        }
>>>> -
>>>> -    create_options = append_option_parameters(create_options,
>>>> -                                              drv->create_options);
>>>> -    create_options = append_option_parameters(create_options,
>>>> -                                              proto_drv->create_options);
>>>> -
>>>> +    create_options = append_opts_list(drv->bdrv_create_options,
>>>> +                                      proto_drv->bdrv_create_options);
>>>
>>> Before: format's options get appended first, then protocol's options.
>>> Because get_option_parameter() searches in order, format options shadow
>>> protocol options.
>>>
>>> After: append_opts_list() gives first argument's options precedence.
>>>
>>> Thus, no change.  Good.
>>>
>>> Should bdrv_create_options option name clashes be avoided?  Outside the
>>> scope of this patch.
>> Sorry, I do not understand this line, clash? Can you explain some more?
>> Do you mean bdrv_create_options should be renamed such as bdrv_create_opts?
> 
> No.  I was talking about what happens when drv->create_options and
> proto->create_options both have an entry with the same name.
> 
> First, I reasoned why your patch doesn't change the way such a clash is
> handled.
> 
> Then I wondered whether we should avoid such clashes, but hastened to
> add that I'm not asking you to do that now.
> 
>>>>        /* Create parameter list with default values */
>>>> -    param = parse_option_parameters("", create_options, param);
>>>> +    opts = qemu_opts_create_nofail(create_options);
>>>>    
>>>> -    set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
>>>> +    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size);
>>>>    
>>>>        /* Parse -o options */
>>>>        if (options) {
>>>> -        param = parse_option_parameters(options, create_options, param);
>>>> -        if (param == NULL) {
>>>> +        if (qemu_opts_do_parse(opts, options, NULL) != 0) {
>>>>                error_setg(errp, "Invalid options for file format '%s'.", 
>>>> fmt);
>>>>                goto out;
>>>>            }
>>>>        }
>>>
>>> Before: size from -o replaces img_size in param.
>>>
>>> After: size from -o gets appended to opts, is therefore shadowed by
>>> img_size.  I think.
>>>
>>> User-visible change, if my reading is correct.  Should be avoided.
>>>
>> Okay. will also "replace", not "append".
> 
> Prepending could also work.
> 
> Doing things in a different order could make appending work.
> 
> Whatever is easiest.
> 
>>>>    
>>>>        if (base_filename) {
>>>> -        if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE,
>>>> +        if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE,
>>>>                                     base_filename)) {
>>>>                error_setg(errp, "Backing file not supported for file 
>>>> format '%s'",
>>>>                           fmt);
>>>
>>> Before: base_filename replaces the backing_file from -o.
>>>
>>> After: base_filename gets appended to opts, shadowed by backing_file
>>> from -o.
>>>
>>>> @@ -4542,39 +4538,37 @@ void bdrv_img_create(const char *filename, const 
>>>> char *fmt,
>>>>        }
>>>>    
>>>>        if (base_fmt) {
>>>> -        if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) 
>>>> {
>>>> +        if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) {
>>>>                error_setg(errp, "Backing file format not supported for 
>>>> file "
>>>>                                 "format '%s'", fmt);
>>>>                goto out;
>>>>            }
>>>>        }
>>>
>>> Likewise.
>>>
>>>>    
>>>> -    backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
>>>> -    if (backing_file && backing_file->value.s) {
>>>> -        if (!strcmp(filename, backing_file->value.s)) {
>>>> +    backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
>>>> +    if (backing_file) {
>>>> +        if (!strcmp(filename, backing_file)) {
>>>>                error_setg(errp, "Error: Trying to create an image with the 
>>>> "
>>>>                                 "same filename as the backing file");
>>>>                goto out;
>>>>            }
>>>>        }
>>>>    
>>>> -    backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
>>>> -    if (backing_fmt && backing_fmt->value.s) {
>>>> -        backing_drv = bdrv_find_format(backing_fmt->value.s);
>>>> +    backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
>>>> +    if (backing_fmt) {
>>>> +        backing_drv = bdrv_find_format(backing_fmt);
>>>>            if (!backing_drv) {
>>>> -            error_setg(errp, "Unknown backing file format '%s'",
>>>> -                       backing_fmt->value.s);
>>>> +            error_setg(errp, "Unknown backing file format '%s'", 
>>>> backing_fmt);
>>>>                goto out;
>>>>            }
>>>>        }
>>>>    
>>>>        // The size for the image must always be specified, with one 
>>>> exception:
>>>>        // If we are using a backing file, we can obtain the size from there
>>>> -    size = get_option_parameter(param, BLOCK_OPT_SIZE);
>>>> -    if (size && size->value.n == -1) {
>>>> -        if (backing_file && backing_file->value.s) {
>>>> +    size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, -1);
>>>> +    if (size == -1) {
>>>> +        if (backing_file) {
>>>
>>> Code has the same before your patch, so it's not your fault, but here
>>> goes anyway:
>>>
>>> QemuOpts support only *unsigned* numbers.  Argument -1 is for an
>>> uint64_t parameter, and gets converted to UINT64_MAX.  If the option
>>> isn't set, it's returned.  The assignment to int64_t size converts it
>>> back to -1.
>>>
>>> Using the an honest UINT64_MAX instead of -1 would be clearer.  Should
>>> make size uint64_t then.
>>>
>>> However, all other places do
>>>
>>>       qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0)
>>>
>>> Why is this one different?
>>>
>> Because previous use -1, so, I passed default value -1.
> 
> I have no idea what you're talking about, and I'm not 100% sure you do
> :)
> 
> The last argument to qemu_opt_get_number() is *not* used here.  That's
> because a few lines above, you do
> 
>      qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size);
> 
> So, opts always has the BLOCK_OPT_SIZE option, and the default value
> never applies.
> 
> Let's take a step back and figure out how the current code works before
> we further review your patch.
> 
> bdrv_img_create() merges parameters img_size, base_filename, base_fmt
> with options as follows:
> 
> 1. Get format and protocol driver from parameters @fmt and @filename.
> 
> 2. Combine the two drivers' option lists.
> 
> 3. Copy the resulting option list for no discernible reason.
> 
> 4. If %BLOCK_OPT_SIZE is a valid option, set its value to parameter
>     @img_size.
> 
> 5. For all KEY=VAL in parameter @options:
>         KEY must be a valid option (error out if not)
>         set its value to VAL
> 
> 6. If parameter @base_filename isn't null:
>         %BLOCK_OPT_BACKING_FILE must be a valid option (error out if not)
>         set its value to @base_filename
> 
> 7. If parameter @base_fmt isn't null:
>         %BLOCK_OPT_BACKING_FMT must be a valid option (error out if not)
>         set its value to @base_fmt
> 
> In short, @base_filename and @base_fmt take precedence over @options
> (unless they're null, of course).  @options takes precedence over
> @img_size.  What a mess :)  Not your fault.
> 
> Now let's look at the code retrieving BLOCK_OPT_SIZE:
> 
>      // The size for the image must always be specified, with one exception:
>      // If we are using a backing file, we can obtain the size from there
>      size = get_option_parameter(param, BLOCK_OPT_SIZE);
>      if (size && size->value.n == -1) {
>          if (backing_file && backing_file->value.s) {
>             // compute size, put it into buf
>              set_option_parameter(param, BLOCK_OPT_SIZE, buf);
>          } else {
>             // error out
>          }
> 
> If %BLOCK_OPT_SIZE is a valid option, and has the special value
> (uint64_t)-1, then we must have a backing file (error out if not), and
> we use its size instead.  In other words, the special value means "use
> the backing file's size".
> 
> The value can come either from @img_size or from @options.
> 
> Some callers indeed pass (uint64_t)-1 for @img_size.  For instance,
> qemu-img create passes it when run without the optional size argument.
> Works.
> 
> qemu-img is the only caller passing @options.  With -o size=-1, it
> actually puts (uint64_t)-1 into @param (parse_option_size() sucks).
> Weird.
> 
> The whole bloody mess should be cleaned up.  But I'm not asking you to
> do that now.
> 
> I do think, however, that you should stick to
> 
>      qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0)
> 
> instead of your voodoo
> 
>      qemu_opt_get_number(opts, BLOCK_OPT_SIZE, -1)
> 
>>>>                uint64_t size;
>>>> -            char buf[32];
>>>>                int back_flags;
>>>>    
>>>>                /* backing files always opened read-only */
>>>> @@ -4583,17 +4577,16 @@ void bdrv_img_create(const char *filename, const 
>>>> char *fmt,
>>>>    
>>>>                bs = bdrv_new("");
>>>>    
>>>> -            ret = bdrv_open(bs, backing_file->value.s, back_flags, 
>>>> backing_drv);
>>>> +            ret = bdrv_open(bs, backing_file, back_flags, backing_drv);
>>>>                if (ret < 0) {
>>>>                    error_setg_errno(errp, -ret, "Could not open '%s'",
>>>> -                                 backing_file->value.s);
>>>> +                                 backing_file);
>>>>                    goto out;
>>>>                }
>>>>                bdrv_get_geometry(bs, &size);
>>>>                size *= 512;
>>>>    
>>>> -            snprintf(buf, sizeof(buf), "%" PRId64, size);
>>>> -            set_option_parameter(param, BLOCK_OPT_SIZE, buf);
>>>> +            qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size);
>>>>            } else {
>>>>                error_setg(errp, "Image creation needs a size parameter");
>>>>                goto out;
>>>> @@ -4601,10 +4594,10 @@ void bdrv_img_create(const char *filename, const 
>>>> char *fmt,
>>>>        }
>>>>    
>>>>        printf("Formatting '%s', fmt=%s ", filename, fmt);
>>>> -    print_option_parameters(param);
>>>> +    qemu_opts_print(opts, NULL);
>>>>        puts("");
>>>>    
>>>> -    ret = bdrv_create(drv, filename, param);
>>>> +    ret = bdrv_create(drv, filename, opts);
>>>>        if (ret < 0) {
>>>>            if (ret == -ENOTSUP) {
>>>>                error_setg(errp,"Formatting or formatting option not 
>>>> supported for "
>>>> @@ -4619,8 +4612,10 @@ void bdrv_img_create(const char *filename, const 
>>>> char *fmt,
>>>>        }
>>>>    
>>>>    out:
>>>> -    free_option_parameters(create_options);
>>>> -    free_option_parameters(param);
>>>> +    free_opts_list(create_options);
>>>> +    if (opts) {
>>>> +        qemu_opts_del(opts);
>>>> +    }
>>>>    
>>>>        if (bs) {
>>>>            bdrv_delete(bs);
>>>> diff --git a/block/cow.c b/block/cow.c
>>>> index a33ce95..5442c9c 100644
>>>> --- a/block/cow.c
>>>> +++ b/block/cow.c
>>>> @@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs)
>>>>    {
>>>>    }
>>>>    
>>>> -static int cow_create(const char *filename, QEMUOptionParameter *options)
>>>> +static int cow_create(const char *filename, QemuOpts *opts)
>>>>    {
>>>>        struct cow_header_v2 cow_header;
>>>>        struct stat st;
>>>> @@ -264,17 +264,13 @@ static int cow_create(const char *filename, 
>>>> QEMUOptionParameter *options)
>>>>        int ret;
>>>>        BlockDriverState *cow_bs;
>>>>    
>>>> -    /* Read out options */
>>>> -    while (options && options->name) {
>>>> -        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>>>> -            image_sectors = options->value.n / 512;
>>>> -        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
>>>> -            image_filename = options->value.s;
>>>> -        }
>>>> -        options++;
>>>> +    /* Read out opts */
>>>> +    if (opts) {
>>>
>>> I suspect you need the if (opts) here and in many other places only
>>> because you special-cased "both lists empty" in append_opts_list().  I
>>> suspect things become easier if you drop that.
>>
>> No, in this version, if(opt) is needed in "protocol", not needed in
>> "format", I want to have the same code style, so I also judged if opts
>> is NULL in "format" _create functions. Do you think is it acceptable?
> 
> I still don't get it.  Can you explain how opts can be null here even
> when append_opts_list() is changed not to return null?
> 
I mean: When I use protocol, such as gluster:
/usr/bin/qemu-img create -f qed -o cluster_size=65536
gluster://kvm11/single-volume/12.qed 10M

Then opts will be null:
qemu_gluster_create (filename=0x555555c11930
"gluster://kvm11/single-volume/12.qed", opts=0x0) at block/gluster.c:339

So it checked if opts is NULL now:
352         if (opts) {
353             total_size =
354                 qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) /
BDRV_SECTOR_SIZE;
355         }

I want to make these code in only one kind of style, so I judged if opts
is NULL in block format code.

>>>
>>>> +        image_sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 
>>>> 512;
>>>> +        image_filename = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
>>>>        }
>>>>    
>>>> -    ret = bdrv_create_file(filename, options);
>>>> +    ret = bdrv_create_file(filename, opts);
>>>>        if (ret < 0) {
>>>>            return ret;
>>>>        }
>>>> @@ -318,18 +314,22 @@ exit:
>>>>        return ret;
>>>>    }
>>>>    
>>>> -static QEMUOptionParameter cow_create_options[] = {
>>>> -    {
>>>> -        .name = BLOCK_OPT_SIZE,
>>>> -        .type = OPT_SIZE,
>>>> -        .help = "Virtual disk size"
>>>> -    },
>>>> -    {
>>>> -        .name = BLOCK_OPT_BACKING_FILE,
>>>> -        .type = OPT_STRING,
>>>> -        .help = "File name of a base image"
>>>> -    },
>>>> -    { NULL }
>>>> +static QemuOptsList cow_create_opts = {
>>>> +    .name = "cow-create-opts",
>>>> +    .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head),
>>>> +    .desc = {
>>>> +        {
>>>> +            .name = BLOCK_OPT_SIZE,
>>>> +            .type = QEMU_OPT_NUMBER,
>>>
>>> QEMU_OPT_SIZE, or else we lose support for suffixes, don't we?
>>>
>> In qemu-img.c, strtosz_suffix convert suffixes to regular digitals.
> 
> That's correct for the optional size argument, but not for -o size=...
> 
>> Okay. I will change to QEMU_OPT_SIZE.
> 
> Please do, and test that both the size argument and -o size=... works
> just like before, with and without suffixes.
> 
>>>> +            .help = "Virtual disk size"
>>>> +        },
>>>> +        {
>>>> +            .name = BLOCK_OPT_BACKING_FILE,
>>>> +            .type = QEMU_OPT_STRING,
>>>> +            .help = "File name of a base image"
>>>> +        },
>>>> +        { /* end of list */ }
>>>> +    }
>>>>    };
>>>>    
>>>>    static BlockDriver bdrv_cow = {
>>>> @@ -345,7 +345,7 @@ static BlockDriver bdrv_cow = {
>>>>        .bdrv_write             = cow_co_write,
>>>>        .bdrv_co_is_allocated   = cow_co_is_allocated,
>>>>    
>>>> -    .create_options = cow_create_options,
>>>> +    .bdrv_create_options    = &cow_create_opts,
>>>>    };
>>>>    
>>>>    static void bdrv_cow_init(void)
>>>> diff --git a/block/gluster.c b/block/gluster.c
>>>> index 0f2c32a..a41c684 100644
>>>> --- a/block/gluster.c
>>>> +++ b/block/gluster.c
>>>> @@ -335,8 +335,7 @@ out:
>>>>        return ret;
>>>>    }
>>>>    
>>>> -static int qemu_gluster_create(const char *filename,
>>>> -        QEMUOptionParameter *options)
>>>> +static int qemu_gluster_create(const char *filename, QemuOpts* opts)
>>>
>>> Space before the *, not after: QemuOpts *opts
>>>
>> Okay.
> 
> [...]
> 
> 
>