Re: [Qemu-devel] [PULL 47/58] qdev-monitor: Unref device when device_add fails

[Amos Kong]

On Tue, Oct 08, 2013 at 07:44:45PM +0200, Andreas Färber wrote:
> From: Stefan Hajnoczi <address@hidden>
> 
> qdev_device_add() leaks the created device upon failure.  I suspect this
> problem crept in because qdev_free() unparents the device but does not
> drop a reference - confusing name.
> 
> Cc: address@hidden
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> Signed-off-by: Andreas Färber <address@hidden>

Hi Stefan,

This commit caused a regression bug:

hotplug more than 32 disks to vm, qemu crash

---

address@hidden qemu]$ cat radd.sh 
for i in `seq 3 9` a b c d e f 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f;do
for j in `seq 1 7` 0;do
/bin/cp /images/none.qcow2 /tmp/resize$i$j.qcow2

echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none
echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none | nc -U 
/tmp/m

echo device_add 
virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on
echo device_add 
virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on | nc -U 
/tmp/m
done
done

----

#0  0x00005555558b7f95 in flatview_ref (view=0x0) at 
/home/devel/qemu/memory.c:300
#1  0x00005555558b9689 in address_space_get_flatview (as=0x55555645d660) at 
/home/devel/qemu/memory.c:656
#2  0x00005555558ba416 in address_space_update_topology (as=0x55555645d660) at 
/home/devel/qemu/memory.c:760
#3  0x00005555558ba5cf in memory_region_transaction_commit () at 
/home/devel/qemu/memory.c:799
#4  0x00005555558bcfcc in memory_region_set_enabled (mr=0x55555647af08, 
enabled=false) at /home/devel/qemu/memory.c:1503
#5  0x000055555571a0af in do_pci_register_device (pci_dev=0x55555647ac10, 
bus=0x5555564132b0, name=0x555556261100 "virtio-blk-pci", devfn=26) at 
hw/pci/pci.c:846
#6  0x000055555571c6cc in pci_qdev_init (qdev=0x55555647ac10) at 
hw/pci/pci.c:1751
#7  0x0000555555694d70 in device_realize (dev=0x55555647ac10, 
err=0x7fffffffc8e8) at hw/core/qdev.c:178
#8  0x00005555556966fc in device_set_realized (obj=0x55555647ac10, value=true, 
err=0x7fffffffca60) at hw/core/qdev.c:699
#9  0x00005555557e7b57 in property_set_bool (obj=0x55555647ac10, 
v=0x55555679a830, opaque=0x555556461b10, name=0x5555559922ae "realized", 
errp=0x7fffffffca60)
    at qom/object.c:1315
#10 0x00005555557e665b in object_property_set (obj=0x55555647ac10, 
v=0x55555679a830, name=0x5555559922ae "realized", errp=0x7fffffffca60) at 
qom/object.c:803
#11 0x00005555557e816e in object_property_set_qobject (obj=0x55555647ac10, 
value=0x555556678880, name=0x5555559922ae "realized", errp=0x7fffffffca60) at 
qom/qom-qobject.c:24
#12 0x00005555557e6950 in object_property_set_bool (obj=0x55555647ac10, 
value=true, name=0x5555559922ae "realized", errp=0x7fffffffca60) at 
qom/object.c:866
#13 0x0000555555694ca7 in qdev_init (dev=0x55555647ac10) at hw/core/qdev.c:163
#14 0x00005555557c60ee in qdev_device_add (opts=0x555556525370) at 
qdev-monitor.c:543
#15 0x00005555557c6730 in do_device_add (mon=0x5555562fb760, 
qdict=0x55555645d440, ret_data=0x7fffffffcb80) at qdev-monitor.c:656
#16 0x00005555558c8892 in handle_user_command (mon=0x5555562fb760, 
cmdline=0x5555563f0f60 "device_add 
virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on")
    at /home/devel/qemu/monitor.c:4137
#17 0x00005555558ca10f in monitor_command_cb (mon=0x5555562fb760, 
cmdline=0x5555563f0f60 "device_add 
virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on", 
    opaque=0x0) at /home/devel/qemu/monitor.c:4757
#18 0x00005555557e9491 in readline_handle_byte (rs=0x5555563f0f60, ch=10) at 
readline.c:373
#19 0x00005555558ca045 in monitor_read (opaque=0x5555562fb760, 
buf=0x7fffffffccf0 "\n\315\377\377\377\177", size=1) at 
/home/devel/qemu/monitor.c:4743
#20 0x00005555557c6cc8 in qemu_chr_be_write (s=0x555556269040, 
buf=0x7fffffffccf0 "\n\315\377\377\377\177", len=1) at qemu-char.c:165
#21 0x00005555557cb026 in tcp_chr_read (chan=0x55555645fe40, cond=G_IO_IN, 
opaque=0x555556269040) at qemu-char.c:2487
#22 0x00007ffff76ede06 in g_main_context_dispatch () from 
/lib64/libglib-2.0.so.0
#23 0x000055555578ef33 in glib_pollfds_poll () at main-loop.c:189
#24 0x000055555578f028 in os_host_main_loop_wait (timeout=77312299) at 
main-loop.c:234
#25 0x000055555578f100 in main_loop_wait (nonblocking=0) at main-loop.c:483
#26 0x000055555582e234 in main_loop () at vl.c:2014
#27 0x0000555555835697 in main (argc=14, argv=0x7fffffffe298, 
envp=0x7fffffffe310) at vl.c:4362

> ---
>  qdev-monitor.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index b1ce26a..531b258 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -518,6 +518,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      }
>      if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
>          qdev_free(qdev);
> +        object_unref(OBJECT(qdev));
>          return NULL;
>      }
>      if (qdev->id) {
> @@ -531,6 +532,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>          g_free(name);
>      }        
>      if (qdev_init(qdev) < 0) {
> +        object_unref(OBJECT(qdev));
>          qerror_report(QERR_DEVICE_INIT_FAILED, driver);
>          return NULL;
>      }
> -- 
> 1.8.1.4
> 

-- 
                        Amos.