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 > Signed-off-by: Chunyan Liu > --- > @@ -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