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

[Chun Yan Liu]

>>> On 6/23/2014 at 10:06 PM, in message <address@hidden>, Peter Lieven
<address@hidden> 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? 
>  
> address@hidden:~/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

>