Re: [Qemu-devel] [PATCH v28 00/33] replace QEMUOptionParameter with QemuOpts

[Peter Lieven <pl@kamp.de>]

On 26.06.2014 07:01, Chun Yan Liu wrote:
>
>>>> On 6/23/2014 at 10:06 PM, in message <53A8347C.6010709@dlhnet.de>, Peter Lieven
> <lieven-lists@dlhnet.de> wrote:
>> On 05.06.2014 11:20, Chunyan Liu wrote:
>>> This patch series is to replace QEMUOptionParameter with QemuOpts, so that
>> only
>>> one Qemu Option structure is kept in QEMU code.
>>>
>>> ---
>>> Changes to v27:
>>>     * rebase to git master (code has been updated in this part since v27
>> sent
>>>       so rebase to make the patch series work.)
>>>
>>> All patches are also available from:
>>> https://github.com/chunyanliu/qemu/commits/QemuOpts
>>>
>>>
>>> Chunyan Liu (33):
>>>     QemuOpts: move find_desc_by_name ahead for later calling
>>>     QemuOpts: repurpose qemu_opts_print to replace
>>>       print_option_parameters
>>>     QemuOpts: add def_value_str to QemuOptDesc
>>>     qapi: output def_value_str when query command line options
>>>     QemuOpts: change opt->name|str from (const char *) to (char *)
>>>     QemuOpts: move qemu_opt_del ahead for later calling
>>>     QemuOpts: add qemu_opt_get_*_del functions for replace work
>>>     QemuOpts: add qemu_opts_print_help to replace print_option_help
>>>     QemuOpts: add conversion between QEMUOptionParameter to QemuOpts
>>>     QemuOpts: add qemu_opts_append to replace append_option_parameters
>>>     QemuOpts: check NULL input for qemu_opts_del
>>>     change block layer to support both QemuOpts and QEMUOptionParamter
>>>     vvfat.c: handle cross_driver's create_options and create_opts
>>>     cow.c: replace QEMUOptionParameter with QemuOpts
>>>     gluster.c: replace QEMUOptionParameter with QemuOpts
>>>     iscsi.c: replace QEMUOptionParameter with QemuOpts
>>>     nfs.c: replace QEMUOptionParameter with QemuOpts
>>>     qcow.c: replace QEMUOptionParameter with QemuOpts
>>>     QemuOpts: export qemu_opt_find
>>>     qcow2.c: replace QEMUOptionParameter with QemuOpts
>>>     qed.c: replace QEMUOptionParameter with QemuOpts
>>>     raw-posix.c: replace QEMUOptionParameter with QemuOpts
>>>     raw-win32.c: replace QEMUOptionParameter with QemuOpts
>>>     raw_bsd.c: replace QEMUOptionParameter with QemuOpts
>>>     rbd.c: replace QEMUOptionParameter with QemuOpts
>>>     sheepdog.c: replace QEMUOptionParameter with QemuOpts
>>>     ssh.c: replace QEMUOptionParameter with QemuOpts
>>>     vdi.c: replace QEMUOptionParameter with QemuOpts
>>>     vhdx.c: replace QEMUOptionParameter with QemuOpts
>>>     vmdk.c: replace QEMUOptionParameter with QemuOpts
>>>     vpc.c: replace QEMUOptionParameter with QemuOpts
>>>     cleanup QEMUOptionParameter
>>>     QemuOpts: cleanup tmp 'allocated' member from QemuOptsList
>>>
>>>    block.c                    |  99 ++++----
>>>    block/cow.c                |  52 ++--
>>>    block/gluster.c            |  73 +++---
>>>    block/iscsi.c              |  32 ++-
>>>    block/nfs.c                |  10 +-
>>>    block/qcow.c               |  72 +++---
>>>    block/qcow2.c              | 259 ++++++++++----------
>>>    block/qed.c                | 112 +++++----
>>>    block/qed.h                |   3 +-
>>>    block/raw-posix.c          |  55 ++---
>>>    block/raw-win32.c          |  38 +--
>>>    block/raw_bsd.c            |  25 +-
>>>    block/rbd.c                |  61 +++--
>>>    block/sheepdog.c           | 105 ++++----
>>>    block/ssh.c                |  30 ++-
>>>    block/vdi.c                |  71 +++---
>>>    block/vhdx.c               |  97 ++++----
>>>    block/vhdx.h               |   1 +
>>>    block/vmdk.c               | 121 +++++-----
>>>    block/vpc.c                |  60 ++---
>>>    block/vvfat.c              |  14 +-
>>>    include/block/block.h      |   7 +-
>>>    include/block/block_int.h  |   9 +-
>>>    include/qemu/option.h      |  53 +---
>>>    include/qemu/option_int.h  |   4 +-
>>>    qapi-schema.json           |   5 +-
>>>    qapi/opts-visitor.c        |  10 +-
>>>    qemu-img.c                 |  91 ++++---
>>>    qmp-commands.hx            |   2 +
>>>    tests/qemu-iotests/049.out |   2 +-
>>>    tests/qemu-iotests/061.out |   2 +-
>>>    util/qemu-config.c         |   4 +
>>>    util/qemu-option.c         | 590 ++++++++++++++++++++-------------------------
>>>    33 files changed, 1037 insertions(+), 1132 deletions(-)
>>>
>>   
>> It seems that for qemu-img this series introduces stack corruption. Any
>> ideas?
>>   
>> lieven@lieven-pc:~/git/qemu$ valgrind --leak-check=full ./qemu-img convert -p
>> nfs://172.21.200.61/vcore-dev-cdrom/pool3/VC_Ubuntu_LTS_12.04.2_64bit.qcow2
>> /tmp/x.raw
>> ==23086== Memcheck, a memory error detector
>> ==23086== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
>> ==23086== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
>> ==23086== Command: ./qemu-img convert -p
>> nfs://172.21.200.61/vcore-dev-cdrom/pool3/VC_Ubuntu_LTS_12.04.2_64bit.qcow2
>> /tmp/x.raw
>> ==23086==
>> ==23086== Warning: client switching stacks?  SP change: 0x7650f08 -->
>> 0x7feffe9f0
>> ==23086==          to suppress, use: --max-stackframe=34218892008 or greater
>> ==23086== Invalid write of size 8
>> ==23086==    at 0x1C4980: qemu_opts_create (qemu-option.c:692)
>> ==23086==    by 0x119918: img_convert (qemu-img.c:1385)
>> ==23086==    by 0x11CEEC: main (qemu-img.c:2830)
>> ==23086==  Address 0x7c65fe8 is 24 bytes inside a block of size 104 free'd
>> ==23086==    at 0x4C2B7B2: realloc (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==23086==    by 0x54C1AF6: g_realloc (in
>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
>> ==23086==    by 0x1C5C52: qemu_opts_append (qemu-option.c:1129)
>> ==23086==    by 0x1198EE: img_convert (qemu-img.c:1383)
>> ==23086==    by 0x11CEEC: main (qemu-img.c:2830)
>> ==23086==
>>   
>>   
>> Peter
>>   
> Thanks for reporting. I think it's the same issue and please refer to:
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg06013.html

Yes, thats it.

Thanks you,
Peter