Re: [Qemu-devel] [PATCH v22 07/25] change block layer to support both QemuOpts and QEMUOptionParamter

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]

On 03/10/2014 01:31 AM, Chunyan Liu wrote:
> Change block layer to support both QemuOpts and QEMUOptionParameter.
> After this patch, it will change backend drivers one by one. At the end,
> QEMUOptionParameter will be removed and only QemuOpts is kept.
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---

> @@ -420,7 +421,11 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
>      CreateCo *cco = opaque;
>      assert(cco->drv);
>  
> -    ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
> +    if (cco->drv->bdrv_create2) {
> +        ret = cco->drv->bdrv_create2(cco->filename, cco->opts, &local_err);

Is it worth assert(!cco->options) here,

> +    } else {
> +        ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);

and assert(!cco->opts) here, to ensure that we aren't ignoring options
from the other style of create?  Then again, it gets cleaned up in later
patches.

> @@ -5248,28 +5266,31 @@ void bdrv_img_create(const char *filename, const char *fmt,
>          return;
>      }
>  
> -    create_options = append_option_parameters(create_options,
> -                                              drv->create_options);
> -    create_options = append_option_parameters(create_options,
> -                                              proto_drv->create_options);
> +    if (drv->bdrv_create2) {
> +        create_opts = qemu_opts_append(create_opts, drv->create_opts);
> +        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);

Memory leak.  As implemented earlier in the series, qemu_opts_append()
creates a new object, but as you are overwriting create_opts with the
second result without hanging onto the pointer returned by the first
call, you have just leaked the first instance.

I ran out of time to review further today, but this series still needs
more work, and I am starting to doubt that it will make 2.0.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]