Re: [Qemu-devel] [PATCH v20 07/26] cow.c: replace QEMUOptionParameter with QemuOpts

[Chunyan Liu]

2014-02-13 8:19 GMT+08:00 Eric Blake <address@hidden>:

On 02/11/2014 11:33 PM, Chunyan Liu wrote:
> cow.c: replace QEMUOptionParameter with QemuOpts
>
> Signed-off-by: Dong Xu Wang <address@hidden>
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
>  block/cow.c |   46 ++++++++++++++++++++++------------------------
>  1 files changed, 22 insertions(+), 24 deletions(-)
>

> +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_SIZE,
> +            .help = "Virtual disk size"

Oh, these QemuOpts really ARE constant strings in the general case, and
passing these strings to free() could be a disaster.  I hope we're okay
in what we do; in fact, maybe it's worth a patch to QemuOptsList that
adds a bool record of whether the list is dynamically allocated (false
by default for all static initialization, and true when malloc'ing a
list) and then asserting that attempts to free (parts of) a list are
only done on a dynamically allocated list.

Well, create_opts specified here won't be freed by free() in existing code I thi
nk. Those using it would malloc and memcpy from it, and that would be freed. The
same way as existing create_options handling.
 

> @@ -414,14 +412,14 @@ static BlockDriver bdrv_cow = {
>      .bdrv_probe     = cow_probe,
>      .bdrv_open      = cow_open,
>      .bdrv_close     = cow_close,
> -    .bdrv_create    = cow_create,
> +    .bdrv_create2    = cow_create,

Might as well keep alignment looking nice.

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